Patchwork merge.graft: add option to keep second parent

login
register
mail settings
Submitter Andrew Halberstadt
Date Dec. 4, 2015, 4:57 a.m.
Message ID <212f93c2367203cadfd6.1449205065@localhost.localdomain>
Download mbox | patch
Permalink /patch/11798/
State Accepted
Headers show

Comments

Andrew Halberstadt - Dec. 4, 2015, 4:57 a.m.
# HG changeset patch
# User Andrew Halberstadt <ahalberstadt@mozilla.com>
# Date 1449201719 18000
#      Thu Dec 03 23:01:59 2015 -0500
# Node ID 212f93c2367203cadfd6103f444899929b8643ae
# Parent  71aa5a26162d6e4a165b68f07b331e3e2eedc117
merge.graft: add option to keep second parent

Currently merge.graft re-writes the dirstate so only a single
parent is kept. For some cases, like evolving a merge commit,
this behaviour is not desired. More specifically, this is
needed to fix issue4389.
Augie Fackler - Dec. 4, 2015, 7:52 p.m.
On Thu, Dec 03, 2015 at 11:57:45PM -0500, Andrew Halberstadt wrote:
> # HG changeset patch
> # User Andrew Halberstadt <ahalberstadt@mozilla.com>
> # Date 1449201719 18000
> #      Thu Dec 03 23:01:59 2015 -0500
> # Node ID 212f93c2367203cadfd6103f444899929b8643ae
> # Parent  71aa5a26162d6e4a165b68f07b331e3e2eedc117
> merge.graft: add option to keep second parent

Queued, thanks. Congratulations on your first hg patch.

