Patchwork branch: allow changing branch of merge commits with --rev

login
register
mail settings
Submitter Anton Shestakov
Date Nov. 17, 2018, 2:45 p.m.
Message ID <353bb16507d9824888ba.1542465916@neuro>
Download mbox | patch
Permalink /patch/36633/
State Accepted
Headers show

Comments

Anton Shestakov - Nov. 17, 2018, 2:45 p.m.
# HG changeset patch
# User Anton Shestakov <av6@dwimlabs.net>
# Date 1541743033 -28800
#      Fri Nov 09 13:57:13 2018 +0800
# Node ID 353bb16507d9824888ba0abb4d35a4ed75189303
# Parent  b93157f69f46a1359726be32bde4afb1e5af4384
# EXP-Topic branch-rev
branch: allow changing branch of merge commits with --rev

Tests show that changing branch of merge commits works fine with evolution and
without, so let's allow it. Other safeguards should prevent users from shooting
themselves in the foot.
Yuya Nishihara - Nov. 18, 2018, 2:28 a.m.
On Sat, 17 Nov 2018 22:45:16 +0800, Anton Shestakov wrote:
> # HG changeset patch
> # User Anton Shestakov <av6@dwimlabs.net>
> # Date 1541743033 -28800
> #      Fri Nov 09 13:57:13 2018 +0800
> # Node ID 353bb16507d9824888ba0abb4d35a4ed75189303
> # Parent  b93157f69f46a1359726be32bde4afb1e5af4384
> # EXP-Topic branch-rev
> branch: allow changing branch of merge commits with --rev

Queued, thanks.

> diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
> --- a/mercurial/cmdutil.py
> +++ b/mercurial/cmdutil.py
> @@ -732,11 +732,10 @@ def changebranch(ui, repo, revs, label):
>          rewriteutil.precheck(repo, revs, 'change branch of')
>  
>          root = repo[roots.first()]
> -        if not root.p1().branch() == label and label in repo.branchmap():
> +        rpb = {parent.branch() for parent in root.parents()}
> +        if label not in rpb and label in repo.branchmap():
>              raise error.Abort(_("a branch of the same name already exists"))

Generally a merge commit inherits the p1 branch, but it's allowed to change
the wdir branch to p2 while merging. So it should be fine to allow using
p2 branch here.

  $ hg merge
  $ hg branch <p2-branch>  # I didn't know this is allowed
Pulkit Goyal - Nov. 19, 2018, 12:27 p.m.
On Sun, Nov 18, 2018 at 5:37 AM Yuya Nishihara <yuya@tcha.org> wrote:

> On Sat, 17 Nov 2018 22:45:16 +0800, Anton Shestakov wrote:
> > # HG changeset patch
> > # User Anton Shestakov <av6@dwimlabs.net>
> > # Date 1541743033 -28800
> > #      Fri Nov 09 13:57:13 2018 +0800
> > # Node ID 353bb16507d9824888ba0abb4d35a4ed75189303
> > # Parent  b93157f69f46a1359726be32bde4afb1e5af4384
> > # EXP-Topic branch-rev
> > branch: allow changing branch of merge commits with --rev
>

Just for the record, I now think that having `branch --rev <rev>` change
branch of a rev is not a good UI.
Yuya Nishihara - Nov. 19, 2018, 12:58 p.m.
On Mon, 19 Nov 2018 15:27:33 +0300, Pulkit Goyal wrote:
> On Sun, Nov 18, 2018 at 5:37 AM Yuya Nishihara <yuya@tcha.org> wrote:
> 
> > On Sat, 17 Nov 2018 22:45:16 +0800, Anton Shestakov wrote:
> > > # HG changeset patch
> > > # User Anton Shestakov <av6@dwimlabs.net>
> > > # Date 1541743033 -28800
> > > #      Fri Nov 09 13:57:13 2018 +0800
> > > # Node ID 353bb16507d9824888ba0abb4d35a4ed75189303
> > > # Parent  b93157f69f46a1359726be32bde4afb1e5af4384
> > > # EXP-Topic branch-rev
> > > branch: allow changing branch of merge commits with --rev
> 
> Just for the record, I now think that having `branch --rev <rev>` change
> branch of a rev is not a good UI.

I agree. It's more like amend or rebase, than branch.

Patch

diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
--- a/mercurial/cmdutil.py
+++ b/mercurial/cmdutil.py
@@ -732,11 +732,10 @@  def changebranch(ui, repo, revs, label):
         rewriteutil.precheck(repo, revs, 'change branch of')
 
         root = repo[roots.first()]
