( )⚙ D11038 amend: make `hg amend -r` fold temporary commit into target commit

This is an archive of the discontinued Mercurial Phabricator instance.

Differential D11038

amend: make `hg amend -r` fold temporary commit into target commit
AbandonedPublic

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

Details

Reviewers
Alphare
baymax
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 accepted this revision as: Alphare.Jul 12 2021, 5:23 AM
martinvonz updated this revision to Diff 29212.Jul 12 2021, 2:43 PM
marmoute added a subscriber: marmoute.Jul 15 2021, 8:22 PM

Not much feedback here but for the previous one of "why are we going through the full trouble of doing cross extensions delegation instead of using rewriteutil function instead?

hgext/amend.py
258–262

This would probably win to be prepared outside of the call to help readability.

(also, this seems weird and increase my questioning about why we are using rebase directly instead of a more appropriate API).

263–265

This would probably win to be prepared outside of the call to help readability.

(also, this seems weird and increase my questioning about why we are using rebase directly instead of a more appropriate API).

270–271

Is there any cases where this could effectively fails ?

martinvonz updated this revision to Diff 29295.Jul 15 2021, 8:35 PM
baymax requested changes to this revision.Feb 2 2022, 8:06 PM

There seems to have been no activities on this Diff for the past 3 Months.

By policy, we are automatically moving it out of the need-review state.

Please, move it back to need-review without hesitation if this diff should still be discussed.

:baymax:need-review-idle:

This revision now requires changes to proceed.Feb 2 2022, 8:06 PM
martinvonz abandoned this revision.Feb 11 2022, 6:36 PM

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 (43 lines)
Mtests/test-amend-rev.t (25 lines)

Diff 29051

hgext/amend.py

_do_continue_amend_rev(ui, repo, state, rebase) _do_continue_amend_rev(ui, repo, state, rebase)
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']]
_rebase_temp_node(ui, repo, state, rebase, temp_ctx, target_ctx) rebased_temp_ctx = _rebase_temp_node(
ui, repo, state, rebase, temp_ctx, target_ctx
)
_fold_temp_node(ui, repo, state, rebase, rebased_temp_ctx, target_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 rebase temporary commit')) raise error.Abort(_(b'failed to rebase temporary commit'))
rebased_temp_ctx = repo[b'tip'] rebased_temp_ctx = repo[b'tip']
state[b'rebased_temp_node'] = rebased_temp_ctx.node() state[b'rebased_temp_node'] = rebased_temp_ctx.node()
return rebased_temp_ctx return rebased_temp_ctx
def _fold_temp_node(ui, repo, state, rebase, rebased_temp_ctx, target_ctx):
if b'amended_node' in state:
return repo.unfiltered()[state[b'amended_node']]
elif statemod.ischildunfinished(repo, b'amend', b'rebase'):
with ui.silent(), statemod.delegating(repo, b'amend', b'rebase'):
ret = statemod.continuechild(ui, repo, b'amend', b'rebase')
else:
# Use rebase with collapse=True to fold the temporary commit into
# the target.
# Temporarily allow orphans; we'll rebase them ourselves.
overrides = {(b'experimental', b'evolution.allowunstable'): b"true"}
with ui.configoverride(
overrides, b'amend'
), ui.silent(), statemod.delegating(repo, b'amend', b'rebase'):
ret = rebase.rebase(
ui,
repo,
rev=[
revsetlang.formatspec(
b'%d + %d', target_ctx.rev(), rebased_temp_ctx.rev()
)
],
marmouteUnsubmitted
Not Done

This would probably win to be prepared outside of the call to help readability.

(also, this seems weird and increase my questioning about why we are using rebase directly instead of a more appropriate API).

marmoute: This would probably win to be prepared outside of the call to help readability. (also, this…
dest=revsetlang.formatspec(
b'%ld', [pctx.rev() for pctx in target_ctx.parents()]
),
marmouteUnsubmitted
Not Done

This would probably win to be prepared outside of the call to help readability.

(also, this seems weird and increase my questioning about why we are using rebase directly instead of a more appropriate API).

marmoute: This would probably win to be prepared outside of the call to help readability. (also, this…
collapse=True,
message=target_ctx.description(),
)
if ret:
raise error.Abort(_(b'failed to fold temporary commit'))
marmouteUnsubmitted
Not Done

Is there any cases where this could effectively fails ?

marmoute: Is there any cases where this could effectively fails ?
amended_ctx = repo[b'tip']
state[b'amended_node'] = amended_ctx.node()
return amended_ctx
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 = [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
$ hg log -G -T '{rev} {desc}' $ hg log -G -T '{rev} {desc}'
@ 4 temporary commit for "amend --rev" (known-bad-output !) @ 5 modify a (known-bad-output !)
| (known-bad-output !) | (known-bad-output !)
| o 2 add b (known-bad-output !) | * 2 add b (known-bad-output !)
| | (known-bad-output !)
| x 1 modify a (known-bad-output !)
|/ (known-bad-output !) |/ (known-bad-output !)
o 1 modify a
|
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
a2 (known-bad-output !) a3
a3 (missing-correct-output !)
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
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
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 continue $ hg continue
1 new orphan changesets
$ hg log -G -T '{rev} {desc}' $ hg log -G -T '{rev} {desc}'
@ 3 temporary commit for "amend --rev" (known-bad-output !) @ 4 add a
* 1 modify a (known-bad-output !)
| (known-bad-output !) | (known-bad-output !)
| o 1 modify a (known-bad-output !) x 0 add a
|/ (known-bad-output !)
o 0 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
a (known-bad-output !) resolved
resolved (missing-correct-output !)
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