>
> Currently merge.graft re-writes the dirstate so only a single
> parent is kept. For some cases, like evolving a merge commit,
> this behaviour is not desired. More specifically, this is
> needed to fix issue4389.
>
> diff --git a/mercurial/merge.py b/mercurial/merge.py
> --- a/mercurial/merge.py
> +++ b/mercurial/merge.py
> @@ -1484,41 +1484,47 @@ def update(repo, node, branchmerge, forc
>              repo.dirstate.endparentchange()
>      finally:
>          wlock.release()
>
>      if not partial:
>          repo.hook('update', parent1=xp1, parent2=xp2, error=stats[3])
>      return stats
>
> -def graft(repo, ctx, pctx, labels):
> +def graft(repo, ctx, pctx, labels, keepparent=False):
>      """Do a graft-like merge.
>
>      This is a merge where the merge ancestor is chosen such that one
>      or more changesets are grafted onto the current changeset. In
>      addition to the merge, this fixes up the dirstate to include only
> -    a single parent and tries to duplicate any renames/copies
> -    appropriately.
> +    a single parent (if keepparent is False) and tries to duplicate any
> +    renames/copies appropriately.
>
>      ctx - changeset to rebase
>      pctx - merge base, usually ctx.p1()
>      labels - merge labels eg ['local', 'graft']
> +    keepparent - keep second parent if any
>
>      """
>      # If we're grafting a descendant onto an ancestor, be sure to pass
>      # mergeancestor=True to update. This does two things: 1) allows the merge if
>      # the destination is the same as the parent of the ctx (so we can use graft
>      # to copy commits), and 2) informs update that the incoming changes are
>      # newer than the destination so it doesn't prompt about "remote changed foo
>      # which local deleted".
>      mergeancestor = repo.changelog.isancestor(repo['.'].node(), ctx.node())
>
>      stats = update(repo, ctx.node(), True, True, False, pctx.node(),
>                     mergeancestor=mergeancestor, labels=labels)
>
> -    # drop the second merge parent
> +    pother = nullid
> +    parents = ctx.parents()
> +    if keepparent and len(parents) == 2 and pctx in parents:
> +        parents.remove(pctx)
> +        pother = parents[0].node()
> +
>      repo.dirstate.beginparentchange()
> -    repo.setparents(repo['.'].node(), nullid)
> +    repo.setparents(repo['.'].node(), pother)
>      repo.dirstate.write(repo.currenttransaction())
>      # fix up dirstate for copies and renames
>      copies.duplicatecopies(repo, ctx.rev(), pctx.rev())
>      repo.dirstate.endparentchange()
>      return stats
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> https://selenic.com/mailman/listinfo/mercurial-devel
Pierre-Yves David - Dec. 5, 2015, 1:38 a.m.
On 12/04/2015 11:52 AM, Augie Fackler wrote:
> On Thu, Dec 03, 2015 at 11:57:45PM -0500, Andrew Halberstadt wrote:
>> # HG changeset patch
>> # User Andrew Halberstadt <ahalberstadt@mozilla.com>
>> # Date 1449201719 18000
>> #      Thu Dec 03 23:01:59 2015 -0500
>> # Node ID 212f93c2367203cadfd6103f444899929b8643ae
>> # Parent  71aa5a26162d6e4a165b68f07b331e3e2eedc117
>> merge.graft: add option to keep second parent
>
> Queued, thanks. Congratulations on your first hg patch.

Actually, I'm a bit confused about how just keeping the parent will be 
enough to the solve the problem we are trying to solve.

So I would like to look at it to double check its logic and how it is 
used in Evolve.
Augie Fackler - Dec. 5, 2015, 1:39 a.m.
> On Dec 4, 2015, at 8:38 PM, Pierre-Yves David <pierre-yves.david@ens-lyon.org> wrote:
> 
> 
> 
> On 12/04/2015 11:52 AM, Augie Fackler wrote:
>> On Thu, Dec 03, 2015 at 11:57:45PM -0500, Andrew Halberstadt wrote:
>>> # HG changeset patch
>>> # User Andrew Halberstadt <ahalberstadt@mozilla.com>
>>> # Date 1449201719 18000
>>> #      Thu Dec 03 23:01:59 2015 -0500
>>> # Node ID 212f93c2367203cadfd6103f444899929b8643ae
>>> # Parent  71aa5a26162d6e4a165b68f07b331e3e2eedc117
>>> merge.graft: add option to keep second parent
>> 
>> Queued, thanks. Congratulations on your first hg patch.
> 
> Actually, I'm a bit confused about how just keeping the parent will be enough to the solve the problem we are trying to solve.
> 
> So I would like to look at it to double check its logic and how it is used in Evolve.

Fair enough. Feel free to drop it from the clowncopter and reinstate it in patchwork if you like.

AF

> 
> --
> Pierre-Yves David
Andrew Halberstadt - Dec. 5, 2015, 1:41 a.m.
On 04/12/15 08:38 PM, Pierre-Yves David wrote:
>
>
> On 12/04/2015 11:52 AM, Augie Fackler wrote:
>> On Thu, Dec 03, 2015 at 11:57:45PM -0500, Andrew Halberstadt wrote:
>>> # HG changeset patch
>>> # User Andrew Halberstadt <ahalberstadt@mozilla.com>
>>> # Date 1449201719 18000
>>> #      Thu Dec 03 23:01:59 2015 -0500
>>> # Node ID 212f93c2367203cadfd6103f444899929b8643ae
>>> # Parent  71aa5a26162d6e4a165b68f07b331e3e2eedc117
>>> merge.graft: add option to keep second parent
>>
>> Queued, thanks. Congratulations on your first hg patch.
>
> Actually, I'm a bit confused about how just keeping the parent will be
> enough to the solve the problem we are trying to solve.
>
> So I would like to look at it to double check its logic and how it is
> used in Evolve.

I just submitted the evolve portion of this patch. Please assume I have 
no idea what I'm doing :).
Pierre-Yves David - Dec. 7, 2015, 6:17 p.m.
On 12/04/2015 05:41 PM, Andrew Halberstadt wrote:
> On 04/12/15 08:38 PM, Pierre-Yves David wrote:
>>
>>
>> On 12/04/2015 11:52 AM, Augie Fackler wrote:
>>> On Thu, Dec 03, 2015 at 11:57:45PM -0500, Andrew Halberstadt wrote:
>>>> # HG changeset patch
>>>> # User Andrew Halberstadt <ahalberstadt@mozilla.com>
>>>> # Date 1449201719 18000
>>>> #      Thu Dec 03 23:01:59 2015 -0500
>>>> # Node ID 212f93c2367203cadfd6103f444899929b8643ae
>>>> # Parent  71aa5a26162d6e4a165b68f07b331e3e2eedc117
>>>> merge.graft: add option to keep second parent
>>>
>>> Queued, thanks. Congratulations on your first hg patch.
>>
>> Actually, I'm a bit confused about how just keeping the parent will be
>> enough to the solve the problem we are trying to solve.
>>
>> So I would like to look at it to double check its logic and how it is
>> used in Evolve.
>
> I just submitted the evolve portion of this patch. Please assume I have
> no idea what I'm doing :).

This patch is actually pretty good, nice work.
It is pushed to the clowncopter, (again)

--
Pierre-Yves David

Patch

diff --git a/mercurial/merge.py b/mercurial/merge.py
--- a/mercurial/merge.py
+++ b/mercurial/merge.py
@@ -1484,41 +1484,47 @@  def update(repo, node, branchmerge, forc
             repo.dirstate.endparentchange()
     finally:
         wlock.release()
 
     if not partial:
         repo.hook('update', parent1=xp1, parent2=xp2, error=stats[3])
     return stats
 
-def graft(repo, ctx, pctx, labels):
+def graft(repo, ctx, pctx, labels, keepparent=False):
     """Do a graft-like merge.
 
     This is a merge where the merge ancestor is chosen such that one
     or more changesets are grafted onto the current changeset. In
     addition to the merge, this fixes up the dirstate to include only
-    a single parent and tries to duplicate any renames/copies
-    appropriately.
+    a single parent (if keepparent is False) and tries to duplicate any
+    renames/copies appropriately.
 
     ctx - changeset to rebase
     pctx - merge base, usually ctx.p1()
     labels - merge labels eg ['local', 'graft']
+    keepparent - keep second parent if any
 
     """
     # If we're grafting a descendant onto an ancestor, be sure to pass
     # mergeancestor=True to update. This does two things: 1) allows the merge if
     # the destination is the same as the parent of the ctx (so we can use graft
     # to copy commits), and 2) informs update that the incoming changes are
     # newer than the destination so it doesn't prompt about "remote changed foo
     # which local deleted".
     mergeancestor = repo.changelog.isancestor(repo['.'].node(), ctx.node())
 
     stats = update(repo, ctx.node(), True, True, False, pctx.node(),
                    mergeancestor=mergeancestor, labels=labels)
 
-    # drop the second merge parent
+    pother = nullid
+    parents = ctx.parents()
+    if keepparent and len(parents) == 2 and pctx in parents:
+        parents.remove(pctx)
+        pother = parents[0].node()
+
     repo.dirstate.beginparentchange()
-    repo.setparents(repo['.'].node(), nullid)
+    repo.setparents(repo['.'].node(), pother)
     repo.dirstate.write(repo.currenttransaction())
     # fix up dirstate for copies and renames
     copies.duplicatecopies(repo, ctx.rev(), pctx.rev())
     repo.dirstate.endparentchange()
     return stats