[darcs-devel] [patch1806] add an explicit type for the output of resolveConflicts

Ben Franksen bugs at darcs.net
Fri Jul 12 11:56:38 UTC 2019


Ben Franksen <ben.franksen at online.de> added the comment:

> I'm happy with the general idea of identifying when we couldn't 
> properly mangle the patches. 

BTW, there is more to it than being nice(r) to users. I added a QC test
for the property that says "mangled conflicts never conflict with each
other", and found that this regularly fails. Investigating revealed that
the reason are replace patches that involve the single-letter word "v"
or "^" or any other word used in the mangling of hunks.

These failures go away if we refuse to add pseudo-mangling involving
non-hunks (and instead display them to the user and/or write them to a
file).

> Looking at the representation
> I proposed, could it just be expressed by making the 
> 'conflictMangled' field a Maybe? Or is it more complicated than 
> that?

Possible, but I think either mangled or unmangled better captures how we
want to use the data. At least I can't see a reason to keep the
unmangled version around if mangling was successful. But it's okay for a
first step if we want to split this into a number of smaller refactors.

> My personal preference is for proceeding in small refactorings
> or changes that I can quickly understand in isolation, so if
> I was doing the refactoring I would probably first go to my type
> or something like it, then change conflictMangled to a Maybe.

Yeah, I have been doing it similarly, of course, but the intermediate
steps have been lost because I have done this in a completely messed up
repo where I mixed it with trying to fix conflict resolution for v3...

I'll take your suggestion into account and split the refactor into
smaller steps that are hopefully easy to understand.

> I didn't understand what the SHA1s in your type are about. If we
> have them (i.e. a V3 repo) aren't they in the prims anyway?

Unfortunately not! For RepoPatchV3 we have

  PrimOf (RepoPatchV3 prim) ~ prim

and not

  PrimOf (RepoPatchV3 prim) ~ NamedPrim prim

The reason for this is that many operations on plain prims (e.g.
coalescing and shrinking of sequences, constructing prims from hunks
etc) cannot be implemented for named prims without resorting to
dangerous unsafe operations: you need to conjure a universally unique
PrimPatchId out of thin air; and when you do that you must be /very/
careful that the resulting named prims never leak into any patches that
may later be stored on disk. Indeed most of the code in Darcs.Repository
that directly deals with prims really operates on anonymous prims that
have never been part of a Named patch: diffTrees freshly creates unnamed
prims, the pending patch consists of unnamed prims.

So the NamedPrim wrapper is currently treated as an implementation
detail of RepoPatchV3, and named prims don't appear anywhere in the
RepoPatch API.

This was a design decision I made in order to keep the full integration
of v3 manageable. Things have settled a bit since then and we could
re-consider that decision. For instance, the Rebase implementation
internally deals with prims (for Fixups) and these should rightfully be
named prims in a v3 repo, rather than plain prims as they are now.
However, tackling this raises some horny design questions: the
(potentially) named prims in a Named patch don't support the same API
(i.e. class instances) that unnamed prims do; they really are different
types, so we would need a new type family (PotentiallyNamedPrimOf?) and
a new version of class PrimPatch (PotentiallyNamedPrimPatch?). The ugly
names should serve to illustrate my hesitation in adding them...

__________________________________
Darcs bug tracker <bugs at darcs.net>
<http://bugs.darcs.net/patch1806>
__________________________________


More information about the darcs-devel mailing list