Patchwork [3,of,3,evolve-ext,RFC] commands: introduce a new command to edit commit metadata

login
register
mail settings
Submitter Siddharth Agarwal
Date March 10, 2016, 9:24 a.m.
Message ID <baf6b4129735a703fd35.1457601859@dev666.prn1.facebook.com>
Download mbox | patch
Permalink /patch/13739/
State Changes Requested
Delegated to: Laurent Charignon
Headers show

Comments

Siddharth Agarwal - March 10, 2016, 9:24 a.m.
# HG changeset patch
# User Siddharth Agarwal <sid0@fb.com>
# Date 1457601780 28800
#      Thu Mar 10 01:23:00 2016 -0800
# Branch stable
# Node ID baf6b4129735a703fd356077e0c73c49c0d13f20
# Parent  acb48e8d739cec3b824a088843d8c432a34d2fd6
commands: introduce a new command to edit commit metadata

The eventual goal of this command is to allow editing one or more changesets'
metadata without being on the particular changeset.

This implementation is by no means complete, but it does allow folding multiple
commits into one. 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.
Laurent Charignon - March 11, 2016, 9:51 p.m.
Hi,

I put two comments inline for things we could do differently. Let me know
what you think.

It took me a while to understand the purpose of this series.
Maybe we could add some context to the commit message, for example:

"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 particularily relevant for repository where changing the parent of
the working copy is time-consumming.
We could add more stack manipulation operations to metaedit in the future."

Thanks!


Laurent

On 3/10/16, 1:24 AM, "Mercurial-devel on behalf of Siddharth Agarwal"
<mercurial-devel-bounces@mercurial-scm.org on behalf of sid0@fb.com> wrote:

