Patchwork backout: add --commit option

login
register
mail settings
Submitter Mateusz Kwapich
Date Dec. 18, 2014, 1:39 a.m.
Message ID <aa4f1fa1bab5dca4848e.1418866765@dev1429.prn1.facebook.com>
Download mbox | patch
Permalink /patch/7160/
State Accepted
Headers show

Comments

Mateusz Kwapich - Dec. 18, 2014, 1:39 a.m.
# HG changeset patch
# User Mateusz Kwapich <mitrandir@fb.com>
# Date 1418865972 28800
#      Wed Dec 17 17:26:12 2014 -0800
# Node ID aa4f1fa1bab5dca4848eeaea36ca5485994fc4f2
# Parent  495bc1b65d25872324a0220354f048b220304bd1
backout: add --commit option

Mercurial backout command makes a commmit by default only when the backed out
revision is the parent of working directory and doesn't commit in any other
case.

The --commit option changes behaviour of backout to make a commit whenever
possible (i.e. there is no unresolved conflicts). This behaviour seems more
intuitive to many use (especially git users migrating to hg).
Pierre-Yves David - Dec. 18, 2014, 1:40 a.m.
On 12/17/2014 05:39 PM, Mateusz Kwapich wrote:
> # HG changeset patch
> # User Mateusz Kwapich <mitrandir@fb.com>
> # Date 1418865972 28800
> #      Wed Dec 17 17:26:12 2014 -0800
> # Node ID aa4f1fa1bab5dca4848eeaea36ca5485994fc4f2
> # Parent  495bc1b65d25872324a0220354f048b220304bd1
> backout: add --commit option
>
> Mercurial backout command makes a commmit by default only when the backed out
> revision is the parent of working directory and doesn't commit in any other
> case.
>
> The --commit option changes behaviour of backout to make a commit whenever
> possible (i.e. there is no unresolved conflicts). This behaviour seems more
> intuitive to many use (especially git users migrating to hg).

+1 I was looking for such flag a few hours ago.
Katsunori FUJIWARA - Dec. 20, 2014, 12:31 p.m.
At Wed, 17 Dec 2014 17:40:22 -0800,
Pierre-Yves David wrote:
> 
> 
> 
> On 12/17/2014 05:39 PM, Mateusz Kwapich wrote:
> > # HG changeset patch
> > # User Mateusz Kwapich <mitrandir@fb.com>
> > # Date 1418865972 28800
> > #      Wed Dec 17 17:26:12 2014 -0800
> > # Node ID aa4f1fa1bab5dca4848eeaea36ca5485994fc4f2
> > # Parent  495bc1b65d25872324a0220354f048b220304bd1
> > backout: add --commit option
> >
> > Mercurial backout command makes a commmit by default only when the backed out
> > revision is the parent of working directory and doesn't commit in any other
> > case.
> >
> > The --commit option changes behaviour of backout to make a commit whenever
> > possible (i.e. there is no unresolved conflicts). This behaviour seems more
> > intuitive to many use (especially git users migrating to hg).
> 
> +1 I was looking for such flag a few hours ago.

I was just asked by an user why "hg merge" doesn't have the
functionality to commit soon after successful merging.

I answered that:

  - Mercurial policy is "one functionality for one command"

    "branch merge, don't forget to commit" reminder message after "hg
    merge" should navigate users, too.

  - the result of merging should be confirmed before committing

    "successful merging" ensures that file contents are merged
    mechanically (if using "internal:merge") without conflicts, but
    not that they are merged correctly in semantics view.
    
  - TortoiseHg can navigate users to commit just after merging, if no
    conflict is detected.

I myself like current separation between "merge" and "commit". But it
is fact that some users want "hg merge" to commit automatically if no
conflict is detected (may "--commit" of "git merge" make this trend ?)

I listed up revision creation and internal merging of each commands
for reference.

  - commands creating single revision

    - "backout" for other than current parent (regardless of
      "--merge"), "merge"

      These imply (internal) merging, and user should "hg commit"
      manually.

      Mateusz plans allowing "backout" without "--merge" to commit new
      revision in this case.

    - "backout" for current parent, "tag"

      These don't imply internal merging, and new revision is created
      automatically.

  - commands creating multiple revisions

    - "bundle", "import", "pull"

      These don't imply internal merging, and new revisions are
      created automatically.

    - "graft", "histedit", "rebase", "transplant" without "--merge"

      These imply internal merging, but new revisions are committed
      automatically if no conflict is detected.

    - "fetch" for another anonymous head

      This implies (internal) merging. Merging revision is committed
      automatically, if no conflict is detected.

    - "transplant" with "--merge"

      This doesn't imply internal merging, even though merging
      revision is created: files in the working directory are changed
      by patch-ing. And new revisions are created automatically.


At first, I don't disagree with adding "--commit" itself. I just want
to clarify about command policy around (internal) merging before
adding it.

Regardless of "--merge" option, "backout" should create same
(internal) merging result from the point of view of file contents,
shouldn't it ?

