[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] allow pre-lock hook to specify token for locking

From: Karl Fogel <kfogel_at_red-bean.com>
Date: Mon, 21 Jul 2008 15:15:32 -0400

"Chia-liang Kao" <clkao_at_clkao.org> writes:
> This is mainly cleanup. token uniqueness are mentioned in fs_fs layer
> lock unction as TODO:
>
> /* If the caller provided a TOKEN, we *really* need to see
> if a lock already exists with that token, and if so, verify that
> the lock's path matches PATH. Otherwise we run the risk of
> breaking the 1-to-1 mapping of lock tokens to locked paths. */
> /* ### TODO: actually do this check. This is tough, because the
> schema doesn't supply a lookup-by-token mechanism. */
>
> I think it's a bit overkilling for this feature to implement the check
> that is currently lacking underlying facilities. I am also wondering
> why do we need to lock-token to be globally unique? not just unique
> per-path? Given the comment stating we don't do any lookup-by-token,
> I don't suppose lock would work across renamed nodes either? Or am i
> being naïve and missing something obvious here?

The reason the current code does not check ("guard") to make sure the
token is unique is that the current code generates those tokens itself,
and so it knows they're unique. If external code becomes responsible
for generating tokens, then that guarantee goes away.

The requirement that the tokens be unique is not for Subversion, it's
for Subversion's consumers -- the whole point of a lock token is to
unambiguously, universally, uniquely identify the lock. (I can't think
of any more words beginning with "u" to help make the point. :-) ) It's
not Subversion that needs that guarantee -- it's other software that
might be helping users communicate, and that might depend on unique lock
tokens to do so.

You're probably going to ask for for an example of some software now,
and I don't have one. I just know that we've been making a promise of
uniqueness (or at least, uniqueness-per-repository), and now we're
contemplating taking that away.

There might be no consequences, I just don't know.

Can you describe why you want this feature? It might help us all think
better...

-Karl

> 2008/7/16 Chia-liang Kao <clkao_at_clkao.org>:
>> IMHO if the hook script decides to use such feature, it should
>> guarantee the uniqueness of the tokens. But kfogel mentioned on irc
>> we should do some guarding about it. I haven't checked if the fs
>> level does any guarding already, and it appears that no one is
>> actually using the svn_repos_fs_lock's token argument in the code
>> base, so there's some room for exploration.
>>
>> the patch also includes my pre-{,un}lock changes, which should
>> probably be committed separately and i will update this patch
>> accordingly.
>>
>> [[[
>> * subversion/libsvn_repos/fs-wrap.c
>> (svn_repos_fs_lock): allow returned new_token from pre-lock hook.
>>
>> * subversion/libsvn_repos/repos.h
>> (svn_repos__hooks_pre_lock): now has additional returned value for token.
>>
>> * subversion/libsvn_repos/hooks.c
>> (run_hook_cmd2): renamed from run_hook_cmd, allow stdout of hook to be
>> captured and returned.
>> (run_hook_cmd): wrapper for run_hook_cmd2 to provide compatibility.
>> (svn_repos__hooks_pre_lock): call run_hook_cmd2 and return the token.
>>
>> * subversion/libsvn_repos/repos.c: Document the pre-lock feature for
>> token from stdout.
>>
>> ]]]
>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
> For additional commands, e-mail: dev-help_at_subversion.tigris.org

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-07-21 21:16:17 CEST

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.