[darcs-devel] unsuspend-reify [was: [patch1824] v3: basic integration]

Ganesh Sittampalam ganesh at earth.li
Sun Sep 1 11:26:08 UTC 2019


On 01/09/2019 00:48, Ben Franksen wrote:

>> I've had a play with changing ToEdit to be Named (PrimOf p) instead of
>> Named p, and it seems to basically work.
> 
> So you replace patches to be edited with their effect right away instead
> of only when we unsuspend. Correct?

Yes. But I think I could improve that to turn any patches undone by
conflicts in the patch into fixups instead. This would address the point
you make in your followup mail. At the moment I have no idea what
happens to any embedded conflicts we suspend, but I doubt it's good. I
expect they get lost when we call effect later.

>> The fromAnonymousPrim calls end
>> up only at the "boundary" of the rebase world, where we are either
>> extracting a conflicted patch (extractRebaseSelect) or looking at
>> changes (D.P.R.Viewing.changeAsMerge, used by
>> RebaseChange.conflictedEffect).
> 
> I am not sure why you need fromAnonymousPrim at all. At the boundary
> (out of rebase) we can build a Named patch in the normal way.

The main reason is that the fixups don't have a name.

>> In both cases the conflicts are straightforward ones which would always
>> originate from Prim patches on both sides and are ephemeral.
> 
> This is is quite cryptic to me. Care to explain?

>> I wonder
>> about using a specific new patch type to represent these types of
>> conflicts, 
> 
> What do you mean "these types of conflicts"? I don't get it. How are
> they different from "normal" conflicts?

Our existing conflict-handling types (V1/V2/V3) are designed to be very
general: we want to be able to merge patches that are already in
conflict, we want to be able to undo conflicts etc.

The conflict we get on unsuspend is between an unconflicted ToEdit
patch, and an unconflicted set of fixups. Not only that, but as things
stand now we don't actually make a conflicted patch in the repo, we just
want the conflict resolutions.

So my idea is that instead of returning an actual patch from
extractRebaseSelect, we define a new type to just capture the "simple"
conflict. I'm not quite sure what classes etc this type would have to
implement, but hopefully not too many. The type would only be used to
pass the conflict back to the Rebase command which would then handle
marking up the conflict (or whatever else).

The main motivation for this was thinking about what we would need to do
to directly commute a prim with a RepoPatchV3, which would be another
way to avoid the need for the fromAnonymousPrim. It certainly could be
done, but it'd involve duplicating code or complicating it, and I
realised it's really unnecessary to have that extra complexity.

If, as discussed below, we did start leaving a Named patch int he
repository to represent the unsuspended fixups, then we'd need to
convert to RepoPatchV3 etc at that point.

>> I think leaving something concrete in the repository to represent a
>> conflict would be a really good step forward.
> 
> We already have that and we call them conflictors. Re-inventing
> something different for a special case is not a good step forward.

I mean having a concrete object in the repository to store the
conflicts, like your suggestion of constructing a Named. It could and
probably should be the natural conflict type for the repository
(RepoPatchV1/V2/V3).

> No we don't. Representing conflicts with unrecorded is not something I
> have ever heaŕd any user ask for. Trying to do that is a completely
> unnecessary complication.

We may be talking at cross-purposes, because I thought we'd already
discussed that. I certainly find it a problem when I pull in some remote
patches and they make my local changes a mess of conflict markup and I
then find it hard to get back to what I had before.

>> Should this be a Named patch? I'm not sure. Inventing new patches for
>> the user is a bit weird, and as you say in patch1871 would be quite
>> weird for their unrecorded changes. I don't see it as out of the
>> question though. The Named patch could just contain the things that were
>> actually in conflict, so it'd have a fairly localised effect and of
>> course we'd have to explain it to the user.
> 
> I don't like the direction this takes. It reminds me of the idea to mix
> the rebase patch with normal patches. SOunds like now you want to mix
> somehow-conflicted-but-not-quite patches in there. With vague notions of
> their effect being "fairly localised". But perhaps I misunderstood.

"Fairly localised" just in terms of the changes to working. We wouldn't
be recording all the users changes against their will, just the ones
that happened to have conflicts.

>> Not really sure, just thinking out loud :-) Perhaps it wouldn't be
>> pending but something else. But the common thread between pending as it
>> is now, conflicts with working, and conflicts with fixups, is that we
>> want some kind of structure to represent some changes that the user
>> hasn't yet asked to put in a real patch.
> 
> Perhaps all we need to do is to save the result of resolveConflicts i.e.
> the unravelled, not-yet-mangled conflicts, somewhere? I mean on
> unsuspend.

Yes, that might work. But ideally we'd keep track of the context of the
conflicting parts, so if the repository changes before we do something
with them, we can still make sense of them.

> BTW, what conflicts with fixups? We don't have conflicts with
> fixups, we depend on them. We only get conflicts when we force-commute
> them. Can you explain how that actually works in our current rebase?

I meant the conflicts we get when we force-commute them.

Ganesh


More information about the darcs-devel mailing list