[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