This is an archive of the discontinued Mercurial Phabricator instance.

Differential D817

histedit: add move histedit action
AbandonedPublic

Authored by mbthomas on Sep 26 2017, 6:14 AM.

Details

Reviewers
ryanmce
Group Reviewers
hg-reviewers
Summary

Add a new histedit action: move. This allows moving of changesets from
outside of the history that is being edited, using the node hash of the other
changeset.

The changeset is copied to the location in the edited history, and the other
changeset is removed from the repository if possible.

If removing the other changeset is not possible (for example, if removing
the changeset would create an orphaned revision, and this is not allowed)
then the move becomes a copy and a warning is emitted.

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

mbthomas created this revision.Sep 26 2017, 6:14 AM
ryanmce requested changes to this revision.Sep 26 2017, 7:53 AM
ryanmce added a subscriber: ryanmce.

I think we need to think about this one a bit more.

hgext/histedit.py
861

hide is really only FB-internal naming. In regular/core hg, this would still likely be a strip.

867–868

Thinking about this more, I think this is pretty non-obvious to an average user.

In fact, the "move" versus "pick" difference is really unclear in general. Why must I "pick" something in the set, even if I'm actually moving it up or down, and "move" something from outside? If I "move" something up or down a slot, it's unclear why I want to use "pick" instead of "move" to an average user.

It seems to me that before we do this we should take a step back and talk about the end state we want to get to.

