On Tue, Mar 29, 2005 at 04:43:30PM -0600, Ken Williams wrote:
I think there's one really good argument in favor of splitting it out and one really good argument against.
In favor: if we knew the subset of build_requires that were actually needed for testing, then it would be easier for people to squirrel away the regression tests and run them again after the module is installed. I think people have been vaguely wanting that for a long time.
Against: in the perl culture (largely because of the way MakeMaker has always been implemented), testing has always been seen as an integral part of the build process. By having people declare testing dependencies as part of build_requires, we reinforce this notion.
On the whole, though, I think it's probably a good idea.
To throw weight onto the "in favor" side.
A quickie sample implementation to add more meat. I didn't apply yet mainly because I'm wondering if we shouldn't bail and do a complete roll-back (eg. don't generate a Build script) if there are any failed requirements. Or should we bail, for example, during ./Build test if there are any test_requires failures? Or continue as is and just let it fail when it tries to use the missing requirements?
Randy.
Index: lib/Module/Build/Base.pm
===================================================================
RCS file: /cvsroot/module-build/Module-Build/lib/Module/Build/Base.pm,v
retrieving revision 1.400
diff -c -u -r1.400 Base.pm
--- lib/Module/Build/Base.pm 24 Mar 2005 16:36:09 -0000 1.400
+++ lib/Module/Build/Base.pm 30 Mar 2005 01:05:47 -0000
@@ -27,13 +27,13 @@
die "Too early to specify a build action '$self->{action}'. Do 'Build
$self->{action}' instead.\n"
if $self->{action};
- $self->_set_install_paths;
- $self->_find_nested_builds;
$self->dist_name;
+ $self->dist_version;
$self->check_manifest;
$self->check_prereq;
$self->set_autofeatures;
- $self->dist_version;
+ $self->_set_install_paths;
+ $self->_find_nested_builds;
return $self;
}
@@ -426,16 +426,12 @@
__PACKAGE__->add_property(build_script => 'Build');
__PACKAGE__->add_property(config_dir => '_build');
__PACKAGE__->add_property(blib => 'blib');
-__PACKAGE__->add_property(requires => {});
-__PACKAGE__->add_property(recommends => {});
-__PACKAGE__->add_property(build_requires => {});
-__PACKAGE__->add_property(conflicts => {});
__PACKAGE__->add_property('mb_version');
__PACKAGE__->add_property(build_elements => [qw(PL support pm xs pod script)]);
__PACKAGE__->add_property(installdirs => 'site');
__PACKAGE__->add_property(install_path => {});
__PACKAGE__->add_property(include_dirs => []);
-__PACKAGE__->add_property('config', {});
+__PACKAGE__->add_property(config => {});
__PACKAGE__->add_property(recurse_into => []);
__PACKAGE__->add_property(build_class => 'Module::Build');
__PACKAGE__->add_property(html_css => ($^O =~ /Win32/) ? 'Active.css' : '');
@@ -475,6 +471,23 @@
quiet
);
+{ # prerequisites
+ my @prereq_actions = ( 'Build.PL', 'build', 'test' );
+ my @prereq_types = qw( requires recommends conflicts );
+ __PACKAGE__->add_property(prereq_actions => [EMAIL PROTECTED]);
+ __PACKAGE__->add_property(prereq_types => [EMAIL PROTECTED]);
+ my @prereq_action_types;
+ foreach my $action ( @prereq_actions ) {
+ foreach my $type ( @prereq_types ) {
+ my $req = $action eq 'Build.PL' ? '' : $action . '_';
+ $req .= $type;
+ __PACKAGE__->add_property( $req => {} );
+ push( @prereq_action_types, $req );
+ }
+ }
+ __PACKAGE__->add_property(prereq_action_types => [EMAIL PROTECTED]);
+}
+
sub mb_parents {
# Code borrowed from Class::ISA.
my @in_stack = (shift);
@@ -657,7 +670,7 @@
File::Path::mkpath($self->{properties}{config_dir});
-d $self->{properties}{config_dir} or die "Can't mkdir
$self->{properties}{config_dir}: $!";
- my @items = qw(requires build_requires conflicts recommends);
+ my @items = @{ $self->prereq_action_types };
$self->_write_dumper('prereqs', { map { $_, $self->$_() } @items });
$self->_write_dumper('build_params', [$self->{args}, $self->{config},
$self->{properties}]);
@@ -680,7 +693,7 @@
my $failures = $self->prereq_failures($info);
if ($failures) {
$self->log_warn("Feature '$name' disabled because of the following
prerequisite failures:\n");
- foreach my $type (qw(requires build_requires conflicts recommends)) {
+ foreach my $type ( @{$self->prereq_action_types} ) {
next unless $failures->{$type};
while (my ($module, $status) = each %{$failures->{$type}}) {
$self->log_warn(" * $status->{message}\n");
@@ -697,7 +710,7 @@
sub prereq_failures {
my ($self, $info) = @_;
- my @types = qw(requires recommends build_requires conflicts);
+ my @types = @{ $self->prereq_action_types };
$info ||= {map {$_, $self->$_()} @types};
@@ -708,12 +721,12 @@
while ( my ($modname, $spec) = each %$prereqs ) {
my $status = $self->check_installed_status($modname, $spec);
- if ($type eq 'conflicts') {
+ if ($type =~ /conflicts$/) {
next if !$status->{ok};
$status->{conflicts} = delete $status->{need};
$status->{message} = "Installed version '$status->{have}' of $modname
conflicts with this distribution";
- } elsif ($type eq 'recommends') {
+ } elsif ($type =~ /recommends$/) {
next if $status->{ok};
$status->{message} = ($status->{have} eq '<none>'
? "Optional prerequisite $modname isn't installed"
@@ -735,9 +748,9 @@
my $failures = $self->prereq_failures;
return 1 unless $failures;
- foreach my $type (qw(requires build_requires conflicts recommends)) {
+ foreach my $type ( @{$self->prereq_action_types} ) {
next unless $failures->{$type};
- my $prefix = $type eq 'recommends' ? '' : 'ERROR: ';
+ my $prefix = $type =~ /recommends$/ ? '' : 'ERROR: ';
while (my ($module, $status) = each %{$failures->{$type}}) {
$self->log_warn(" * $prefix$status->{message}\n");
}
@@ -2239,7 +2252,7 @@
$node->{$name} = $self->$_();
}
- foreach (qw(requires recommends build_requires conflicts)) {
+ foreach ( @{$self->prereq_action_types} ) {
$node->{$_} = $p->{$_} if exists $p->{$_} and keys %{ $p->{$_} };
}
