This is an archive of the discontinued Mercurial Phabricator instance.

Differential D2913

rebase: store rebase state after each commit
ClosedPublic

Authored by martinvonz on Mar 21 2018, 2:10 PM.

Details

Reviewers
phillco
indygreg
Group Reviewers
hg-reviewers
Commits
rHG98663bed146e: rebase: store rebase state after each commit
Summary

Before this patch, we stored the rebase state early in the processing
of a node, before we updated the rebase state to indicate that the
node was processed. This meant that we could redo the working copy
merge and run into conflicts. However, this only happened in the
--collapse case if the rebase was interrupted while editing the final
commit message; in the case earlier interruptions, we would instead
detect the in-process revision by finding two dirstate parents.

This patch moves the writing of the rebase state to after we have
completed the revision completely, and, importantly, after we have
updated the rebase state to mark it done. This means we'll realize
that all nodes have been rebased in the case mentioned above of
editing the final commit message of a --collapse. See change to test
case.

I also moved the writing outside of the large if/elif block in
_rebasenode(). This shouldn't matter much, but seems cleaner. One
observable effect is if rebase was interrupted just after ignoring an
obsolete node ("not rebasing ##, already in destination"), we used to come up with the same decision after --continue too, but after this patch we'll instead say "already rebased #". This seems more
consistent, since that's what we would do with obsolete nodes that had
been marked done earlier in the process (not only just before the
interruption).

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 21 2018, 2:10 PM
indygreg accepted this revision.Mar 21 2018, 7:29 PM
This revision is now accepted and ready to land.Mar 21 2018, 7:29 PM
Closed by commit rHG98663bed146e: rebase: store rebase state after each commit (authored by martinvonz). · Explain WhyMar 21 2018, 7:34 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 7205

hgext/rebase.py

