On Saturday 15 March 2008 5:55 pm, Christopher J. Madsen wrote:
> Here are my thoughts on adding plugins to M::B. Comments, anyone?
Yup. :)
> Places to specify plugins to be loaded:
>
> * Build.PL
> (must distinguish between required plugins and optional ones)
>
> * Module::Build subclass
> (is this one necessary?)
>
> * system-level config file
>
> * user-level config file
>
> * on the Build.PL command line
>
> I don't see any need for optional plugins at the system or user
> level. If you didn't install a plugin, then don't request it.
IMO, this could be vastly simplified if you just limit yourself to the first
two items (Build.PL, and M::B subclass).
Personally, I've yet to encounter a situation myself where I've needed to set
system-level or user-level configuration for all of the packages that I'm
installing. That said, though, I may be just be the odd one out here...
> What plugins can do:
>
> I'm going to borrow some ideas from Damian Conway's Hook::LexWrap, and
> modify them as I see fit.
>
> Plugins can install pre and post hooks on any method. Hooks get
> called like this:
>
> $plugin->pre_METHOD($builder, $parameters, $context, $return)
> $plugin->post_METHOD($builder, $parameters, $context, $return)
I actually did something similar here and am using it in my own custom plugin
aware M::B subclass. I did handle things slightly differently, though...
In my approach I started with a M::B subclass (Module::Build::Pluggable) which
gives me the baseline replacement for "Module::Build" for any build module
that I want to create.
I then have a series of "Module::Build::Plugin::*" plugins that are sucked in
and that via a custom "import()" routine add appropriate callbacks to the
build -class- (as opposed to object). My setup does require that I basically
create a custom derived build class for each project I'm working on, but
that's ok with me; if I'm needing anything funky enough to warrant my using a
pluggable M::B I'm probably already deriving a custom build class anyways.
And, similarly to what you've borrowed from Hook::LexWrap above, I also went
with the notion of having plugins simply tie themselves as pre/post to an
existing ACTION_* method.
Here's what a sample derived build module would look like for me:
package MY::Build;
use base qw(Module::Build::Pluggable);
use Module::Build::Plugin::cpan2rpm;
use Module::Build::Plugin::dist_inc;
use Module::Build::Plugin::install_extras;
use Module::Build::Plugin::clean_dist_tarball;
use Module::Build::Plugin::replace
'###VERSION###' => sub { shift->{properties}{dist_version} },
'###TIME###' => sub { time() };
1;
To help each of the plugins register themselves with
my "Module::Build::Pluggable" class, I defined an "add_callback()" method
that works like this:
- "pre" actions:
$caller->add_callback( 'pre:???', sub { ... } );
- "post" actions:
$caller->add_callback( 'post:???', sub { ... } );
- New actions (treated as "post", if the action already exists):
$caller->add_callback( '???', sub { ... } );
- Actions to be invoked at instantiation of my M::B::Pluggable subclass:
$caller->add_init_callback( sub { ... } );
Isn't perfect, but its worked for me thus far.
Here's an example of one of my plugins:
package Module::Build::Plugin::clean_dist_tarball;
sub import {
my $caller = scalar caller;
$caller->add_callback( 'pre:clean', \&_pre_clean );
}
sub _pre_clean {
my $self = shift;
my $tarball = $self->dist_dir() . '.tar.gz';
$self->add_to_cleanup( $tarball );
}
Short and simple. Made my life a whole lot easier when I could start to
separate out all of the common behaviour for my packages into separate
plugins; I quit rewriting the same code again and again and just slurped in
the plugin and was done with it.
> In a pre hook:
>
> * $return is always undef.
>
> * You can modify the $parameters array to change what the method (and
> any subsequent hooks) see.
>
> * To completely override a method, set $_[-1] to an appropriate value
> and return 'done'. No more pre hooks will be called. The normal
> method will not be called. All post hooks will still be called.
> (I don't think setting $_[-1] is sufficient, because it makes it
> impossible to override a method and return undef.)
>
> You must set $_[-1] to an arrayref if $context is 1. (I can't
> decide if the wrapping code should auto-upgrade any non-arrayref
> return value in this case, to simplify returning a scalar from a
> method that might be called in list context or scalar context.)
>
> * Otherwise, a pre-hook must return 'continue'. Returning anything
> else is an error. (This is to prevent hooks that just happen to
> return the value of their last expression from causing mysterious
> problems.)
>
> In a post hook:
>
> * If $context is undef, $return is always undef.
> If $context is false, $return is a scalar.
> If $context is true, $return is an arrayref.
>
> * Modifying $_[-1] changes the return value.
> (Unless $context is undef, of course.)
>
> * The return value of a post hook is ignored.
> (Or does it make sense to have done/continue here also?)
>
> * You can modify the $parameters array to change what any subsequent
> hooks see, but this is HIGHLY discouraged.
In my setup, I went simple and just worked on the premise that if I needed to
report errors of any sort that I called "die". Otherwise, it is presumed
that the plugin was successful in doing whatever it was supposed to be doing.
Plugins weren't allowed to modify the parameters, but you could manipulate
the build object however you saw fit.
> To allow adding new actions:
>
> When a plugin hooks ACTION_whatever, we check for
> Module::Build::Base::ACTION_whatever. If it doesn't exist, we define
> it as sub ACTION_whatever {}.
>
> I don't think it should be an error to hook a method that doesn't
> exist. (It might have been introduced in a newer version of M::B.)
> It only becomes a runtime error if the non-existent method is ever
> called (and it hasn't been completely overridden by a pre-hook).
The "add_callback()" routine I use to set up pre/post callbacks handled this
automatically; if the ACTION_* method didn't exist it was created using the
provided coderef as the initial starting point.
> The order in which plugins get called:
>
> When you request a plugin, you specify a priority for it.
>
> * Build.PL can specify priorities from -100 .. 100.
>
> * The system config can use priorities from -1000 .. 1000.
>
> * The user config can use priorities from -10000 .. 10000.
>
> * Pre-hooks get called in decreasing order of priority.
>
> * Post-hooks get called in increasing order of priority.
>
> * Plugins do not get to pick their own priority.
> The person loading the plugin does that.
>
> * The default priority is 0.
> (Assuming the method of specifying plugins allows for a default.)
>
> * Plugins with equal priority are sorted by name (just to ensure
> deterministic results).
>
> * Do we need to allow per-method priorities, or is per-module sufficient?
I kept things really simple here; priority was based on the order of how the
plugins were loaded; if they were loaded first, they got the chance to be
invoked first.
In essence, loading up plugins in my M::B::Pluggable just appended coderefs
to "pre" and "post" lists for each of the ACTION_* methods. If multiple
plugins hooked into the same action (e.g. "post:code"), then the one that was
loaded first got to run first.
> Questions for further thought:
>
> * How can configuration be passed to plugins?
I cheated... I did it all at import time, like this:
use Module::Build::Plugin::replace
'###VERSION###' => sub { shift->{properties}{dist_version} },
'###TIME###' => sub { time() };
If a plugin needed anything that couldn't immediately be provided when it was
loaded into my M::B::Pluggable class, it had to get it out of %ENV; I didn't
provide any other means of stuffing other data/config into it.
----------------------------------------------------------------------
IMHO Christopher, I think you're over-engineering things. All this fancy
system and user level config and flexibility sounds great, but it also makes
it a whole lot more complicated and harder to understand what's going on at
any given point.
I borrowed a bunch of ideas from how CGI::Application handles its plugin
scheme, and basically just built what I needed to scratch my own itch. That
said, I'm quite sure that what I've built is -not- the be all and end all of
plugin systems, it just happens to work for me and was simple enough that our
other devs got ramped up on it without much effort at all.
--
Graham TerMarsch
Howling Frog Internet Development, Inc.
