Mercurial > hg
changeset 33863:3160876c6e4e
rebase: choose merge base without unwanted revisions
Previously, when there are 2 merge base candidates, we choose p1 blindly,
which may make the merge result to have "unwanted content". This patch makes
rebase smarter - choose a merge base that does not have "unwanted revs" if
possible. Since we don't really have a good solution when there are
"unwanted revs", abort in that case.
Differential Revision: https://phab.mercurial-scm.org/D340
| author | Jun Wu <quark@fb.com> |
|---|---|
| date | Thu, 10 Aug 2017 22:17:15 -0700 |
| parents | 3cfc9070245f |
| children | 70354bd4f19b |
| files | hgext/rebase.py tests/test-rebase-newancestor.t tests/test-rebase-obsolete.t |
| diffstat | 3 files changed, 129 insertions(+), 39 deletions(-) [+] |
line wrap: on
line diff
--- a/hgext/rebase.py Wed Aug 16 10:44:06 2017 -0700 +++ b/hgext/rebase.py Thu Aug 10 22:17:15 2017 -0700 @@ -1041,11 +1041,14 @@ for j, x in enumerate(newps[:i]): if x == nullrev: continue - if isancestor(np, x): + if isancestor(np, x): # CASE-1 np = nullrev - elif isancestor(x, np): + elif isancestor(x, np): # CASE-2 newps[j] = np np = nullrev + # New parents forming an ancestor relationship does not + # mean the old parents have a similar relationship. Do not + # set bases[x] to nullrev. bases[j], bases[i] = bases[i], bases[j] newps[i] = np @@ -1069,8 +1072,6 @@ 'moving at least one of its parents') % (rev, repo[rev])) - repo.ui.debug(" future parents are %d and %d\n" % tuple(newps)) - # "rebasenode" updates to new p1, use the corresponding merge base. if bases[0] != nullrev: base = bases[0] @@ -1093,28 +1094,47 @@ # better than the default (ancestor(F, Z) == null). Therefore still # pick one (so choose p1 above). if sum(1 for b in bases if b != nullrev) > 1: - assert base is not None + unwanted = [None, None] # unwanted[i]: unwanted revs if choose bases[i] + for i, base in enumerate(bases): + if base == nullrev: + continue + # Revisions in the side (not chosen as merge base) branch that + # might contain "surprising" contents + siderevs = list(repo.revs('((%ld-%d) %% (%d+%d))', + bases, base, base, dest)) - # Revisions in the side (not chosen as merge base) branch that might - # contain "surprising" contents - siderevs = list(repo.revs('((%ld-%d) %% (%d+%d))', - bases, base, base, dest)) + # If those revisions are covered by rebaseset, the result is good. + # A merge in rebaseset would be considered to cover its ancestors. + if siderevs: + rebaseset = [r for r, d in state.items() if d > 0] + merges = [r for r in rebaseset + if cl.parentrevs(r)[1] != nullrev] + unwanted[i] = list(repo.revs('%ld - (::%ld) - %ld', + siderevs, merges, rebaseset)) - # If those revisions are covered by rebaseset, the result is good. - # A merge in rebaseset would be considered to cover its ancestors. - if siderevs: - rebaseset = [r for r, d in state.items() if d > 0] - merges = [r for r in rebaseset if cl.parentrevs(r)[1] != nullrev] - unwantedrevs = list(repo.revs('%ld - (::%ld) - %ld', - siderevs, merges, rebaseset)) + # Choose a merge base that has a minimal number of unwanted revs. + l, i = min((len(revs), i) + for i, revs in enumerate(unwanted) if revs is not None) + base = bases[i] + + # newps[0] should match merge base if possible. Currently, if newps[i] + # is nullrev, the only case is newps[i] and newps[j] (j < i), one is + # the other's ancestor. In that case, it's fine to not swap newps here. + # (see CASE-1 and CASE-2 above) + if i != 0 and newps[i] != nullrev: + newps[0], newps[i] = newps[i], newps[0] - # For revs not covered, it is worth a warning. - if unwantedrevs: - repo.ui.warn( - _('warning: rebasing %d:%s may include unwanted changes ' - 'from %s\n') - % (rev, repo[rev], ', '.join('%d:%s' % (r, repo[r]) - for r in unwantedrevs))) + # The merge will include unwanted revisions. Abort now. Revisit this if + # we have a more advanced merge algorithm that handles multiple bases. + if l > 0: + unwanteddesc = _(' or ').join( + (', '.join('%d:%s' % (r, repo[r]) for r in revs) + for revs in unwanted if revs is not None)) + raise error.Abort( + _('rebasing %d:%s will include unwanted changes from %s') + % (rev, repo[rev], unwanteddesc)) + + repo.ui.debug(" future parents are %d and %d\n" % tuple(newps)) return newps[0], newps[1], base
--- a/tests/test-rebase-newancestor.t Wed Aug 16 10:44:06 2017 -0700 +++ b/tests/test-rebase-newancestor.t Thu Aug 10 22:17:15 2017 -0700 @@ -354,16 +354,8 @@ rebasing 5:5f2c926dfecf "D" (D) rebasing 6:b296604d9846 "E" (E) rebasing 7:caa9781e507d "F" (F tip) - warning: rebasing 7:caa9781e507d may include unwanted changes from 4:d6003a550c2c - saved backup bundle to $TESTTMP/dual-merge-base1/.hg/strip-backup/b296604d9846-0516f6d2-rebase.hg (glob) - $ hg log -r 4 -T '{files}\n' - C - $ hg manifest -r 'desc(F)' - C - D - E - R - Z + abort: rebasing 7:caa9781e507d will include unwanted changes from 4:d6003a550c2c or 3:c1e6b162678d + [255] The warning does not get printed if there is no unwanted change detected:
--- a/tests/test-rebase-obsolete.t Wed Aug 16 10:44:06 2017 -0700 +++ b/tests/test-rebase-obsolete.t Thu Aug 10 22:17:15 2017 -0700 @@ -1065,10 +1065,8 @@ note: not rebasing 3:7fb047a69f22 "E" (E), already in destination as 1:112478962961 "B" rebasing 2:b18e25de2cf5 "D" (D) 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 | | x 5:66f1a38021c9 F @@ -1102,11 +1100,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 | | x 5:66f1a38021c9 F @@ -1123,6 +1119,88 @@ $ 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) + abort: rebasing 7:dac5d11c5a7d will include unwanted changes from 3:59c792af609c, 5:b23a2cc00842 or 2:ba2b7fa7166d, 4:a3d17304151f + [255] + $ cd .. + +Rebase a non-clean merge. One parent has successor in destination, the other +parent moves as requested. + + $ 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 + note: not rebasing 0:426bada5c675 "A" (A), already in destination as 2:96cc3511f894 "C" + rebasing 1:fc2b737bb2e5 "B" (B) + rebasing 3:b8ed089c80ad "D" (D) + + $ rm .hg/localtags + $ hg log -G + o 6:e4f78693cc88 D + | + o 5:76840d832e98 B + | + o 4:50e41c1f3950 Z + | + o 2:96cc3511f894 C + + $ hg files -r tip + B + C + D + Z + + $ cd .. + + $ 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 + note: not rebasing 1:fc2b737bb2e5 "B" (B), already in destination as 2:96cc3511f894 "C" + rebasing 0:426bada5c675 "A" (A) + rebasing 3:b8ed089c80ad "D" (D) + + $ rm .hg/localtags + $ hg log -G + o 6:1b355ed94d82 D + | + o 5:a81a74d764a6 A + | + o 4:50e41c1f3950 Z + | + o 2:96cc3511f894 C + + $ hg files -r tip + A + 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