Patchwork [2,of,2] update: allow dirty update to successors

# HG changeset patch
# User Pierre-Yves David <pierre-yves.david@logilab.fr>
# Date 1365784871 -7200
#      Fri Apr 12 18:41:11 2013 +0200
# Node ID 0f44f99fb5c1d9b05fa0b703e11aa10d7dc0f926
# Parent  5bd7702f8e9613a70b2dcf511aef85fd221eff78
update: allow dirty update to successors


Update to successors are no longer seen as cross branch update. This allows to
merge with uncommited changes.

This changeset is a small step forward. We want to allow dirty update to
precursors and takes obsolescence in account when finding the default update
destination. But those requires deeper changes and will comes in later
changesets.

On Fri, Apr 12, 2013 at 06:55:22PM +0200, pierre-yves.david@logilab.fr wrote:
> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david@logilab.fr>
> # Date 1365784871 -7200
> #      Fri Apr 12 18:41:11 2013 +0200
> # Node ID 0f44f99fb5c1d9b05fa0b703e11aa10d7dc0f926
> # Parent  5bd7702f8e9613a70b2dcf511aef85fd221eff78
> update: allow dirty update to successors
>

Behavior sounds reasonable, but I'd like someone else to agree with me
before I push, since I'm not wholly able to channel mpm on this kind
of thing.

One nit in this patch about import style, otherwise series LGTM.

>
>
> Update to successors are no longer seen as cross branch update. This allows to
> merge with uncommited changes.
>
> This changeset is a small step forward. We want to allow dirty update to
> precursors and takes obsolescence in account when finding the default update
> destination. But those requires deeper changes and will comes in later
> changesets.
>
> diff --git a/mercurial/merge.py b/mercurial/merge.py
> --- a/mercurial/merge.py
> +++ b/mercurial/merge.py
> @@ -5,10 +5,11 @@
>  # This software may be used and distributed according to the terms of the
>  # GNU General Public License version 2 or any later version.
>
>  from node import nullid, nullrev, hex, bin
>  from i18n import _
> +from mercurial import obsolete

stylistically, we generally prefer "import obsolete" to "from mercurial import obsolete" in hg