># HG changeset patch
># User Siddharth Agarwal <sid0@fb.com>
># Date 1457601780 28800
>#      Thu Mar 10 01:23:00 2016 -0800
># Branch stable
># Node ID baf6b4129735a703fd356077e0c73c49c0d13f20
># Parent  acb48e8d739cec3b824a088843d8c432a34d2fd6
>commands: introduce a new command to edit commit metadata
>
>The eventual goal of this command is to allow editing one or more
>changesets'
>metadata without being on the particular changeset.
>
>This implementation is by no means complete, but it does allow folding
>multiple
>commits into one. 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,127 @@ def fold(ui, repo, *revs, **opts):
>     finally:
>         lockmod.release(lock, wlock)
> 
>+@command('^metaedit',
>+         [('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@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'))
>+
>+    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'))
>+        # check above ensures only one revision
>+        root = head = repo[revs.first()]
>+        if root.phase() == phases.public:

Let's use mutable here:

 if not root.mutable():

>+            raise error.Abort(_('cannot edit commit information for
>public '
>+                                'revisions'))
>+
>+    wlock = lock = None
>+    try:
>+        wlock = repo.wlock()
>+        lock = repo.lock()
>+        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: don't create a new commit if there are no non-trivial
>+            # changes

What do you mean?

>+            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.
>+            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:
>diff --git a/tests/test-evolve.t b/tests/test-evolve.t
>--- a/tests/test-evolve.t
>+++ b/tests/test-evolve.t
>@@ -2,6 +2,7 @@
>   > [defaults]
>   > amend=-d "0 0"
>   > fold=-d "0 0"
>+  > metaedit=-d "0 0"
>   > [web]
>   > push_ssl = false
>   > allow_push = *
>@@ -1449,3 +1450,113 @@ Check that dirstate changes are kept at
> 
>   $ hg status newlyadded
>   A newlyadded
>+
>+hg metaedit
>+-----------
>+
>+deliberately leave the working copy with dirty merges so that we know
>there are
>+no updates going on
>+  $ hg update .
>+  abort: outstanding merge conflicts
>+  [255]
>+check that metaedit respects allowunstable
>+  $ hg metaedit '36 + 42' --fold
>+  abort: cannot fold non-linear revisions (multiple roots given)
>+  [255]
>+  $ hg metaedit '36::39 + 41' --fold
>+  abort: cannot fold non-linear revisions (multiple heads given)
>+  [255]
>+  $ hg metaedit -r 0
>+  abort: cannot edit commit information for public revisions
>+  [255]
>+  $ hg metaedit -r 0 --fold
>+  abort: cannot fold public revisions
>+  [255]
>+  $ hg metaedit '18::20' --fold --config
>'experimental.evolution=createmarkers, allnewcommands'
>+  abort: cannot fold chain not ending with a head or with branching
>+  [255]
>+  $ hg metaedit '.^' --config 'experimental.evolution=createmarkers,
>allnewcommands'
>+  abort: cannot edit commit information in the middle of a stack
>+  [255]
>+  $ hg metaedit --fold
>+  abort: revisions must be specified with --fold
>+  [255]
>+  $ hg metaedit --user foobar
>+  $ hg log --template '{rev}: {author}\n' -r '42:' --hidden
>+  42: test
>+  43: foobar
>+  $ hg log --template '{rev}: {author}\n' -r .
>+  43: foobar
>+  $ hg status newlyadded
>+  A newlyadded
>+  $ hg resolve --list
>+  U newfile
>+
>+TODO: support this
>+  $ hg metaedit '.^::.'
>+  abort: editing multiple revisions without --fold is not currently
>supported
>+  [255]
>+
>+  $ HGEDITOR=cat hg metaedit '.^::.' --fold
>+  HG: This is a fold of 2 changesets.
>+  HG: Commit message of changeset 41.
>+  
>+  amended
>+  
>+  HG: Commit message of changeset 43.
>+  
>+  will be evolved safely
>+  
>+  
>+  
>+  HG: Enter commit message.  Lines beginning with 'HG:' are removed.
>+  HG: Leave message empty to abort commit.
>+  HG: --
>+  HG: user: test
>+  HG: branch 'default'
>+  HG: changed a
>+  HG: changed newfile
>+  2 changesets folded
>+
>+  $ glog -r .
>+  @  44:41bf1183869c@default(draft) amended
>+  |
>+
>+no new commit is created here because the date is the same
>+  $ HGEDITOR=cat hg metaedit
>+  amended
>+  
>+  
>+  will be evolved safely
>+  
>+  
>+  HG: Enter commit message.  Lines beginning with 'HG:' are removed.
>+  HG: Leave message empty to abort commit.
>+  HG: --
>+  HG: user: test
>+  HG: branch 'default'
>+  HG: changed a
>+  HG: changed newfile
>+  nothing changed
>+
>+  $ glog -r '.^::.'
>+  @  44:41bf1183869c@default(draft) amended
>+  |
>+  o  36:43c3f5ef149f@default(draft) add uu
>+  |
>+
>+'fold' one commit
>+  $ hg metaedit 39 --fold --user foobar2
>+  1 changesets folded
>+  $ hg log -r 45 --template '{rev}: {author}\n'
>+  45: foobar2
>+
>+TODO: don't create a new commit in this case
>+  $ hg metaedit --config defaults.metaedit=
>+  $ hg log -r '.^::.' --template '{rev}: {desc|firstline}\n'
>+  36: add uu
>+  46: amended
>+  $ hg status newlyadded
>+  A newlyadded
>+  $ hg resolve --list
>+  U newfile
>_______________________________________________
>Mercurial-devel mailing list
>Mercurial-devel@mercurial-scm.org
>https://urldefense.proofpoint.com/v2/url?u=https-3A__www.mercurial-2Dscm.o
>rg_mailman_listinfo_mercurial-2Ddevel&d=CwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=
>qmwlQ6ljsf0--v3ANP53-V-RM6PPUtJ5zK5Y1fStJGg&m=oUZ2G_tCET_pgpZBh8g76dcs_txC
>RJ7klaiBEudWgBk&s=2SPWmaswg-nDKtTR6xX6QAEz-ilmhvgyMz0OQPFrouQ&e=
Danek Duvall - March 11, 2016, 10:15 p.m.
Siddharth Agarwal wrote:

> commands: introduce a new command to edit commit metadata
> 
> The eventual goal of this command is to allow editing one or more changesets'
> metadata without being on the particular changeset.
> 
> This implementation is by no means complete, but it does allow folding multiple
> commits into one. 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.

Why is this not simply an enhancement to histedit and amend?

Danek
Siddharth Agarwal - March 11, 2016, 10:20 p.m.
On 3/11/16 14:15, Danek Duvall wrote:
> Why is this not simply an enhancement to histedit and amend?
>
> Danek

Both of those require you to be on the particular commit being edited or 
amended, and both of them do working copy operations. In our experience 
we've found those restrictions to be a 2-6x hit on performance.
Danek Duvall - March 11, 2016, 10:38 p.m.
Siddharth Agarwal wrote:

> On 3/11/16 14:15, Danek Duvall wrote:
> >Why is this not simply an enhancement to histedit and amend?
> >
> >Danek
> 
> Both of those require you to be on the particular commit being edited or
> amended, and both of them do working copy operations. In our experience
> we've found those restrictions to be a 2-6x hit on performance.

I guess my point is that if it's not a backwards-compatibility issue (for
histedit, at least), then why couldn't those subcommands be tweaked a) not
to require being on the changeset in question and b) not do to working copy
operations (if not required)?  Is there any reason anyone would
specifically want one of those things to be true?  If not, why grow the
interface instead of enhancing the existing one?

Danek
Siddharth Agarwal - March 11, 2016, 10:44 p.m.
On 3/11/16 14:38, Danek Duvall wrote:
> I guess my point is that if it's not a backwards-compatibility issue (for
> histedit, at least), then why couldn't those subcommands be tweaked a) not
> to require being on the changeset in question and b) not do to working copy
> operations (if not required)?  Is there any reason anyone would
> specifically want one of those things to be true?  If not, why grow the
> interface instead of enhancing the existing one?

