Re: svn_fs_lock multiple paths

Philip Martin wrote:
>> I'm working on a new svn_fs_lock that allows multiple paths to be
>> locked.  This is to avoid the inefficiency of writing the large digest
>> files multiple times when locking multiple paths one at a time.  I have
>> the low level file handling written but the plumbing to connect it all
>> up is a bit tricky.
>
> Another bit of the API that's causing a problem: canonical paths.  The
> existing svn_fs_path implementation calls svn_fspath__canonicalize on
> its input paths which means that a variety of different inputs produce
> the same result, e.g. "/A/f", "A/f", "/A///f",
> etc. all work.
>
> When svn_fs_lock supports multiple paths how should it behave when more
> than one input path refers to the same canonical path?  Should it detect
> duplicates and fail up front?  Should it act on the canonical path only
> once and produce fewer "results" than inputs?  Should it act on the
> canonical path multiple times, perhaps locking the first time and
> failing subsequently, or locking every time with --force?  Should it be
> stricter that the single path function and fail on all non-canonical
> paths?

Just to throw in an opinion on this question... *some* code somewhere should avoid locking the same canonical path more than once, but I think the responsibility for that should be at a higher level.  You wouldn't try to make this function detect the caller calling it more than once with the same path, so don't bother trying to detect if the caller specifies the same path more than once in a single call.  That's my way of looking at it.

I don't know why the FS API allows non-canonical paths in the first place but (assuming the whole FS API is fairly consistent about this, which I haven't checked) I suggest we leave consideration of requiring canonical paths as a separate issue for the whole API.

- Julian

> The repos layer runs the pre-lock hooks on each path so whatever the FS
> layer does probably has to duplicated in the repos layer.
Received on 2014-02-07 12:59:01 CET