Re: cvs commit: httpd-2.0/modules/dav/lock config6.m4

[Greg Stein <gs...@lyra.org>]

On Mon, Jan 06, 2003 at 06:42:51PM -0000, jerenkrantz@apache.org wrote:
> jerenkrantz    2003/01/06 10:42:51
> 
>   Modified:    modules/dav/lock config6.m4
>   Log:
>   Um, that should have been a 'no' - you should have to explicitly enable
>   mod_dav_lock not always include it whenever mod_dav is built.

The other thing that could happen is to enable locking services *only* when
the DAVGenericLockDB directive is present. However, that will change mod_dav
a bit, as it assumes that locking *is* available if the hooks are available.
The hooks would need to be expanded with an "enabled?" query function.

That allows you to build in mod_dav_lock, yet not force people to add a new
directive simply because other DAV providers now see the module.

btw, note that mod_dav_lock is still maintaining the notion of a "key type"
(inode vs fname). That isn't required any more. Another question: does it
record a file name or a location name?

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/

Re: cvs commit: httpd-2.0/modules/dav/lock config6.m4

[Greg Stein <gs...@lyra.org>]

On Mon, Jan 06, 2003 at 03:17:48PM -0800, Justin Erenkrantz wrote:
> --On Monday, January 06, 2003 14:10:55 -0800 Greg Stein <gs...@lyra.org> 
> wrote:
> 
> > The other thing that could happen is to enable locking services *only*
> > when the DAVGenericLockDB directive is present. However, that will change
> > mod_dav a bit, as it assumes that locking *is* available if the hooks are
> > available. The hooks would need to be expanded with an "enabled?" query
> > function.
> 
> Yeah, this was the approach I was thinking of, too.  I'm not clear if we 
> can do this in a backwards-compatible way, but it does seem to make sense 
> to me.

I just look at the dav_hooks_locks structure. Actually, this should be quite
easy, and backwards-compatible. We can change the get_supportedlock() hook
to state that NULL means "no locking available". Lock providers which are
capable of on/off can start returning NULL (of course, they shouldn't do
that to an older mod_dav; not sure if we need to prevent that).

However, that is resource-specific; mod_dav is very lax :-( with associating
locking functionality with specific resources. It just kind of assumes a
global lock database (that's why it is a lockdb rather than a property of
the resource...). So that kind of means we need a generic query about
whether locking is enabled for "something" (dunno what that is)

Sigh.

Well, it looks like we have a dav_resource everywhere that we fetch the lock
hooks. Thus, we can always do a two-step check. One to grab the hooks, the
second to query whether that resource has the locks enabled. If not, then we
return NULL as if the hooks are not present.

Some internal mod_dav API tweaking will need to occur, but it should be
fine.

Thoughts? I can help by making these changes [plus whatever??]. Then if you
adjust mod_dav_lock, then we're set...

> If the change only stays on the 2.1 branch, I have no problem 
> keeping it like this.  I'd also like to tweak the insert_prop hook to take 
> a pool parameter.

Actually, it should take two pools: one for the pool for the result text,
and one for temporary allocations.

Unfortunately, that is definitely an API change. Can we defer that one until
we know all of the pool params that are needed? For example, can existing
providers assume r->pool for the result, and create/destroy a pool within
the function? (the create/destroy-in-func pattern is "not recommended" but
how far would that get us?)

> > btw, note that mod_dav_lock is still maintaining the notion of a "key
> > type" (inode vs fname). That isn't required any more. Another question:
> > does it record a file name or a location name?
> 
> Yeah, I meant to rip that key type out.  But, I just wanted to get 
> something that worked for now and get it out there.

Yah. I assumed that :-)  And wow... it worked quite well :-)

> It currently records 
> the full-path location.  As far as I could tell, there wasn't a good way to 
> get the repository-specific path in the lock code.  But, I could be missing 
> something that would help out.  -- justin

Oh no... we *do* want the full URL location. Recording private paths would
be Badness(tm). I was asking to see whether the result was bad or good :-)

(altho... I could see if somebody had a multiple-vhost server, but one
 database, they could run into conflicts... never thought about that until
 just now)

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/

Re: cvs commit: httpd-2.0/modules/dav/lock config6.m4

[Justin Erenkrantz <je...@apache.org>]

--On Monday, January 06, 2003 14:10:55 -0800 Greg Stein <gs...@lyra.org> 
wrote:

> The other thing that could happen is to enable locking services *only*
> when the DAVGenericLockDB directive is present. However, that will change
> mod_dav a bit, as it assumes that locking *is* available if the hooks are
> available. The hooks would need to be expanded with an "enabled?" query
> function.

Yeah, this was the approach I was thinking of, too.  I'm not clear if we 
can do this in a backwards-compatible way, but it does seem to make sense 
to me.  If the change only stays on the 2.1 branch, I have no problem 
keeping it like this.  I'd also like to tweak the insert_prop hook to take 
a pool parameter.

> btw, note that mod_dav_lock is still maintaining the notion of a "key
> type" (inode vs fname). That isn't required any more. Another question:
> does it record a file name or a location name?

Yeah, I meant to rip that key type out.  But, I just wanted to get 
something that worked for now and get it out there.  It currently records 
the full-path location.  As far as I could tell, there wasn't a good way to 
get the repository-specific path in the lock code.  But, I could be missing 
something that would help out.  -- justin