[PATCH 1 of 2] graft: allow creating sibling grafts

Matt Mackall mpm at selenic.com
Mon Apr 6 15:29:13 CDT 2015


On Sun, 2015-04-05 at 19:24 -0700, Durham Goode wrote:
> # HG changeset patch
> # User Durham Goode <durham at fb.com>
> # Date 1428260138 25200
> #      Sun Apr 05 11:55:38 2015 -0700
> # Node ID 409103499e22860c64bd334e671753dfcf0072a6
> # Parent  4a4018831d2ebc3c9cae9c6613e6a2497b4f0993
> graft: allow creating sibling grafts
> 
> Previously it was impossible to graft a commit onto it's own parent (i.e. create
> a copy of the commit). This is useful when wanting to create a backup of the
> commit before continuing to amend it. This patch enables that behavior.

I'm about +0 on these two patches from functionality level, but I'm
finding the patch itself confusing.

> diff --git a/mercurial/merge.py b/mercurial/merge.py
> --- a/mercurial/merge.py
> +++ b/mercurial/merge.py
> @@ -1184,12 +1184,14 @@ def graft(repo, ctx, pctx, labels):
>      labels - merge labels eg ['local', 'graft']
>  
>      """
> +    wpctx = repo['.']
> +    mergeancestor = repo.changelog.isancestor(wpctx.node(), ctx.node())
> +    stats = update(repo, ctx.node(), True, True, False, pctx.node(),
> +                   mergeancestor=mergeancestor, labels=labels)

So.. we should allow merging with an ancestor whenever the target is an
ancestor? If that's actually the case, then this patch should be a
single line adding mergeancestor=True. But the only case we've described
so far is just when the target is p1(source).

(I think we should in fact be able to graft patches onto ancestors. For
instance, grafting a bug fix back on to the not-yet-diverged release
point, but am less motivated by the particular abuse^Wuse case you have
in mind. Similarly, I think there's value in tracking repeated grafts.)

>      repo.dirstate.beginparentchange()
> -    repo.setparents(repo['.'].node(), nullid)
> +    repo.setparents(wpctx.node(), nullid)

Why is this changing? Is it just a clean-up because we've made wpctx a
variable above? If so, it actually obscures the intent a bit. We just
want to drop the second parent _of the current state_, and it's not
immediately obvious to someone reading the code that wpctx is still
relevant after an update().

-- 
Mathematics is the supreme nostalgia of our time.




More information about the Mercurial-devel mailing list