This is an archive of the discontinued Mercurial Phabricator instance.

Differential D1784

fbamend: autorestack children for simple rebases
AcceptedPublic

Authored by marco.leogrande on Dec 27 2017, 8:23 PM.
Tags
None
Subscribers
quark

Details

Reviewers
quark
Group Reviewers
Restricted Project
Summary

Today, if you run 'hg amend' while in the middle of a stack of
commits, it amends the commit and leaves the top of the stack
behind. Users then have to run 'hg amend --restack' or 'hg restack' to
bring the old top of the stack onto the new version of the commit.

Add heuristic code so that an automatic restack can be performed if
amending a commit did not change its manifest, nor did it leave behind
a dirty working copy. This optimization can be avoided by providing
the --no-rebase commandline option.

Fix the existing tests that relied on the previous behaviour (so their
original intent is kept), and add new ones to cover the new heuristic.

Diff Detail

Repository
rFBHGX Facebook Mercurial Extensions
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

Herald added a reviewer: Restricted Project. · View Herald TranscriptDec 27 2017, 8:23 PM
quark requested changes to this revision.Dec 27 2017, 9:03 PM
quark added a subscriber: quark.

This looks pretty good. I just have a small request about the user-facing message.

hgext3rd/fbamend/__init__.py
278–279

Since this is user-facing and not everyone knows what "manifest" is. It's more friendly to avoid the terminology. Power users may want to disable the feature (ex. if they want to amend multiple commits and then just use restack once to fix them all) but it's not obvious how. Maybe something like "(auto-rebasing descendants, use --no-rebase to disable this)"

Note "children" and "descendants" are different in hg. See hg help revset for details.

tests/test-fbamend-restack.t
873–875

hg debugbuilddag '+3' is a more efficient way to create multiple commits. You can use hg update 'tip^' in the next line to update to the second commit. That said, I'm fine with just using mkcommit.

899

debugbuilddag creates empty commits that requires explicit "add" if you changed "b". That's doable by using hg amend -A b here.

This revision now requires changes to proceed.Dec 27 2017, 9:03 PM
marco.leogrande marked 3 inline comments as done.Dec 27 2017, 9:29 PM
quark accepted this revision.Dec 27 2017, 11:33 PM

Great!

This revision is now accepted and ready to land.Dec 27 2017, 11:33 PM

Revision Contents
Changeset List

Diff 4657

hgext3rd/fbamend/__init__.py

raise error.Abort(_('--%s must be called with --amend') % raise error.Abort(_('--%s must be called with --amend') %
badflags[0]) badflags[0])
return orig(ui, repo, *pats, **opts) return orig(ui, repo, *pats, **opts)
def amend(ui, repo, *pats, **opts): def amend(ui, repo, *pats, **opts):
'''amend the current changeset with more changes '''amend the current changeset with more changes
''' '''
# 'rebase' is a tristate option: None=auto, True=force, False=disable
rebase = opts.get('rebase') rebase = opts.get('rebase')
to = opts.get('to') to = opts.get('to')
if rebase and _histediting(repo): if rebase and _histediting(repo):
# if a histedit is in flight, it's dangerous to remove old commits # if a histedit is in flight, it's dangerous to remove old commits
hint = _('during histedit, use amend without --rebase') hint = _('during histedit, use amend without --rebase')
raise error.Abort('histedit in progress', hint=hint) raise error.Abort('histedit in progress', hint=hint)
else: else:
node = cmdutil.amend(ui, repo, old, {}, pats, opts) node = cmdutil.amend(ui, repo, old, {}, pats, opts)
if node == old.node(): if node == old.node():
ui.status(_("nothing changed\n")) ui.status(_("nothing changed\n"))
return 1 return 1
if haschildren and rebase is None and not _histediting(repo):
# If the user has chosen the default behaviour for the
# rebase, then see if we can apply any heuristics. This
# will not performed if a histedit is in flight.
newcommit = repo[node]
# If the rebase did not change the manifest and the
# working copy is clean, force the children to be
# restacked.
if (old.manifestnode() == newcommit.manifestnode() and
not repo[None].dirty()):
ui.status(_('(auto-rebasing descendants, use --no-rebase '
'to disable this)\n'))
quarkUnsubmitted
Done

