This is an archive of the discontinued Mercurial Phabricator instance.

Differential D10249

rebase: filter out descendants of divergence-causing commits earlier
ClosedPublic

Authored by martinvonz on Mar 22 2021, 1:36 PM.

Details

Reviewers
None
Group Reviewers
hg-reviewers
Commits
rHG535de0e34a79: rebase: filter out descendants of divergence-causing commits earlier
Summary

hg rebase treats obsolete commits differently depending what has
happened to the commit:

  1. Obsolete commit without non-obsolete successors: Skipped, and a note is printed ("it has no successor").
  2. Obsolete commit with a successor in the destination (ancestor of it): Skipped, and a note is printed ("already in destination").
  3. Obsolete commit with a successor in the rebase set: The commit and its descendants are skipped, and a note is printed ("not rebasing <commit> and its descendants as this would cause divergence"), unless allowdivergence config set.
  4. Obsolete commit with a successor elsewhere: Error ("this rebase will cause divergences"), unless allowdivergence config set.

Before this patch, we did all those checks up front, except for (3),
which was checked later. The later check consisted of two parts: 1)
filtering out of descendants, and 2) conditionally printing message if
the allowdivergence config was not set. This patch makes it so we do
the filtering early.

A consequence of filtering out divergence-causing commits earlier is
that we rebase commits in slightly different order, which has some
impact on tests.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

martinvonz created this revision.Mar 22 2021, 1:36 PM
martinvonz edited the summary of this revision. (Show Details)Mar 23 2021, 2:06 AM
martinvonz planned changes to this revision.Mar 24 2021, 12:37 AM

I may make some changes to this patch. I keep getting confused about the two different divergence cases. Despite have stared at this code for days, I still don't know which case is which (i.e. I'm not sure my description of cases (3) and (4) is correct).

martinvonz added a comment.Mar 24 2021, 2:26 AM
In D10249#156144, @martinvonz wrote:

I may make some changes to this patch. I keep getting confused about the two different divergence cases. Despite have stared at this code for days, I still don't know which case is which (i.e. I'm not sure my description of cases (3) and (4) is correct).

I think I actually got the description right. I've updated the series a bit and it's ready for review again.

martinvonz added a commit: rHG535de0e34a79: rebase: filter out descendants of divergence-causing commits earlier.Mar 25 2021, 5:30 PM
This revision was not accepted when it landed; it landed in state Needs Review.
Closed by commit rHG535de0e34a79: rebase: filter out descendants of divergence-causing commits earlier (authored by martinvonz). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mhgext/rebase.py (31 lines)
Mtests/test-rebase-obsolete3.t (20 lines)

Diff 26591

hgext/rebase.py