Then, are there any reason that merging result isn't automatically
committed in the case of "backout --merge --commit" ? because of
"creating new topological branch" ?  (or just forgot ? > Mateusz)

If "backout --merge --commit" is allowed to commit merging revision
automatically, what about adding "--commit" to "hg merge" ?
Correctness of internal merging shouldn't be so different between
"backout --merge" and "merge".


BTW, it seems to be reasonable that "automated committing" commands
commit revisions merging internally without user interactions, because
user interactions decrease convenience of automation, even though
internal merging doesn't ensure correctness in semantics view also in
this case :-)

> -- 
> Pierre-Yves David
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel

----------------------------------------------------------------------
[FUJIWARA Katsunori]                             foozy@lares.dti.ne.jp
Matt Mackall - Dec. 21, 2014, 10:32 p.m.
On Sat, 2014-12-20 at 21:31 +0900, FUJIWARA Katsunori wrote:
> At Wed, 17 Dec 2014 17:40:22 -0800,
> Pierre-Yves David wrote:
> > 
> > 
> > 
> > On 12/17/2014 05:39 PM, Mateusz Kwapich wrote:
> > > # HG changeset patch
> > > # User Mateusz Kwapich <mitrandir@fb.com>
> > > # Date 1418865972 28800
> > > #      Wed Dec 17 17:26:12 2014 -0800
> > > # Node ID aa4f1fa1bab5dca4848eeaea36ca5485994fc4f2
> > > # Parent  495bc1b65d25872324a0220354f048b220304bd1
> > > backout: add --commit option
> > >
> > > Mercurial backout command makes a commmit by default only when the backed out
> > > revision is the parent of working directory and doesn't commit in any other
> > > case.
> > >
> > > The --commit option changes behaviour of backout to make a commit whenever
> > > possible (i.e. there is no unresolved conflicts). This behaviour seems more
> > > intuitive to many use (especially git users migrating to hg).
> > 
> > +1 I was looking for such flag a few hours ago.
> 
> I was just asked by an user why "hg merge" doesn't have the
> functionality to commit soon after successful merging.
> 
> I answered that:
> 
>   - Mercurial policy is "one functionality for one command"
> 
>     "branch merge, don't forget to commit" reminder message after "hg
>     merge" should navigate users, too.
> 
>   - the result of merging should be confirmed before committing
> 
>     "successful merging" ensures that file contents are merged
>     mechanically (if using "internal:merge") without conflicts, but
>     not that they are merged correctly in semantics view.
>     
>   - TortoiseHg can navigate users to commit just after merging, if no
>     conflict is detected.
> 
> I myself like current separation between "merge" and "commit". But it
> is fact that some users want "hg merge" to commit automatically if no
> conflict is detected (may "--commit" of "git merge" make this trend ?)
> 
> I listed up revision creation and internal merging of each commands
> for reference.
> 
>   - commands creating single revision
> 
>     - "backout" for other than current parent (regardless of
>       "--merge"), "merge"
> 
>       These imply (internal) merging, and user should "hg commit"
>       manually.
> 
>       Mateusz plans allowing "backout" without "--merge" to commit new
>       revision in this case.
> 
>     - "backout" for current parent, "tag"
> 
>       These don't imply internal merging, and new revision is created
>       automatically.
> 
>   - commands creating multiple revisions
> 
>     - "bundle", "import", "pull"
> 
>       These don't imply internal merging, and new revisions are
>       created automatically.
> 
>     - "graft", "histedit", "rebase", "transplant" without "--merge"
> 
>       These imply internal merging, but new revisions are committed
>       automatically if no conflict is detected.
> 
>     - "fetch" for another anonymous head
> 
>       This implies (internal) merging. Merging revision is committed
>       automatically, if no conflict is detected.
> 
>     - "transplant" with "--merge"
> 
>       This doesn't imply internal merging, even though merging
>       revision is created: files in the working directory are changed
>       by patch-ing. And new revisions are created automatically.
> 
> 
> At first, I don't disagree with adding "--commit" itself. I just want
> to clarify about command policy around (internal) merging before
> adding it.
> 
> Regardless of "--merge" option, "backout" should create same
> (internal) merging result from the point of view of file contents,
> shouldn't it ?
> 
> Then, are there any reason that merging result isn't automatically
> committed in the case of "backout --merge --commit" ? because of
> "creating new topological branch" ?  (or just forgot ? > Mateusz)
> 
> If "backout --merge --commit" is allowed to commit merging revision
> automatically, what about adding "--commit" to "hg merge" ?
> Correctness of internal merging shouldn't be so different between
> "backout --merge" and "merge".
> 
> 
> BTW, it seems to be reasonable that "automated committing" commands
> commit revisions merging internally without user interactions, because
> user interactions decrease convenience of automation, even though
> internal merging doesn't ensure correctness in semantics view also in
> this case :-)

The history is that there were originally three backout cases:

a) backout a head -> revert + automatic commit
b) backout of non-head -> revert + automatic commit, manual merge needed
c) (b) + --merge -> as above + merge, commit of merge still needed