Since this is user-facing and not everyone knows what "manifest" is. It's more friendly to avoid the terminology. Power users may want to disable the feature (ex. if they want to amend multiple commits and then just use restack once to fix them all) but it's not obvious how. Maybe something like "(auto-rebasing descendants, use --no-rebase to disable this)"

Note "children" and "descendants" are different in hg. See hg help revset for details.

quark: Since this is user-facing and not everyone knows what "manifest" is. It's more friendly to…
rebase = True
if haschildren and not rebase: if haschildren and not rebase:
msg = _("warning: the changeset's children were left behind\n") msg = _("warning: the changeset's children were left behind\n")
if _histediting(repo): if _histediting(repo):
ui.warn(msg) ui.warn(msg)
ui.status(_('(this is okay since a histedit is in progress)\n')) ui.status(_('(this is okay since a histedit is in progress)\n'))
else: else:
_usereducation(ui) _usereducation(ui)
ui.warn(msg) ui.warn(msg)

tests/test-fbamend-nextrebase.t

$ hg next --rebase --merge $ hg next --rebase --merge
abort: cannot use both --merge and --rebase abort: cannot use both --merge and --rebase
[255] [255]
Rebasing single changeset. Rebasing single changeset.
$ hg debugbuilddag -n +4 $ hg debugbuilddag -n +4
$ hg up 1 $ hg up 1
2 files updated, 0 files merged, 0 files removed, 0 files unresolved 2 files updated, 0 files merged, 0 files removed, 0 files unresolved
$ hg amend -m "amended" $ hg amend -m "amended" --no-rebase
warning: the changeset's children were left behind warning: the changeset's children were left behind
(use 'hg restack' to rebase them) (use 'hg restack' to rebase them)
$ hg next $ hg next
abort: current changeset has no children abort: current changeset has no children
[255] [255]
$ hg next --rebase $ hg next --rebase
rebasing 2:776c07fa2b12 "r2" rebasing 2:776c07fa2b12 "r2"
1 files updated, 0 files merged, 0 files removed, 0 files unresolved 1 files updated, 0 files merged, 0 files removed, 0 files unresolved
o 0 r0 o 0 r0
Rebasing multiple changesets at once. Rebasing multiple changesets at once.
$ reset $ reset
$ hg debugbuilddag -n +5 $ hg debugbuilddag -n +5
$ hg up 1 $ hg up 1
2 files updated, 0 files merged, 0 files removed, 0 files unresolved 2 files updated, 0 files merged, 0 files removed, 0 files unresolved
$ hg amend -m "amended" $ hg amend -m "amended" --no-rebase
warning: the changeset's children were left behind warning: the changeset's children were left behind
(use 'hg restack' to rebase them) (use 'hg restack' to rebase them)
$ hg next --rebase --top $ hg next --rebase --top
rebasing 2:776c07fa2b12 "r2" rebasing 2:776c07fa2b12 "r2"
rebasing 3:137d867d71d5 "r3" rebasing 3:137d867d71d5 "r3"
rebasing 4:daa37004f338 "r4" rebasing 4:daa37004f338 "r4"
3 files updated, 0 files merged, 0 files removed, 0 files unresolved 3 files updated, 0 files merged, 0 files removed, 0 files unresolved
[55b98e] r4 [55b98e] r4
o 0 r0 o 0 r0
Rebasing a stack one changeset at a time. Rebasing a stack one changeset at a time.
$ reset $ reset
$ hg debugbuilddag -n +5 $ hg debugbuilddag -n +5
$ hg up 1 $ hg up 1
2 files updated, 0 files merged, 0 files removed, 0 files unresolved 2 files updated, 0 files merged, 0 files removed, 0 files unresolved
$ hg amend -m "amended" $ hg amend -m "amended" --no-rebase
warning: the changeset's children were left behind warning: the changeset's children were left behind
(use 'hg restack' to rebase them) (use 'hg restack' to rebase them)
$ hg next --rebase $ hg next --rebase
rebasing 2:776c07fa2b12 "r2" rebasing 2:776c07fa2b12 "r2"
1 files updated, 0 files merged, 0 files removed, 0 files unresolved 1 files updated, 0 files merged, 0 files removed, 0 files unresolved
[fe8ffc] r2 [fe8ffc] r2
$ hg next --rebase $ hg next --rebase
rebasing 3:137d867d71d5 "r3" rebasing 3:137d867d71d5 "r3"
o 0 r0 o 0 r0
Rebasing a stack two changesets at a time. Rebasing a stack two changesets at a time.
$ reset $ reset
$ hg debugbuilddag -n +6 $ hg debugbuilddag -n +6
$ hg up 1 $ hg up 1
2 files updated, 0 files merged, 0 files removed, 0 files unresolved 2 files updated, 0 files merged, 0 files removed, 0 files unresolved
$ hg amend -m "amended" $ hg amend -m "amended" --no-rebase
warning: the changeset's children were left behind warning: the changeset's children were left behind
(use 'hg restack' to rebase them) (use 'hg restack' to rebase them)
$ hg next --rebase 2 $ hg next --rebase 2
rebasing 2:776c07fa2b12 "r2" rebasing 2:776c07fa2b12 "r2"
rebasing 3:137d867d71d5 "r3" rebasing 3:137d867d71d5 "r3"
2 files updated, 0 files merged, 0 files removed, 0 files unresolved 2 files updated, 0 files merged, 0 files removed, 0 files unresolved
[7d603c] r3 [7d603c] r3
$ showgraph $ showgraph
o 0 r0 o 0 r0
Rebasing after multiple amends. Rebasing after multiple amends.
$ reset $ reset
$ hg debugbuilddag -n +5 $ hg debugbuilddag -n +5
$ hg up 1 $ hg up 1
2 files updated, 0 files merged, 0 files removed, 0 files unresolved 2 files updated, 0 files merged, 0 files removed, 0 files unresolved
$ hg amend -m "amend 1" $ hg amend -m "amend 1" --no-rebase
warning: the changeset's children were left behind warning: the changeset's children were left behind
(use 'hg restack' to rebase them) (use 'hg restack' to rebase them)
$ hg amend -m "amend 2" $ hg amend -m "amend 2"
$ hg amend -m "amend 3" $ hg amend -m "amend 3"
$ showgraph $ showgraph
@ 7 amend 3 @ 7 amend 3
| |
| o 4 r4 | o 4 r4
o 0 r0 o 0 r0
Rebasing from below the amended changeset with the --newest flag. Rebasing from below the amended changeset with the --newest flag.
$ reset $ reset
$ hg debugbuilddag -n +6 $ hg debugbuilddag -n +6
$ hg up 2 $ hg up 2
3 files updated, 0 files merged, 0 files removed, 0 files unresolved 3 files updated, 0 files merged, 0 files removed, 0 files unresolved
$ hg amend -m "amended" $ hg amend -m "amended" --no-rebase
warning: the changeset's children were left behind warning: the changeset's children were left behind
(use 'hg restack' to rebase them) (use 'hg restack' to rebase them)
$ hg up 0 $ hg up 0
0 files updated, 0 files merged, 2 files removed, 0 files unresolved 0 files updated, 0 files merged, 2 files removed, 0 files unresolved
$ showgraph $ showgraph
o 6 amended o 6 amended
| |
| o 5 r5 | o 5 r5
Test aborting due to ambiguity caused by a rebase. The rebase should be Test aborting due to ambiguity caused by a rebase. The rebase should be
rolled back and the final state should be as it was before `hg next --rebase`. rolled back and the final state should be as it was before `hg next --rebase`.
$ reset $ reset
$ hg debugbuilddag -n +6 $ hg debugbuilddag -n +6
$ hg up 1 $ hg up 1
2 files updated, 0 files merged, 0 files removed, 0 files unresolved 2 files updated, 0 files merged, 0 files removed, 0 files unresolved
$ hg amend -m "amended" $ hg amend -m "amended" --no-rebase
warning: the changeset's children were left behind warning: the changeset's children were left behind
(use 'hg restack' to rebase them) (use 'hg restack' to rebase them)
$ mkcommit a $ mkcommit a
$ hg prev $ hg prev
0 files updated, 0 files merged, 1 files removed, 0 files unresolved 0 files updated, 0 files merged, 1 files removed, 0 files unresolved
[dc00ac] amended [dc00ac] amended
$ showgraph $ showgraph
o 7 add a o 7 add a
$ mkcommit a $ mkcommit a
$ mkcommit b $ mkcommit b
$ mkcommit c $ mkcommit c
$ mkcommit d $ mkcommit d
$ hg up 1 $ hg up 1
0 files updated, 0 files merged, 2 files removed, 0 files unresolved 0 files updated, 0 files merged, 2 files removed, 0 files unresolved
$ echo "conflict" > c $ echo "conflict" > c
$ hg add c $ hg add c
$ hg amend -m "amended to add c" $ hg amend -m "amended to add c" --no-rebase
warning: the changeset's children were left behind warning: the changeset's children were left behind
(use 'hg restack' to rebase them) (use 'hg restack' to rebase them)
$ showgraph $ showgraph
@ 4 amended to add c @ 4 amended to add c
| |
| o 3 add d | o 3 add d
| | | |
| o 2 add c | o 2 add c

