[darcs-devel] rebase squash

[Ben Franksen]

Am 14.05.20 um 00:25 schrieb Ganesh Sittampalam:
> On 13/05/2020 14:21, Ben Franksen wrote:
> 
>> Let's reduce the problem to two patches to make this a bit clearer. So
>> conceptually I have
>>
>>   Suspended (n1; f1) (Named d1 p1); Suspended (n2; f2) (Named d2 p2)
>>
>> where ni are the name fixups and fi the prim fixups. Flattening this
>> sequence gives
>>
>>   n1; f1; d1; p1; n2; f2; d2; p2
>>
>> which we commute to
>>
>>   n1; n2; d1'; d2; f1; p1; f2; p2
>>
>> and then re-build into
>>
>>   new = Suspended NilFL (Named (d1'+d2) (f1; p1; f2; p2))
>>
>> and then push (n1;n2) through (new;rest_of_suspended_patches).
> 
> Why do we need to push anything? Once we've built new we can just leave
> n1;n2 before it in the rebase state, and handle it on unsuspend as now.

Okay, good, that makes it simpler.

>>> Regardless, I'm a bit unsure how you will safely handle prim fixups. I
>>> suspect that your answer to that would explain the right way to handle
>>> name fixups.
>>
>> I don't handle prim fixups in any special way, they are simply gathered
>> together with the regular prims into one large sequence, no commutation.
>> I guess I should also sortCoalesceFL that sequence to normalize it.
>>
>> This is very much like what rebase inject does. In fact, rebase inject
>> is now a special case of rebase squash: it is what you get if you squash
>> a single patch and don't change any of the patch meta-data.
> 
> I am not sure that just turning the fixups into real patch content is
> that sensible. I added rebase inject as a hidden command because it
> helped me get out of some tricky situations when I knew exactly what I
> wanted to achieve, but I'm not convinced it's really part of a sensible UI.
> 
> It does sort of make sense in the specific case of a conflict that has a
> resolution, but what if we have an unresolved conflict as well? Adding
> in all the fixups (and coalescing) is a bit of a blunt instrument then
> as you'll essentially be destroying the content of the still-conflicted
> patch without even generating the conflict markers.

I agree that it would probably be a bad idea to squash/inject an
unresolved conflicted patch.

Can we detect if an initial segment of suspended patches has unresolved
conflicts? (And then refuse to squash such a segment?)

I don't think we can, at least I don't know how.

>> (2) Change the meaning of the list of explicit dependencies from a set
>> to a multiset i.e. bag. Then we could have more than one name X in it
>> and commuting a Rename past merely renames (any) one of them. This would
>> mean the commute
>>
> 
>> I tend to view (2) as the ideal solution as it is the cleanest one,
>> doesn't remove any existing feature, and allows a maximum of commutation.
> 
> I agree. I think that's a really nice idea. I wonder if it's worth
> tracking "allowed to be a bag" in the type system. We could for example
> abstract Named over the type of names to make the treatment more
> modular. So the existing Named would be instantiated with (PatchInfo,
> [PatchInfo]) -- really a set and a new variant of the parameter would
> allow bags.

I am not opposed to the idea, but would leave it to you to design this
properly.

>> BTW, do we test the usual properties (recommute, permutivity) for
>> Fixups? Or for Suspended patches?
> 
> No. We probably should for NameFixup. For PrimFixup I hope they would
> trivially follow from the underlying Prims though maybe we should verify
> that.

Yes.

> And pushFixup, which is quite fundamental to rebase has no laws that I
> know of. It would be nice to see if there are some. Now it's been made
> more modular I think that's a bit more plausible than before.

Yes, too.

Here is my current status: I have implemented all the missing parts and
have a complete rebase squash command that replaces (and is based on)
rebase inject. It properly commutes the name fixups backward and also
pushes the rename fixups as per solution (2). I have not tested it yet,
nor reviewed D.P.Named and also haven't looked at all the error cases
when we commute name fixups.

Cheers
Ben