Re: [Module::Build] M::B actions design

[Randy W. S. <Ra...@Th...>]

Thanks for the discussion; It has caused me think this through a little 
more thoroughly. But the thoughts below should still be considered 
idealizations and may differ from the possible or practical. ;)

On 10/16/2003 6:50 AM, Steve Purkis wrote:
> On Thursday, October 16, 2003, at 12:06  am, Randy W. Sims wrote:
> 
>> There is another issue here in that the list of actions is growing 
>> quite large, and M::B is (IMHO) turning into a large monolithic 
>> module. Would it be worth the effort to re-architect actions, so that 
>> they are plug-ins (M::B::ACTION::html, M::B::ACTION::dist). This would 
>> keep M::B from becoming monolithic and would allow others to develop 
>> plug-ins to expand functionality without requiring invasive code changes.
> 
> 
> Yes, M::B: is becoming quite monolithic.  So was MakeMaker.  The main 
> difference I find between the two is that M::B is much easier to read, 
> understand and extend.

I didn't realize until now that my original choice of words may have 
been misconstrued as an insulting remark--That was not my intent. I 
meant monolithic in the sense that most of the code is in a single 
file/class (M::B::Base) that it is growing quite large, and it is not 
likely to get smaller. I didn't intend it as an insult or as any kind of 
judgemental statement, just as an observation.

I don't know that I would describe MakeMaker as monolithic. Michael 
Schwern has done a great job of cleaning it up. It's main problem is 
that it is limited to the underlying tool--make, and to it's different 
versions, implementations, and platforms.

> Splitting out the actions is not a bad idea - I bounced a similar one 
> around with some friends in the past:
> 
>     package My::Action;
> 
>     use base qw( Module::Build::Action );
>     use constant depends_on => qw( foo, bar );
> 
>     sub builder { }    # the parent M::B obj
> 
>     sub check_dependencies {
>         ...
>     }
> 
>     sub execute {
>         ...
>     }
> 
> There are a few problems with these ideas: how do you write platform 
> specific code?  One answer is to sub-class each action, or write a 
> method for each platform in the action's class.  That could work, but it 
> could also get a bit hairy.

The idea(l) is that M::B provide as much as possible an API for the 
action plugins that is platform independent. If there are still OS 
differences and they are small, they can be put inline. Otherwise, you 
would create subclasses that handled the differences and use the 'base' 
class as a proxy, forwarding the calls to the appropriate 'subclass'.

i.e. if you write an action, say for reporting build status to 
cpan-testers called M::B::Action::Reports. If there are differnces 
between *nix systems & Windows, you'd create classes 
M::B::Action::Reports::Unix & M::B::Action::Reports::Windows. Now, in 
M::B::Action::Reports you'd write stub functions that determinse the 
appropriate subclass to call based on OS and forward the call to the 
real function in the determined subclass.

> Then, how do you access commonly used methods?  A solution might be to 
> bung all these methods in a common place that everybody inherits from, 
> say M::B::Common:
> 
>     M::B::Common
>         |
>         +- M::B::Base
>         |
>         +- M::B::Action
>               |
>               +- M::B::Action::Foo

Again, the common methods would become part of M::B's API for actions. 
(Nit: It's a bad idea to subclass just to share code.)

> And, how do you subclass actions?  If I want to write my own 'install' 
> action, how do I do it?  At the moment, I sub-class M::B and do it 
> there.  Sub-classing each action is easy, but how do you get M::B to use 
> them?  One way is to say M::B::Base must have stub methods for each 
> action so that they can be overridden.

As an Build.PL author, you would do one of two things. 1) You'd use a 
helper class, something like

my $action = new M::B::Action::Builder(
   name => 'install',
   code => sub {...},
   help => 'A help message to display for users.'
);

M::B::Action::register( $action );

or 2) You'd create an inline class and register it:

package MY::Action::install;
use Module::Build::Action::Install;
use vars qw(@ISA);
@ISA = qw(Module::Build::Action::Install);
sub new { return Module::Build::Action::register( bless \my $arse ) }
sub ACTION_install { ... }

package main
new MY::Action::install;
...

> I think they all stem from the fact that M::B was not written with this 
> design in mind, so it would take a lot of effort to refactor things.  It 
> certainly would involve a *lot* of changes, and little (if any) 
> backwards compatibility.  There is potentially a huge gain as far as 
> simplicity is concerned, but it may be more work than it's worth.

Yeah, I have no doubt it would be difficult to change, but M::B is not 
going to get smaller. So, any refactoring is best done sooner than later.

> That said...
> 
>> This is only half thought-out, so I'm not sure if it's practical. For 
>> example, I don't know how easy it would be to come up with an 
>> interface so that actions could get info from M::B without digging 
>> into the internals.
> 
> 
> Making them plugins is a good idea, but it's even more complicated.
> 
> 
>> <thinking out loud>
>>
>> Each module would contain one or more actions.
> 
> 
> I'd limit it to one a package.  That way you can get the action name 
> from the package name itself:
> 
>     'build' => M::B::Action::Build

It would be more convenient to allow multiple related actions to share a 
file so that they can share common functions/data. Also, it's best if 
they do not share the name because the most appropriate name for an 
action might be a perl keyword. The module should have a descriptive 
name appropriate for the actions it contains. The actions 
(packages/classes) themselves should probably follow the current 
standard of prefacing them with 'ACTION_' to avoid conflicts.

>>  M::B would search for plugin actions at startup, querying each module 
>> for a list of supported actions.
> 
> 
> So M::B builds up a list of actions and their classes?  And I assume it 
> would search particular sub-classes, starting with 
> Module::Build::Action?  Maybe that's how you could do user level 
> sub-classing: push on your own 'search' package:
> 
>     @M::B::SearchPackages = qw( My::Action Module::Build::Action );

This is where I still have uncertainties. One way that seems kind of 
hackish to me is to scan for '*.pm' under M::B::Action. Then try to 
execute a register function for each module found. If an author provides 
an action that is intended to subclass/override another action, then the 
register function for that action would have to report that in some way. 
I think this kind of usage would be rare in practice. ???

> Then it would find your actions first.  If you have 1 action per 
> package, then M::B already knows what it does.  So eventually you build 
> a map:
> 
>     $build->{ActionClassMap} = {
>         Build   => 'My::Action::Build'
>         Install => 'M::B::Action::Install'
>     };
> 
> Then to dispatch, you do something like:
> 
>     sub dispatch {
>         my $self   = shift;
>         my $action = shift;
>         my $aclass = $self->{ActionClassMap}->{$action} || die;
>         $aclass->new->builder( $self )->check_dependencies->execute;
>     }
> 
> 
>>  Each action knows what parameters it accepts and can display its own 
>> help info. There could be a helper class for generating actions and 
>> linking them by dependencies, i.e.
>>
>> my $action = new M::B::ACTION::Builder(
>>   name => 'My_lib',
>>   code => sub {},
>>   ...
>> );
> 
> 
> Dunno if that's needed - M::B already does it.

Do you mean the subclass() method? In the way I'm describing, you would 
subclass on an action by action basis rather than subclass M::B; in 
fact, you would probably rarely if ever need to subclass M::B itself.

>> my $top = new M::B::ACTION::Builtin( name => 'all' );
>> $top->add_dependency($action);
> 
> 
> I'd go with 'M::B::Action::All', it's more consistent.

Actually, right now I'm thinking more along the lines of

my $top = new M::B::Action::find_action( name => 'all' );
$top->add_dependency($action);

to get a reference to an existing action as it seems more flexible.

> Food for thought...
> 
> -Steve
> 

Randy.