obsoleteset = {r for r in self.state if self.repo[r].obsolete()} obsoleteset = {r for r in self.state if self.repo[r].obsolete()}
( (
self.obsolete_with_successor_in_destination, self.obsolete_with_successor_in_destination,
self.obsolete_with_successor_in_rebase_set, self.obsolete_with_successor_in_rebase_set,
) = _compute_obsolete_sets(self.repo, obsoleteset, self.destmap) ) = _compute_obsolete_sets(self.repo, obsoleteset, self.destmap)
skippedset = set(self.obsolete_with_successor_in_destination) skippedset = set(self.obsolete_with_successor_in_destination)
skippedset.update(self.obsolete_with_successor_in_rebase_set) skippedset.update(self.obsolete_with_successor_in_rebase_set)
_checkobsrebase(self.repo, self.ui, obsoleteset, skippedset) _checkobsrebase(self.repo, self.ui, obsoleteset, skippedset)
allowdivergence = self.ui.configbool(
b'experimental', b'evolution.allowdivergence'
)
if allowdivergence:
self.obsolete_with_successor_in_rebase_set = set()
else:
for rev in self.repo.revs(
b'descendants(%ld) and not %ld',
self.obsolete_with_successor_in_rebase_set,
self.obsolete_with_successor_in_rebase_set,
):
self.state.pop(rev, None)
self.destmap.pop(rev, None)
def _prepareabortorcontinue( def _prepareabortorcontinue(
self, isabort, backup=True, suppwarns=False, dryrun=False, confirm=False self, isabort, backup=True, suppwarns=False, dryrun=False, confirm=False
): ):
self.resume = True self.resume = True
try: try:
self.restorestatus() self.restorestatus()
# Calculate self.obsolete_* sets # Calculate self.obsolete_* sets
cands = [k for k, v in pycompat.iteritems(self.state) if v == revtodo] cands = [k for k, v in pycompat.iteritems(self.state) if v == revtodo]
p = repo.ui.makeprogress( p = repo.ui.makeprogress(
_(b"rebasing"), unit=_(b'changesets'), total=len(cands) _(b"rebasing"), unit=_(b'changesets'), total=len(cands)
) )
def progress(ctx): def progress(ctx):
p.increment(item=(b"%d:%s" % (ctx.rev(), ctx))) p.increment(item=(b"%d:%s" % (ctx.rev(), ctx)))
allowdivergence = self.ui.configbool(
b'experimental', b'evolution.allowdivergence'
)
for subset in sortsource(self.destmap): for subset in sortsource(self.destmap):
sortedrevs = self.repo.revs(b'sort(%ld, -topo)', subset) sortedrevs = self.repo.revs(b'sort(%ld, -topo)', subset)
if not allowdivergence:
sortedrevs -= self.repo.revs(
b'descendants(%ld) and not %ld',
self.obsolete_with_successor_in_rebase_set,
self.obsolete_with_successor_in_rebase_set,
)
for rev in sortedrevs: for rev in sortedrevs:
self._rebasenode(tr, rev, allowdivergence, progress) self._rebasenode(tr, rev, progress)
p.complete() p.complete()
ui.note(_(b'rebase merging completed\n')) ui.note(_(b'rebase merging completed\n'))
def _concludenode(self, rev, editor, commitmsg=None): def _concludenode(self, rev, editor, commitmsg=None):
"""Commit the wd changes with parents p1 and p2. """Commit the wd changes with parents p1 and p2.
Reuse commit info from rev but also store useful information in extra. Reuse commit info from rev but also store useful information in extra.
Return node of committed revision.""" Return node of committed revision."""
commitmsg=commitmsg, commitmsg=commitmsg,
editor=editor, editor=editor,
user=ctx.user(), user=ctx.user(),
date=date, date=date,
) )
return newnode return newnode
def _rebasenode(self, tr, rev, allowdivergence, progressfn): def _rebasenode(self, tr, rev, progressfn):
repo, ui, opts = self.repo, self.ui, self.opts repo, ui, opts = self.repo, self.ui, self.opts
ctx = repo[rev] ctx = repo[rev]
desc = _ctxdesc(ctx) desc = _ctxdesc(ctx)
if self.state[rev] == rev: if self.state[rev] == rev:
ui.status(_(b'already rebased %s\n') % desc) ui.status(_(b'already rebased %s\n') % desc)
elif ( elif rev in self.obsolete_with_successor_in_rebase_set:
not allowdivergence
and rev in self.obsolete_with_successor_in_rebase_set
):
msg = ( msg = (
_( _(
b'note: not rebasing %s and its descendants as ' b'note: not rebasing %s and its descendants as '
b'this would cause divergence\n' b'this would cause divergence\n'
) )
% desc % desc
) )
repo.ui.status(msg) repo.ui.status(msg)

tests/test-rebase-obsolete3.t

