Sharing Config Commands

[Paul Querna <ch...@force-elite.com>]

Currently, you can have multiple modules share a single directive, if
they return DECLINE_CMD, instead of NULL.  The problem is that this
behavior depends on the order of the modules being loaded.

I am attempting to have my mod_svn_view share the 'SVNParentPath'
directive with mod_dav_svn.  Since mod_dav_svn is out of my control, it
always returns NULL for SVNParentPath.  If mod_svn_view is loaded
_after_ mod_dav_svn, it works, since my module returns DECLINE_CMD.
mod_dav_svn will then consume it after me and return NULL.

This is caused by the linked list of modules being in reverse order of
the LoadModule directive.

I think we want to encourage that the order of LoadModule doesn't
matter.  I also think any module should be able to receive the values
for any configuration command.

Looking at the current code, it scans the linked list of modules, and
then scans the commands for each module.  Once it finds a matching
command, it runs that module's function.

Possible solutions:

1) Scan the entire linked list of modules for any matching commands.

2) Put all commands into an apr_hash_t.  The value would be a linked
list of modules that are interested in that command.

Solution #1 could be in a few lines of code changes, by equating
returning NULL to DECLINE_CMD.  It has the disadvantage of scanning a
double linked list of modules/commands...

Solution #2 is faster when you have many modules with many commands.
People with large configurations might even notice a speed improvement...

Thoughts?

-Paul

Re: Sharing Config Commands

[Paul Querna <ch...@force-elite.com>]

Joshua Slive wrote:
> On Wed, 25 May 2005 14:10:38 -0700, "Paul Querna" <ch...@force-elite.com>
> said:
> 
>>Currently, you can have multiple modules share a single directive, if
>>they return DECLINE_CMD, instead of NULL.  The problem is that this
>>behavior depends on the order of the modules being loaded.
>>
>>I am attempting to have my mod_svn_view share the 'SVNParentPath'
>>directive with mod_dav_svn.  Since mod_dav_svn is out of my control, it
>>always returns NULL for SVNParentPath.  If mod_svn_view is loaded
>>_after_ mod_dav_svn, it works, since my module returns DECLINE_CMD.
>>mod_dav_svn will then consume it after me and return NULL.
>>
>>This is caused by the linked list of modules being in reverse order of
>>the LoadModule directive.
>>
>>I think we want to encourage that the order of LoadModule doesn't
>>matter.  I also think any module should be able to receive the values
>>for any configuration command.
> 
> 
> Why don't you just explicitly specify that your module's hook should run
> before the one for mod_dav_svn?

For configuration directives?

Directives are not hooks.  I already am set to run before mod_dav in the
handler hooks, but this has no use if I cannot receive the configuration
values.

Re: Sharing Config Commands

[Joshua Slive <jo...@slive.ca>]

On Wed, 25 May 2005 14:10:38 -0700, "Paul Querna" <ch...@force-elite.com>
said:
> Currently, you can have multiple modules share a single directive, if
> they return DECLINE_CMD, instead of NULL.  The problem is that this
> behavior depends on the order of the modules being loaded.
> 
> I am attempting to have my mod_svn_view share the 'SVNParentPath'
> directive with mod_dav_svn.  Since mod_dav_svn is out of my control, it
> always returns NULL for SVNParentPath.  If mod_svn_view is loaded
> _after_ mod_dav_svn, it works, since my module returns DECLINE_CMD.
> mod_dav_svn will then consume it after me and return NULL.
> 
> This is caused by the linked list of modules being in reverse order of
> the LoadModule directive.
> 
> I think we want to encourage that the order of LoadModule doesn't
> matter.  I also think any module should be able to receive the values
> for any configuration command.

Why don't you just explicitly specify that your module's hook should run
before the one for mod_dav_svn?

Joshua.
-- 
Joshua Slive
joshua@slive.ca

[PATCH] Hash Config Commands was: Sharing Config Commands

[Paul Querna <ch...@force-elite.com>]

Paul Querna wrote:
....
> 1) Scan the entire linked list of modules for any matching commands.
> 
> 2) Put all commands into an apr_hash_t.  The value would be a linked
> list of modules that are interested in that command.
> 
> Solution #1 could be in a few lines of code changes, by equating
> returning NULL to DECLINE_CMD.  It has the disadvantage of scanning a
> double linked list of modules/commands...
> 
> Solution #2 is faster when you have many modules with many commands.
> People with large configurations might even notice a speed improvement...

Attached is a patch that does solution #2.  It works in my test case,
and it seems to work for all the old configuration directives.

If no one cares, I will commit it in a couple days.

Thanks,

-Paul