tests/test-fbamend-noinhibit.t

Set up test environment. Set up test environment.
$ cat >> $HGRCPATH << EOF $ cat >> $HGRCPATH << EOF
> [extensions] > [extensions]
> fbamend=$TESTDIR/../hgext3rd/fbamend > fbamend=$TESTDIR/../hgext3rd/fbamend
> rebase= > rebase=
> [experimental] > [experimental]
> evolution = createmarkers > evolution = createmarkers
> EOF > EOF
$ hg init repo && cd repo $ hg init repo && cd repo
Perform restack without inhibit extension. Perform restack without inhibit extension.
$ hg debugbuilddag -m +3 $ hg debugbuilddag -m +3
$ hg update 1 $ hg update 1
1 files updated, 0 files merged, 0 files removed, 0 files unresolved 1 files updated, 0 files merged, 0 files removed, 0 files unresolved
$ hg amend -m "Amended" $ hg amend -m "Amended" --no-rebase
warning: the changeset's children were left behind warning: the changeset's children were left behind
(use 'hg restack' to rebase them) (use 'hg restack' to rebase them)
$ hg rebase --restack $ hg rebase --restack
rebasing 2:* "r2" (glob) rebasing 2:* "r2" (glob)

tests/test-fbamend-restack.t

onto the newest successor of their parent. onto the newest successor of their parent.
$ reset $ reset
$ mkcommit a $ mkcommit a
$ mkcommit b $ mkcommit b
$ mkcommit c $ mkcommit c
$ hg prev $ hg prev
0 files updated, 0 files merged, 1 files removed, 0 files unresolved 0 files updated, 0 files merged, 1 files removed, 0 files unresolved
[*] add b (glob) [*] add b (glob)
$ hg amend -m "successor 1" $ hg amend -m "successor 1" --no-rebase
warning: the changeset's children were left behind warning: the changeset's children were left behind
(use 'hg restack' to rebase them) (use 'hg restack' to rebase them)
$ hg up 1 $ hg up 1
0 files updated, 0 files merged, 0 files removed, 0 files unresolved 0 files updated, 0 files merged, 0 files removed, 0 files unresolved
$ hg amend -m "successor 2" $ hg amend -m "successor 2" --no-rebase
warning: the changeset's children were left behind warning: the changeset's children were left behind
(use 'hg restack' to rebase them) (use 'hg restack' to rebase them)
$ hg up 1 $ hg up 1
0 files updated, 0 files merged, 0 files removed, 0 files unresolved 0 files updated, 0 files merged, 0 files removed, 0 files unresolved
$ showgraph $ showgraph
o 4 successor 2 o 4 successor 2
| |
| o 3 successor 1 | o 3 successor 1
> | > |
> C > C
> | > |
> B > B
> | > |
> A > A
> EOS > EOS
$ hg update B -q $ hg update B -q
$ hg commit --amend -m B2 -q 2>/dev/null $ hg commit --amend -m B2 -q --no-rebase 2>/dev/null
$ hg tag --local B2 $ hg tag --local B2
$ hg rebase -r C -d B2 -q $ hg rebase -r C -d B2 -q
$ hg commit --amend -m B3 -q 2>/dev/null $ hg commit --amend -m B3 -q --no-rebase 2>/dev/null
$ hg tag --local B3 $ hg tag --local B3
$ showgraph $ showgraph
@ 6 B3 @ 6 B3
| |
| o 5 C | o 5 C
| | | |
| x 4 B2 | x 4 B2
|/ |/
| |
| @ 7 F | @ 7 F
| | | |
o | 4 E o | 4 E
| | | |
| o 2 C | o 2 C
|/ |/
o 0 A o 0 A
Test auto-restack heuristics - no changes to manifest and clean working directory
$ reset
$ hg debugdrawdag<<'EOS'
> C
> |
quarkUnsubmitted
Done

