On Mon, Feb 15, 2010 at 10:51, Julian Foad <julian.foad_at_wandisco.com> wrote:
>...
>> If you could be so kind to glance over it and straighten out my picture, if
>> necessary. Upon approval, I'll check it into notes/ so we can edit.
>>
>> Note, if my view is correct, this design text implies small changes to the
>> current state of the API:
>> - no need for _pristine_write()
>
> Can I suggest that, instead, _install() and _get_temp_dir() should not
> be exposed. _write() should accept NULL for the checksum parameter and,
> in that case, calculate the checksum itself. The implementation could
> (at least in that case) use _get_temp_dir() and _install() like your
> pseudo-code for the use case "store". Even when the checksum is provided
> it could do the same and then verify the provided checksum.
pristine_write() is broken and should go away. The implementation
cannot determine if the stream was closed due to *successful*
completion, or if it is being closed due to an error.
>...
>> USE CASES
>> =========
>>
>> Pseudocode implies args for reference to working copy, pool etc. as needed.
>> pristine_* = svn_wc__db_pristine_*
>> _usable = svn_wc__db_checkmode_usable, etc.
>>
>>
>> Use case "new": "I have locally-created content which should be stored
>> -------------- as a pristine."
>> Another pristine with identical content may exist coincidentally, in which
>> case we technically don't need to write the pristine at all. But, as discussed
>> in "On the method to write pristine files" above, we should anyway copy the
>> new content to a tempfile to make sure nothing changes it in-between us
>> checksumming and copying. So, this is exactly the same as use case "store"!
>
> I think the important distinction between your use cases "new" and
> "store" is whether you have the checksum available a-priori. (Whether
> the source is a file or a stream is not so important.)
Yes, it *is* important. If only a stream is present, then we can
checksum as we go (if we don't have it). If a file is present, then we
could do a filesystem copy, rather than streaming operations (ie. no
detranslation or checksumming needed).
This is why I suggested the use cases focus around the source constraints.
>...
>> Use case "could use": "I have more complex options, and I want to get a
>> -------------------- *fast* count on how many of certain pristines I can
>> expect to exist. Does this one show?"
>
> That's not really a use case, now, is it? *Why* do I care whether the
> store *might* have the pristine text I'm asking about?
IIRC, the use case is more "are you *missing* <this> pristine?"
Higher-level code may need to contact a repository if certain
pristines are missing.
If the pristine is there, but unusable... that's not a likely question.
>...
> API comment: I hope we could reduce the checking modes to just two: a
> "normal" check which tells whether the store has a pristine copy (and if
> it says it does, we assume it is usable!), and a "verify" which verifies
> all the metadata including the checksum. These could (should, I think)
> be two separate functions rather than modes of a single function.
I believe so, yes, which is why I added that comment.
>...
>> Use case "repair": "I've seen this pristine is børken. Restore or remove!"
>> -----------------
>> pseudocode:
>> pristine_repair(&present, checksum) (7)
>
> Commenting on the API: The idea of having a separate "repair" API
> function and use case sounds flawed - it just creates unnecessary work.
> We should do something simpler such as having the _check(_valid) call
> automatically delete any entry that is found to be invalid.
Repair may also involve fetching broken entries. Of course, that isn't
possible from a pure-WC standpoint, but the repair operation (as part
of 'svn cleanup'?) will need to do that.
>...
Cheers,
-g
Received on 2010-02-16 21:28:53 CET