[darcs-devel] [patch1730] add RepoPatchV3 aka camp conflictors (and 3 more)

[Ben Franksen]

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

This is a literal copy of my email to @darcs-devel, posted here for
reference.

------------
As mentioned in my comments to http://bugs.darcs.net/issue2614 it is now
clear that we cannot solve the inconsistencies of the existing darcs
patch formats using "merge-by-value" as this leads to a loss of
permutivity. The only solution is to move forward toward
RepoPatchV3/Camp which is based on /named/ prim patches. These are
wrappers around our current prim patch types that add an identifier
which consists of the hash of the meta data of the named patch to which
they belong, together with a counter to distinguish between the several
prims inside a named patch (assigned when it is recorded). The counter
would not be strictly necessary to get the theory right, but adding it
makes many of the operations on V3 patches a lot more efficient:
equality of named prim patches in the same (pre or post) context reduces
to equality of their identifiers; we can also define an Ord instance and
use Data.Set to store e.g. the set of conflicting (contexted) patches in
a Conflictor.

It is important to note, however, that when we work with prim patches
directly, i.e. outside the implementation of RepoPatchVX, we usually
want to handle the underlying unnamed prim patches: unrecorded changes
and rebase fixups are not part of a Named patch, they are anonymous.
Thus I believe it is best to let (PrimOf p) remain the unnamed prim type
underlying a RepoPatch, even for RepoPatchV3.

The major stumbling block to the integration of named prim patches (i.e.
prim patches with identifiers) into the class based patch interfaces is
the class FromPrim with its fromPrim method to construct a patch of type
p out of a prim patch of type (PrimOf p). Named prims cannot have
instances for that class, at least not cleanly, because to construct a
named prim we need the ingredients for the identifier.

I have been working on minimizing the use of fromPrim throughout the
code base. After a few rounds of refactorings I arrived at a situation
where the overloaded fromPrim is really only needed for the rebase
implementation and --for the time being-- the functions anonymous and
infopatch from Darcs.Patch.Named (which now take prims as input). (Not
counted here are uses of fromPrim inside the implementation of
RepoPatchV1 and RepoPatchV2 as these do not affect the general RepoPatch
API.) The Named constructor infopatch will need to be adapted to create
appropriate identifiers for the prims it gets passed.

Problematic are the anonymous constructor and the remaining uses of
fromPrim inside the rebase implementation. The use case here is the
same: to commute/merge prims with Named patches. I have been thinking of
using a really dirty hack here: randomly generate (hopefully) unique
prim patch identifiers and wrap that with unsafePerformIO. This would
allow us to lift an arbitrary prim to a RepoPatchV3. But note that the
implementation of RepoPatchV3 stores prim patches /with/ identifiers
inside conflictors; if a conflictor stores a reference to some anonymous
prim patch this would be very bad!

So we can do that only if we can guarantee that the identifiers we
conjured out of thin air are never stored inside a regular Named patch
of the repository. In particular, we should never use merge with
anonymously created Named patches and then store the resulting patches
in the repo!

I believe that for the rebase patch this will work because the only way
to leave the rebase patch is via unsuspend and that should flatten out
all existing conflictors (using effect) and thus never create a patch
that contains a Conflictor. I have not yet investigated all uses of the
anonymous constructor, so I can't say yet whether these may prove critical.
------------

Update: I have checked all uses of anonymous and I believe they are all
safe. Will send a preparation bundle for screened with the FromPrim
refactors and another one to update the V3 implementation, with the
first steps taken towards full integration.

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