On Jan 7, 2005, at 11:41 AM, Greg Hudson wrote:
> On Fri, 2005-01-07 at 08:38, Ben Collins-Sussman wrote:
>> Looking over the two mod_dav_svn vtable funcs (dav_svn_refresh_locks()
>> and dav_svn_find_lock())... they *could* be reimplemented to call
>> _lock_from_path() instead of _lock_from_token() if we really wanted
>> that. IOW, if we wanted to delete the _lock_from_token() API from
>> svn_fs.h altogether, we'd be just fine. I don't think there are any
>> other callers. But as a matter of completeness, it seems odd for our
>> fs locking API to return tokens that "represent locks", and yet not
>> have an API to lookup by token.
>
> I'm strongly in favor of removing _lock_from_token.
I'm removing it now.
But: I just realized that mod_dav_svn isn't the only caller. The
repos wrapper around svn_fs_unlock() -- the one which calls pre- and
post- hooks -- needs this routine.
Why?
1. svn_fs_unlock() takes only a token, since the fs can do any sort of
internal lookup it wants.
2. svn_repos_fs_unlock() also takes only a token, imitating the fs
API. It needs to convert a token to a lock_t so that it can pass
lock_t->path to the hook scripts.
cmpilato and I just chatted, and we thing the smartest thing to do here
is still remove svn_fs_get_lock_from_token(), but then also add a path
argument to both svn_fs_unlock() and svn_repos_fs_lock(). It's a much
cleaner API, anyway: it's weird that svn_fs_lock() takes a path, but
svn_fs_unlock() doesn't. svn_fs_unlock() should take a path (just like
every other fs routine in the universe), and it should also take a
token *only* for authz purposes.
So I'm working on this now, unless somebody screams.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri Jan 7 19:46:46 2005