>  import error, util, filemerge, copies, subrepo, worker, dicthelpers
>  import errno, os, shutil
>
>  class mergestate(object):
>      '''track 3-way merge state of individual files'''
> @@ -695,21 +696,28 @@ def update(repo, node, branchmerge, forc
>                  if wc.sub(s).dirty():
>                      raise util.Abort(_("outstanding uncommitted changes in "
>                                         "subrepository '%s'") % s)
>
>          elif not overwrite:
> -            if pa == p1 or pa == p2: # linear
> -                pass # all good
> -            elif wc.dirty(missing=True):
> -                raise util.Abort(_("crosses branches (merge branches or use"
> -                                   " --clean to discard changes)"))
> -            elif onode is None:
> -                raise util.Abort(_("crosses branches (merge branches or update"
> -                                   " --check to force update)"))
> -            else:
> -                # Allow jumping branches if clean and specific rev given
> -                pa = p1
> +            if pa not in (p1, p2):  # nolinear
> +                dirty = wc.dirty(missing=True)
> +                if dirty or onode is None:
> +                    # Branching is a bit strange to ensure we do the minimal
> +                    # amount of call to obsolete.validdest as it may be costly.
> +                    if obsolete.validdest(repo, p1, repo[node]):
> +                        pa = p1  # allow updating to successors
> +                    elif dirty:
> +                        msg = _("crosses branches (merge branches or use"
> +                                " --clean to discard changes)")
> +                        raise util.Abort(msg)
> +                    else:  # node is none
> +                        msg = _("crosses branches (merge branches or update"
> +                                " --check to force update)")
> +                        raise util.Abort(msg)
> +                else:
> +                    # Allow jumping branches if clean and specific rev given
> +                    pa = p1
>
>          ### calculate phase
>          actions = calculateupdates(repo, wc, p2, pa,
>                                     branchmerge, force, partial, mergeancestor)
>
> diff --git a/mercurial/obsolete.py b/mercurial/obsolete.py
> --- a/mercurial/obsolete.py
> +++ b/mercurial/obsolete.py
> @@ -404,11 +404,11 @@ def allsuccessors(obsstore, nodes, ignor
>                      remaining.add(suc)
>
>  def validdest(repo, old, new):
>      """Is <new> a valid destination from the <old> one
>
> -    Used by bookmark logic.
> +    Used by bookmark and update logic.
>      """
>      repo = repo.unfiltered()
>      if old == new:
>          # Old == new -> nothing to update.
>          return False
> diff --git a/tests/test-update-branches.t b/tests/test-update-branches.t
> --- a/tests/test-update-branches.t
> +++ b/tests/test-update-branches.t
> @@ -162,5 +162,71 @@ Cases are run as shown in that table, ro
>    $ revtest '-cC dirty linear'  dirty 1 2 -cC
>    abort: cannot specify both -c/--check and -C/--clean
>    parent=1
>    M foo
>
> +Test obsolescence behavior
> +---------------------------------------------------------------------
> +
> +successors should be taken in account when checking head destination
> +
> +  $ cat << EOF >> $HGRCPATH
> +  > [extensions]
> +  > obs=$TESTTMP/obs.py
> +  > [ui]
> +  > logtemplate={rev}:{node|short} {desc|firstline}
> +  > EOF
> +  $ cat > $TESTTMP/obs.py << EOF
> +  > import mercurial.obsolete
> +  > mercurial.obsolete._enabled = True
> +  > EOF
> +
> +Test no-argument update to a successor of an obsoleted changeset
> +
> +  $ hg log -G
> +  o  5:ff252e8273df 5
> +  |
> +  o  4:d047485b3896 4
> +  |
> +  | o  3:6efa171f091b 3
> +  | |
> +  | | o  2:bd10386d478c 2
> +  | |/
> +  | @  1:0786582aa4b1 1
> +  |/
> +  o  0:60829823a42a 0
> +
> +  $ hg status
> +  M foo
> +
> +We add simple obsolescence marker between 3 and 4 (indirect successors)
> +
> +  $ hg id --debug -i -r 3
> +  6efa171f091b00a3c35edc15d48c52a498929953
> +  $ hg id --debug -i -r 4
> +  d047485b3896813b2a624e86201983520f003206
> +  $ hg debugobsolete 6efa171f091b00a3c35edc15d48c52a498929953 aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
> +  $ hg debugobsolete aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa d047485b3896813b2a624e86201983520f003206
> +
> +Test that 5 is detected as a valid destination from 3
> +  $ hg up --hidden 3
> +  1 files updated, 0 files merged, 0 files removed, 0 files unresolved
> +  $ hg up 5
> +  1 files updated, 0 files merged, 0 files removed, 0 files unresolved
> +
> +Test that 5 is detected as a valid destination from 1
> +  $ hg up 0 # we should be possible to update to 3 directly
> +  >         # but not implemented yet.
> +  1 files updated, 0 files merged, 0 files removed, 0 files unresolved
> +  $ hg up --hidden 3
> +  1 files updated, 0 files merged, 0 files removed, 0 files unresolved
> +  $ hg up 5
> +  1 files updated, 0 files merged, 0 files removed, 0 files unresolved
> +
> +Test that 5 is not detected as a valid destination from 2
> +  $ hg up 0
> +  1 files updated, 0 files merged, 0 files removed, 0 files unresolved
> +  $ hg up 2
> +  1 files updated, 0 files merged, 0 files removed, 0 files unresolved
> +  $ hg up 5
> +  abort: crosses branches (merge branches or use --clean to discard changes)
> +  [255]
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel

On 12 avr. 2013, at 22:30, Augie Fackler wrote:

> On Fri, Apr 12, 2013 at 06:55:22PM +0200, pierre-yves.david@logilab.fr wrote:
>> # HG changeset patch
>> # User Pierre-Yves David <pierre-yves.david@logilab.fr>
>> # Date 1365784871 -7200
>> #      Fri Apr 12 18:41:11 2013 +0200
>> # Node ID 0f44f99fb5c1d9b05fa0b703e11aa10d7dc0f926
>> # Parent  5bd7702f8e9613a70b2dcf511aef85fd221eff78
>> update: allow dirty update to successors
>> 
> 
> Behavior sounds reasonable, but I'd like someone else to agree with me
> before I push, since I'm not wholly able to channel mpm on this kind
> of thing.

I can testify that this had been discussed and agreed at the sprint. Levi Bard started working on it then. 

This is the first step toward graceful handling of "pull turn your current working directory parent obsolete."

On Fri, 2013-04-12 at 16:30 -0400, Augie Fackler wrote:
> On Fri, Apr 12, 2013 at 06:55:22PM +0200, pierre-yves.david@logilab.fr wrote:
> > # HG changeset patch
> > # User Pierre-Yves David <pierre-yves.david@logilab.fr>
> > # Date 1365784871 -7200
> > #      Fri Apr 12 18:41:11 2013 +0200
> > # Node ID 0f44f99fb5c1d9b05fa0b703e11aa10d7dc0f926
> > # Parent  5bd7702f8e9613a70b2dcf511aef85fd221eff78
> > update: allow dirty update to successors
> >
> 
> Behavior sounds reasonable, but I'd like someone else to agree with me
> before I push, since I'm not wholly able to channel mpm on this kind
> of thing.

Yeah. Perhaps we need to write down some rationales somewhere (together
with the table of all the update behaviors).

Here are the rationales I think it's important to be aware of:

a) we allow linear update with dirty working directory because I copied
CVS and we can't go back

b) we don't allow jumping across branches on update because people
forget they had a branch and wonder where their code went

c) we don't quietly not jump in (b) because the opposite happens

d) we allow cross-jumping when explicit targets are given because
there's some indication the user knows what they're doing..

e) ..except when we have a dirty working directory, because merges are
likely to mess things up


Also, I often feel we need a name for sub-named-branch topological
branches, like 'twig'.