D340: rebase: prefer choosing merge base with successor in destination
martinvonz added a comment. In https://phab.mercurial-scm.org/D340#6330, @quark wrote: > Since the "unwanted" warning "covers" the case this patch handles. The behavior is still "correct" (since the warning is printed) without this patch. I'm leaning towards dropping this patch. > > For errors vs warnings, I still think it's better to give power users a chance to proceed. How do you think about this approach? > > - If there are multiple merge base candidates, calculate "unwanted" revsets for both, and if only one of them is empty, pick that one automatically and proceed. > - If both merge base candidates have "unwanted" revsets, raise `error.InterventionRequired` so power users could still have a chance to proceed fixing them manually. Sounds good to me. Thanks! The test cases still seem useful, so maybe you can keep that part of the patch? REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D340 To: quark, #hg-reviewers Cc: martinvonz, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D340: rebase: prefer choosing merge base with successor in destination
quark planned changes to this revision. quark added a comment. Since the "unwanted" warning "covers" the case this patch handles. The behavior is still "correct" (since the warning is printed) without this patch. I'm leaning towards dropping this patch. For errors vs warnings, I still think it's better to give power users a chance to proceed. How do you think about this approach? - If there are multiple merge base candidates, calculate "unwanted" revsets for both, and if only one of them is empty, pick that one automatically and proceed. - If both merge base candidates have "unwanted" revsets, raise `error.InterventionRequired` so power users could still have a chance to proceed fixing them manually. INLINE COMMENTS > martinvonz wrote in rebase.py:1115 > And if there are more ancestors of B, will the "rebasing %d:%s may include > unwanted changes" warning appear? > > Regardless, if B adds file B and the merge in C involves removing file B, > then the total diff will be empty. Does that mean that C' will be empty? It > should be reverting B', right? (The same thing applies if it's not a whole > file that gets added/removed, of course.) I think I'd prefer to be > conservative here and error out until we have a safe answer to these merges > that won't risk resulting in surprising contents. In this particular case, > the user can work around it by first rebasing "B+C" only (but that may be > tricky to realize). > And if there are more ancestors of B, will the "rebasing %d:%s may include > unwanted changes" warning appear? Yes. In that case maybe either way is suboptimal. The troublesome cases are: - A merge base candidate has a successor in destination with *different* content - A merge base candidate has "unwanted" ancestors that are not covered by rebaseset I think the problem of this patch is it assumes "different" content blindly. It would be better to actually have a quick check about whether that is the case. Thinking about it again, most cases the reason a commit is in destination is because of a rebase so its content does not change. Therefore this patch is about a very corner case that may not worth the complexity. > Regardless, if B adds file B and the merge in C involves removing file B, > then the total diff will be empty. That is a surprise to me. Apparently you know merge better than I am :) > martinvonz wrote in test-rebase-obsolete.t:1180 > Why do we want/need rebaseskipobsolete? I was expecting to see a test case > like then one in the documentation you added i rebase.py. Not sure why I added that. But it does seem to be unnecessary. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D340 To: quark, #hg-reviewers Cc: martinvonz, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D340: rebase: prefer choosing merge base with successor in destination
martinvonz added inline comments. INLINE COMMENTS > rebase.py:1115 > +#/| | # because it re-introduces obsoleted content. If we choose > +# A B D # A as merge base, it works as expected - C' may be empty. > +if all(b != nullrev for b in bases): And if there are more ancestors of B, will the "rebasing %d:%s may include unwanted changes" warning appear? Regardless, if B adds file B and the merge in C involves removing file B, then the total diff will be empty. Does that mean that C' will be empty? It should be reverting B', right? (The same thing applies if it's not a whole file that gets added/removed, of course.) I think I'd prefer to be conservative here and error out until we have a safe answer to these merges that won't risk resulting in surprising contents. In this particular case, the user can work around it by first rebasing "B+C" only (but that may be tricky to realize). > test-rebase-obsolete.t:1180 > + > EOS > + $ hg rebase -r A+B+D -d Z --config experimental.rebaseskipobsolete=0 > + rebasing 0:426bada5c675 "A" (A) Why do we want/need rebaseskipobsolete? I was expecting to see a test case like then one in the documentation you added i rebase.py. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D340 To: quark, #hg-reviewers Cc: martinvonz, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D340: rebase: prefer choosing merge base with successor in destination
quark updated this revision to Diff 870.
REPOSITORY
rHG Mercurial
CHANGES SINCE LAST UPDATE
https://phab.mercurial-scm.org/D340?vs=868=870
REVISION DETAIL
https://phab.mercurial-scm.org/D340
AFFECTED FILES
hgext/rebase.py
tests/test-rebase-obsolete.t
CHANGE DETAILS
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
@@ -1069,10 +1069,8 @@
rebasing 2:b18e25de2cf5 "D" (D)
note: not rebasing 3:7fb047a69f22 "E" (E), already in destination as
1:112478962961 "B"
rebasing 5:66f1a38021c9 "F" (F tip)
- warning: rebasing 5:66f1a38021c9 may include unwanted changes from
3:7fb047a69f22
+ note: rebase of 5:66f1a38021c9 created no changes to commit
$ hg log -G
- o 7:9ed45af61fa0 F
- |
o 6:8f47515dda15 D
|
| x5:66f1a38021c9 F
@@ -1106,11 +1104,9 @@
note: not rebasing 2:b18e25de2cf5 "D" (D), already in destination as
1:112478962961 "B"
rebasing 3:7fb047a69f22 "E" (E)
rebasing 5:66f1a38021c9 "F" (F tip)
- warning: rebasing 5:66f1a38021c9 may include unwanted changes from
2:b18e25de2cf5
+ note: rebase of 5:66f1a38021c9 created no changes to commit
$ hg log -G
- o 7:502540f44880 F
- |
o 6:533690786a86 E
|
| x5:66f1a38021c9 F
@@ -1127,6 +1123,121 @@
$ cd ..
+Rebase merge where both parents have successors in destination
+
+ $ hg init p12-succ-in-dest
+ $ cd p12-succ-in-dest
+ $ hg debugdrawdag <<'EOS'
+ > E F
+ > /| /| # replace: A -> C
+ > A B C D # replace: B -> D
+ > | |
+ > X Y
+ > EOS
+ $ hg rebase -r A+B+E -d F
+ note: not rebasing 4:a3d17304151f "A" (A), already in destination as
0:96cc3511f894 "C"
+ note: not rebasing 5:b23a2cc00842 "B" (B), already in destination as
1:058c1e1fb10a "D"
+ rebasing 7:dac5d11c5a7d "E" (E tip)
+ warning: rebasing 7:dac5d11c5a7d may include unwanted changes from
3:59c792af609c, 5:b23a2cc00842
+ $ hg manifest -r tip
+ B
+ C
+ D
+ Y
+ $ hg log -G -T '{rev}:{node|short} {desc|firstline} +{file_adds}\n'
+ o 8:3306ff4ff997 E +B Y
+ |
+ | x7:dac5d11c5a7d E +B Y
+ | |\
+ o \ \6:e0c929a964ce F +C
+ |\ \ \
+ | | | x 5:b23a2cc00842 B +B
+ | | | |
+ | | x | 4:a3d17304151f A +A
+ | | | |
+ | | | o 3:59c792af609c Y +Y
+ | | |
+ | | o 2:ba2b7fa7166d X +X
+ | |
+ | o 1:058c1e1fb10a D +D
+ |
+ o 0:96cc3511f894 C +C
+
+ $ cd ..
+
+Rebase one parent with successor in destination, the other parent moves as "-d"
+requests. These two tests cover the case when the second merge base candidate
+gets selected, new parents get updated accordingly so new p1 matches the merge
+base.
+
+ $ hg init p1-succ-p2-move
+ $ cd p1-succ-p2-move
+ $ hg debugdrawdag <<'EOS'
+ > D Z
+ > /| | # replace: A -> C
+ > A B C # D/D = D
+ > EOS
+ $ hg rebase -r A+B+D -d Z --config experimental.rebaseskipobsolete=0
+ rebasing 0:426bada5c675 "A" (A)
+ rebasing 1:fc2b737bb2e5 "B" (B)
+ rebasing 3:b8ed089c80ad "D" (D)
+
+ $ rm .hg/localtags
+ $ hg log -G
+ o7:7636985f7fde D
+ |\
+ | o 6:76840d832e98 B
+ | |
+ o | 5:a81a74d764a6 A
+ |/
+ o 4:50e41c1f3950 Z
+ |
+ o 2:96cc3511f894 C
+
+ $ hg files -r tip
+ A
+ B
+ C
+ D
+ Z
+
+ $ cd ..
+
+With p1 and p2 swapped from the above case
+
+ $ hg init p1-move-p2-succ
+ $ cd p1-move-p2-succ
+ $ hg debugdrawdag <<'EOS'
+ > D Z
+ > /| | # replace: B -> C
+ > A B C # D/D = D
+ > EOS
+ $ hg rebase -r B+A+D -d Z --config experimental.rebaseskipobsolete=0
+ rebasing 0:426bada5c675 "A" (A)
+ rebasing 1:fc2b737bb2e5 "B" (B)
+ rebasing 3:b8ed089c80ad "D" (D)
+
+ $ rm .hg/localtags
+ $ hg log -G
+ o7:7636985f7fde D
+ |\
+ | o 6:76840d832e98 B
+ | |
+ o | 5:a81a74d764a6 A
+ |/
+ o 4:50e41c1f3950 Z
+ |
+ o 2:96cc3511f894 C
+
+ $ hg files -r tip
+ A
+ B
+ C
+ D
+ Z
+
+ $ cd ..
+
Test that bookmark is moved and working dir is updated when all changesets have
equivalents in destination
$ hg init rbsrepo && cd rbsrepo
diff --git a/hgext/rebase.py b/hgext/rebase.py
--- a/hgext/rebase.py
+++ b/hgext/rebase.py
@@ -1098,6 +1098,30 @@
repo.ui.debug(" future parents are %d and %d\n" % tuple(newps))
+# If there are multiple merge base candidates, try to remove one of them.
+# Prefer keeping obsoleted node with successor in destination. Otherwise we
+# might re-introduce unwanted obsoleted changes. For example,
+#
+# C # rebase -r A+B+C -d D
+#/| # Suppose A has content "+A", B has "+B", D has "+D".
+# A B D # replace: A is replaced by D
+#
+# B gets moved on top of D, A gets skipped, C gets moved on top of B':
+#
+# C' # When choosing merge base for C, A and B are candidates.
+# | # If we choose B, the difference between C and B are "+A",
+# C B' # and C' will have the content "+A", which is suboptimal
+#/| | # because it re-introduces obsoleted content. If we choose
+#
D340: rebase: prefer choosing merge base with successor in destination
quark updated this revision to Diff 868.
REPOSITORY
rHG Mercurial
CHANGES SINCE LAST UPDATE
https://phab.mercurial-scm.org/D340?vs=830=868
REVISION DETAIL
https://phab.mercurial-scm.org/D340
AFFECTED FILES
hgext/rebase.py
tests/test-rebase-obsolete.t
CHANGE DETAILS
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
@@ -1069,10 +1069,8 @@
rebasing 2:b18e25de2cf5 "D" (D)
note: not rebasing 3:7fb047a69f22 "E" (E), already in destination as
1:112478962961 "B"
rebasing 5:66f1a38021c9 "F" (F tip)
- warning: rebasing 5:66f1a38021c9 may inlcude unwanted changes from revision 3
+ note: rebase of 5:66f1a38021c9 created no changes to commit
$ hg log -G
- o 7:9ed45af61fa0 F
- |
o 6:8f47515dda15 D
|
| x5:66f1a38021c9 F
@@ -1106,11 +1104,9 @@
note: not rebasing 2:b18e25de2cf5 "D" (D), already in destination as
1:112478962961 "B"
rebasing 3:7fb047a69f22 "E" (E)
rebasing 5:66f1a38021c9 "F" (F tip)
- warning: rebasing 5:66f1a38021c9 may inlcude unwanted changes from revision 2
+ note: rebase of 5:66f1a38021c9 created no changes to commit
$ hg log -G
- o 7:502540f44880 F
- |
o 6:533690786a86 E
|
| x5:66f1a38021c9 F
@@ -1127,6 +1123,137 @@
$ cd ..
+Rebase merge where both parents have successors in destination
+
+ $ hg init p12-succ-in-dest
+ $ cd p12-succ-in-dest
+ $ hg debugdrawdag <<'EOS'
+ > E F
+ > /| /| # replace: A -> C
+ > A B C D # replace: B -> D
+ > | |
+ > X Y
+ > EOS
+ $ hg rebase -r A+B+E -d F
+ note: not rebasing 4:a3d17304151f "A" (A), already in destination as
0:96cc3511f894 "C"
+ note: not rebasing 5:b23a2cc00842 "B" (B), already in destination as
1:058c1e1fb10a "D"
+ rebasing 7:dac5d11c5a7d "E" (E tip)
+ warning: rebasing 7:dac5d11c5a7d may inlcude unwanted changes from revision
3, 5
+ $ hg manifest -r tip
+ B
+ C
+ D
+ Y
+ $ hg log -G -T '{rev}:{node|short} {desc|firstline} +{file_adds}\n'
+ o 8:3306ff4ff997 E +B Y
+ |
+ | x7:dac5d11c5a7d E +B Y
+ | |\
+ o \ \6:e0c929a964ce F +C
+ |\ \ \
+ | | | x 5:b23a2cc00842 B +B
+ | | | |
+ | | x | 4:a3d17304151f A +A
+ | | | |
+ | | | o 3:59c792af609c Y +Y
+ | | |
+ | | o 2:ba2b7fa7166d X +X
+ | |
+ | o 1:058c1e1fb10a D +D
+ |
+ o 0:96cc3511f894 C +C
+
+ $ cd ..
+
+Rebase one parent with successor in destination, the other parent moves as "-d"
+requests. These two tests are to cover the case when the second merge base
+candidate gets selected, new parents get updated accordingly so new p1 matches
+the merge base.
+
+ $ hg init p1-succ-p2-move
+ $ cd p1-succ-p2-move
+ $ hg debugdrawdag <<'EOS'
+ > D
+ > /|
+ > A B Z # replace: A -> C
+ > | | | # D/D = D
+ > X Y C
+ > EOS
+ $ hg rebase -r A+B+D -d Z --config experimental.rebaseskipobsolete=0
+ rebasing 3:a3d17304151f "A" (A)
+ rebasing 4:b23a2cc00842 "B" (B)
+ rebasing 6:141c08ee43e3 "D" (D tip)
+ warning: rebasing 6:141c08ee43e3 may inlcude unwanted changes from revision 2
+
+ $ rm .hg/localtags
+ $ hg log -G
+ o9:791364acf1f7 D
+ |\
+ | o 8:eb6ceb8cd2c1 B
+ | |
+ o | 7:fe98b87cd6e0 A
+ |/
+ o 5:50e41c1f3950 Z
+ |
+ | o 2:59c792af609c Y
+ |
+ | o 1:ba2b7fa7166d X
+ |
+ o 0:96cc3511f894 C
+
+ $ hg files -r tip
+ A
+ B
+ C
+ D
+ Y
+ Z
+
+ $ cd ..
+
+With p1 and p2 swapped from the above case
+
+ $ hg init p1-move-p2-succ
+ $ cd p1-move-p2-succ
+ $ hg debugdrawdag <<'EOS'
+ > D
+ > /|
+ > A B Z # replace: B -> C
+ > | | | # D/D = D
+ > X Y C
+ > EOS
+ $ hg rebase -r B+A+D -d Z --config experimental.rebaseskipobsolete=0
+ rebasing 3:a3d17304151f "A" (A)
+ rebasing 4:b23a2cc00842 "B" (B)
+ rebasing 6:141c08ee43e3 "D" (D tip)
+ warning: rebasing 6:141c08ee43e3 may inlcude unwanted changes from revision 1
+
+ $ rm .hg/localtags
+ $ hg log -G
+ o9:433cf923621e D
+ |\
+ | o 8:eb6ceb8cd2c1 B
+ | |
+ o | 7:fe98b87cd6e0 A
+ |/
+ o 5:50e41c1f3950 Z
+ |
+ | o 2:59c792af609c Y
+ |
+ | o 1:ba2b7fa7166d X
+ |
+ o 0:96cc3511f894 C
+
+ $ hg files -r tip
+ A
+ B
+ C
+ D
+ X
+ Z
+
+ $ cd ..
+
Test that bookmark is moved and working dir is updated when all changesets have
equivalents in destination
$ hg init rbsrepo && cd rbsrepo
diff --git a/hgext/rebase.py b/hgext/rebase.py
--- a/hgext/rebase.py
+++ b/hgext/rebase.py
@@ -1096,6 +1096,30 @@
repo.ui.debug(" future parents are %d and %d\n" % tuple(newps))
+# If there are multiple merge base candidates, try to remove one of them.
+# Prefer keeping obsoleted node with successor in destination. Otherwise we
+# might re-introduce unwanted obsoleted changes. For example,
+#
+# C # rebase -r A+B+C -d D
+#/| # Suppose A has content "+A", B has "+B", D has "+D".
+# A B D # replace: A is replaced by D
+#
+# B gets moved on top of D, A gets skipped, C gets moved on top of B':
D340: rebase: prefer choosing merge base with successor in destination
quark updated this revision to Diff 770.
REPOSITORY
rHG Mercurial
CHANGES SINCE LAST UPDATE
https://phab.mercurial-scm.org/D340?vs=768=770
REVISION DETAIL
https://phab.mercurial-scm.org/D340
AFFECTED FILES
hgext/rebase.py
tests/test-rebase-obsolete.t
CHANGE DETAILS
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
@@ -1069,9 +1069,8 @@
rebasing 2:b18e25de2cf5 "D" (D)
note: not rebasing 3:7fb047a69f22 "E" (E), already in destination as
1:112478962961 "B"
rebasing 5:66f1a38021c9 "F" (F tip)
+ note: rebase of 5:66f1a38021c9 created no changes to commit
$ hg log -G
- o 7:9ed45af61fa0 F
- |
o 6:8f47515dda15 D
|
| x5:66f1a38021c9 F
@@ -1105,12 +1104,11 @@
note: not rebasing 2:b18e25de2cf5 "D" (D), already in destination as
1:112478962961 "B"
rebasing 3:7fb047a69f22 "E" (E)
rebasing 5:66f1a38021c9 "F" (F tip)
+ note: rebase of 5:66f1a38021c9 created no changes to commit
-Rebased F should have one parent, just like in the test case above
+F should disappear, just like in the test case above
$ hg log -G
- o 7:502540f44880 F
- |
o 6:533690786a86 E
|
| x5:66f1a38021c9 F
@@ -1127,6 +1125,79 @@
$ cd ..
+Rebase merge where both parents have successors in destination
+
+ $ hg init p12-succ-in-dest
+ $ cd p12-succ-in-dest
+ $ hg debugdrawdag <<'EOS'
+ > E F
+ > /| /| # replace: A -> C
+ > A B C D # replace: B -> D
+ > EOS
+ $ hg rebase -r ::E -d F
+ note: not rebasing 0:426bada5c675 "A" (A), already in destination as
2:96cc3511f894 "C"
+ note: not rebasing 1:fc2b737bb2e5 "B" (B), already in destination as
3:058c1e1fb10a "D"
+ rebasing 4:d6e82823588a "E" (E)
+ warning: cannot decide a unique merge base for 4:d6e82823588a, merge result
may be suboptimal
+ $ hg log -Gp -T '{rev}:{node|short} {desc|firstline}\n'
+ o 6:d0827eab33f0 E
+ | diff -r e0c929a964ce -r d0827eab33f0 A
+ | --- /dev/null Thu Jan 01 00:00:00 1970 +
+ | +++ b/A Thu Jan 01 00:00:00 1970 +
+ | @@ -0,0 +1,1 @@
+ | +A
+ | \ No newline at end of file
+ |
+ o5:e0c929a964ce F
+ |\ diff -r 058c1e1fb10a -r e0c929a964ce C
+ | | --- /dev/null Thu Jan 01 00:00:00 1970 +
+ | | +++ b/C Thu Jan 01 00:00:00 1970 +
+ | | @@ -0,0 +1,1 @@
+ | | +C
+ | | \ No newline at end of file
+ | |
+ | | x4:d6e82823588a E
+ | | |\ diff -r 426bada5c675 -r d6e82823588a B
+ | | | | --- /dev/null Thu Jan 01 00:00:00 1970 +
+ | | | | +++ b/B Thu Jan 01 00:00:00 1970 +
+ | | | | @@ -0,0 +1,1 @@
+ | | | | +B
+ | | | | \ No newline at end of file
+ | | | |
+ | o | | 3:058c1e1fb10a D
+ | / / diff -r -r 058c1e1fb10a D
+ | | |--- /dev/null Thu Jan 01 00:00:00 1970 +
+ | | |+++ b/D Thu Jan 01 00:00:00 1970 +
+ | | |@@ -0,0 +1,1 @@
+ | | |+D
+ | | |\ No newline at end of file
+ | | |
+ o | | 2:96cc3511f894 C
+ / / diff -r -r 96cc3511f894 C
+ | |--- /dev/null Thu Jan 01 00:00:00 1970 +
+ | |+++ b/C Thu Jan 01 00:00:00 1970 +
+ | |@@ -0,0 +1,1 @@
+ | |+C
+ | |\ No newline at end of file
+ | |
+ | x 1:fc2b737bb2e5 B
+ |diff -r -r fc2b737bb2e5 B
+ |--- /dev/null Thu Jan 01 00:00:00 1970 +
+ |+++ b/B Thu Jan 01 00:00:00 1970 +
+ |@@ -0,0 +1,1 @@
+ |+B
+ |\ No newline at end of file
+ |
+ x 0:426bada5c675 A
+ diff -r -r 426bada5c675 A
+ --- /dev/null Thu Jan 01 00:00:00 1970 +
+ +++ b/A Thu Jan 01 00:00:00 1970 +
+ @@ -0,0 +1,1 @@
+ +A
+ \ No newline at end of file
+
+ $ cd ..
+
Test that bookmark is moved and working dir is updated when all changesets have
equivalents in destination
$ hg init rbsrepo && cd rbsrepo
diff --git a/hgext/rebase.py b/hgext/rebase.py
--- a/hgext/rebase.py
+++ b/hgext/rebase.py
@@ -1087,9 +1087,35 @@
if len(bases) > 1:
bases.difference_update(ancestor(rev, d) for d in set(dests))
-# Only pick the merge base if we have a unique candidate
-if len(bases) == 1:
-base = next(iter(bases))
+# If there are still multiple candidates, prefer obsoleted ones with
+# successor in destination. Otherwise we might re-introduce unwanted
+# obsoleted changes. For example,
+#
+# C # rebase -r A+B+C -d D
+#/| # Suppose A has content "+A", B has "+B", D has "+D".
+# A B D # replace: A is replaced by D
+#
+# B gets moved on top of D, A gets skipped, C gets moved on top of B':
+#
+# C' # When choosing merge base for C, A and B are candidates.
+# | # If we choose B, the difference between C and B are "+A",
+# C B' # and C' will have the content "+A", which is suboptimal
+#/| | # because it re-introduces obsoleted content. If we
D340: rebase: prefer choosing merge base with successor in destination
quark updated this revision to Diff 768.
REPOSITORY
rHG Mercurial
CHANGES SINCE LAST UPDATE
https://phab.mercurial-scm.org/D340?vs=766=768
REVISION DETAIL
https://phab.mercurial-scm.org/D340
AFFECTED FILES
hgext/rebase.py
tests/test-rebase-obsolete.t
CHANGE DETAILS
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
@@ -1069,9 +1069,8 @@
rebasing 2:b18e25de2cf5 "D" (D)
note: not rebasing 3:7fb047a69f22 "E" (E), already in destination as
1:112478962961 "B"
rebasing 5:66f1a38021c9 "F" (F tip)
+ note: rebase of 5:66f1a38021c9 created no changes to commit
$ hg log -G
- o 7:9ed45af61fa0 F
- |
o 6:8f47515dda15 D
|
| x5:66f1a38021c9 F
@@ -1105,12 +1104,11 @@
note: not rebasing 2:b18e25de2cf5 "D" (D), already in destination as
1:112478962961 "B"
rebasing 3:7fb047a69f22 "E" (E)
rebasing 5:66f1a38021c9 "F" (F tip)
+ note: rebase of 5:66f1a38021c9 created no changes to commit
-Rebased F should have one parent, just like in the test case above
+F should disappear, just like in the test case above
$ hg log -G
- o 7:502540f44880 F
- |
o 6:533690786a86 E
|
| x5:66f1a38021c9 F
@@ -1127,6 +1125,77 @@
$ cd ..
+Rebase merge where both parents have successors in destination
+
+ $ hg init p12-succ-in-dest
+ $ cd p12-succ-in-dest
+ $ hg debugdrawdag <<'EOS'
+ > E F
+ > /| /| # replace: A -> C
+ > A B C D # replace: B -> D
+ > EOS
+ $ hg rebase -r ::E -d F
+ note: not rebasing 0:426bada5c675 "A" (A), already in destination as
2:96cc3511f894 "C"
+ note: not rebasing 1:fc2b737bb2e5 "B" (B), already in destination as
3:058c1e1fb10a "D"
+ rebasing 4:d6e82823588a "E" (E)
+ warning: cannot decide a unique merge base for 4:d6e82823588a, merge result
may be suboptimal
+ $ hg log -Gp -T '{rev}:{node|short} {desc|firstline}\n'
+ o 6:d0827eab33f0 E
+ | diff -r e0c929a964ce -r d0827eab33f0 A
+ | --- /dev/null Thu Jan 01 00:00:00 1970 +
+ | +++ b/A Thu Jan 01 00:00:00 1970 +
+ | @@ -0,0 +1,1 @@
+ | +A
+ | \ No newline at end of file
+ |
+ o5:e0c929a964ce F
+ |\ diff -r 058c1e1fb10a -r e0c929a964ce C
+ | | --- /dev/null Thu Jan 01 00:00:00 1970 +
+ | | +++ b/C Thu Jan 01 00:00:00 1970 +
+ | | @@ -0,0 +1,1 @@
+ | | +C
+ | | \ No newline at end of file
+ | |
+ | | x4:d6e82823588a E
+ | | |\ diff -r 426bada5c675 -r d6e82823588a B
+ | | | | --- /dev/null Thu Jan 01 00:00:00 1970 +
+ | | | | +++ b/B Thu Jan 01 00:00:00 1970 +
+ | | | | @@ -0,0 +1,1 @@
+ | | | | +B
+ | | | | \ No newline at end of file
+ | | | |
+ | o | | 3:058c1e1fb10a D
+ | / / diff -r -r 058c1e1fb10a D
+ | | |--- /dev/null Thu Jan 01 00:00:00 1970 +
+ | | |+++ b/D Thu Jan 01 00:00:00 1970 +
+ | | |@@ -0,0 +1,1 @@
+ | | |+D
+ | | |\ No newline at end of file
+ | | |
+ o | | 2:96cc3511f894 C
+ / / diff -r -r 96cc3511f894 C
+ | |--- /dev/null Thu Jan 01 00:00:00 1970 +
+ | |+++ b/C Thu Jan 01 00:00:00 1970 +
+ | |@@ -0,0 +1,1 @@
+ | |+C
+ | |\ No newline at end of file
+ | |
+ | x 1:fc2b737bb2e5 B
+ |diff -r -r fc2b737bb2e5 B
+ |--- /dev/null Thu Jan 01 00:00:00 1970 +
+ |+++ b/B Thu Jan 01 00:00:00 1970 +
+ |@@ -0,0 +1,1 @@
+ |+B
+ |\ No newline at end of file
+ |
+ x 0:426bada5c675 A
+ diff -r -r 426bada5c675 A
+ --- /dev/null Thu Jan 01 00:00:00 1970 +
+ +++ b/A Thu Jan 01 00:00:00 1970 +
+ @@ -0,0 +1,1 @@
+ +A
+ \ No newline at end of file
+
Test that bookmark is moved and working dir is updated when all changesets have
equivalents in destination
$ hg init rbsrepo && cd rbsrepo
diff --git a/hgext/rebase.py b/hgext/rebase.py
--- a/hgext/rebase.py
+++ b/hgext/rebase.py
@@ -1087,9 +1087,35 @@
if len(bases) > 1:
bases.difference_update(ancestor(rev, d) for d in set(dests))
-# Only pick the merge base if we have a unique candidate
-if len(bases) == 1:
-base = next(iter(bases))
+# If there are still multiple candidates, prefer obsoleted ones with
+# successor in destination. Otherwise we might re-introduce unwanted
+# obsoleted changes. For example,
+#
+# C # rebase -r A+B+C -d D
+#/| # Suppose A has content "+A", B has "+B", D has "+D".
+# A B D # replace: A is replaced by D
+#
+# B gets moved on top of D, A gets skipped, C gets moved on top of B':
+#
+# C' # When choosing merge base for C, A and B are candidates.
+# | # If we choose B, the difference between C and B are "+A",
+# C B' # and C' will have the content "+A", which is suboptimal
+#/| | # because it re-introduces obsoleted content. If we choose
+#
D340: rebase: prefer choosing merge base with successor in destination
quark created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.
REVISION SUMMARY
As demonstrated by the test case change, and the new comment. Choosing a
merge base candidate with a successor in destination would allow us to avoid
re-introducing obsoleted changes and is therefore considered better.
REPOSITORY
rHG Mercurial
REVISION DETAIL
https://phab.mercurial-scm.org/D340
AFFECTED FILES
hgext/rebase.py
tests/test-rebase-obsolete.t
CHANGE DETAILS
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
@@ -1069,9 +1069,8 @@
rebasing 2:b18e25de2cf5 "D" (D)
note: not rebasing 3:7fb047a69f22 "E" (E), already in destination as
1:112478962961 "B"
rebasing 5:66f1a38021c9 "F" (F tip)
+ note: rebase of 5:66f1a38021c9 created no changes to commit
$ hg log -G
- o 7:9ed45af61fa0 F
- |
o 6:8f47515dda15 D
|
| x5:66f1a38021c9 F
@@ -1105,12 +1104,11 @@
note: not rebasing 2:b18e25de2cf5 "D" (D), already in destination as
1:112478962961 "B"
rebasing 3:7fb047a69f22 "E" (E)
rebasing 5:66f1a38021c9 "F" (F tip)
+ note: rebase of 5:66f1a38021c9 created no changes to commit
Rebased F should have one parent, just like in the test case above
$ hg log -G
- o 7:502540f44880 F
- |
o 6:533690786a86 E
|
| x5:66f1a38021c9 F
@@ -1127,6 +1125,77 @@
$ cd ..
+Rebase merge where both parents have successors in destination
+
+ $ hg init p12-succ-in-dest
+ $ cd p12-succ-in-dest
+ $ hg debugdrawdag <<'EOS'
+ > E F
+ > /| /| # replace: A -> C
+ > A B C D # replace: B -> D
+ > EOS
+ $ hg rebase -r ::E -d F
+ note: not rebasing 0:426bada5c675 "A" (A), already in destination as
2:96cc3511f894 "C"
+ note: not rebasing 1:fc2b737bb2e5 "B" (B), already in destination as
3:058c1e1fb10a "D"
+ rebasing 4:d6e82823588a "E" (E)
+ warning: cannot decide a unique merge base for 4:d6e82823588a, merge result
may be suboptimal
+ $ hg log -Gp -T '{rev}:{node|short} {desc|firstline}\n'
+ o 6:d0827eab33f0 E
+ | diff -r e0c929a964ce -r d0827eab33f0 A
+ | --- /dev/null Thu Jan 01 00:00:00 1970 +
+ | +++ b/A Thu Jan 01 00:00:00 1970 +
+ | @@ -0,0 +1,1 @@
+ | +A
+ | \ No newline at end of file
+ |
+ o5:e0c929a964ce F
+ |\ diff -r 058c1e1fb10a -r e0c929a964ce C
+ | | --- /dev/null Thu Jan 01 00:00:00 1970 +
+ | | +++ b/C Thu Jan 01 00:00:00 1970 +
+ | | @@ -0,0 +1,1 @@
+ | | +C
+ | | \ No newline at end of file
+ | |
+ | | x4:d6e82823588a E
+ | | |\ diff -r 426bada5c675 -r d6e82823588a B
+ | | | | --- /dev/null Thu Jan 01 00:00:00 1970 +
+ | | | | +++ b/B Thu Jan 01 00:00:00 1970 +
+ | | | | @@ -0,0 +1,1 @@
+ | | | | +B
+ | | | | \ No newline at end of file
+ | | | |
+ | o | | 3:058c1e1fb10a D
+ | / / diff -r -r 058c1e1fb10a D
+ | | |--- /dev/null Thu Jan 01 00:00:00 1970 +
+ | | |+++ b/D Thu Jan 01 00:00:00 1970 +
+ | | |@@ -0,0 +1,1 @@
+ | | |+D
+ | | |\ No newline at end of file
+ | | |
+ o | | 2:96cc3511f894 C
+ / / diff -r -r 96cc3511f894 C
+ | |--- /dev/null Thu Jan 01 00:00:00 1970 +
+ | |+++ b/C Thu Jan 01 00:00:00 1970 +
+ | |@@ -0,0 +1,1 @@
+ | |+C
+ | |\ No newline at end of file
+ | |
+ | x 1:fc2b737bb2e5 B
+ |diff -r -r fc2b737bb2e5 B
+ |--- /dev/null Thu Jan 01 00:00:00 1970 +
+ |+++ b/B Thu Jan 01 00:00:00 1970 +
+ |@@ -0,0 +1,1 @@
+ |+B
+ |\ No newline at end of file
+ |
+ x 0:426bada5c675 A
+ diff -r -r 426bada5c675 A
+ --- /dev/null Thu Jan 01 00:00:00 1970 +
+ +++ b/A Thu Jan 01 00:00:00 1970 +
+ @@ -0,0 +1,1 @@
+ +A
+ \ No newline at end of file
+
Test that bookmark is moved and working dir is updated when all changesets have
equivalents in destination
$ hg init rbsrepo && cd rbsrepo
diff --git a/hgext/rebase.py b/hgext/rebase.py
--- a/hgext/rebase.py
+++ b/hgext/rebase.py
@@ -1087,9 +1087,35 @@
if len(bases) > 1:
bases.difference_update(ancestor(rev, d) for d in set(dests))
-# Only pick the merge base if we have a unique candidate
-if len(bases) == 1:
-base = next(iter(bases))
+# If there are still multiple candidates, prefer obsoleted ones with
+# successor in destination. Otherwise we might re-introduce unwanted
+# obsoleted changes. For example,
+#
+# C # rebase -r A+B+C -d D
+#/| # Suppose A has content "+A", B has "+B", D has "+D".
+# A B D # replace: A is replaced by D
+#
+# B gets moved on top of D, A gets skipped, C gets moved on top of B':
+#
+# C' # When choosing merge base for C, A and B are candidates.
+# | # If we choose B, the difference
