( )⚙ D11039 amend: make `hg amend -r` rebase descendants of target onto amended target

This is an archive of the discontinued Mercurial Phabricator instance.

Differential D11039

amend: make `hg amend -r` rebase descendants of target onto amended target
AbandonedPublic

Authored by martinvonz on Jul 9 2021, 4:28 PM.

Details

Reviewers
Alphare
marmoute
Group Reviewers
hg-reviewers

Diff Detail

Repository
rHG Mercurial
Branch
default
Lint
No Linters Available
Unit
No Unit Test Coverage

Event Timeline

martinvonz created this revision.Jul 9 2021, 4:28 PM
Alphare added a subscriber: Alphare.Jul 12 2021, 5:28 AM

Can we test that further descendants (potentially not linear) are not rebased automatically? This can be done in a separate, simpler test case as an additional changeset.

Alphare requested changes to this revision.Jul 12 2021, 5:30 AM
This revision now requires changes to proceed.Jul 12 2021, 5:30 AM
martinvonz updated this revision to Diff 29213.Jul 12 2021, 2:43 PM
martinvonz added a comment.Jul 12 2021, 2:43 PM
In D11039#169655, @Alphare wrote:

Can we test that further descendants (potentially not linear) are not rebased automatically? This can be done in a separate, simpler test case as an additional changeset.

I think that's now done. I also added tests for amending into non-ancestors (in the first patch in this series).

martinvonz updated this revision to Diff 29296.Jul 15 2021, 8:35 PM
marmoute requested changes to this revision.Jul 15 2021, 8:38 PM
marmoute added a subscriber: marmoute.
marmoute added inline comments.
hgext/amend.py
182–184

It would be nice to have some documentation about what each of these value mean, there start to be many of them and I am not sure what each of them is about.

301