In my "idealized" world, histedit might have:

  • keep (instead of "pick"), which I think is more clear. "pick" could still be a backwards-compatible name though (git's "pick" comes from "cherry-pick", and copying it to histedit seems like a mistake to me, in hindsight.
  • I like "copy" as-implemented in the previous patch. I think it's pretty clear and a nice four-letter name.
  • some kind of verb that allows me to take whatever commit, from wherever, and apply it where I am now with no unreasonable restrictions. Like a superset of "keep" and "move" (or whatever we name that). We could call it "grab" (?). grab is a term that we used for a while to "move" things from one point to another. It kind of evokes "graft" with it's prefix but is clearly different and implies taking it away from wherever it is.
  • (maybe) a "safe" way to move from somewhere else. I'm not sold on the "move" name the more I think about it because, desptie nicely being 4 letters, it doesn't really make clear to me how it is to be used.
This revision now requires changes to proceed.Sep 26 2017, 7:53 AM
mbthomas added inline comments.Sep 26 2017, 1:14 PM
hgext/histedit.py
867–868

Another four-letter candidate might be take. I don't think t is taken as an abbreviation, either.

quark added a subscriber: quark.Sep 26 2017, 1:33 PM
quark added inline comments.
hgext/histedit.py
861

Strictly speaking, it is "rebase", which is not 100% equal to graft + hide. The latter does not create an obsmarker.

867–868

How about just letting pick accept external commits? We can gate the feature by a config option if it looks risky.

880

error.Abort may be more appropriate. ParseError is more about a syntax error (ex. having [^0-9a-f] in commit hash).

mbthomas added a comment.Oct 3 2017, 5:24 AM

I'm thinking now that maybe we don't want move at all. If someone does want to move a commit using histedit, they can copy it in and then strip/hide the old commit. At the moment there is no way for histedit to affect commits outside the history range that it is editing, and I think that's a good thing.

quark added a comment.Oct 5 2017, 11:56 PM
In D817#15385, @mbthomas wrote:

I'm thinking now that maybe we don't want move at all. If someone does want to move a commit using histedit, they can copy it in and then strip/hide the old commit. At the moment there is no way for histedit to affect commits outside the history range that it is editing, and I think that's a good thing.

With the current design, copy X to Y then prune (which could be different from "hide", I think our "hide" implementation is still subject to change) X will not create an obsmarker from X to Y. That is the reason why move is needed. That's also mentioned in IRC:

Sep 25 11:14:00 <durin42>	I think they probably both need to exist
Sep 25 11:14:05 <durin42>	since move should record an obsolete marker

It might be possible to make things smarter so copy X to Y then hide X is equivalent to move X to Y. But the existing code does not support that yet.

mbthomas abandoned this revision.Oct 12 2017, 5:10 AM

Revision Contents
Changeset List

Diff 2076

hgext/histedit.py

@action(["mess", "m"], @action(["mess", "m"],
_('edit commit message without changing commit content'), _('edit commit message without changing commit content'),
priority=True) priority=True)
class message(histeditaction): class message(histeditaction):
def commiteditor(self): def commiteditor(self):
return cmdutil.getcommiteditor(edit=True, editform='histedit.mess') return cmdutil.getcommiteditor(edit=True, editform='histedit.mess')
@action(['move', 'v'],
_('move (graft and hide) a changeset from elsewhere'))
ryanmceUnsubmitted
Not Done

hide is really only FB-internal naming. In regular/core hg, this would still likely be a strip.

ryanmce: `hide` is really only FB-internal naming. In regular/core hg, this would still likely be a…
quarkUnsubmitted
Not Done

Strictly speaking, it is "rebase", which is not 100% equal to graft + hide. The latter does not create an obsmarker.

quark: Strictly speaking, it is "rebase", which is not 100% equal to graft + hide. The latter does not…
class move(histeditaction):
def _verifynodeconstraints(self, prev, expected, seen, basenodes):
if self.node in expected:
msg = _('%s "%s" changeset was an edited list candidate')
raise error.ParseError(
msg % (self.verb, node.short(self.node)),
hint=_('move must only use unlisted changesets'))
ryanmceUnsubmitted
Not Done

Thinking about this more, I think this is pretty non-obvious to an average user.

In fact, the "move" versus "pick" difference is really unclear in general. Why must I "pick" something in the set, even if I'm actually moving it up or down, and "move" something from outside? If I "move" something up or down a slot, it's unclear why I want to use "pick" instead of "move" to an average user.

It seems to me that before we do this we should take a step back and talk about the end state we want to get to.

In my "idealized" world, histedit might have:

  • keep (instead of "pick"), which I think is more clear. "pick" could still be a backwards-compatible name though (git's "pick" comes from "cherry-pick", and copying it to histedit seems like a mistake to me, in hindsight.
  • I like "copy" as-implemented in the previous patch. I think it's pretty clear and a nice four-letter name.
  • some kind of verb that allows me to take whatever commit, from wherever, and apply it where I am now with no unreasonable restrictions. Like a superset of "keep" and "move" (or whatever we name that). We could call it "grab" (?). grab is a term that we used for a while to "move" things from one point to another. It kind of evokes "graft" with it's prefix but is clearly different and implies taking it away from wherever it is.
  • (maybe) a "safe" way to move from somewhere else. I'm not sold on the "move" name the more I think about it because, desptie nicely being 4 letters, it doesn't really make clear to me how it is to be used.
ryanmce: Thinking about this more, I think this is pretty non-obvious to an average user. In fact, the…
mbthomasAuthorUnsubmitted
Not Done

Another four-letter candidate might be take. I don't think t is taken as an abbreviation, either.

mbthomas: Another four-letter candidate might be `take`. I don't think `t` is taken as an abbreviation…
quarkUnsubmitted
Not Done

How about just letting pick accept external commits? We can gate the feature by a config option if it looks risky.

quark: How about just letting `pick` accept external commits? We can gate the feature by a config…
repo = self.state.repo
ctx = repo[self.node]
if not ctx.mutable():
raise error.ParseError(
_('%s "%s" is public') % (self.verb, node.short(self.node)),
hint=_('cannot move public changesets'))
for basenode in basenodes:
ancestors = repo.changelog.commonancestorsheads(ctx.node(),
basenode)
if ctx.node() in ancestors:
msg = _('%s "%s" is an ancestor of base changeset "%s"')
raise error.ParseError(
quarkUnsubmitted
Not Done

error.Abort may be more appropriate. ParseError is more about a syntax error (ex. having [^0-9a-f] in commit hash).

quark: `error.Abort` may be more appropriate. `ParseError` is more about a syntax error (ex. having `…
msg % (self.verb, node.short(self.node),
node.short(basenode)),
hint=_('use histedit with a larger range to move older '
'changesets'))
@action(['copy', 'c'], @action(['copy', 'c'],
_('copy (graft) a changeset from elsewhere')) _('copy (graft) a changeset from elsewhere'))
class copy(histeditaction): class copy(histeditaction):
def _verifynodeconstraints(self, prev, expected, seen, basenodes): def _verifynodeconstraints(self, prev, expected, seen, basenodes):
pass pass
def continueclean(self): def continueclean(self):
ctx, replacement = super(copy, self).continueclean() ctx, replacement = super(copy, self).continueclean()

tests/test-histedit-arguments.t

# Commands: # Commands:
# #
# e, edit = use commit, but stop for amending # e, edit = use commit, but stop for amending
# m, mess = edit commit message without changing commit content # m, mess = edit commit message without changing commit content
# p, pick = use commit # p, pick = use commit
# c, copy = copy (graft) a changeset from elsewhere # c, copy = copy (graft) a changeset from elsewhere
# d, drop = remove commit from history # d, drop = remove commit from history
# f, fold = use commit, but combine it with the one above # f, fold = use commit, but combine it with the one above
# v, move = move (graft and hide) a changeset from elsewhere
# r, roll = like fold, but discard this commit's description and date # r, roll = like fold, but discard this commit's description and date
# #
Run on a revision not ancestors of the current working directory. Run on a revision not ancestors of the current working directory.
-------------------------------------------------------------------- --------------------------------------------------------------------
$ hg up 2 $ hg up 2
1 files updated, 0 files merged, 0 files removed, 0 files unresolved 1 files updated, 0 files merged, 0 files removed, 0 files unresolved
# Commands: # Commands:
# #
# e, edit = use commit, but stop for amending # e, edit = use commit, but stop for amending
# m, mess = edit commit message without changing commit content # m, mess = edit commit message without changing commit content
# p, pick = use commit # p, pick = use commit
# c, copy = copy (graft) a changeset from elsewhere # c, copy = copy (graft) a changeset from elsewhere
# d, drop = remove commit from history # d, drop = remove commit from history
# f, fold = use commit, but combine it with the one above # f, fold = use commit, but combine it with the one above
# v, move = move (graft and hide) a changeset from elsewhere
# r, roll = like fold, but discard this commit's description and date # r, roll = like fold, but discard this commit's description and date
# #
Test --continue with --keep Test --continue with --keep
$ hg strip -q -r . --config extensions.strip= $ hg strip -q -r . --config extensions.strip=
$ hg histedit '.^' -q --keep --commands - << EOF $ hg histedit '.^' -q --keep --commands - << EOF
> edit eb57da33312f 2 three > edit eb57da33312f 2 three
# Commands: # Commands:
# #
# e, edit = use commit, but stop for amending # e, edit = use commit, but stop for amending
# m, mess = edit commit message without changing commit content # m, mess = edit commit message without changing commit content
# p, pick = use commit # p, pick = use commit
# c, copy = copy (graft) a changeset from elsewhere # c, copy = copy (graft) a changeset from elsewhere
# d, drop = remove commit from history # d, drop = remove commit from history
# f, fold = use commit, but combine it with the one above # f, fold = use commit, but combine it with the one above
# v, move = move (graft and hide) a changeset from elsewhere
# r, roll = like fold, but discard this commit's description and date # r, roll = like fold, but discard this commit's description and date
# #
$ cd .. $ cd ..

tests/test-histedit-bookmark-motion.t

# Commands: # Commands:
# #
# e, edit = use commit, but stop for amending # e, edit = use commit, but stop for amending
# m, mess = edit commit message without changing commit content # m, mess = edit commit message without changing commit content
# p, pick = use commit # p, pick = use commit
# c, copy = copy (graft) a changeset from elsewhere # c, copy = copy (graft) a changeset from elsewhere
# d, drop = remove commit from history # d, drop = remove commit from history
# f, fold = use commit, but combine it with the one above # f, fold = use commit, but combine it with the one above
# v, move = move (graft and hide) a changeset from elsewhere
# r, roll = like fold, but discard this commit's description and date # r, roll = like fold, but discard this commit's description and date
# #
$ hg histedit 1 --commands - --verbose << EOF | grep histedit $ hg histedit 1 --commands - --verbose << EOF | grep histedit
> pick 177f92b77385 2 c > pick 177f92b77385 2 c
> drop d2ae7f538514 1 b > drop d2ae7f538514 1 b
> pick 055a42cdd887 3 d > pick 055a42cdd887 3 d
> fold e860deea161a 4 e > fold e860deea161a 4 e
> pick 652413bf663e 5 f > pick 652413bf663e 5 f
# Commands: # Commands:
# #
# e, edit = use commit, but stop for amending # e, edit = use commit, but stop for amending
# m, mess = edit commit message without changing commit content # m, mess = edit commit message without changing commit content
# p, pick = use commit # p, pick = use commit
# c, copy = copy (graft) a changeset from elsewhere # c, copy = copy (graft) a changeset from elsewhere
# d, drop = remove commit from history # d, drop = remove commit from history
# f, fold = use commit, but combine it with the one above # f, fold = use commit, but combine it with the one above
# v, move = move (graft and hide) a changeset from elsewhere
# r, roll = like fold, but discard this commit's description and date # r, roll = like fold, but discard this commit's description and date
# #
$ hg histedit 1 --commands - --verbose << EOF | grep histedit $ hg histedit 1 --commands - --verbose << EOF | grep histedit
> pick b346ab9a313d 1 c > pick b346ab9a313d 1 c
> pick cacdfd884a93 3 f > pick cacdfd884a93 3 f
> pick 59d9f330561f 2 d > pick 59d9f330561f 2 d
> EOF > EOF
saved backup bundle to $TESTTMP/r/.hg/strip-backup/59d9f330561f-073008af-histedit.hg (glob) saved backup bundle to $TESTTMP/r/.hg/strip-backup/59d9f330561f-073008af-histedit.hg (glob)

tests/test-histedit-commute.t

# Commands: # Commands:
# #
# e, edit = use commit, but stop for amending # e, edit = use commit, but stop for amending
# m, mess = edit commit message without changing commit content # m, mess = edit commit message without changing commit content
# p, pick = use commit # p, pick = use commit
# c, copy = copy (graft) a changeset from elsewhere # c, copy = copy (graft) a changeset from elsewhere
# d, drop = remove commit from history # d, drop = remove commit from history
# f, fold = use commit, but combine it with the one above # f, fold = use commit, but combine it with the one above
# v, move = move (graft and hide) a changeset from elsewhere
# r, roll = like fold, but discard this commit's description and date # r, roll = like fold, but discard this commit's description and date
# #
edit the history edit the history
(use a hacky editor to check histedit-last-edit.txt backup) (use a hacky editor to check histedit-last-edit.txt backup)
$ EDITED="$TESTTMP/editedhistory" $ EDITED="$TESTTMP/editedhistory"
$ cat > $EDITED <<EOF $ cat > $EDITED <<EOF
# Commands: # Commands:
# #
# e, edit = use commit, but stop for amending # e, edit = use commit, but stop for amending
# m, mess = edit commit message without changing commit content # m, mess = edit commit message without changing commit content
# p, pick = use commit # p, pick = use commit
# c, copy = copy (graft) a changeset from elsewhere # c, copy = copy (graft) a changeset from elsewhere
# d, drop = remove commit from history # d, drop = remove commit from history
# f, fold = use commit, but combine it with the one above # f, fold = use commit, but combine it with the one above
# v, move = move (graft and hide) a changeset from elsewhere
# r, roll = like fold, but discard this commit's description and date # r, roll = like fold, but discard this commit's description and date
# #
should also work if a commit message is missing should also work if a commit message is missing
$ BUNDLE="$TESTDIR/missing-comment.hg" $ BUNDLE="$TESTDIR/missing-comment.hg"
$ hg init missing $ hg init missing
$ cd missing $ cd missing
$ hg unbundle $BUNDLE $ hg unbundle $BUNDLE

tests/test-histedit-edit.t

# Commands: # Commands:
# #
# e, edit = use commit, but stop for amending # e, edit = use commit, but stop for amending
# m, mess = edit commit message without changing commit content # m, mess = edit commit message without changing commit content
# p, fold = use commit # p, fold = use commit
# c, copy = copy (graft) a changeset from elsewhere # c, copy = copy (graft) a changeset from elsewhere
# d, drop = remove commit from history # d, drop = remove commit from history
# f, fold = use commit, but combine it with the one above # f, fold = use commit, but combine it with the one above
# v, move = move (graft and hide) a changeset from elsewhere
# r, roll = like fold, but discard this commit's description and date # r, roll = like fold, but discard this commit's description and date
# #

tests/test-histedit-obsolete.t

# Commands: # Commands:
# #
# e, edit = use commit, but stop for amending # e, edit = use commit, but stop for amending
# m, mess = edit commit message without changing commit content # m, mess = edit commit message without changing commit content
# p, pick = use commit # p, pick = use commit
# c, copy = copy (graft) a changeset from elsewhere # c, copy = copy (graft) a changeset from elsewhere
# d, drop = remove commit from history # d, drop = remove commit from history
# f, fold = use commit, but combine it with the one above # f, fold = use commit, but combine it with the one above
# v, move = move (graft and hide) a changeset from elsewhere
# r, roll = like fold, but discard this commit's description and date # r, roll = like fold, but discard this commit's description and date
# #
$ hg histedit 1 --commands - --verbose <<EOF | grep histedit $ hg histedit 1 --commands - --verbose <<EOF | grep histedit
> pick 177f92b77385 2 c > pick 177f92b77385 2 c
> drop d2ae7f538514 1 b > drop d2ae7f538514 1 b
> pick 055a42cdd887 3 d > pick 055a42cdd887 3 d
> fold e860deea161a 4 e > fold e860deea161a 4 e
> pick 652413bf663e 5 f > pick 652413bf663e 5 f

tests/test-histedit-outgoing.t

# Commands: # Commands:
# #
# e, edit = use commit, but stop for amending # e, edit = use commit, but stop for amending
# m, mess = edit commit message without changing commit content # m, mess = edit commit message without changing commit content
# p, pick = use commit # p, pick = use commit
# c, copy = copy (graft) a changeset from elsewhere # c, copy = copy (graft) a changeset from elsewhere
# d, drop = remove commit from history # d, drop = remove commit from history
# f, fold = use commit, but combine it with the one above # f, fold = use commit, but combine it with the one above
# v, move = move (graft and hide) a changeset from elsewhere
# r, roll = like fold, but discard this commit's description and date # r, roll = like fold, but discard this commit's description and date
# #
$ cd .. $ cd ..
show the error from unrelated repos show the error from unrelated repos
$ cd r3 $ cd r3
$ HGEDITOR=cat hg histedit --outgoing ../r | grep -v comparing | grep -v searching $ HGEDITOR=cat hg histedit --outgoing ../r | grep -v comparing | grep -v searching
abort: repository is unrelated abort: repository is unrelated
# Commands: # Commands:
# #
# e, edit = use commit, but stop for amending # e, edit = use commit, but stop for amending
# m, mess = edit commit message without changing commit content # m, mess = edit commit message without changing commit content
# p, pick = use commit # p, pick = use commit
# c, copy = copy (graft) a changeset from elsewhere # c, copy = copy (graft) a changeset from elsewhere
# d, drop = remove commit from history # d, drop = remove commit from history
# f, fold = use commit, but combine it with the one above # f, fold = use commit, but combine it with the one above
# v, move = move (graft and hide) a changeset from elsewhere
# r, roll = like fold, but discard this commit's description and date # r, roll = like fold, but discard this commit's description and date
# #
$ cd .. $ cd ..
test sensitivity to branch in URL: test sensitivity to branch in URL:
$ cd r2 $ cd r2
$ hg -q update 2 $ hg -q update 2
# Commands: # Commands:
# #
# e, edit = use commit, but stop for amending # e, edit = use commit, but stop for amending
# m, mess = edit commit message without changing commit content # m, mess = edit commit message without changing commit content
# p, pick = use commit # p, pick = use commit
# c, copy = copy (graft) a changeset from elsewhere # c, copy = copy (graft) a changeset from elsewhere
# d, drop = remove commit from history # d, drop = remove commit from history
# f, fold = use commit, but combine it with the one above # f, fold = use commit, but combine it with the one above
# v, move = move (graft and hide) a changeset from elsewhere
# r, roll = like fold, but discard this commit's description and date # r, roll = like fold, but discard this commit's description and date
# #
test to check number of roots in outgoing revisions test to check number of roots in outgoing revisions
$ hg -q outgoing -G --template '{node|short}({branch})' '../r' $ hg -q outgoing -G --template '{node|short}({branch})' '../r'
@ f26599ee3441(foo) @ f26599ee3441(foo)