Patchwork [4,of,7] destutil: explicitly remove descendants from destmerge candidates

login
register
mail settings
Submitter Pierre-Yves David
Date Feb. 14, 2016, 2:16 a.m.
Message ID <4cccc4e95970f32885a3.1455416186@marginatus.alto.octopoid.net>
Download mbox | patch
Permalink /patch/13175/
State Superseded
Headers show

Comments

Pierre-Yves David - Feb. 14, 2016, 2:16 a.m.
# HG changeset patch
# User Pierre-Yves David <pierre-yves.david@fb.com>
# Date 1454951526 -3600
#      Mon Feb 08 18:12:06 2016 +0100
# Node ID 4cccc4e95970f32885a3e896c668f3e00b11bd45
# Parent  b989484b0b89e62846830c0ce20bb567b2d866df
# EXP-Topic destination
# Available At http://hg.netv6.net/marmoute-wip/mercurial/
#              hg pull http://hg.netv6.net/marmoute-wip/mercurial/ -r 4cccc4e95970
destutil: explicitly remove descendants from destmerge candidates

While 'hg merge' will refuse to pick a default destination if the working copy
is not on a head, this will be a common and valid case for rebase. In this
case, we will need to exclude from the candidate destination all descendants
from the rebased set.

We make a step forward in that direction by removing descendants of the working
copy from the candidate destinations instead of manually filtering the working
copy parent at the end of the process.

A later changeset will make it possible to specify the source set.
Martin von Zweigbergk - Feb. 14, 2016, 8:05 a.m.
On Sat, Feb 13, 2016 at 6:16 PM, Pierre-Yves David
<pierre-yves.david@ens-lyon.org> wrote:
> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david@fb.com>
> # Date 1454951526 -3600
> #      Mon Feb 08 18:12:06 2016 +0100
> # Node ID 4cccc4e95970f32885a3e896c668f3e00b11bd45
> # Parent  b989484b0b89e62846830c0ce20bb567b2d866df
> # EXP-Topic destination
> # Available At http://hg.netv6.net/marmoute-wip/mercurial/
> #              hg pull http://hg.netv6.net/marmoute-wip/mercurial/ -r 4cccc4e95970
> destutil: explicitly remove descendants from destmerge candidates
>
> While 'hg merge' will refuse to pick a default destination if the working copy
> is not on a head, this will be a common and valid case for rebase. In this
> case, we will need to exclude from the candidate destination all descendants
> from the rebased set.
>
> We make a step forward in that direction by removing descendants of the working
> copy from the candidate destinations instead of manually filtering the working
> copy parent at the end of the process.

The patch looks like just a refactoring removing the current head from
set (list) of non-bookmarked branch heads. I don't see anything about
descendants. Please explain.

>
> A later changeset will make it possible to specify the source set.
>
> diff --git a/mercurial/destutil.py b/mercurial/destutil.py
> --- a/mercurial/destutil.py
> +++ b/mercurial/destutil.py
> @@ -210,45 +210,46 @@ def _destmergebranch(repo, action='merge
>      """find merge destination based on branch heads"""
>      node = None
>      parent = repo.dirstate.p1()
>      branch = repo.dirstate.branch()
>      bheads = repo.branchheads(branch)
> -    nbhs = [bh for bh in bheads if not repo[bh].bookmarks()]
>
>      if parent not in bheads:
>          # Case A: working copy if not on a head.
>          #
>          # This is probably a user mistake We bailout pointing at 'hg update'
>          if len(repo.heads()) <= 1:
>              msg, hint = msgdestmerge['nootherheadsbehind'][action]
>          else:
>              msg, hint = msgdestmerge['notatheads'][action]
>          raise error.Abort(msg, hint=hint)
> -    elif len(nbhs) > 2:
> -        # Case B: There is more than 2 anonymous heads
> +    # remove current head from the set
> +    bheads = [bh for bh in bheads if bh != parent]

If bheads actually were a set, you wouldn't need the comment (because
"bheads.remove(parent)" is pretty obvious).

