[darcs-devel] [issue2553] rebase amend

Ben Franksen ben.franksen at online.de
Tue Oct 24 08:06:18 UTC 2017


Hi Ganesh

Am 22.10.2017 um 16:38 schrieb Ganesh Sittampalam:
> On 18/10/2017 16:08, Benjamin Franksen wrote:
>> I went ahead and implemented a very simple force-commute command just to
>> see if the idea works at all. It force-commutes the top two patches
>> (unless there is a tag in the way). Here is the test run:
> [...]
>> instance PrimPatch prim => Merge (RepoPatchV2 prim) where
>>     merge (InvConflictor{} :\/: _) = impossible
>>     merge (_ :\/: InvConflictor{}) = impossible
>>     merge (Etacilpud _ :\/: _) = impossible
>>     merge (_ :\/: Etacilpud _) = impossible
>>
>> So these cases are simply not handled because we think they cannot
>> happen. Do you think we could add these cases so we can play further and
>> see where this goes?
> 
> We should think about how to test it if you do - the conflictor code is
> already pretty complicated so I'd be quite nervous about merging it into
> darcs.

Oh, no worries, I bet I am at least as nervous about it as you are. At
the moment all I am doing is experimenting. I was surprised that my bold
attempt succeeded at least partly ;-)

When I detected the missing cases I thought adding them would be a
simple matter of "doing the same thing only backwards" and then fixing
the type errors. However, after a closer look and some experimental
attempts I found that I could not do it without a deeper understanding
of what the existing code does and why. Perhaps this effort would be
better spent on starting to integrate the camp theory.

>>> Let me correct that: I don't think our current implementation supports
>>> it at all, although I agree it's theoretically very attractive and it's
>>> something I've wanted for a long time - I remember asking David about it
>>> when he was first working on conflictors.
>>>
>>> In particular, I don't think our patch implementation itself supports
>>> it. Of course for a conflict-free patch, A^-1 is indistinguishable from
>>> a "normal" patch, but once you have conflicts, inverses are
>>> distinguishable because of the Conflictor/InvConflictor distinction.
>>
>> I don't see how it is relevant whether inverses are distinguishable from
>> "normal" patches.
> 
> I just meant the cases in the merge code you've now run into above.
> 
> If you commute A;B to get B';A' via "merging inverses", and then
> re-merge A+B', then if B' and A' are conflict-free, they are
> indistinguishable from normal patches, so logically the code in
> RealPatch and Prim shouldn't go wrong otherwise it might go wrong on
> normal patches.

Ah, I see what you mean.

> Once conflicts are involved, our representation does distinguish
> inverses, and it's plausible for the merge code to simply not support it.

Yes. I did not think about the possibility of cases for InvConflictor or
Etacilpud not being handled.

> FWIW, as you probably know, rebase produces conflicts by merging
> inverses when you unsuspend patches. My initial implementation involved
> keeping those conflicts in the repo but in the end I decided to flatten
> them out by calling 'effect' before making the final patch. If we decide
> to support force-commuting we could also change that back.

No, I did not know this. But it explains what I found out the hard way:
when I unsuspend and get conflicts and then revert I can't get the
conflicts back with mark-conflicts. This was unexpected for me and it's
a trap waiting for people to fall into, if you ask me.

How hard would it be to fix that, independent of force-commute?

Cheers
Ben



More information about the darcs-devel mailing list