[PATCH 6 of 6 RFC] destutil: choose non-closed branch head as default update destination (BC)

[Pierre-Yves David]

On 02/24/2016 03:17 PM, FUJIWARA Katsunori wrote:
> # HG changeset patch
> # User FUJIWARA Katsunori <foozy at lares.dti.ne.jp>
> # Date 1456322433 -32400
> #      Wed Feb 24 23:00:33 2016 +0900
> # Node ID ae5d291a6dcb96683eae3c4e33575297fa113fbb
> # Parent  1f9b3d38e380b018cbe10ed1acee882c014e3763
> destutil: choose non-closed branch head as default update destination (BC)
>
> Before this patch, destupdate() returns the tipmost (descendant)
> branch head regardless of closed or not. But updating to closed branch
> head isn't reasonable for ordinary workflow, because:
>
>    - "hg heads" doesn't show closed heads (= updated parent itself) by
>      default
>
>    - subsequent committing on it re-opens closed branch
>
>      even if inactivation of closed head is needed, update destination
>      isn't it, because it should be merged into to another branch in
>      such case.

Excluding closed head from update makes a lot of sense to me. But we 
probably need some better messaging around the no-op update. I'm not 
exactly sure no-op update is the right choice if there is nothing but 
closed head above. However giving priority to non-closed head seems an 
obvious way to go here.

I need to think about the "no non-closed heads in .::" case a bit more.

Thanks for looking into that.

>
> This patch chooses non-closed branch head as default update
> destination.
>
> This patch changes not only normal lookup code path, but also "no
> default branch" code path, for consistency.
>
> If no appropriate default update destination is found, destupdate()
> returns '.' as destination to work as no-op at updating. 'None' isn't
> reasonable for this purpose, because:
>
>    - repo[node] returns useless rev/node if node=None
>    - merge.update() re-invokes destupdate() for None destination
>
> diff --git a/mercurial/destutil.py b/mercurial/destutil.py
> --- a/mercurial/destutil.py
> +++ b/mercurial/destutil.py
> @@ -93,14 +93,15 @@ def _destupdatebranch(repo, clean, check
>       movemark = node = None
>       currentbranch = wc.branch()
>       if currentbranch in repo.branchmap():
> -        heads = repo.branchheads(currentbranch, closed=True)
> +        heads = repo.branchheads(currentbranch)
>           if heads:
>               node = repo.revs('max(.::(%ln))', heads).first()
>           if bookmarks.isactivewdirparent(repo):
>               movemark = repo['.'].node()
>       else:
>           if currentbranch == 'default': # no default branch!
> -            node = repo.lookup('tip') # update to tip
> +            # update to the tipmost non-closed branch head
> +            node = repo.revs('max(head() and not closed())').first()
>           else:
>               raise error.Abort(_("branch %s not found") % currentbranch)
>       return node, movemark, None
> @@ -130,6 +131,11 @@ def destupdate(repo, clean=False, check=
>           node, movemark, activemark = destupdatestepmap[step](repo, clean, check)
>           if node is not None:
>               break
> +    else:
> +        # udate to current parent (= no-op), because no appropriate
> +        # default destination is found
> +        node = '.'
> +
>       rev = repo[node].rev()
>
>       _destupdatevalidate(repo, rev, clean, check)
> @@ -366,6 +372,16 @@ def _statusotherbranchheads(ui, repo):
>           if otherheads:
>               ui.status(_('%i other heads for branch "%s"\n') %
>                         (len(otherheads), currentbranch))
> +    elif len(repo): # omit redundant message in empty repository
> +        if not heads:
> +            if currentbranch not in repo.branchmap():
> +                # this is "update to the tipmost non-closed branch head" case
> +                ui.status(_('all branches are closed\n'))
> +            else:
> +                ui.status(_('all heads of branch "%s" are closed\n') %
> +                          currentbranch)
> +        elif not len(repo.revs('parents()::(%ln)', heads)):
> +            ui.status(_('all descendant branch heads are closed\n'))
>
>   def statusotherdests(ui, repo):
>       """Print message about other head"""
> 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
> @@ -196,6 +196,24 @@ if on the closed branch head:
>     1 other heads for branch "default"
>     parent=6
>
> +if only one descendant non-closed branch head exists:
> +- update to it, even if its revision is less than closed one
> +- "N other heads for ...." message isn't displayed
> +
> +  $ norevtest "non-closed 2 should be chosen" clean 1
> +  1 files updated, 0 files merged, 0 files removed, 0 files unresolved
> +  parent=2
> +
> +if all descendant branch heads are closed:
> +- updating is no-op
> +- "N other heads for ...." message isn't displayed
> +- "all descendant branch ...." is displayed
> +
> +  $ norevtest "all descendant branch heads are closed" clean 3
> +  0 files updated, 0 files merged, 0 files removed, 0 files unresolved
> +  all descendant branch heads are closed
> +  parent=3
> +
>   Test updating if all branch heads are closed
>
>   if on the closed branch head:
> @@ -208,6 +226,55 @@ if on the closed branch head:
>     0 files updated, 0 files merged, 0 files removed, 0 files unresolved
>     parent=6
>
> +if not on the closed branch head:
> +- updating is no-op
> +- "all heads of branch ...." message is displayed
> +
> +  $ norevtest "all heads of branch default are closed" clean 1
> +  0 files updated, 0 files merged, 0 files removed, 0 files unresolved
> +  all heads of branch "default" are closed
> +  parent=1
> +
> +  $ cd ..
> +
> +Test updating if "default" branch doesn't exist and no revision is
> +checked out (= "default" is used as current branch)
> +
> +  $ hg init no-default-branch
> +  $ cd no-default-branch
> +
> +  $ hg branch foobar
> +  marked working directory as branch foobar
> +  (branches are permanent and global, did you want a bookmark?)
> +  $ echo a > a
> +  $ hg commit -m "#0" -A
> +  adding a
> +  $ echo 1 >> a
> +  $ hg commit -m "#1"
> +  $ hg update -q 0
> +  $ echo 3 >> a
> +  $ hg commit -m "#2"
> +  created new head
> +  $ hg commit --close-branch -m "#3"
> +
> +if there is at least one non-closed branch head:
> +- update to the tipmost branch head
> +
> +  $ norevtest "non-closed 1 should be chosen" clean null
> +  1 files updated, 0 files merged, 0 files removed, 0 files unresolved
> +  parent=1
> +
> +if all branch heads are closed
> +- updating is no-op
> +- "all branches are closed" message is displayed
> +
> +  $ hg update -q -C 1
> +  $ hg commit --close-branch -m "#4"
> +
> +  $ norevtest "all branches are closed" clean null
> +  0 files updated, 0 files merged, 0 files removed, 0 files unresolved
> +  all branches are closed
> +
>     $ cd ../b1
>
>   Test obsolescence behavior
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>

-- 
Pierre-Yves David