> +    # filters out bookmarked heads
> +    nbhs = [bh for bh in bheads if not repo[bh].bookmarks()]
> +    if len(nbhs) > 1:
> +        # Case B: There is more than 1 other anonymous heads
>          #
>          # This means that there will be more than 1 candidate. This is
>          # ambiguous. We abort asking the user to pick as explicit destination
>          # instead.
>          msg, hint = msgdestmerge['toomanyheads'][action]
> -        msg %= (branch, len(bheads))
> +        msg %= (branch, len(bheads) + 1)
>          raise error.Abort(msg, hint=hint)
> -    elif len(nbhs) <= 1:
> -        # Case B: There is no other anonymous head that the one we are one
> +    elif not nbhs:
> +        # Case B: There is no other anonymous heads
>          #
>          # This means that there is no natural candidate to merge with.
>          # We abort, with various messages for various cases.
> -        if len(bheads) > 1:
> +        if bheads:
>              msg, hint = msgdestmerge['bookmarkedheads'][action]
>          elif len(repo.heads()) > 1:
>              msg, hint = msgdestmerge['nootherbranchheads'][action]
>              msg %= branch
>          else:
>              msg, hint = msgdestmerge['nootherheads'][action]
>          raise error.Abort(msg, hint=hint)
> -    elif parent == nbhs[0]:
> -        node = nbhs[-1]
>      else:
>          node = nbhs[0]
>      assert node is not None
>      return node
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Pierre-Yves David - Feb. 14, 2016, 11:28 a.m.
On 02/14/2016 08:05 AM, Martin von Zweigbergk wrote:
> On Sat, Feb 13, 2016 at 6:16 PM, Pierre-Yves David
> <pierre-yves.david@ens-lyon.org> wrote:
>> # HG changeset patch
>> # User Pierre-Yves David <pierre-yves.david@fb.com>
>> # Date 1454951526 -3600
>> #      Mon Feb 08 18:12:06 2016 +0100
>> # Node ID 4cccc4e95970f32885a3e896c668f3e00b11bd45
>> # Parent  b989484b0b89e62846830c0ce20bb567b2d866df
>> # EXP-Topic destination
>> # Available At http://hg.netv6.net/marmoute-wip/mercurial/
>> #              hg pull http://hg.netv6.net/marmoute-wip/mercurial/ -r 4cccc4e95970
>> destutil: explicitly remove descendants from destmerge candidates
>>
>> While 'hg merge' will refuse to pick a default destination if the working copy
>> is not on a head, this will be a common and valid case for rebase. In this
>> case, we will need to exclude from the candidate destination all descendants
>> from the rebased set.
>>
>> We make a step forward in that direction by removing descendants of the working
>> copy from the candidate destinations instead of manually filtering the working
>> copy parent at the end of the process.
>
> The patch looks like just a refactoring removing the current head from
> set (list) of non-bookmarked branch heads. I don't see anything about
> descendants. Please explain.

While writting commit message I've mixed the change done in this patch 
with the change done in the next one. I'll the dispatch the text and 
send a V2.

Do you want to me include patch 1-3 in this V2 or are they fine by you?

Cheers,
Martin von Zweigbergk - Feb. 14, 2016, 2:57 p.m.
1-3 look good to me, no need to resend, thanks.

On Sun, Feb 14, 2016, 03:28 Pierre-Yves David <
pierre-yves.david@ens-lyon.org> wrote:

