This is an archive of the discontinued Mercurial Phabricator instance.

Differential D6880

rebase: track new nodes when --keep is set
ClosedPublic

Authored by pgossman on Sep 25 2019, 11:35 AM.

Details

Reviewers
martinvonz
pulkit
Group Reviewers
hg-reviewers
Commits
rHG43c84b816445: rebase: track new nodes when --keep is set
Summary

When --keep is passed with rebase, the new nodes created are not
accessible from templates.

This change enables accessing the newly-created nodes from nodechanges,
just as if --keep was not set.

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

pgossman created this revision.Sep 25 2019, 11:35 AM
pulkit accepted this revision.Sep 25 2019, 11:38 AM
This revision is now accepted and ready to land.Sep 25 2019, 11:38 AM
pgossman updated this revision to Diff 16609.Sep 25 2019, 11:38 AM
pgossman added a commit: rHG43c84b816445: rebase: track new nodes when --keep is set.Sep 25 2019, 11:57 AM
Closed by commit rHG43c84b816445: rebase: track new nodes when --keep is set (authored by pgossman). · Explain Why
This revision was automatically updated to reflect the committed changes.
martinvonz added inline comments.Sep 26 2019, 7:20 PM
hgext/rebase.py
1808–1810

It looks like this still moves bookmarks, right (in the moves dict)? Should scmutil.cleanupnodes() simply only be called if not keepf?

pgossman added inline comments.Sep 26 2019, 7:29 PM
hgext/rebase.py
1808–1810

Yes this moves them. With --keep, bookmarks should still be moved.

See rebase help or https://bz.mercurial-scm.org/show_bug.cgi?id=5682

martinvonz added inline comments.Sep 26 2019, 8:16 PM
hgext/rebase.py
1808–1810