What happens (and what's is the intend) if any of the nodes here are obsolete ?

310

Using tip here seem a bit fragile.

The only(tip, <amended>) range seems strange since this will include changeset that are not descendant of <target>, so over reporting. (we need a test for that).

In addition if some node got skipped or "disappeared for being empty post rebase". It feel like we have a good odd of bad/confusing report.

This revision now requires changes to proceed.Jul 15 2021, 8:38 PM
martinvonz marked an inline comment as done.Feb 11 2022, 6:37 PM
martinvonz abandoned this revision.

It looks like this is being worked on in the evolve extension, so I'll drop this patch.

Revision Contents
Changeset List

PathPackages
Mhgext/amend.py (38 lines)
Mtests/test-amend-rev.t (240 lines)

Diff 29213

hgext/amend.py

with repo.wlock(), repo.lock(), util.acceptintervention( with repo.wlock(), repo.lock(), util.acceptintervention(
repo.transaction(b'amend') repo.transaction(b'amend')
), state_store.save_on_conflicts(1, state): ), state_store.save_on_conflicts(1, state):
cmdutil.checkunfinished(repo) cmdutil.checkunfinished(repo)
orig_pctx = repo[b'.'] orig_pctx = repo[b'.']
target_ctx = scmutil.revsingle(repo, opts['rev']) target_ctx = scmutil.revsingle(repo, opts['rev'])
to_rewrite = repo.revs(b'%d::.', target_ctx.rev()) to_rewrite = repo.revs(b'%d::.', target_ctx.rev())
rewriteutil.precheck(repo, to_rewrite, b'amend') rewriteutil.precheck(repo, to_rewrite, b'amend')
descendants = repo.set(b'%ld - %d', to_rewrite, target_ctx.rev())
try: try:
# Create a temporary commit of the working copy. # Create a temporary commit of the working copy.
ret = commands._docommit( ret = commands._docommit(
ui, repo, message=b'temporary commit for "amend --rev"' ui, repo, message=b'temporary commit for "amend --rev"'
) )
if ret: if ret:
return ret return ret
# Update to the previous working copy (without the changes in the # Update to the previous working copy (without the changes in the
# temporary commit). We'll let rebase figure out how to update # temporary commit). We'll let rebase figure out how to update
# the working copy parent as commits are rebased around. # the working copy parent as commits are rebased around.
merge.clean_update(orig_pctx) merge.clean_update(orig_pctx)
temp_ctx = repo[b'tip'] temp_ctx = repo[b'tip']
state[b'target_node'] = target_ctx.node() state[b'target_node'] = target_ctx.node()
state[b'temp_node'] = temp_ctx.node() state[b'temp_node'] = temp_ctx.node()
state[b'descendant_nodes'] = [ctx.node() for ctx in descendants]
marmouteUnsubmitted
Done

It would be nice to have some documentation about what each of these value mean, there start to be many of them and I am not sure what each of them is about.

marmoute: It would be nice to have some documentation about what each of these value mean, there start to…
_do_continue_amend_rev(ui, repo, state, rebase) _do_continue_amend_rev(ui, repo, state, rebase)
except error.InterventionRequired: except error.InterventionRequired:
raise raise
except Exception: except Exception:
_do_abort_amend_rev(ui, repo, state) _do_abort_amend_rev(ui, repo, state)
raise raise
def _do_continue_amend_rev(ui, repo, state, rebase): def _do_continue_amend_rev(ui, repo, state, rebase):
unfi = repo.unfiltered() unfi = repo.unfiltered()
target_ctx = unfi[state[b'target_node']] target_ctx = unfi[state[b'target_node']]
temp_ctx = unfi[state[b'temp_node']] temp_ctx = unfi[state[b'temp_node']]
rebased_temp_ctx = _rebase_temp_node( rebased_temp_ctx = _rebase_temp_node(
ui, repo, state, rebase, temp_ctx, target_ctx ui, repo, state, rebase, temp_ctx, target_ctx
) )
_fold_temp_node(ui, repo, state, rebase, rebased_temp_ctx, target_ctx) amended_ctx = _fold_temp_node(
ui, repo, state, rebase, rebased_temp_ctx, target_ctx
)
_rebase_descendants(ui, repo, state, rebase, amended_ctx)
def _rebase_temp_node(ui, repo, state, rebase, temp_ctx, target_ctx): def _rebase_temp_node(ui, repo, state, rebase, temp_ctx, target_ctx):
if b'rebased_temp_node' in state: if b'rebased_temp_node' in state:
return repo.unfiltered()[state[b'rebased_temp_node']] return repo.unfiltered()[state[b'rebased_temp_node']]
elif statemod.ischildunfinished(repo, b'amend', b'rebase'): elif statemod.ischildunfinished(repo, b'amend', b'rebase'):
with ui.silent(), statemod.delegating(repo, b'amend', b'rebase'): with ui.silent(), statemod.delegating(repo, b'amend', b'rebase'):
ret = statemod.continuechild(ui, repo, b'amend', b'rebase') ret = statemod.continuechild(ui, repo, b'amend', b'rebase')
if ret: if ret:
raise error.Abort(_(b'failed to fold temporary commit')) raise error.Abort(_(b'failed to fold temporary commit'))
amended_ctx = repo[b'tip'] amended_ctx = repo[b'tip']
state[b'amended_node'] = amended_ctx.node() state[b'amended_node'] = amended_ctx.node()
return amended_ctx return amended_ctx
def _rebase_descendants(ui, repo, state, rebase, amended_ctx):
if b'rebased_descendant_nodes' in state:
return
elif statemod.ischildunfinished(repo, b'amend', b'rebase'):
with statemod.delegating(repo, b'amend', b'rebase'):
ret = statemod.continuechild(ui, repo, b'amend', b'rebase')
else:
descendant_nodes = state[b'descendant_nodes']
if descendant_nodes:
with statemod.delegating(repo, b'amend', b'rebase'):
ret = rebase.rebase(
ui,
repo,
rev=[revsetlang.formatspec(b'%ln', descendant_nodes)],
marmouteUnsubmitted
Not Done

What happens (and what's is the intend) if any of the nodes here are obsolete ?

marmoute: What happens (and what's is the intend) if any of the nodes here are obsolete ?
dest=revsetlang.formatspec(b'%d', amended_ctx.rev()),
)
else:
ret = 0
if ret:
raise error.Abort(_(b'failed to rebase descendants of target'))
rebased_descendants = repo.set(b'only(tip, %d)', amended_ctx.rev())
marmouteUnsubmitted
Not Done

Using tip here seem a bit fragile.

The only(tip, <amended>) range seems strange since this will include changeset that are not descendant of <target>, so over reporting. (we need a test for that).

In addition if some node got skipped or "disappeared for being empty post rebase". It feel like we have a good odd of bad/confusing report.

marmoute: Using `tip` here seem a bit fragile. The `only(tip, <amended>)` range seems strange since this…
state[b'rebased_descendant_nodes'] = [
ctx.node() for ctx in rebased_descendants
]
def _abort_amend_rev(ui, repo): def _abort_amend_rev(ui, repo):
with repo.wlock(), repo.lock(), repo.transaction(b'amend'): with repo.wlock(), repo.lock(), repo.transaction(b'amend'):
state_store = statemod.cmdstate(repo, b'amend-state') state_store = statemod.cmdstate(repo, b'amend-state')
state = state_store.read() state = state_store.read()
_do_abort_amend_rev(ui, repo, state) _do_abort_amend_rev(ui, repo, state)
state_store.delete() state_store.delete()
def _do_abort_amend_rev(ui, repo, state): def _do_abort_amend_rev(ui, repo, state):
unfi = repo.unfiltered() unfi = repo.unfiltered()
if statemod.ischildunfinished(repo, b'amend', b'rebase'): if statemod.ischildunfinished(repo, b'amend', b'rebase'):
with ui.silent(): with ui.silent():
statemod.abortchild(ui, repo, b'amend', b'rebase') statemod.abortchild(ui, repo, b'amend', b'rebase')
to_strip = [] to_strip = []
temp_node = state.get(b'temp_node') temp_node = state.get(b'temp_node')
if temp_node and temp_node in unfi: if temp_node and temp_node in unfi:
to_strip.append(temp_node) to_strip.append(temp_node)
temp_ctx = unfi[temp_node] temp_ctx = unfi[temp_node]
merge.clean_update(temp_ctx) merge.clean_update(temp_ctx)
with repo.dirstate.parentchange(): with repo.dirstate.parentchange():
scmutil.movedirstate(unfi, temp_ctx.p1()) scmutil.movedirstate(unfi, temp_ctx.p1())
rebased_temp_node = state.get(b'rebased_temp_node') rebased_temp_node = state.get(b'rebased_temp_node')
to_strip.append(rebased_temp_node) to_strip.append(rebased_temp_node)
to_strip.append(state.get(b'amended_node')) to_strip.append(state.get(b'amended_node'))
rebased_descendant_nodes = state.get(b'rebased_descendant_nodes')
if rebased_descendant_nodes:
to_strip.extend(rebased_descendant_nodes)
to_strip = [node for node in to_strip if node and node in unfi] to_strip = [node for node in to_strip if node and node in unfi]
if to_strip: if to_strip:
repair.delayedstrip(ui, unfi, to_strip) repair.delayedstrip(ui, unfi, to_strip)
def extsetup(ui): def extsetup(ui):
def _after_rebase_loaded(loaded): def _after_rebase_loaded(loaded):
if loaded: if loaded:

tests/test-amend-rev.t

$ hg amend -r 'desc("modify a")' --config experimental.evolution.createmarkers=False $ hg amend -r 'desc("modify a")' --config experimental.evolution.createmarkers=False
abort: --rev requires evolution.createmarkers to be enabled abort: --rev requires evolution.createmarkers to be enabled
[20] [20]
Can amend into grandparent Can amend into grandparent
$ hg amend -r 'desc("modify a")' $ hg amend -r 'desc("modify a")'
1 new orphan changesets rebasing 2:42e29cb5ca48 "add b"
$ hg log -G -T '{rev} {desc}' $ hg log -G -T '{rev} {desc}'
@ 6 add b
|
o 5 modify a o 5 modify a
| |
| @ 2 add b
| | (known-bad-output !)
| x 1 modify a (known-bad-output !)
|/ (known-bad-output !)
o 0 add a o 0 add a
Target commit has new content Target commit has new content
$ hg cat -r 'desc("modify a")' a $ hg cat -r 'desc("modify a")' a
a3 a3
The working copy is clean and there is no unfinished operation The working copy is clean and there is no unfinished operation
$ hg st -v $ hg st -v
Target commit has new content Target commit has new content
$ hg cat -r 'desc("add b")' a $ hg cat -r 'desc("add b")' a
a2 a2
The working copy is clean and there is no unfinished operation The working copy is clean and there is no unfinished operation
$ hg st -v $ hg st -v
Descendants of the working copy parent don't get rebased
--------------------------------------------------------------------------------
Set up a repo with a few commits
$ cd $TESTTMP
$ hg init side-branches
$ cd side-branches
$ echo a > a
$ hg ci -Aqm 'add a'
$ echo a2 > a
$ hg ci -Aqm 'modify a'
$ echo a3 > a
$ hg ci -Aqm 'change a'
$ hg co '.^^'
1 files updated, 0 files merged, 0 files removed, 0 files unresolved
$ echo b > b
$ hg ci -Aqm 'add b'
$ hg merge 'desc("modify a")'
1 files updated, 0 files merged, 0 files removed, 0 files unresolved
(branch merge, don't forget to commit)
$ hg ci -m merge
$ echo b2 > b
$ hg ci -Aqm 'modify b'
$ hg co '.^'
1 files updated, 0 files merged, 0 files removed, 0 files unresolved
$ hg log -G -T '{rev} {desc}'
o 5 modify b
|
@ 4 merge
|\
| o 3 add b
| |
+---o 2 change a
| |
o | 1 modify a
|/
o 0 add a
$ echo a4 > a
$ hg amend -r 'desc("modify a")' --config experimental.evolution.allowunstable=yes
created new head
rebasing 4:c850352cb575 "merge"
2 new orphan changesets
$ hg log -G -T '{rev} {desc}'
@ 9 merge
|\
| o 8 modify a
| |
| | * 5 modify b
| | |
+---x 4 merge
| | |
o | | 3 add b
|/ /
| | * 2 change a
| |/
| x 1 modify a
|/
o 0 add a
Target commit has new content
$ hg cat -r 'desc("modify a")' a
a4
The working copy is clean and there is no unfinished operation
$ hg st -v
Can abort or continue after conflict while rebasing temporary commit Can abort or continue after conflict while rebasing temporary commit
-------------------------------------------------------------------------------- --------------------------------------------------------------------------------
Common setup for abort and continue Common setup for abort and continue
$ cd "$TESTTMP" $ cd "$TESTTMP"
$ hg init conflict-rebasing-temp-commit $ hg init conflict-rebasing-temp-commit
$ cd conflict-rebasing-temp-commit $ cd conflict-rebasing-temp-commit
$ echo a > a $ echo a > a
$ hg ci -Aqm 'add a' $ hg ci -Aqm 'add a'
$ echo a2 > a $ echo a2 > a
$ hg ci -m 'modify a' $ hg ci -m 'modify a'
$ echo a3 > a $ echo a3 > a
$ hg log -G -T '{rev} {desc}' $ hg log -G -T '{rev} {desc}'
@ 1 modify a @ 1 modify a
| |
o 0 add a o 0 add a
$ hg amend -r 'desc("add a")' $ hg amend -r 'desc("add a")'
warning: conflicts while merging a! (edit, then use 'hg resolve --mark') warning: conflicts while merging a! (edit, then use 'hg resolve --mark')
unresolved conflicts (see 'hg resolve', then 'hg amend --continue') unresolved conflicts (see 'hg resolve', then 'hg amend --continue')
[240] [240]
$ hg log -G -T '{rev} {desc}'
% 2 temporary commit for "amend --rev"
|
o 1 modify a
|
@ 0 add a
$ hg st -v $ hg st -v
M a M a
? a.orig ? a.orig
# The repository is in an unfinished *amend* state. # The repository is in an unfinished *amend* state.
# Unresolved merge conflicts: # Unresolved merge conflicts:
# #
# a # a
? a.orig ? a.orig
Can continue Can continue
$ cd ../conflict-rebasing-temp-commit-continue $ cd ../conflict-rebasing-temp-commit-continue
$ echo resolved > a $ echo resolved > a
$ hg resolve -m $ hg resolve -m
(no more unresolved files) (no more unresolved files)
continue: hg amend --continue continue: hg amend --continue
$ hg log -G -T '{rev} {desc}'
o 2 temporary commit for "amend --rev"
|
o 1 modify a
|
@ 0 add a
$ hg continue $ hg continue
rebasing 1:41c4ea50d4cf "modify a"
merging a
warning: conflicts while merging a! (edit, then use 'hg resolve --mark')
1 new orphan changesets 1 new orphan changesets
unresolved conflicts (see 'hg resolve', then 'hg amend --continue')
[240]
$ hg log -G -T '{rev} {desc}' $ hg log -G -T '{rev} {desc}'
o 4 add a @ 4 add a
@ 1 modify a % 1 modify a
| |
x 0 add a x 0 add a
$ echo resolved again > a
$ hg resolve -m
(no more unresolved files)
continue: hg amend --continue
$ hg continue
rebasing 1:41c4ea50d4cf "modify a"
$ hg log -G -T '{rev} {desc}'
@ 5 modify a
|
o 4 add a
Target commit has new content Target commit has new content
$ hg cat -r 'desc("add a")' a $ hg cat -r 'desc("add a")' a
resolved resolved
The working copy is clean and there is no unfinished operation The working copy is clean and there is no unfinished operation
$ hg st -v $ hg st -v
? a.orig
Can abort or continue after conflict while rebasing descendant commit
--------------------------------------------------------------------------------
Common setup for abort and continue
$ cd "$TESTTMP"
$ hg init conflict-rebasing-descendant-commit
$ cd conflict-rebasing-descendant-commit
$ echo a > a
$ hg ci -Aqm 'add a'
$ echo a2 > a
$ hg ci -m 'modify a'
$ echo a > a
$ hg ci -m 'revert a'
$ echo a3 > a
$ hg log -G -T '{rev} {desc}'
@ 2 revert a
|
o 1 modify a
|
o 0 add a
$ hg amend -r 'desc("add a")'
rebasing 1:41c4ea50d4cf "modify a"
merging a
warning: conflicts while merging a! (edit, then use 'hg resolve --mark')
2 new orphan changesets
unresolved conflicts (see 'hg resolve', then 'hg amend --continue')
[240]
$ hg log -G -T '{rev} {desc}'
@ 5 add a
* 2 revert a
|
% 1 modify a
|
x 0 add a
$ hg st -v
M a
? a.orig
# The repository is in an unfinished *amend* state.
# Unresolved merge conflicts:
#
# a
#
# To mark files as resolved: hg resolve --mark FILE
# To continue: hg amend --continue
# To abort: hg amend --abort
Make a copy of the repo and working copy to test continuing
$ cp -R . ../conflict-rebasing-descendant-commit-continue
Can abort
$ hg abort
rebase aborted
saved backup bundle to $TESTTMP/conflict-rebasing-descendant-commit/.hg/strip-backup/f242bdc041a8-4658c323-backup.hg
The log output looks like it did before we started
$ hg log -G -T '{rev} {desc}'
@ 2 revert a
|
o 1 modify a
|
o 0 add a
The working copy has the change it had before we started
$ hg diff
diff -r 43217edd8bde a
--- a/a Thu Jan 01 00:00:00 1970 +0000
+++ b/a Thu Jan 01 00:00:00 1970 +0000
@@ -1,1 +1,1 @@
-a
+a3
There is no unfinished operation
$ hg st -v
M a
? a.orig
Can continue
$ cd ../conflict-rebasing-descendant-commit-continue
$ echo resolved > a
$ hg resolve -m
(no more unresolved files)
continue: hg amend --continue
$ hg log -G -T '{rev} {desc}'
@ 5 add a
* 2 revert a
|
* 1 modify a
|
x 0 add a
$ hg continue
rebasing 1:41c4ea50d4cf "modify a"
rebasing 2:43217edd8bde "revert a"
merging a
warning: conflicts while merging a! (edit, then use 'hg resolve --mark')
unresolved conflicts (see 'hg resolve', then 'hg amend --continue')
[240]
$ echo resolved again > a
$ hg resolve -m
(no more unresolved files)
continue: hg amend --continue
$ hg continue
already rebased 1:41c4ea50d4cf "modify a" as 79b758e3d369
rebasing 2:43217edd8bde "revert a"
$ hg log -G -T '{rev} {desc}'
@ 7 revert a
|
o 6 modify a
|
o 5 add a
Target commit has new content
$ hg cat -r 'desc("add a")' a
a3
The working copy is clean and there is no unfinished operation
$ hg st -v
? a.orig