>
>
> On 02/14/2016 08:05 AM, Martin von Zweigbergk wrote:
> > On Sat, Feb 13, 2016 at 6:16 PM, Pierre-Yves David
> > <pierre-yves.david@ens-lyon.org> wrote:
> >> # HG changeset patch
> >> # User Pierre-Yves David <pierre-yves.david@fb.com>
> >> # Date 1454951526 -3600
> >> #      Mon Feb 08 18:12:06 2016 +0100
> >> # Node ID 4cccc4e95970f32885a3e896c668f3e00b11bd45
> >> # Parent  b989484b0b89e62846830c0ce20bb567b2d866df
> >> # EXP-Topic destination
> >> # Available At http://hg.netv6.net/marmoute-wip/mercurial/
> >> #              hg pull http://hg.netv6.net/marmoute-wip/mercurial/ -r
> 4cccc4e95970
> >> destutil: explicitly remove descendants from destmerge candidates
> >>
> >> While 'hg merge' will refuse to pick a default destination if the
> working copy
> >> is not on a head, this will be a common and valid case for rebase. In
> this
> >> case, we will need to exclude from the candidate destination all
> descendants
> >> from the rebased set.
> >>
> >> We make a step forward in that direction by removing descendants of the
> working
> >> copy from the candidate destinations instead of manually filtering the
> working
> >> copy parent at the end of the process.
> >
> > The patch looks like just a refactoring removing the current head from
> > set (list) of non-bookmarked branch heads. I don't see anything about
> > descendants. Please explain.
>
> While writting commit message I've mixed the change done in this patch
> with the change done in the next one. I'll the dispatch the text and
> send a V2.
>
> Do you want to me include patch 1-3 in this V2 or are they fine by you?
>
> Cheers,
>
> --
> Pierre-Yves David
>

Patch

diff --git a/mercurial/destutil.py b/mercurial/destutil.py
--- a/mercurial/destutil.py
+++ b/mercurial/destutil.py
@@ -210,45 +210,46 @@  def _destmergebranch(repo, action='merge
     """find merge destination based on branch heads"""
     node = None
     parent = repo.dirstate.p1()
     branch = repo.dirstate.branch()
     bheads = repo.branchheads(branch)
-    nbhs = [bh for bh in bheads if not repo[bh].bookmarks()]
 
     if parent not in bheads:
         # Case A: working copy if not on a head.
         #
         # This is probably a user mistake We bailout pointing at 'hg update'
         if len(repo.heads()) <= 1:
             msg, hint = msgdestmerge['nootherheadsbehind'][action]
         else:
             msg, hint = msgdestmerge['notatheads'][action]
         raise error.Abort(msg, hint=hint)
-    elif len(nbhs) > 2:
-        # Case B: There is more than 2 anonymous heads
+    # remove current head from the set
+    bheads = [bh for bh in bheads if bh != parent]
+    # filters out bookmarked heads
+    nbhs = [bh for bh in bheads if not repo[bh].bookmarks()]
+    if len(nbhs) > 1:
+        # Case B: There is more than 1 other anonymous heads
         #
         # This means that there will be more than 1 candidate. This is
         # ambiguous. We abort asking the user to pick as explicit destination
         # instead.
         msg, hint = msgdestmerge['toomanyheads'][action]
-        msg %= (branch, len(bheads))
+        msg %= (branch, len(bheads) + 1)
         raise error.Abort(msg, hint=hint)
-    elif len(nbhs) <= 1:
-        # Case B: There is no other anonymous head that the one we are one
+    elif not nbhs:
+        # Case B: There is no other anonymous heads
         #
         # This means that there is no natural candidate to merge with.
         # We abort, with various messages for various cases.
-        if len(bheads) > 1:
+        if bheads:
             msg, hint = msgdestmerge['bookmarkedheads'][action]
         elif len(repo.heads()) > 1:
             msg, hint = msgdestmerge['nootherbranchheads'][action]
             msg %= branch
         else:
             msg, hint = msgdestmerge['nootherheads'][action]
         raise error.Abort(msg, hint=hint)
-    elif parent == nbhs[0]:
-        node = nbhs[-1]
     else:
         node = nbhs[0]
     assert node is not None
     return node