On Thu, Feb 10, 2011 at 6:47 PM, Nick Kew <[email protected]> wrote:
> On Thu, 10 Feb 2011 18:23:18 -0500
> Jeff Trawick <[email protected]> wrote:
>
>> or am I missing something?
>>
>> Is it too disruptive to fix for 2.4?
>
> Well, we *could* just dispense with it, and let modules
> retrieve optional functions in a hook of their choice
> such as post_config.
>
> Are you looking to a radical change to the semantics of "optional"?
Quick summary:
The hook itself is helpful in some situations but we could have
managed without it with a bit of awkwardness for some modules. (Also,
removing it now means some modules will have to rearrange code, sort
out some dependencies, and in some cases convince other module authors
to change where they register their optional functions).
If we do have the hook, we're crippling it by not allowing it to
return OK/fail, and lack of access to a server_rec parameter for
logging purposes is ugly and inconsistent with other hooks.
Now excruciatingly slow:
"Optional" is one thing; the use of the optional function API is another ;)
The optional function API is used in two situations:
1. The function is purely optional from the standpoint of the module,
such as mod_ssl registering log formats if mod_log_config (or
stand-in) is available.
2. The function is absolutely required from the standpoint of the
module, such as mod_authnz_ldap finding functions exported by
mod_ldap. By using the optional function API, we can
a) remove restrictions on the order of LoadModule directives
b) issue more helpful messages than simply failure to load if the
providing module isn't available
Another variation somewhere between #1 and #2: Because of a feature
enabled in a module's configuration, it absolutely requires an
optional function exported by another module.
Is the optional_fn_retrieve hook absolutely required?
Let's say that importers had to look up these functions in the
post_config hook. That means that exporters must register it in an
earlier hook, regardless of their configuration. The earlier hook
might as well be the register_hooks callback, since registering the
function can't be impacted by configuration.
I see a couple of concerns with that:
1. A module may only have a certain capability tied to its exported
optional function based on its configuration. There's no clean way to
implement that if a module has to export the function before examining
its configuration. (I think it has to return not-implemented errors
from the function at steady-state.)
2. Pain: Axing the hook now would require some existing modules to
rearrange their code and in some cases require changes in other
modules.
(Perhaps this isn't a good justification for the hook. I'll proceed
anyway :) ).
Why should the hook be able to return OK/fail?
If the optional function mechanism is used to avoid load order issues
-- i.e., the module requires the function, getting NULL back from the
retrieve should allow a log message to be written and startup aborted.
Why should the hook have access to server_rec? Not required; the hook
can use the existing global variable ap_server_conf if it needs to
log. Accessing it via a parameter is consistent with other hooks
though.
Why would the hook need to log?
a) report whether an optional capability is {absent|present} based on
the availability of an optional function
b) report a fatal error if required functions (possibly required due
to configuration) can't be found
--/--
A couple of existing examples:
mod_log_config exports a log format registration function in its
register-hooks callback; mod_ssl looks it up in its post_config hook;
if not present, nobody cares; optional-fn-retrieve hook not
used/needed
mod_ldap exports a bunch of functions in its register-hooks callback;
mod_authnz_ldap looks it up in its optional_fn_retrieve hook. They
are absolutely required to operate, and it can't properly handle an
error there, so in its post-config hook it checks if mod_ldap.c is
loaded. It could instead "know" that mod_ldap registered the
functions prior to the post-config hook and look them up then, where
error reporting is available. That's a little awkward. Checking for
the name of the module at the point errors can be detected is
obviously bad as an example since it should be possible to design an
LDAP solution where the admin chooses from alternatives like
{mod_ldap_openldap.so, mod_ldap_mozldap.so, etc.} which use different
libraries but export the functions required by mod_authnz_ldap.
(Whether LDAP library plugin capability should be in httpd modules or
APR is not something I'm trying to discuss here -- I'm just thinking
of it as a type of design which should work in a straightforward
manner using optional functions.)
