I was studying Jarkko's patch this morning in preparation for applying
it. I had a number of concerns.
1. It turned out that at the point the patch was originally submitted,
HEAD had moved a bit beyond the version against which Jarkko diffed. I
had refactored some of the patched code into internal subroutines. So I
had to adjust the patch to reflect that.
2. If I understand the thrust of the patch correctly, it says, "Only
set these warnings if we're using GCC as our C compiler. Otherwise,
skip it." If that's correct, then the value which we need to consult
inside the Parrot::Configure object at the start of runstep() is not
options->{gccversion} but data->{gccversion}. If I understand
config/auto/gcc.pm correctly, if a user has GCC set up as her default C
compiler, then data->{gccversion} will be set to the GCC version number
in that config step -- regardless of whether a value was set for
'gccversion' on the command-line. So I further revised the patch to
reflect this understanding.
3. Added some verbose output and explicitly set result strings in both
cases.
4. Added two test files to test the non-GCC case both silent and verbose.
Andy D, Jarkko: Does this work for you?
Index: MANIFEST
===================================================================
--- MANIFEST (revision 24835)
+++ MANIFEST (working copy)
@@ -1,7 +1,7 @@
# ex: set ro:
# $Id$
#
-# generated by tools/dev/mk_manifest_and_skip.pl Sun Jan 13 14:16:31 2008 UT
+# generated by tools/dev/mk_manifest_and_skip.pl Sun Jan 13 16:32:54 2008 UT
#
# See tools/dev/install_files.pl for documentation on the
# format of this file.
@@ -3510,6 +3510,8 @@
t/steps/auto_warnings-04.t []
t/steps/auto_warnings-05.t []
t/steps/auto_warnings-06.t []
+t/steps/auto_warnings-07.t []
+t/steps/auto_warnings-08.t []
t/steps/gen_config_h-01.t []
t/steps/gen_config_pm-01.t []
t/steps/gen_core_pmcs-01.t []
Index: t/steps/auto_warnings-07.t
===================================================================
--- t/steps/auto_warnings-07.t (revision 0)
+++ t/steps/auto_warnings-07.t (revision 0)
@@ -0,0 +1,85 @@
+#! perl
+# Copyright (C) 2007, The Perl Foundation.
+# $Id$
+# auto_warnings-07.t
+
+use strict;
+use warnings;
+use Test::More tests => 22;
+use Carp;
+use lib qw( lib t/configure/testlib );
+use_ok('config::init::defaults');
+use_ok('config::init::hints');
+use_ok('config::inter::progs');
+use_ok('config::auto::warnings');
+use Parrot::Configure;
+use Parrot::Configure::Options qw( process_options );
+use Parrot::Configure::Test qw( test_step_thru_runstep);
+
+my $args = process_options(
+ {
+ argv => [ ],
+ mode => q{configure},
+ }
+);
+
+my $conf = Parrot::Configure->new;
+
+test_step_thru_runstep( $conf, q{init::defaults}, $args );
+test_step_thru_runstep( $conf, q{init::hints}, $args );
+test_step_thru_runstep( $conf, q{inter::progs}, $args );
+
+my $pkg = q{auto::warnings};
+
+$conf->add_steps($pkg);
+$conf->options->set( %{$args} );
+
+my ( $task, $step_name, $step);
+$task = $conf->steps->[-1];
+$step_name = $task->step;
+
+$step = $step_name->new();
+ok( defined $step, "$step_name constructor returned defined value" );
+isa_ok( $step, $step_name );
+ok( $step->description(), "$step_name has description" );
+
+# Mock case where C compiler is not gcc.
+$conf->data->set( gccversion => undef );
+ok($step->runstep($conf), "runstep() returned true value");
+is($step->result(), q{skipped}, "Got expected result");
+
+pass("Completed all tests in $0");
+
+################### DOCUMENTATION ###################
+
+=head1 NAME
+
+auto_warnings-07.t - test config::auto::warnings
+
+=head1 SYNOPSIS
+
+ % prove t/steps/auto_warnings-07.t
+
+=head1 DESCRIPTION
+
+The files in this directory test functionality used by F<Configure.pl>.
+
+The tests in this file test config::auto::warnings when the C compiler
+being used is not I<gcc>.
+
+=head1 AUTHOR
+
+James E Keenan
+
+=head1 SEE ALSO
+
+config::auto::warnings, F<Configure.pl>.
+
+=cut
+
+# Local Variables:
+# mode: cperl
+# cperl-indent-level: 4
+# fill-column: 100
+# End:
+# vim: expandtab shiftwidth=4:
Property changes on: t/steps/auto_warnings-07.t
___________________________________________________________________
Name: svn:mime-type
+ text/plain
Name: svn:keywords
+ Author Date Id Revision
Name: svn:eol-style
+ native
Index: t/steps/auto_warnings-08.t
===================================================================
--- t/steps/auto_warnings-08.t (revision 0)
+++ t/steps/auto_warnings-08.t (revision 0)
@@ -0,0 +1,98 @@
+#! perl
+# Copyright (C) 2007, The Perl Foundation.
+# $Id$
+# auto_warnings-08.t
+
+use strict;
+use warnings;
+use Test::More tests => 23;
+use Carp;
+use lib qw( lib t/configure/testlib );
+use_ok('config::init::defaults');
+use_ok('config::init::hints');
+use_ok('config::inter::progs');
+use_ok('config::auto::warnings');
+use Parrot::Configure;
+use Parrot::Configure::Options qw( process_options );
+use Parrot::Configure::Test qw( test_step_thru_runstep);
+use IO::CaptureOutput qw | capture |;
+
+my $args = process_options(
+ {
+ argv => [ ],
+ mode => q{configure},
+ }
+);
+
+my $conf = Parrot::Configure->new;
+
+test_step_thru_runstep( $conf, q{init::defaults}, $args );
+test_step_thru_runstep( $conf, q{init::hints}, $args );
+test_step_thru_runstep( $conf, q{inter::progs}, $args );
+
+my $pkg = q{auto::warnings};
+
+$conf->add_steps($pkg);
+$conf->options->set( %{$args} );
+
+my ( $task, $step_name, $step);
+$task = $conf->steps->[-1];
+$step_name = $task->step;
+
+$step = $step_name->new();
+ok( defined $step, "$step_name constructor returned defined value" );
+isa_ok( $step, $step_name );
+ok( $step->description(), "$step_name has description" );
+
+{
+ my ($stdout, $rv);
+ # Mock case where C compiler is not gcc.
+ $conf->data->set( gccversion => undef );
+ $conf->options->set( verbose => 1 );
+ capture(
+ sub { $rv = $step->runstep($conf); },
+ \$stdout,
+ );
+ ok($rv, "runstep() returned true value");
+ is($step->result(), q{skipped}, "Got expected result");
+ like($stdout,
+ qr/Currently we only set warnings/,
+ "Got expected verbose output"
+ );
+}
+
+pass("Completed all tests in $0");
+
+################### DOCUMENTATION ###################
+
+=head1 NAME
+
+auto_warnings-08.t - test config::auto::warnings
+
+=head1 SYNOPSIS
+
+ % prove t/steps/auto_warnings-08.t
+
+=head1 DESCRIPTION
+
+The files in this directory test functionality used by F<Configure.pl>.
+
+The tests in this file test config::auto::warnings when the C compiler
+being used is not I<gcc> and when C<--verbose> option has been selected.
+
+=head1 AUTHOR
+
+James E Keenan
+
+=head1 SEE ALSO
+
+config::auto::warnings, F<Configure.pl>.
+
+=cut
+
+# Local Variables:
+# mode: cperl
+# cperl-indent-level: 4
+# fill-column: 100
+# End:
+# vim: expandtab shiftwidth=4:
Property changes on: t/steps/auto_warnings-08.t
___________________________________________________________________
Name: svn:mime-type
+ text/plain
Name: svn:keywords
+ Author Date Id Revision
Name: svn:eol-style
+ native
Index: t/steps/auto_warnings-01.t
===================================================================
--- t/steps/auto_warnings-01.t (revision 24835)
+++ t/steps/auto_warnings-01.t (working copy)
@@ -49,6 +49,7 @@
%potential_warnings_seen = map { $_, 1 } @{ $step->{potential_warnings} };
ok($potential_warnings_seen{'-std=c89'}, "Cage warning added");
+
pass("Keep Devel::Cover happy");
pass("Completed all tests in $0");
Index: config/auto/warnings.pm
===================================================================
--- config/auto/warnings.pm (revision 24835)
+++ config/auto/warnings.pm (working copy)
@@ -130,15 +130,23 @@
my $verbose = $conf->options->get('verbose');
print "\n" if $verbose;
+ if ( defined $conf->data->get('gccversion') ) {
- # add on some extra warnings if requested
- $self->_add_cage_warnings($conf);
- $self->_add_maintainer_warnings($conf);
-
- # now try out our warnings
- for my $maybe_warning (@{ $self->{potential_warnings} }) {
- $self->try_warning( $conf, $maybe_warning, $verbose );
+ # add on some extra warnings if requested
+ $self->_add_cage_warnings($conf);
+ $self->_add_maintainer_warnings($conf);
+
+ # now try out our warnings
+ for my $maybe_warning (@{ $self->{potential_warnings} }) {
+ $self->try_warning( $conf, $maybe_warning, $verbose );
+ }
+ $self->set_result("set for gcc");
}
+ else {
+ print "Currently we only set warnings if using gcc as C compiler\n"
+ if $verbose;
+ $self->set_result("skipped");
+ }
return 1;
}
