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

[Gábor STEFANIK]

>


--------------------------------------------------------------------------
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: Mercurial-devel [mailto:mercurial-devel-bounces at mercurial-scm.org]
> On Behalf Of Gábor STEFANIK
> Sent: Thursday, September 15, 2016 12:07 PM
> To: Yuya Nishihara <yuya at tcha.org>
> Cc: mercurial-devel at mercurial-scm.org
> Subject: RE: [PATCH 1 of 2 v8] graft: support grafting across move/copy
> (issue4028)
>
> >
>
>
> --------------------------------------------------------------------------
> 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"

Ouch. That got mangled.
See a readable version in https://bz.mercurial-scm.org/show_bug.cgi?id=5369

>
> I have a plan for fixing this, and it doesn't involve further extending
> checkcopies.
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel