[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