self.skipped)) self.skipped))
self.state[rev] = dest self.state[rev] = dest
elif self.state[rev] == revtodo: elif self.state[rev] == revtodo:
ui.status(_('rebasing %s\n') % desc) ui.status(_('rebasing %s\n') % desc)
progressfn(ctx) progressfn(ctx)
p1, p2, base = defineparents(repo, rev, self.destmap, p1, p2, base = defineparents(repo, rev, self.destmap,
self.state, self.skipped, self.state, self.skipped,
self.obsoletenotrebased) self.obsoletenotrebased)
if not tr:
self.storestatus()
if len(repo[None].parents()) == 2: if len(repo[None].parents()) == 2:
repo.ui.debug('resuming interrupted rebase\n') repo.ui.debug('resuming interrupted rebase\n')
else: else:
overrides = {('ui', 'forcemerge'): opts.get('tool', '')} overrides = {('ui', 'forcemerge'): opts.get('tool', '')}
with ui.configoverride(overrides, 'rebase'): with ui.configoverride(overrides, 'rebase'):
stats = rebasenode(repo, rev, p1, base, self.collapsef, stats = rebasenode(repo, rev, p1, base, self.collapsef,
dest, wctx=self.wctx) dest, wctx=self.wctx)
if stats[3] > 0: if stats[3] > 0:
ui.warn(_('note: rebase of %d:%s created no changes ' ui.warn(_('note: rebase of %d:%s created no changes '
'to commit\n') % (rev, ctx)) 'to commit\n') % (rev, ctx))
self.skipped.add(rev) self.skipped.add(rev)
self.state[rev] = p1 self.state[rev] = p1
ui.debug('next revision set to %d\n' % p1) ui.debug('next revision set to %d\n' % p1)
else: else:
ui.status(_('already rebased %s as %s\n') % ui.status(_('already rebased %s as %s\n') %
(desc, repo[self.state[rev]])) (desc, repo[self.state[rev]]))
if not tr:
# When not using single transaction, store state after each
# commit is completely done. On InterventionRequired, we thus
# won't store the status. Instead, we'll hit the "len(parents) == 2"
# case and realize that the commit was in progress.
self.storestatus()
def _finishrebase(self): def _finishrebase(self):
repo, ui, opts = self.repo, self.ui, self.opts repo, ui, opts = self.repo, self.ui, self.opts
fm = ui.formatter('rebase', opts) fm = ui.formatter('rebase', opts)
fm.startitem() fm.startitem()
if self.collapsef: if self.collapsef:
p1, p2, _base = defineparents(repo, min(self.state), self.destmap, p1, p2, _base = defineparents(repo, min(self.state), self.destmap,
self.state, self.skipped, self.state, self.skipped,

tests/test-rebase-collapse.t

o 3: 63668d570d21 'C' o 3: 63668d570d21 'C'
| |
| @ 2: 82b8abf9c185 'D' | @ 2: 82b8abf9c185 'D'
| | | |
o | 1: f899f3910ce7 'B' o | 1: f899f3910ce7 'B'
|/ |/
o 0: 4a2df7238c3b 'A' o 0: 4a2df7238c3b 'A'
BROKEN: should not result in a conflict
$ hg rebase --continue $ hg rebase --continue
already rebased 1:f899f3910ce7 "B" (B) as 82b8abf9c185 already rebased 1:f899f3910ce7 "B" (B) as 82b8abf9c185
rebasing 3:63668d570d21 "C" (C tip) already rebased 3:63668d570d21 "C" (C tip) as 82b8abf9c185
merging A saved backup bundle to $TESTTMP/aborted-editor/.hg/strip-backup/f899f3910ce7-7cab5e15-rebase.hg
warning: conflicts while merging A! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)
[1]
$ cat A
<<<<<<< dest: 82b8abf9c185 D - test: D
BD
||||||| base
B
=======
C
>>>>>>> source: 63668d570d21 C tip - test: C

tests/test-rebase-conflicts.t

summary: added default.txt summary: added default.txt
$ hg rebase -s9 -d2 --debug # use debug to really check merge base used $ hg rebase -s9 -d2 --debug # use debug to really check merge base used
rebase onto 4bc80088dc6b starting from e31216eec445 rebase onto 4bc80088dc6b starting from e31216eec445
rebasing on disk rebasing on disk
rebase status stored rebase status stored
rebasing 9:e31216eec445 "more changes to f1" rebasing 9:e31216eec445 "more changes to f1"
future parents are 2 and -1 future parents are 2 and -1
rebase status stored
update to 2:4bc80088dc6b update to 2:4bc80088dc6b
resolving manifests resolving manifests
branchmerge: False, force: True, partial: False branchmerge: False, force: True, partial: False
ancestor: d79e2059b5c0+, local: d79e2059b5c0+, remote: 4bc80088dc6b ancestor: d79e2059b5c0+, local: d79e2059b5c0+, remote: 4bc80088dc6b
f2.txt: other deleted -> r f2.txt: other deleted -> r
removing f2.txt removing f2.txt
f1.txt: remote created -> g f1.txt: remote created -> g
getting f1.txt getting f1.txt
merge against 9:e31216eec445 merge against 9:e31216eec445
detach base 8:8e4e2c1a07ae detach base 8:8e4e2c1a07ae
searching for copies back to rev 3 searching for copies back to rev 3
unmatched files in other (from topological common ancestor): unmatched files in other (from topological common ancestor):
f2.txt f2.txt
resolving manifests resolving manifests
branchmerge: True, force: True, partial: False branchmerge: True, force: True, partial: False
ancestor: 8e4e2c1a07ae, local: 4bc80088dc6b+, remote: e31216eec445 ancestor: 8e4e2c1a07ae, local: 4bc80088dc6b+, remote: e31216eec445
f1.txt: remote is newer -> g f1.txt: remote is newer -> g
getting f1.txt getting f1.txt
committing files: committing files:
f1.txt f1.txt
committing manifest committing manifest
committing changelog committing changelog
updating the branch cache updating the branch cache
rebased as 19c888675e13 rebased as 19c888675e13
rebase status stored
rebasing 10:2f2496ddf49d "merge" (tip) rebasing 10:2f2496ddf49d "merge" (tip)
future parents are 11 and 7 future parents are 11 and 7
rebase status stored
already in destination already in destination
merge against 10:2f2496ddf49d merge against 10:2f2496ddf49d
detach base 9:e31216eec445 detach base 9:e31216eec445
searching for copies back to rev 3 searching for copies back to rev 3
unmatched files in other (from topological common ancestor): unmatched files in other (from topological common ancestor):
f2.txt f2.txt
resolving manifests resolving manifests
branchmerge: True, force: True, partial: False branchmerge: True, force: True, partial: False
ancestor: e31216eec445, local: 19c888675e13+, remote: 2f2496ddf49d ancestor: e31216eec445, local: 19c888675e13+, remote: 2f2496ddf49d
f1.txt: remote is newer -> g f1.txt: remote is newer -> g
getting f1.txt getting f1.txt
committing files: committing files:
f1.txt f1.txt
committing manifest committing manifest
committing changelog committing changelog
updating the branch cache updating the branch cache
rebased as 2a7f09cac94c rebased as 2a7f09cac94c
rebase status stored
rebase merging completed rebase merging completed
update back to initial working directory parent update back to initial working directory parent
resolving manifests resolving manifests
branchmerge: False, force: False, partial: False branchmerge: False, force: False, partial: False
ancestor: 2a7f09cac94c, local: 2a7f09cac94c+, remote: d79e2059b5c0 ancestor: 2a7f09cac94c, local: 2a7f09cac94c+, remote: d79e2059b5c0
f1.txt: other deleted -> r f1.txt: other deleted -> r
removing f1.txt removing f1.txt
f2.txt: remote created -> g f2.txt: remote created -> g