[PATCH 3 of 3 evolve-ext V2] commands: introduce a new command to edit commit metadata

Pierre-Yves David pierre-yves.david at ens-lyon.org
Sun Mar 27 18:14:23 EDT 2016



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`,
- 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)

> +         [('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?

> +    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

> +        # 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.

> +            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).

> +        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?

> +            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:

-- 
Pierre-Yves David


More information about the Mercurial-devel mailing list