[darcs-devel] [patch1902] add failing test for rebase of conflicted patches
Ben Franksen
bugs at darcs.net
Wed Sep 4 07:33:53 UTC 2019
Ben Franksen <ben.franksen at online.de> added the comment:
>> I may be wrong but I think this points out an inherent weakness of doing
>> force-commute with conflictors. What such a force-commute really does is
>> a purely formal commute, that just exchanges the names of the patches,
>> so that patch 'one' now makes the change originally done by 'two' and
>> vice versa. I think this is the reason behind the unexpected content of
>> the remaining patch 'two' here.
>
> I think it's actually because of the 'effect' call when we suspend. When
> we unsuspend, if we have the right fixups and toedits we should get the
> right conflicts.
>
> But I haven't actually tried playing with this by hand myself, I'll do
> that soon.
>
>> I am looking forward to your experiments with representing the conflicts
>> of a suspended conflictor in the rebase patch...
>
> I've made some progress, but the result is nasty - see the bundle I just
> sent to this patch.
I had to pipe one more 'y' into the suspend commands. Then the test
indeed succeeded! Good work so far.
I looked at what unwind does for v3 and I think it makes a lot of sense.
Your idea to separate the conflicted effect into three sections (fixups
before; the actual patch; fixups after) is ingenious.
Your unwind doesn't look at the set of conflicts in a conflictor at all.
This could turn out to be problematic for more complicated conflicts.
For instance, the last conflictor of a three-way conflicted merge has an
empty effect.
I will try to extend the test to cover this scenario, too, My guess is
that it will not give the expected result yet.
> The trickiest point is that when we are suspending, we have something
> like Named (FL RepoPatchV3).
>
> But we need to look at each RepoPatchV3 individually to unwind it. So we
> end up with a list like
>
> context1 +>+ underlying_patch1 +>+ context2 +>+ underlying_patch2 +>+ ...
>
> And then in my quick hack we end up with multiple ToEdits for a single
> patch we want to suspend.
I guess in the end you'll have to bite the bullet and more or less
reflect the structure of what is in a Named patch a bit more faithfully.
That is, ToEdit will have to consist of a PatchInfo plus explicit
dependencies, plus a /sequence/ of (fixup;patch), not just a single one.
This could be done by embedding a Named (RebasePatch prim), where
(RebasePatch prim) lives at the same level as (RepoPatchVx prim) and
consists of (fixups;prim) or perhaps even (fixups;prim;fixups). The
latter would be particularly elegant, as unwind would then become a
patch converter:
unwind : p wX wY -> RebasePatch (PrimOf p) wX wY
I think this would also free you from having to create random junk on
the fly.
__________________________________
Darcs bug tracker <bugs at darcs.net>
<http://bugs.darcs.net/patch1902>
__________________________________
More information about the darcs-devel
mailing list