Patchwork [STABLE] rebase: avoid RevlogError when computing obsoletenotrebased (issue5907)

login
register
mail settings
Submitter Matt Harbison
Date June 6, 2018, 12:56 p.m.
Message ID <69d1cafe75f20a1e8bf0.1528289783@Envy>
Download mbox | patch
Permalink /patch/31986/
State Accepted
Headers show

Comments

Matt Harbison - June 6, 2018, 12:56 p.m.
# HG changeset patch
# User Matt Harbison <matt_harbison@yahoo.com>
# Date 1528256994 14400
#      Tue Jun 05 23:49:54 2018 -0400
# Branch stable
# Node ID 69d1cafe75f20a1e8bf0478d715c4cdd207d8ecb
# Parent  79c54e7c0c5279b315e43540b98fb22c4843d79b
rebase: avoid RevlogError when computing obsoletenotrebased (issue5907)

The key to reproducing this seems to be missing an obsolete node that is not an
ancestor of the destination.
Yuya Nishihara - June 6, 2018, 1:31 p.m.
On Wed, 06 Jun 2018 08:56:23 -0400, Matt Harbison wrote:
> # HG changeset patch
> # User Matt Harbison <matt_harbison@yahoo.com>
> # Date 1528256994 14400
> #      Tue Jun 05 23:49:54 2018 -0400
> # Branch stable
> # Node ID 69d1cafe75f20a1e8bf0478d715c4cdd207d8ecb
> # Parent  79c54e7c0c5279b315e43540b98fb22c4843d79b
> rebase: avoid RevlogError when computing obsoletenotrebased (issue5907)

Queued for stable, thanks.

> diff --git a/hgext/rebase.py b/hgext/rebase.py
> --- a/hgext/rebase.py
> +++ b/hgext/rebase.py
> @@ -1815,7 +1815,8 @@ def _computeobsoletenotrebased(repo, reb
>                  # If 'srcrev' has a successor in rebase set but none in
>                  # destination (which would be catched above), we shall skip it
>                  # and its descendants to avoid divergence.
> -                if any(nodemap[s] in destmap for s in successors):
> +                if any(nodemap[s] in destmap for s in successors
> +                       if s in nodemap):
>                      obsoletewithoutsuccessorindestination.add(srcrev)

I have no idea why we're taking care of missing successors until the last
"if not successors else" condition. We might have to filter them out earlier?
Whichever way is correct, this patch should be an improvement.
Matt Harbison - June 7, 2018, 1:33 a.m.
On Wed, 06 Jun 2018 09:31:25 -0400, Yuya Nishihara <yuya@tcha.org> wrote:

> On Wed, 06 Jun 2018 08:56:23 -0400, Matt Harbison wrote:
>> # HG changeset patch
>> # User Matt Harbison <matt_harbison@yahoo.com>
>> # Date 1528256994 14400
>> #      Tue Jun 05 23:49:54 2018 -0400
>> # Branch stable
>> # Node ID 69d1cafe75f20a1e8bf0478d715c4cdd207d8ecb
>> # Parent  79c54e7c0c5279b315e43540b98fb22c4843d79b
>> rebase: avoid RevlogError when computing obsoletenotrebased (issue5907)
>
> Queued for stable, thanks.
>
>> diff --git a/hgext/rebase.py b/hgext/rebase.py
>> --- a/hgext/rebase.py
>> +++ b/hgext/rebase.py
>> @@ -1815,7 +1815,8 @@ def _computeobsoletenotrebased(repo, reb
>>                  # If 'srcrev' has a successor in rebase set but none in
>>                  # destination (which would be catched above), we shall  
>> skip it
>>                  # and its descendants to avoid divergence.
>> -                if any(nodemap[s] in destmap for s in successors):
>> +                if any(nodemap[s] in destmap for s in successors
>> +                       if s in nodemap):
>>                      obsoletewithoutsuccessorindestination.add(srcrev)
>
> I have no idea why we're taking care of missing successors until the last
> "if not successors else" condition. We might have to filter them out  
> earlier?

I wondered the same thing.  But obsutil.allsuccessors() says not all nodes  
may be known locally.  If we filter out earlier, that would seem to change  
the logic of `successors.issubset(extinctnodes)` (extinctnodes are all  
local), and also `if not successors`.  (I have no idea if the current code  
is correct, but I didn't want to overreach on stable.)

Patch

diff --git a/hgext/rebase.py b/hgext/rebase.py
--- a/hgext/rebase.py
+++ b/hgext/rebase.py
@@ -1815,7 +1815,8 @@  def _computeobsoletenotrebased(repo, reb
                 # If 'srcrev' has a successor in rebase set but none in
                 # destination (which would be catched above), we shall skip it
                 # and its descendants to avoid divergence.
-                if any(nodemap[s] in destmap for s in successors):
+                if any(nodemap[s] in destmap for s in successors
+                       if s in nodemap):
                     obsoletewithoutsuccessorindestination.add(srcrev)
 
     return (
diff --git a/tests/test-rebase-obsolete.t b/tests/test-rebase-obsolete.t
--- a/tests/test-rebase-obsolete.t
+++ b/tests/test-rebase-obsolete.t
@@ -1493,6 +1493,38 @@  Rebase merge where successor of other pa
   
   $ cd ..
 
+  $ hg init p2-succ-in-dest-c
+  $ cd p2-succ-in-dest-c
+
+The scenario here was that B::D were developed on default.  B was queued on
+stable, but amended before being push to hg-committed.  C was queued on default,
+along with unrelated J.
+
+  $ hg debugdrawdag <<EOF
+  > J
+  > |
+  > F
+  > |
+  > E
+  > | D
+  > | |
+  > | C      # replace: C -> F
+  > | |  H I # replace: B -> H -> I
+  > | B  |/
+  > |/   G
+  > A
+  > EOF
+  1 new orphan changesets
+
+This strip seems to be the key to avoid an early divergence warning.
+  $ hg --config extensions.strip= --hidden strip -qr H
+  1 new orphan changesets
+
+  $ hg rebase -b 'desc("D")' -d 'desc("J")'
+  abort: this rebase will cause divergences from: 112478962961
+  (to force the rebase please set experimental.evolution.allowdivergence=True)
+  [255]
+
 Rebase merge where both parents have successors in destination
 
   $ hg init p12-succ-in-dest