[prev in list] [next in list] [prev in thread] [next in thread] 

List:       darcs-devel
Subject:    Re: [darcs-devel] open questions regarding FileUUID
From:       Ben Franksen <ben.franksen () online ! de>
Date:       2018-05-05 9:34:18
Message-ID: pcjtmn$82l$1 () blaine ! gmane ! org
[Download RAW message or body]

Am 13.04.2018 um 22:58 schrieb Guillaume Hoffmann:
> I guess your comment also applies to the PrimClassify class, that is 
> as specific as PrimConstruct.

Yes. And besides PrimConstruct and PrimClassify, there is also:

* class Darcs.Patch.FileHunk.IsHunk
* constraints of the sort ApplyState p ~ Tree

> I don't know what is the best solution, maybe start removing these
> two classes from PrimPatch can see how many error messages you get
> :)

I started to do that for PrimClassify, PrimConstruct, and IsHunk, and
came as far as Darcs.Repository.Job at which point I stopped because I
noted that the ApplyState p ~ Tree constraint already excludes use of
FileUUID and it is rather prevalent (almost all interesting commands
have such a constraint somewhere).

Even getting that far wasn't easy. I had to add constraints to a lot of
functions.

What I found is that PrimConstruct/PrimClassify/IsHunk are needed mainly in:

 (1) D.R.Diff.treeDiff (PrimConstruct only)
 (2) D.P.Split (PrimConstruct/IsHunk, implements hunk editing)
 (3) D.P.Conflict (all three, implements conflict resolution markup)
 (4) D.P.Viewing (IsHunk, implements showContextPatch)

(1) can be handled, I think, by creating a new interface that can be
implemented by FileUUID; the methods will need access to (at least) the
ApplyState in order to interpret file paths properly.

(2) No idea yet what to do about this one

(3) I am still trying to understand how conflict resolution works so
it's hard to comment on this one. One things that is particularly
unclear to me is why method resolveConflicts returns a list of lists;
D.R.Resolution uses only the head of each (inner) list and I have no
idea what the tails are good for. The implementations of this method for
V1 and V2 RepoPatches are very complicated and there is no clear
specification to be found.

(4) is particularly nagging because it infects D.P.Bundle.bundleN which
in turn infects the functions in Darcs.Repository.Hashed because of the
unrevert bundle.

I had one minor success with D.R.Pending: it is possible to eliminate
the PrimClassify dependency by abstracting the functionality for sifting
of prim patches into a class. The hope here is that this interface can
be implemented by FileUUID. The Problem is that the interface is ugly
and not well specified: I just turned the functions isSimple, crudeSift,
and siftForPending into class methods, copying what documentation was
there. I think this is what was previously done for PrimCoalesce, which
is similarly ad-hoc (its docs raise more questions than they answer).
Still this is a small improvement.

I am currently trying to separate out the functions in D.R.Hashed that
work with pristine trees (to a new module D.R.Pristine) in order to get
a better picture for how an interface may look like that is polymorphic
over the ApplyState. I fear this will in the end require advances type
hackery with singleton types via D.P.RepoType because we need a way to
distinguish what to do depending on the patch type (and thus its
ApplyState). If someone (Ganesh?) has an idea how to do that I would be
grateful for hints...

Cheers
Ben

> 2018-04-13 6:15 GMT-03:00 Ben Franksen <ben.franksen@online.de>:
>> The way FileUUID is currently designed leaves a number of
>> questions.
>> 
>> From the definition of the Apply instance, it appears that we treat
>> the object map as --conceptually-- containing an (empty) object for
>> all possible UUIDs. If an object denoted by its UUID is edited and
>> we do not find it in the object map, then we create an empty object
>> out of thin air. This makes a certain sense.
>> 
>> However, it is unclear how to make sure an object has the right
>> type (file or directory) when manifested for the first time. For
>> instance, 'darcs add path' presumably means: generate a new UUID,
>> then manifest it at the given location, where the UUID of the
>> location is found by splitting the path and traversing the object
>> map starting with the root.
>> 
>> But the object does not yet exist in the object map, so its type
>> is unspecified. I don't think this is what the user expects: users
>> will want to know that when they add a file or directory, this is
>> what they get, and not some unspecified thing that can be either of
>> both. I think this is a bug and it should be fixed.
>> 
>> Another problem/question is: how do we make sure the object map 
>> represents a tree and not an arbitrary graph, possibly even with
>> cycles? There is nothing as yet that forbids to manifest a
>> directory within itself or within a parent of itself. Nor is there
>> anything that forbids manifesting an object at multiple locations.
>> This is like a Unix filesystem with completely unrestricted hard
>> links between directories.
>> 
>> If the idea was to allow these pathological object maps on the
>> grounds that ensuring global invariants is the responsibility of
>> higher level code, then we must make sure that there is an
>> efficient way to do that. This probably means we must maintain a
>> cache of UUID -> Maybe Location, e.g. a HashMap UUID Location. I
>> think maintaining the tree invariant works best if we have an
>> abstract interface to patch creation that enforces them. Which
>> brings me to the next point:
>> 
>> The interface we use for patch construction is class PrimConstruct.
>> This is very low-level and geared toward what Prim.V1 offers. In
>> order to properly integrate FileUUID into Darcs we either need a
>> more abstract interface that makes sense for both Prim.V1 and
>> Prim.FileUUID, or we must bite the bullet and add case distinctions
>> everywhere this interface is used. This is a very difficult design
>> problem and I can't offer a good way forward yet. It is not even
>> easy to find all the places where the PrimConstruct interface is
>> used, since the class is hidden behind the PrimPatch class. I am
>> tempted to remove PrimConstruct from the super classes of PrimPatch
>> just to make it more apparent where PrimConstruct is used, but even
>> that is a daunting task and the patch would probably touch many
>> many files.
>> 
>> Cheers Ben
>> 
>> _______________________________________________ darcs-devel mailing
>> list darcs-devel@osuosl.org 
>> https://lists.osuosl.org/mailman/listinfo/darcs-devel


_______________________________________________
darcs-devel mailing list
darcs-devel@osuosl.org
https://lists.osuosl.org/mailman/listinfo/darcs-devel
[prev in list] [next in list] [prev in thread] [next in thread]