[darcs-devel] FullUnwind rebase behavior

Ben Franksen ben.franksen at online.de
Sun May 10 11:19:57 UTC 2020


Am 08.05.20 um 01:01 schrieb Ganesh Sittampalam:
> On 07/05/2020 14:19, Ben Franksen wrote:
>> One way to move forward would be to offer a squash operation on
>> suspended patches, i.e. 'darcs rebase squash'. It interactively presents
>> the user with suspended patches, to select or deselect.
>>
>> What it does is to combine the selected suspended patches to one patch,
>> replacing their content with the combined effect /including/
>> (pre-)fixups, cancelling inverses (or even do a sortCoalesceFL). A
>> squash of more than one named patch should behave as it does in git
>> rebase -i, i.e. invoke the patch log editor pre-loaded with all affected
>> patch logs, so the user can combine them if they wish.
>>
>> Until we have a better story for how to handle the situation where we
>> cannot currently mark conflicts, rebase should refuse to unsuspend
>> conflicted patches (i.e. ones with preceding fixups), unless the user
>> explicitly overrides this default.
> 
> FYI 'darcs rebase inject' does help with this already, as it pushes
> fixups into the next patch. But it's not really very satisfactory as an
> overall solution, especially for cases where some fixups came from
> suspended conflicts and others were naturally generated during rebase.
> 
> This is just a quick note as I haven't had time to think about this
> properly yet. It's very annoying because the unwind approach seemed
> conceptually very nice and of course I spent a lot of time on it, but
> perhaps it will turn out better to abandon it.

I don't think abandoning the approach is feasible or even advisable at
this point. I rather think this should be solved in the UI i.e. we
should gently push users toward doing things inside the rebase state,
adding suitable commands and by default refusing to unsuspend when that
would result in conflicts.

I have been working on a 'rebase suqsh' command. My approach was to
gradually improve the existing. rebase inject command (thanks for the
hint about it, I never used that one before). The first thing I did was
to allow injection for multiple patches in a row. That was the easy part.

The next step is to offer the user the possibility of squashing the
selected patches. When I tried to implement this, I stumbled over the
existence of NameFixups, which make this somewhat more complicated than
I expected.

So I started wondering: Wouldn't it make more sense to warn about lost
dependencies early, that is, right when we push a NameFixup into the
rebase state, instead of later on unsuspend? By that time, we may no
longer remember why we obliterated or unrecorded the lost dependency.
And it is anyway too late to do anything about it, whereas if we warn
early, we could offer to abort the operation, in case dropping the
dependency was unintended (note we are still inside a transaction when
we push fixups).

This would mean we never have to store NameFixups in the rebase state,
as these would be immediately merged with the explicit dependencies of
wherever they get stuck. I think this could simplify the rebase
implementation considerably; for instance it would obviate the need for
WithDroppedDeps.

We may have talked about this question before. I dimly rember having
defended NameFixups back then, and I still think they are a good tool
and we should continue to use them when we push fixups into the rebase
state; we just shouldn't store them.



More information about the darcs-devel mailing list