[darcs-devel] rebase squash

Ganesh Sittampalam ganesh at earth.li
Wed May 13 22:25:20 UTC 2020


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.

>> While there are some cases where Named :> RebaseName doesn't commute
>> (see D.P.R.Name.commuteNamedName) I'm not sure if they can actually
>> arise in a rebase state.
> 
> Then we should add a property stating that they cannot arise and see if
> it fails.

Yes, perhaps. And/or think more carefully about the invariants we expect
of rebase states and how they might help us systematically formulate
laws and commute rules for the individual components in a rebase state.

>> 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.


> (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.

> 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.

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.

Ganesh


More information about the darcs-devel mailing list