So we always had one automatic commit. But merge is tricky because of
conflicts. At the time backout was introduced, there was no precedent
for --continue/--abort, so we punted.

Later, we changed (b) to do:

b') backout of non-head > update backwards, revert, update forward

This drops the automatic (branching) commit, and gives us a nice linear
history. And it basically makes --merge irrelevant. So we're down to two
cases.

There's a pretty strong argument for reducing it to one case by adding
automatic commit on successful merge and treating backout of a head as a
degenerate case of a non-head.

As you say, we do have some precedent in graft for automatically
committing some types of merges.. but I definitely wouldn't want to
entertain the idea of merge --commit since it's so hard to fix bad
merges.

But I'm fine with adding a --commit today and maybe making it an
invisible default later.
Matt Mackall - Dec. 22, 2014, 1:51 a.m.
On Wed, 2014-12-17 at 17:39 -0800, Mateusz Kwapich wrote:
> # HG changeset patch
> # User Mateusz Kwapich <mitrandir@fb.com>
> # Date 1418865972 28800
> #      Wed Dec 17 17:26:12 2014 -0800
> # Node ID aa4f1fa1bab5dca4848eeaea36ca5485994fc4f2
> # Parent  495bc1b65d25872324a0220354f048b220304bd1
> backout: add --commit option

This is queued for default, thanks.

Patch

diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -425,13 +425,14 @@ 
 
 @command('backout',
     [('', 'merge', None, _('merge with old dirstate parent after backout')),
+    ('', 'commit', None, _('always commit if the backout was successful')),
     ('', 'parent', '',
      _('parent to choose when backing out merge (DEPRECATED)'), _('REV')),
     ('r', 'rev', '', _('revision to backout'), _('REV')),
     ('e', 'edit', False, _('invoke editor on commit messages')),
     ] + mergetoolopts + walkopts + commitopts + commitopts2,
     _('[OPTION]... [-r] REV'))
-def backout(ui, repo, node=None, rev=None, **opts):
+def backout(ui, repo, node=None, rev=None, commit=False, **opts):
     '''reverse effect of earlier changeset
 
     Prepare a new changeset with the effect of REV undone in the
@@ -519,11 +520,12 @@ 
                 if stats[3]:
                     repo.ui.status(_("use 'hg resolve' to retry unresolved "
                                      "file merges\n"))
-                else:
+                    return 1
+                elif not commit:
                     msg = _("changeset %s backed out, "
                             "don't forget to commit.\n")
                     ui.status(msg % short(node))
-                return stats[3] > 0
+                    return 0
             finally:
                 ui.setconfig('ui', 'forcemerge', '', '')
         else:
diff --git a/tests/test-backout.t b/tests/test-backout.t
--- a/tests/test-backout.t
+++ b/tests/test-backout.t
@@ -43,6 +43,47 @@ 
   commit: (clean)
   update: (current)
 
+commit option
+
+  $ cd ..
+  $ hg init commit
+  $ cd commit
+
+  $ echo tomatoes > a
+  $ hg add a
+  $ hg commit -d '0 0' -m tomatoes
+
+  $ echo chair > b
+  $ hg add b
+  $ hg commit -d '1 0' -m chair
+
+  $ echo grapes >> a
+  $ hg commit -d '2 0' -m grapes
+
+  $ hg backout --commit -d '4 0' 1 --tool=:fail
+  0 files updated, 0 files merged, 1 files removed, 0 files unresolved
+  changeset 3:1c2161e97c0a backs out changeset 1:22cb4f70d813
+  $ hg summary
+  parent: 3:1c2161e97c0a tip
+   Backed out changeset 22cb4f70d813
+  branch: default
+  commit: (clean)
+  update: (current)
+
+  $ echo ypples > a
+  $ hg commit -d '5 0' -m ypples
+
+  $ hg backout --commit -d '6 0' 2 --tool=:fail
+  0 files updated, 0 files merged, 0 files removed, 1 files unresolved
+  use 'hg resolve' to retry unresolved file merges
+  [1]
+  $ hg summary
+  parent: 4:ed99997b793d tip
+   ypples
+  branch: default
+  commit: 1 unresolved (clean)
+  update: (current)
+
 file that was removed is recreated
 (this also tests that editor is not invoked if the commit message is
 specified explicitly)
diff --git a/tests/test-completion.t b/tests/test-completion.t
--- a/tests/test-completion.t
+++ b/tests/test-completion.t
@@ -217,7 +217,7 @@ 
   update: clean, check, date, rev, tool
   addremove: similarity, subrepos, include, exclude, dry-run
   archive: no-decode, prefix, rev, type, subrepos, include, exclude
-  backout: merge, parent, rev, edit, tool, include, exclude, message, logfile, date, user
+  backout: merge, commit, parent, rev, edit, tool, include, exclude, message, logfile, date, user
   bisect: reset, good, bad, skip, extend, command, noupdate
   bookmarks: force, rev, delete, rename, inactive, template
   branch: force, clean