[darcs-users] darcs patch: Use index-based diffing in Record. (and 57 more)

[Eric Kow]

Phew!  Thanks very much for this, both of you.

Now that everybody has done all the hard work, I'd like to throw in a
few non-technical suggestions, mostly just boring facilitation stuff.

The mental image I have is of a giant boulder that's just starting to
tip over and land in the 'hooray, job well done' zone.  So here I am
vainly trying to poke at the boulder with my finger...

On Sat, Sep 12, 2009 at 23:11:03 +0100, Ganesh Sittampalam wrote:
> I should mention that I haven't looked at the actual changes to darcs-hs  
> in much detail yet, so that side shouldn't be considered properly 
> reviewed yet. I will however do my best to do so asap. In addition my 
> general feeling is that the changes to darcs itself are intrinsically 
> likely to be correct because they just build on the correctness of 
> hashed-storage.

Dmitry: do you have any comments here?

> As Petr says  below, there are some quite significant points of
> disagreement that remain between us over a few aspects of the overall
> design, but those shouldn't obscure the essential success of his work,
> and I would be willing to sign off on the current state of things,
> albeit somewhat reluctantly.

Thanks.  So we have a last resort, but let's try to maximise our
happiness with the code first.

>> Out of these, 2, 3 and 8 are mostly trivialities that can be addressed with
>> just a little bit more time (over the next week, I suppose). I believe 5 is
>> just a documentation issue, and 6 is partially documentation and partially
>> thinking on how to make the situation better.

Easy stuff
----------
Just annotating: these are easy and commonly agreed upon.

2. Split TreeMonad into a read-only and read-write part
3. Clarify virtualTreeIO
8. utility function naming

5. Safety of parallel writes to the filesystem
6. unsafe floatPath function from Storage.Hashed used in Darcs.Gorsvet

7. Performance on old fashioned repos with plain pristineold-fashioned repos
----------------------------------------------------------------------------
>> Point 7 is a lot more work, and the question whether it's worth the investment
>> is a hard one.
...
>> However, I'd like to ask that whatever we decide, we should not block
>> the merge on this.
>
> I also agree about not blocking the merge, because keeping the branch  
> alive for too long is a waste of Petr's time.

Sounds good.

1. Merge hashed-storage into darcs, or keep it separate?
--------------------------------------------------------
>> (a) Ganesh believes that it's vital to move Storage.Hashed.Darcs (the  
>> current module that holds everything darcs-specific in hashed-storage)  
>> into the darcs darcs repository.

>> I have both practical issues (the hashed-storage testsuite relies on
>> this code) and theoretical reservations about such move (I think it's
>> fair to offer this functionality without dragging in libdarcs).

>> A compromise would have a separate cabal package in the darcs repo,
>> but this does not fix the testsuite problem.

> For clients that want to work with the darcs repo format, I think the  
> solution Petr mentions above of having a separate cabal package  
> (hashed-storage-darcs?) in the darcs darcs repo is the way to go.
...
> The downside of this idea is that it further complicates the 
> build process for darcs - but hashed-storage-darcs should be small and 
> quick to build so hopefully this wouldn't be too much of a problem.

The compromise sounds like a nice baby step towards the improved Darcs
API as Ganesh says.

Petr: please confirm: if the test suite was fixed, you can live with the
compromise solution?

4. Safety of hash treatment
---------------------------
>> (b) Ganesh thinks it would be better to overload the Tree type and a number of
>> associated functions over the Hash type, while I favour the current variant
>> with a single global Hash data type. We both think that our preferred version
>> is more readable. I currently can't really think of any serious advantages of
>> one over the other, as of the current status, with darcs-specific code factored
>> into a separate module.

Petr/Ganesh: since this seems like a bit of a toss-up, how would you
feel about the decision coming from a third party?  For example, perhaps
we could solicit Nicolas Pouillard's opinion as a potential non-darcs
hashed-storage user and go with what he suggests?

> In my preferred world, hashed-storage would be a library that users can  
> instantiate with whatever specific hash type and format they choose. 
> Darcs would then be a client with a specific instantiation that takes 
> account of the general weirdnesses of the darcs hashed format (it uses 
> both SHA1 and SHA256 hashes, the hashed filenames in existing repos are 
> prefixed by a 10 digit size specifier, and the way that directories 
> listings are hashed is also somewhat specific to darcs).

This sounds like the right thing to do in principle, but due to
geography, I've been talking to Ganesh more, so I'm biased.  In any
case, I'm not really entitled to have an opinion.

Conclusion
----------
> I should mention that I've already prototyped much of the parametrisation 
> in a throwaway fork of Petr's repo and I'd be happy to bring this up to  
> date and extend it to the testsuite in order to minimise any extra work  
> this change would place on Petr.

It sounds like the only real sticking point that needs a concrete action
is the test suite.

Ganesh: elaborating on your offer.  If you could provide us with an
example or prototype of how the test suite could be generalised and
still be useful we'd be able to wrap this up fairly quickly.  For
example, could we move the current hashed-storage suite into the
darcs-specific package and then have some fairly simple sanity checks
for hashed-storage proper?

-- 
Eric Kow <http://www.nltg.brighton.ac.uk/home/Eric.Kow>
PGP Key ID: 08AC04F9
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: Digital signature
URL: <http://lists.osuosl.org/pipermail/darcs-users/attachments/20090914/b9413f64/attachment.pgp>