Changeset d and its descendants are excluded to avoid divergence of d, which Changeset d and its descendants are excluded to avoid divergence of d, which
would occur because the successor of d (d') is also in rebaseset. As a would occur because the successor of d (d') is also in rebaseset. As a
consequence f (descendant of d) is left behind. consequence f (descendant of d) is left behind.
$ hg rebase -b 'e' -d 'x' $ hg rebase -b 'e' -d 'x'
rebasing 1:488e1b7e7341 b "b" rebasing 1:488e1b7e7341 b "b"
rebasing 3:a82ac2b38757 c "c" rebasing 3:a82ac2b38757 c "c"
note: not rebasing 4:76be324c128b d "d" and its descendants as this would cause divergence
rebasing 5:027ad6c5830d d' "d'" rebasing 5:027ad6c5830d d' "d'"
rebasing 6:d60ebfa0f1cb e "e" rebasing 6:d60ebfa0f1cb e "e"
note: not rebasing 4:76be324c128b d "d" and its descendants as this would cause divergence
$ hg log -G -r 'a':: $ hg log -G -r 'a'::
o 11:eb6d63fc4ed5 e o 11:eb6d63fc4ed5 e
| |
o 10:44d8c724a70c d' o 10:44d8c724a70c d'
| |
o 9:d008e6b4d3fd c o 9:d008e6b4d3fd c
| |
o 8:67e8f4a16c49 b o 8:67e8f4a16c49 b
| | | |
o | 1:488e1b7e7341 b o | 1:488e1b7e7341 b
|/ |/
o 0:b173517d0057 a o 0:b173517d0057 a
$ hg rebase -b 'f' -d 'x' $ hg rebase -b 'f' -d 'x'
rebasing 1:488e1b7e7341 b "b" rebasing 1:488e1b7e7341 b "b"
rebasing 3:a82ac2b38757 c "c" rebasing 3:a82ac2b38757 c "c"
rebasing 5:63324dc512ea e' "e'"
rebasing 7:3ffec603ab53 f "f"
rebasing 4:76be324c128b d "d" rebasing 4:76be324c128b d "d"
note: not rebasing 6:e36fae928aec e "e" and its descendants as this would cause divergence note: not rebasing 6:e36fae928aec e "e" and its descendants as this would cause divergence
rebasing 5:63324dc512ea e' "e'"
rebasing 7:3ffec603ab53 f "f"
$ hg log -G -r 'a': $ hg log -G -r 'a':
o 13:a1707a5b7c2c d o 13:ef6251596616 f
| |
| o 12:ef6251596616 f o 12:b6f172e64af9 e'
| | |
| o 11:b6f172e64af9 e' | o 11:a1707a5b7c2c d
|/ |/
o 10:d008e6b4d3fd c o 10:d008e6b4d3fd c
| |
o 9:67e8f4a16c49 b o 9:67e8f4a16c49 b
| |
| * 8:2876ce66c6eb g | * 8:2876ce66c6eb g
| | | |
| | x 7:3ffec603ab53 f (rewritten using rebase as 12:ef6251596616) | | x 7:3ffec603ab53 f (rewritten using rebase as 13:ef6251596616)
| | | | | |
| x | 6:e36fae928aec e (rewritten using replace as 5:63324dc512ea) | x | 6:e36fae928aec e (rewritten using replace as 5:63324dc512ea)
| | | | | |
| | x 5:63324dc512ea e' (rewritten using rebase as 11:b6f172e64af9) | | x 5:63324dc512ea e' (rewritten using rebase as 12:b6f172e64af9)
| | | | | |
| x | 4:76be324c128b d (rewritten using rebase as 13:a1707a5b7c2c) | x | 4:76be324c128b d (rewritten using rebase as 11:a1707a5b7c2c)
| |/ | |/
| x 3:a82ac2b38757 c (rewritten using rebase as 10:d008e6b4d3fd) | x 3:a82ac2b38757 c (rewritten using rebase as 10:d008e6b4d3fd)
| | | |
o | 2:630d7c95eff7 x o | 2:630d7c95eff7 x
| | | |
| x 1:488e1b7e7341 b (rewritten using rebase as 9:67e8f4a16c49) | x 1:488e1b7e7341 b (rewritten using rebase as 9:67e8f4a16c49)
|/ |/
o 0:b173517d0057 a o 0:b173517d0057 a