[PATCH 3 of 3 evolve-ext V2] commands: introduce a new command to edit commit metadata
Siddharth Agarwal
sid at less-broken.com
Fri Apr 15 20:14:07 UTC 2016
Following up on this --
On 3/27/16 15:14, Pierre-Yves David wrote:
> On 03/19/2016 10:02 PM, Siddharth Agarwal wrote:
>> # HG changeset patch
>> # User Siddharth Agarwal <sid0 at fb.com>
>> # Date 1458450093 25200
>> # Sat Mar 19 22:01:33 2016 -0700
>> # Branch stable
>> # Node ID 6570300a9b764dacb8c5551aebb55251153787c1
>> # Parent 62cee591d6b306cfa8d1f30acbcc4cae972d79ff
>> commands: introduce a new command to edit commit metadata
>
> This patches is both a bit too big and a bit too light. I would prefer
> to have a minimal first patches for this command and then add further
> feature (and testing). In addition, I would like the command to
> support multiple revision before we take a --fold flag. I would like
> to avoid just getting `hg fold` under another name here.
>
> I would expect to see something similar to:
> - patch 1: add a `hg metaedit` command,
> - patch 2: allow multiple revisions with `hg metaedit`,
I'm really not inclined to do this right now -- can I do something like
"accept multiple revisions with `hg metaedit` but abort"?
> - patch 3: add a --fold flag,
>
>> This patch introduces metaedit, a command to metadata of a set of
>> revisions
>> without updating working copy. It also supports folding a stack in
>> the same
>> operation. This is particularly relevant for repository where
>> changing the
>> parent of the working copy is time-consuming. We could add more stack
>> manipulation operations to metaedit in the future.
>>
>> Crucially, it is different from 'hg fold --exact' in that it also allows
>> 'folding' a single commit and rewriting its metadata. This is really
>> useful to
>> have as a single logical operation, for example while preparing a
>> series of
>> multiple local changesets that will need to be pushed as a single
>> changeset.
>>
>> diff --git a/hgext/evolve.py b/hgext/evolve.py
>> --- a/hgext/evolve.py
>> +++ b/hgext/evolve.py
>> @@ -2997,6 +2997,128 @@ def fold(ui, repo, *revs, **opts):
>> finally:
>> lockmod.release(lock, wlock)
>>
>> + at command('^metaedit',
>
> We probably don't want to (yep, evolve is a bit a mess here, I'm
> trying to put some order in that)
We probably don't want to what, precisely?
>
>> + [('r', 'rev', [], _("revisions to edit")),
>> + ('', 'fold', None, _("also fold specified revisions into
>> one"))
>> + ] + commitopts + commitopts2,
>> + _('hg metaedit [OPTION]... [-r] [REV]'))
>> +def metaedit(ui, repo, *revs, **opts):
>> + """edit commit information
>> +
>> + Edits the commit information for the specified revisions. By
>> default, edits
>> + commit information for the working directory parent.
>> +
>> + With --fold, also folds multiple revisions into one if
>> necessary. In this
>> + case, the given revisions must form a linear unbroken chain.
>> +
>> + .. container:: verbose
>> +
>> + Some examples:
>> +
>> + - Edit the commit message for the working directory parent::
>> +
>> + hg metaedit
>> +
>> + - Change the username for the working directory parent::
>> +
>> + hg metaedit --user 'New User <new-email at example.com>'
>> +
>> + - Combine all draft revisions that are ancestors of foo but not
>> of @ into
>> + one::
>> +
>> + hg metaedit --fold 'draft() and only(foo,@)'
>> +
>> + See :hg:`help phases` for more about draft revisions and
>> + :hg:`help revsets` for more about the `draft()` and `only()`
>> keywords
>> + """
>> + revs = list(revs)
>> + revs.extend(opts['rev'])
>> + if not revs:
>> + if opts['fold']:
>> + raise error.Abort(_('revisions must be specified with
>> --fold'))
>> + revs = ['.']
>> +
>> + revs = scmutil.revrange(repo, revs)
>> + if not opts['fold'] and len(revs) > 1:
>> + # TODO: handle the non-fold case with multiple revisions.
>> This is
>> + # somewhat tricky because if we want to edit a series of
>> commits:
>> + #
>> + # a ---- b ---- c
>> + #
>> + # we need to rewrite a first, then directly rewrite b on top
>> of the new
>> + # a, then rewrite c on top of the new b. So we need to
>> handle revisions
>> + # in topological order.
>> + raise error.Abort(_('editing multiple revisions without
>> --fold is not '
>> + 'currently supported'))
>
> I would be happy to see this lifted. It looks like sorting the
> revision should be sufficient to get them on topological order, and a
> small mapping to forward parent. Am I missing someting?
There's also other UI issues like how to accept the commit message for
multiple revisions, etc. That's why I'm inclined to not deal with this
problem right now.
>
>> + if opts['fold']:
>> + root, head = _foldcheck(repo, revs)
>> + else:
>> + if _willcreatedisallowedunstable(repo, revs):
>> + raise error.Abort(_('cannot edit commit information in
>> the middle '
>> + 'of a stack'))
>
> If you can add information about a problematic node, that would help
> the user to debug the situation
OK -- but this code is copied verbatim from other places where we do
similar checks. We'll probably need to do those sorts of fixups anyway.
>
>> + # check above ensures only one revision
>> + root = head = repo[revs.first()]
>> + if not root.mutable():
>
> Directly check repo.revs("%lr and public()", revs) That will be
> simpler and faster.
OK.
>
>> + raise error.Abort(_('cannot edit commit information for
>> public '
>> + 'revisions'))
>> +
>> + wlock = lock = None
>> + try:
>> + wlock = repo.wlock()
>> + lock = repo.lock()
>
> The repository is locked too late. We should compute the target and
> validate its content (especially the unstable detection).
Good point -- though a lot of other code is also buggy I suspect.
>
>> + wctx = repo[None]
>> + p1, p2 = wctx.p1(), wctx.p2()
>> + tr = repo.transaction('metaedit')
>> + newp1 = None
>> + try:
>> + commitopts = opts.copy()
>> + allctx = [repo[r] for r in revs]
>> + targetphase = max(c.phase() for c in allctx)
>> +
>> + if commitopts.get('message') or commitopts.get('logfile'):
>> + commitopts['edit'] = False
>> + else:
>> + if opts['fold']:
>> + msgs = ["HG: This is a fold of %d changesets." %
>> len(allctx)]
>> + msgs += ["HG: Commit message of changeset
>> %s.\n\n%s\n" %
>> + (c.rev(), c.description()) for c in
>> allctx]
>> + else:
>> + msgs = [head.description()]
>> + commitopts['message'] = "\n".join(msgs)
>> + commitopts['edit'] = True
>> +
>> + # TODO: if the author and message are the same, don't
>> create a new
>> + # hash. Right now we create a new hash because the date
>> can be
>> + # different.
>> + newid, created = rewrite(repo, root, allctx, head,
>> + [root.p1().node(),
>> root.p2().node()],
>> + commitopts=commitopts)
>> + # Optimization: if the working copy parent is a *head*
>> (not root,
>> + # not in between) of a commit or commit series that got
>> rewritten,
>> + # just use localrepo.setparents and avoid any working copy
>> + # updates. It's easier to do this if we don't also have
>> to worry
>> + # about p2.
>
> Why do we need this? isn't and update good enough?
It probably is -- I'll get rid of this.
>
>> + if not p2 and head == p1:
>> + newp1 = newid
>> + if created:
>> + phases.retractboundary(repo, tr, targetphase, [newid])
>> + obsolete.createmarkers(repo, [(ctx, (repo[newid],))
>> + for ctx in allctx])
>> + else:
>> + ui.status(_("nothing changed\n"))
>> + tr.close()
>> + finally:
>> + tr.release()
>> + if opts['fold']:
>> + ui.status('%i changesets folded\n' % len(revs))
>> + if newp1 is not None:
>> + repo.setparents(newp1)
>> + elif p1.rev() in revs:
>> + hg.update(repo, newid)
>> + finally:
>> + lockmod.release(lock, wlock)
>> +
>> def _foldcheck(repo, revs):
>> roots = repo.revs('roots(%ld)', revs)
>> if len(roots) > 1:
>
More information about the Mercurial-devel
mailing list