[darcs-devel] conflicted rebase (or rather: rebase with conflicted fixups)

Ben Franksen ben.franksen at online.de
Sun Jun 14 19:48:24 UTC 2020


Am 14.06.20 um 12:36 schrieb Ganesh Sittampalam:
> On 14/06/2020 11:02, Ben Franksen wrote:
>> The other problem is somewhat more fundamental. Pushing inverted
>> differences gives us the expected results in the case of a move simply
>> because the move commutes past the suspended patch. But if we push
>> something that does not commute past, then this suppresses the fact that
>> we have a conflict here.
> 
> That's because you're no longer treating fixups as conflicts on
> unsuspend, right?

Yes.

> I guess that's another thing I intuitively have
> reservations about, though I will aim to make these reservations
> concrete by testing out interesting scenarios on your code.

I think this is not a problem at all, except when we push fixups in an
ad-hoc manner, as we do for amend.

>> This should not surprise us: we know that if p^;q commutes (to, say,
>> q';p'^), then we can calculate merge(p\/q) as cleanMerge(p\/q)=(q'/\p').
>> But if p^;q does not commute, then this is invalid. In light of this,
>> there is an obvious solution: Instead of pushing the inverse difference,
>> we really have to /merge/ the (un-inverted) difference with the rebase
>> items, i.e. we have to call 'removeFixups diff suspended'.

This idea doesn't work :-( It violates the invariant that a patch we
conflict with must be in our history. If we have A;B, suspend B and
amend A with X where X conflicts with B, we get a conflictor B' that
refers to a non-existing X.

I think the root of the problem lies in the fact that amend canonizes
the patch contents. I think this is most clearly seen if we split the
amend into first recording a new patch X on top of A, and then squashing
them (thereby canonizing and assigning a new identity). The first action
clearly works fine with rebase without any special considerations. The
latter is what causes trouble: either we first push fixups for the old
A;X and then merge with (fixups of) the new A' (which destroys file
identities if X contains a move); or we don't modify the rebase state at
all (apart from pushing a (Rename A' A) fixup); but we can do this only
if nothing in the rebase state conflicts with anything in the repo,
including A or X. In particular, if adding X causes the rebase state to
become conflicted, then we can no longer squash A and X.

I think the status quo (of my rebase variant C), i.e. loosing conflicts
when we amend in changes that would normally conflict with the rebase
state is the only viable compromise here.

> Another property I'd expect is that if we amend with some difference X
> and then undo it by amending with X^, the rebase state is unaffected. I
> guess that would require the second merge somehow "undid" the first?

For the case where the merge of X with the rebase state is clean this is
obviously the case (since merging is then equivalent to pushing first
the inverse of X and then X). If we keep the current behavior then it is
true whenever we are able to push inverse difference fixups. Otherwise
if X conflicts with rebase then it fails, because we first merge with X
and then with Y=X^, but Y and X are only inverses content-wise, their
identities are not. So we get a double conflict instead.

>>> At some point I think we need to make a decision between these
>>> strategies for 2.16. We did survive with A for quite a while without it
>>> being a major headache, so if both B and C introduce some form of
>>> regressions compared to A we might actually be better going back to A
>>> for a release and then revisiting the design afterwards.
>>
>> I agree that this is a viable option, but I'd like to see if we can't
>> fix the problems in C with the two changes I sketched above.
>>
> 
> Happy to keep exploring C. While I think a release soonish would be
> good, I'm not pushing for any particular timescale.

I have not yet played with canonizing fixups, but in light of my
observations above I am pretty sure this won't help us solve the problem
with loosing conflicts on amend. I guess it is clear now that adopting C
means we have to live with a few regressions concerning the amend
interaction. On the other hand, we gain fixes for all of the problems
that variant A had with suspending conflicted patches. It is hard to
judge what is more severe and of course I am biased here.

On the one hand, darcs crashing on certain operations is perhaps worse
than loosing conflicts when we amend with a difference that would
normally be expected to conflict with the rebase state. On the other
hand, one could argue that the latter happens more regularly. But that
is only because people, us included, have adopted a work flow that
mostly avoids conflicts. In the long run I would very much like to
overcome this bias towards avoiding conflicts, but this requires that we
finalize the darcs-3 format and find a good compromise for conversion to
darcs-3. We already postponed this for another release, so we can
postpone fixing the rebase problems as well.

So I guess for 2.16 we should stay with rebase variant A.

> A while ago you were in favour of obliterating B from screened. Are you
> still keen on that?

Yes, definitely. Rolling back changes on such a scale is not a good
idea. I am almost certain that this will cause us endless trouble in the
future.

> I'm (still) not, because (a) it's a pain to remove it from all the
> clones particularly as more time has passed and (b) unless we're sure we
> don't want to use it, it's a reasonable state to keep developing from
> for now, e.g. improving tests.

I guess coming up with suitable rollback patches is equally difficult.

As for obliterating from all the clones: I think the only relevant
public clone is the one on hub.darcs.net. This one could simply be
re-cloned from scratch. (How is the mirroring done BTW? Do we push or
does the server have a script that pulls?)

Obliterating only the rebase variant B patches is quite possible and
preserves almost all of the test harness improvements. I know because I
have already done that, more or less: I have pulled everything else from
screened into my variant C branch (unpublished) and for that I only had
to rebase the last two of my patches. Some of the test harness changes
had to be rebased but that is also something already done and can
probably be pulled from my variantC branch. So I think I can quite
easily provide a new screened with all of the rebase variant B patches
removed.

The largest part of the remaining patches in screened are in the one
large bundle that pulls the branch-2.14 changes and resolves its
conflicts. I think we should fast-track most of what remains in screened
to reviewed, make sure we have no regressions, tag the whole bunch
2.15.4 and then create the release branch. Then we should tackle the
things on our todo list for 2.16.

Cheers
Ben



More information about the darcs-devel mailing list