Good point. They probably can, but not without significant evolve support.

The other consideration I'd have is that histedit is also not the most 
usable command from the perspective of automation.

- Siddharth

Patch

diff --git a/hgext/evolve.py b/hgext/evolve.py
--- a/hgext/evolve.py
+++ b/hgext/evolve.py
@@ -2997,6 +2997,127 @@  def fold(ui, repo, *revs, **opts):
     finally:
         lockmod.release(lock, wlock)
 
+@command('^metaedit',
+         [('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@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'))
+
+    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'))
+        # check above ensures only one revision
+        root = head = repo[revs.first()]
+        if root.phase() == phases.public:
+            raise error.Abort(_('cannot edit commit information for public '
+                                'revisions'))
+
+    wlock = lock = None
+    try:
+        wlock = repo.wlock()
+        lock = repo.lock()
+        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: don't create a new commit if there are no non-trivial
+            # changes
+            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.
+            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:
diff --git a/tests/test-evolve.t b/tests/test-evolve.t
--- a/tests/test-evolve.t
+++ b/tests/test-evolve.t
@@ -2,6 +2,7 @@ 
   > [defaults]
   > amend=-d "0 0"
   > fold=-d "0 0"
+  > metaedit=-d "0 0"
   > [web]
   > push_ssl = false
   > allow_push = *
@@ -1449,3 +1450,113 @@  Check that dirstate changes are kept at 
 
   $ hg status newlyadded
   A newlyadded
+
+hg metaedit
+-----------
+
+deliberately leave the working copy with dirty merges so that we know there are
+no updates going on
+  $ hg update .
+  abort: outstanding merge conflicts
+  [255]
+check that metaedit respects allowunstable
+  $ hg metaedit '36 + 42' --fold
+  abort: cannot fold non-linear revisions (multiple roots given)
+  [255]
+  $ hg metaedit '36::39 + 41' --fold
+  abort: cannot fold non-linear revisions (multiple heads given)
+  [255]
+  $ hg metaedit -r 0
+  abort: cannot edit commit information for public revisions
+  [255]
+  $ hg metaedit -r 0 --fold
+  abort: cannot fold public revisions
+  [255]
+  $ hg metaedit '18::20' --fold --config 'experimental.evolution=createmarkers, allnewcommands'
+  abort: cannot fold chain not ending with a head or with branching
+  [255]
+  $ hg metaedit '.^' --config 'experimental.evolution=createmarkers, allnewcommands'
+  abort: cannot edit commit information in the middle of a stack
+  [255]
+  $ hg metaedit --fold
+  abort: revisions must be specified with --fold
+  [255]
+  $ hg metaedit --user foobar
+  $ hg log --template '{rev}: {author}\n' -r '42:' --hidden
+  42: test
+  43: foobar
+  $ hg log --template '{rev}: {author}\n' -r .
+  43: foobar
+  $ hg status newlyadded
+  A newlyadded
+  $ hg resolve --list
+  U newfile
+
+TODO: support this
+  $ hg metaedit '.^::.'
+  abort: editing multiple revisions without --fold is not currently supported
+  [255]
+
+  $ HGEDITOR=cat hg metaedit '.^::.' --fold
+  HG: This is a fold of 2 changesets.
+  HG: Commit message of changeset 41.
+  
+  amended
+  
+  HG: Commit message of changeset 43.
+  
+  will be evolved safely
+  
+  
+  
+  HG: Enter commit message.  Lines beginning with 'HG:' are removed.
+  HG: Leave message empty to abort commit.
+  HG: --
+  HG: user: test
+  HG: branch 'default'
+  HG: changed a
+  HG: changed newfile
+  2 changesets folded
+
+  $ glog -r .
+  @  44:41bf1183869c@default(draft) amended
+  |
+
+no new commit is created here because the date is the same
+  $ HGEDITOR=cat hg metaedit
+  amended
+  
+  
+  will be evolved safely
+  
+  
+  HG: Enter commit message.  Lines beginning with 'HG:' are removed.
+  HG: Leave message empty to abort commit.
+  HG: --
+  HG: user: test
+  HG: branch 'default'
+  HG: changed a
+  HG: changed newfile
+  nothing changed
+
+  $ glog -r '.^::.'
+  @  44:41bf1183869c@default(draft) amended
+  |
+  o  36:43c3f5ef149f@default(draft) add uu
+  |
+
+'fold' one commit
+  $ hg metaedit 39 --fold --user foobar2
+  1 changesets folded
+  $ hg log -r 45 --template '{rev}: {author}\n'
+  45: foobar2
+
+TODO: don't create a new commit in this case
+  $ hg metaedit --config defaults.metaedit=
+  $ hg log -r '.^::.' --template '{rev}: {desc|firstline}\n'
+  36: add uu
+  46: amended
+  $ hg status newlyadded
+  A newlyadded
+  $ hg resolve --list
+  U newfile