[darcs-users] darcs patch: Add forceHashSlurped that hashes the slu... (and 1 more)

[David Roundy]

On Mon, Sep 1, 2008 at 10:39 AM, Petr Rockai <me at mornfall.net> wrote:
> Hi,
>
> "David Roundy" <daveroundy at gmail.com> writes:
>> It sounds to me like this is a bug in hashSlurped that should be
>> fixed.  hashSlurped should always work right, which shouldn't be too
>> hard.  We just have to check that the hash files referred to in Just
>> <hash> are present in the subdirectory of _darcs/ that we're writing
>> to.  It's a quick check (just a doesFileExist) in the common case
>> where it's already present, and it should be able to ensure that this
>> kind of bug won't happen.
> yes, I just have been hesitant to do that, as it is likely to add a few
> thousand stat calls (well, as many as there are files in the repo) to most
> repository-altering operations. On the other hand, it's likely the metadata is
> all in OS cache at that time, so it might not affect things adversely.

I think this is worthwhile, and not too expensive.  You're right that
the metadata is probably in the cache, and stat calls are generally
pretty cheap.

Another alternative would be to simply always keep hash files that
refer to file or directory content in a fixed directory.  The latter
option wouldn't be a bad idea at all, as it would also promote sharing
of disk space.  In fact, that would be my leaning at the moment.  If
we keep all HashedIO files in "pristine.hashed", then the whole
problem can't occur, and I don't see any downside.  Well, I guess
there's the downside that cleaning the pristine.hashed directory could
be more complicated, as we wouldn't know how many other files might be
stashed away there.  But we only cleanRepository in optimize, and
optimize doesn't need to be either amazingly fast or amazingly simple.
 One major advantage of this approach is that it decreases the number
of arguments to these functions, which should make them simpler and
safer to use.

> However, there is another issue with that approach. If we encounter hash
> failures in pristine, we might want repair to fix those -- which it will under
> forceHashSlurped as proposed, but won't under hashSlurped with a doesFileExist
> call in it.

I don't think this is a problem.

David