hg debugbuilddag '+3' is a more efficient way to create multiple commits. You can use hg update 'tip^' in the next line to update to the second commit. That said, I'm fine with just using mkcommit.

quark: `hg debugbuilddag '+3'` is a more efficient way to create multiple commits. You can use `hg…
> B
> |
> A
> EOS
$ hg update B -q
$ hg amend -m 'Unchanged manifest for B'
(auto-rebasing descendants, use --no-rebase to disable this)
rebasing 2:26805aba1e60 "C" (C)
$ hg prev
0 files updated, 0 files merged, 1 files removed, 0 files unresolved
[426bad] A
$ hg amend -m 'Unchanged manifest for A'
(auto-rebasing descendants, use --no-rebase to disable this)
rebasing 3:5357953e3ea3 "Unchanged manifest for B"
rebasing 4:b635bd2cf20b "C"
Test auto-restack heuristics - manifest changes
$ reset
$ hg debugdrawdag<<'EOS'
> C
> |
> B
> |
> A
quarkUnsubmitted
Done

debugbuilddag creates empty commits that requires explicit "add" if you changed "b". That's doable by using hg amend -A b here.

quark: `debugbuilddag` creates empty commits that requires explicit "add" if you changed "b". That's…
> EOS
$ hg update B -q
$ echo 'new b' > B
$ hg amend -m 'Change manifest for B'
warning: the changeset's children were left behind
(use 'hg restack' to rebase them)
Test auto-restack heuristics - no committed changes to manifest but dirty working directory
$ reset
$ hg debugdrawdag<<'EOS'
> C
> |
> B
> |
> A
> EOS
$ hg update B -q
$ echo 'new b' > B
$ hg amend a -m 'Unchanged manifest, but dirty workdir'
warning: the changeset's children were left behind
(use 'hg restack' to rebase them)
Test auto-restack heuristics - no changes to manifest but no children
$ reset
$ hg debugdrawdag<<'EOS'
> B
> |
> A
> EOS
$ hg update B -q
$ hg amend -m 'Unchanged manifest for B'

tests/test-fbamend-userestack.t

Test that no preamend bookmark is created. Test that no preamend bookmark is created.
$ mkcommit a $ mkcommit a
$ mkcommit b $ mkcommit b
$ mkcommit c $ mkcommit c
$ mkcommit d $ mkcommit d
$ hg up 1 $ hg up 1
0 files updated, 0 files merged, 2 files removed, 0 files unresolved 0 files updated, 0 files merged, 2 files removed, 0 files unresolved
$ hg amend -m "amended" $ hg amend -m "amended" --no-rebase
warning: the changeset's children were left behind warning: the changeset's children were left behind
(use 'hg restack' to rebase them) (use 'hg restack' to rebase them)
$ hg book $ hg book
no bookmarks set no bookmarks set
Test hg amend --fixup. Test hg amend --fixup.
$ showgraph $ showgraph
@ 4 amended @ 4 amended

tests/test-smartlog-inhibit.t

Test that changesets with visible precursors are rendered as x's, even Test that changesets with visible precursors are rendered as x's, even
with the inhibit extension enabled. with the inhibit extension enabled.
$ hg init repo $ hg init repo
$ cd repo $ cd repo
$ hg debugbuilddag +4 $ hg debugbuilddag +4
$ hg book -r 3 test $ hg book -r 3 test
$ hg up 1 $ hg up 1
0 files updated, 0 files merged, 0 files removed, 0 files unresolved 0 files updated, 0 files merged, 0 files removed, 0 files unresolved
$ hg amend -m "amended" $ hg amend -m "amended" --no-rebase
warning: the changeset's children were left behind warning: the changeset's children were left behind
(use 'hg restack' to rebase them) (use 'hg restack' to rebase them)
$ hg smartlog -T '{rev} {bookmarks}' $ hg smartlog -T '{rev} {bookmarks}'
@ 4 @ 4
| |
| o 3 test | o 3 test
| | | |
| o 2 | o 2