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; }