Oh, right, that bug... I agree that it's good that this patch preserves that behavior to avoid doing multiple things in one patch. But I think the current behavior is pretty surprising. I'm curious what you think since you apparently use hg rebase --keep (I don't).

pgossman added inline comments.Sep 30 2019, 12:23 PM
hgext/rebase.py
1808–1810

Bookmarks don't affect my use case for --keep, however, I agree the behavior was surprising and it would seem more intuitive if --keep did not move bookmarks.

Revision Contents
Changeset List

PathPackages
Mhgext/rebase.py (25 lines)
Mtests/test-rebase-templates.t (27 lines)

Diff 16615

hgext/rebase.py

stripcleanup = not obsolete.isenabled(repo, obsolete.createmarkersopt) stripcleanup = not obsolete.isenabled(repo, obsolete.createmarkersopt)
collapsednodes = [] collapsednodes = []
for rev, newrev in sorted(state.items()): for rev, newrev in sorted(state.items()):
if newrev >= 0 and newrev != rev: if newrev >= 0 and newrev != rev:
oldnode = tonode(rev) oldnode = tonode(rev)
newnode = collapsedas or tonode(newrev) newnode = collapsedas or tonode(newrev)
moves[oldnode] = newnode moves[oldnode] = newnode
if not keepf:
succs = None succs = None
if rev in skipped: if rev in skipped:
if stripcleanup or not repo[rev].obsolete(): if stripcleanup or not repo[rev].obsolete():
succs = () succs = ()
elif collapsedas: elif collapsedas:
collapsednodes.append(oldnode) collapsednodes.append(oldnode)
else: else:
succs = (newnode,) succs = (newnode,)
if succs is not None: if succs is not None:
replacements[(oldnode,)] = succs replacements[(oldnode,)] = succs
if collapsednodes: if collapsednodes:
replacements[tuple(collapsednodes)] = (collapsedas,) replacements[tuple(collapsednodes)] = (collapsedas,)
scmutil.cleanupnodes(repo, replacements, 'rebase', moves, backup=backup)
if fm: if fm:
hf = fm.hexfunc hf = fm.hexfunc
fl = fm.formatlist fl = fm.formatlist
fd = fm.formatdict fd = fm.formatdict
changes = {} changes = {}
for oldns, newn in replacements.iteritems(): for oldns, newn in replacements.iteritems():
for oldn in oldns: for oldn in oldns:
changes[hf(oldn)] = fl([hf(n) for n in newn], name='node') changes[hf(oldn)] = fl([hf(n) for n in newn], name='node')
nodechanges = fd(changes, key="oldnode", value="newnodes") nodechanges = fd(changes, key="oldnode", value="newnodes")
fm.data(nodechanges=nodechanges) fm.data(nodechanges=nodechanges)
if keepf:
replacements = {}
scmutil.cleanupnodes(repo, replacements, 'rebase', moves, backup=backup)
martinvonzUnsubmitted
Not Done

It looks like this still moves bookmarks, right (in the moves dict)? Should scmutil.cleanupnodes() simply only be called if not keepf?

martinvonz: It looks like this still moves bookmarks, right (in the `moves` dict)? Should `scmutil.
pgossmanAuthorUnsubmitted
Done

Yes this moves them. With --keep, bookmarks should still be moved.

See rebase help or https://bz.mercurial-scm.org/show_bug.cgi?id=5682

pgossman: Yes this moves them. With --keep, bookmarks should still be moved. See rebase help or https…
martinvonzUnsubmitted
Not Done

Oh, right, that bug... I agree that it's good that this patch preserves that behavior to avoid doing multiple things in one patch. But I think the current behavior is pretty surprising. I'm curious what you think since you apparently use hg rebase --keep (I don't).

martinvonz: Oh, right, that bug... I agree that it's good that this patch preserves that behavior to avoid…
pgossmanAuthorUnsubmitted
Done

Bookmarks don't affect my use case for --keep, however, I agree the behavior was surprising and it would seem more intuitive if --keep did not move bookmarks.

pgossman: Bookmarks don't affect my use case for `--keep`, however, I agree the behavior was surprising…
def pullrebase(orig, ui, repo, *args, **opts): def pullrebase(orig, ui, repo, *args, **opts):
'Call rebase after pull if the latter has been invoked with --rebase' 'Call rebase after pull if the latter has been invoked with --rebase'
if opts.get(r'rebase'): if opts.get(r'rebase'):
if ui.configbool('commands', 'rebase.requiredest'): if ui.configbool('commands', 'rebase.requiredest'):
msg = _('rebase destination required by configuration') msg = _('rebase destination required by configuration')
hint = _('use hg pull followed by hg rebase -d DEST') hint = _('use hg pull followed by hg rebase -d DEST')
raise error.Abort(msg, hint=hint) raise error.Abort(msg, hint=hint)

tests/test-rebase-templates.t

| |
o 4:849767420fd5 Added c o 4:849767420fd5 Added c
| |
o 0:18d04c59bb5d Added a o 0:18d04c59bb5d Added a
$ hg rebase -s 6 -d 4 -q -T "{nodechanges % '{oldnode}:{newnodes % ' {node} '}'}" $ hg rebase -s 6 -d 4 -q -T "{nodechanges % '{oldnode}:{newnodes % ' {node} '}'}"
d9d6773efc831c274eace04bc13e8e6412517139: f48cd65c6dc3d2acb55da54402a5b029546e546f (no-eol) d9d6773efc831c274eace04bc13e8e6412517139: f48cd65c6dc3d2acb55da54402a5b029546e546f (no-eol)
$ hg log -G -T "{rev}:{node|short} {desc}"
o 7:f48cd65c6dc3 Added b
|
| @ 5:df21b32134ba Added d
|/
o 4:849767420fd5 Added c
|
o 0:18d04c59bb5d Added a
$ hg rebase -s 7 -d 5 -q --keep -T "{nodechanges % '{oldnode}:{newnodes % ' {node} '}'}"
f48cd65c6dc3d2acb55da54402a5b029546e546f: 6f7dda91e55e728fb798f3e44dbecf0ebaa83267 (no-eol)
$ hg log -G -T "{rev}:{node|short} {desc}"
o 8:6f7dda91e55e Added b
|
| o 7:f48cd65c6dc3 Added b
| |
@ | 5:df21b32134ba Added d
|/
o 4:849767420fd5 Added c
|
o 0:18d04c59bb5d Added a