[PATCH 1 of 2 v8] graft: support grafting across move/copy (issue4028)

Gábor STEFANIK Gabor.STEFANIK at nng.com
Thu Sep 15 06:06:58 EDT 2016


>


--------------------------------------------------------------------------
This message, including its attachments, is confidential. For more information please read NNG's email policy here:
http://www.nng.com/emailpolicy/
By responding to this email you accept the email policy.


-----Original Message-----
> From: Yuya Nishihara [mailto:youjah at gmail.com] On Behalf Of Yuya
> Nishihara
> Sent: Wednesday, September 14, 2016 6:10 PM
> To: Gábor STEFANIK <Gabor.STEFANIK at nng.com>
> Cc: mercurial-devel at mercurial-scm.org
> Subject: Re: [PATCH 1 of 2 v8] graft: support grafting across move/copy
> (issue4028)
>
> On Mon, 12 Sep 2016 17:25:45 +0000, Gábor STEFANIK wrote:
> > > I thought callers could compute ca and cta by themselves since they
> > > explicitly pass a pseudo ca, but maybe I'm wrong. If that's already
> > > been discussed, there's no reason to complain. Sorry for the noise.
> >
> > In certain "hg update" scenarios, mergemod.update may be called with
> > ancestor=None, and we can still end up with a fake ca (in a completely
> > sane way). Also, there are quite a few extensions both in and out of
> > tree, that call mergemod.update with or without an explicit ca, which may
> or may not be a real common ancestor.
> >
> > Then there is also the case when "ca" is a common ancestor of both c1
> > and c2, but ca != c1.ancestor(c2) This case is an ungraftlike merge, and
> should not use the graft logic. It can happen e.g. due to bidmerge.
> >
> > Finally, there are cases of "hg graft" where ca==cta, and thus a graft
> > command can involve an ungraftlike merge. There is no way for the caller
> to detect this, except by calling descendant().
>
> Great explanation, many thanks. I didn't know the bidmerge case.
>
> > > I'm not sure if this is a relevant example, but given the following
> > > history, I got different results by a) "graft 6 to 10" and b) "graft
> > > 7 to 10", which seems suspicious.
> > >
> > > @  10: a
> > > |
> > > | +  9: c
> > > | |
> > > | +  8: d->()
> > > | |
> > > | | +  7: d
> > > | |/
> > > | | +  6: c
> > > | |/
> > > | +    5: c,d
> > > | |\
> > > | | +  4: b->d
> > > | | |
> > > | + |  3: b->c
> > > | |/
> > > | +  2: a->b
> > > |/
> > > o  1: a
> > > |
> > > o  0: ()->a
>
> [snip]
>
> > This looks like a separate (though related) issue to me. Turning the DAG
> around causes "a" to be both "moved from c" and "moved from d", a ypoc-
> like scenario. The problem is, mergecopies' callers expect the "copy" return
> value to be a many-to-one mapping of targets to sources, when it's actually
> many-to-many in this scenario. Fixable, but I would rather handle this in a
> followup issue (that I intend to do still in the 4.0 timeframe), since it's not a
> regression, and the patch has already grown quite big.
>
> Yes, this is ypoc-like, both "graft 6" and "graft 7" are ambiguous since there
> are two sources 'c' and 'd' possibly to be merged to 'a'.
>
> checkcopies() is getting complicated to support rotated DAG, and I'm afraid it
> will be more complicated in future (or we'll have to decide a complete
> rewrite.) That makes me feel extending checkcopies() isn't the best way to
> work around simple graft scenarios for 4.0 timeframe.

The issue you described isn't specific to backwards grafting through renames.
In fact, it can be reproduced also with a regular, ungraftlike merge command:

hg init test
cd test
for i in 1 2 3 4 5 6 7 8; do echo $i >> a; done
for i in a b c d e f g h; do echo $i >> b; done
hg add a b
hg ci -m initial
hg mv a c
hg ci -m 'a->c'
hg up 0
hg mv b c
hg ci -m 'b->c'
hg merge
<resolve the conflict so that both files' contents are preserved, numbers first, then letters>
hg commit -m merge
hg up 0
sed -e s/4/44/ a > a
hg ci -m 44
hg up 0
sed -e s/e/ee/ b > b
hg ci -m ee
hg up 3
hg merge 4 # works
hg ci -m goodmerge
hg up 3
hg merge 5 # fails, "other changed b which local deleted"

I have a plan for fixing this, and it doesn't involve further extending checkcopies.


More information about the Mercurial-devel mailing list