-        if not root.p1().branch() == label and label in repo.branchmap():
+        rpb = {parent.branch() for parent in root.parents()}
+        if label not in rpb and label in repo.branchmap():
             raise error.Abort(_("a branch of the same name already exists"))
 
-        if repo.revs('merge() and %ld', revs):
-            raise error.Abort(_("cannot change branch of a merge commit"))
         if repo.revs('obsolete() and %ld', revs):
             raise error.Abort(_("cannot change branch of a obsolete changeset"))
 
diff --git a/tests/test-branch-change.t b/tests/test-branch-change.t
--- a/tests/test-branch-change.t
+++ b/tests/test-branch-change.t
@@ -308,24 +308,116 @@  revs is same as the new branch name
   o  18:204d2769eca2 Added a
      stable ()
 
-Testing on merge
+Changing branch of a merge commit
 
-  $ hg merge -r 26
-  1 files updated, 0 files merged, 0 files removed, 0 files unresolved
+  $ hg branch -q ghi
+  $ echo f > f
+  $ hg ci -qAm 'Added f'
+  $ hg up -q 27
+  $ hg branch -q jkl
+  $ echo g > g
+  $ hg ci -qAm 'Added g'
+  $ hg glog -r 'heads(:)'
+  @  29:6bc1c6c2c9da Added g
+  |  jkl ()
+  ~
+  o  28:2f1019bd29d2 Added f
+  |  ghi (b1)
+  ~
+
+  $ hg branch -q default
+  $ hg merge -r 28
+  4 files updated, 0 files merged, 0 files removed, 0 files unresolved
   (branch merge, don't forget to commit)
-
   $ hg branch -r . abcd
   abort: outstanding uncommitted merge
   [255]
+
   $ hg ci -m "Merge commit"
-  $ hg branch -r '(.^)::' def
-  abort: cannot change branch of a merge commit
+  $ hg glog -r 'parents(.)::'
+  @    30:4d56e6b1eb6b Merge commit
+  |\   default ()
+  | o  29:6bc1c6c2c9da Added g
+  | |  jkl ()
+  | ~
+  o  28:2f1019bd29d2 Added f
+  |  ghi (b1)
+  ~
+
+  $ hg branch -r . ghi
+  0 files updated, 0 files merged, 4 files removed, 0 files unresolved
+  changed branch on 1 changesets
+  $ hg branch -r . jkl
+  changed branch on 1 changesets
+  $ hg branch -r . default
+  changed branch on 1 changesets
+  $ hg branch -r . stable
+  abort: a branch of the same name already exists
   [255]
 
 Changing branch on public changeset
 
-  $ hg phase -r 27 -p
-  $ hg branch -r 27 def
+  $ hg phase -r . -p
+  $ hg branch -r . def
   abort: cannot change branch of public changesets
   (see 'hg help phases' for details)
   [255]
+
+Merge commit with conflicts, with evolution and without
+
+  $ mklozenge() {
+  >   echo foo > a
+  >   hg ci -qAm foo
+  >   echo bar > a
+  >   hg ci -qm bar
+  >   hg up -q '.^'
+  >   echo baz > a
+  >   hg ci -qm baz
+  >   hg merge -q -t :local
+  >   echo neither > a
+  >   hg ci -qm neither
+  > }
+
+  $ cd ..
+  $ hg init merge-with-evolution
+  $ cd merge-with-evolution
+  $ mklozenge
+
+  $ hg branch -r '(.^)::' abc
+  changed branch on 2 changesets
+  $ hg glog
+  @    5:c07fa8b34d54 neither
+  |\   abc ()
+  | o  4:f2aa51777cc9 baz
+  | |  abc ()
+  o |  1:2e33c4f0856b bar
+  |/   default ()
+  o  0:91cfb6004abf foo
+     default ()
+  $ hg cat a
+  neither
+
+  $ cd ..
+  $ hg init merge-without-evolution
+  $ cd merge-without-evolution
+  $ mklozenge
+  $ cat > .hg/hgrc << EOF
+  > [experimental]
+  > evolution = no
+  > evolution.allowunstable = no
+  > EOF
+
+  $ hg branch -r '(.^)::' abc
+  changed branch on 2 changesets
+  saved backup bundle to $TESTTMP/merge-without-evolution/.hg/strip-backup/9a3a2af368f4-8db1a361-branch-change.hg
+  $ hg glog
+  @    3:c07fa8b34d54 neither
+  |\   abc ()
+  | o  2:f2aa51777cc9 baz
+  | |  abc ()
+  o |  1:2e33c4f0856b bar
+  |/   default ()
+  o  0:91cfb6004abf foo
+     default ()
+  $ hg cat a
+  neither