# New Ticket Created by James Keenan # Please include the string: [perl #43655] # in the subject line of all future correspondence about this issue. # <URL: http://rt.perl.org/rt3/Ticket/Display.html?id=43655 >
--- osname= linux osvers= 2.6.18.3 arch= i486-linux-gnu-thread-multi cc= cc --- Flags: category=core severity=medium ack=no --- In Parrot::Configure, the heavy lifting of running a configuration step is done by an internal method heretofore named _runstep(). This identifier is too easily confused with two public methods, each of which invoke _runstep(): runstep() and runsteps(). (Are you beginning to see the problem?) This patch renames _runstep() to _run_this_step() inside lib/Parrot/Configure.pm as well as in two test files which referenced _runstep(). In addition, this patch modifies the interface to _run_this_step(). Heretofore, _runstep() took a list of 5 scalars as arguments. That's at least 2 arguments too many. _run_this_step() now takes a single hash reference for its argument. The keys of this hash closely follow the scalars previously provided as arguments. This change has been tested and implemented in the reconfigure branch and tested in trunk. The change has no negative effect on configuration and should not be visible to the ordinary user of Configure.pl. Please review. Thank you very much. kid51 Index: lib/Parrot/Configure.pm =================================================================== --- lib/Parrot/Configure.pm (revision 19698) +++ lib/Parrot/Configure.pm (working copy) @@ -184,12 +184,19 @@ sub runsteps { my $self = shift; - my ( $verbose, $verbose_step, $ask ) = $self->options->get(qw(verbose verbose-step ask)); + my $n = 0; # step number + my ( $verbose, $verbose_step, $ask ) = + $self->options->get( qw( verbose verbose-step ask ) ); - my $n = 0; # step number foreach my $task ( $self->steps ) { $n++; - $self->_runstep( $task, $verbose, $verbose_step, $ask, $n ); + $self->_run_this_step( { + task => $task, + verbose => $verbose, + verbose_step => $verbose_step, + ask => $ask, + n => $n, + } ); } return 1; } @@ -208,24 +215,29 @@ my $self = shift; my $taskname = shift; - my ( $verbose, $verbose_step, $ask ) = $self->options->get(qw(verbose verbose-step ask)); + my ( $verbose, $verbose_step, $ask ) = + $self->options->get( qw( verbose verbose-step ask ) ); for my $task ( $self->steps() ) { if ( $task->{"Parrot::Configure::Task::step"} eq $taskname ) { - $self->_runstep( $task, $verbose, $verbose_step, $ask, 1 ); + $self->_run_this_step( { + task => $task, + verbose => $verbose, + verbose_step => $verbose_step, + ask => $ask, + n => 1, + } ); } } } -sub _runstep { +sub _run_this_step { my $self = shift; - my $task = shift; + my $args = shift; - my ( $verbose, $verbose_step, $ask, $n ) = @_; + my $step_name = $args->{task}->step; + my @step_params = @{ $args->{task}->params }; - my $step_name = $task->step; - my @step_params = @{ $task->params }; - eval "use $step_name;"; die $@ if $@; @@ -237,24 +249,24 @@ $description = "" unless defined $description; # set per step verbosity - if ( defined $verbose_step ) { + if ( defined $args->{verbose_step} ) { # by step number - if ( $verbose_step =~ /^\d+$/ && $n == $verbose_step ) { + if ( $args->{verbose_step} =~ /^\d+$/ && $args->{n} == $args->{verbose_step} ) { $self->options->set( verbose => 2 ); } # by description - elsif ( $description =~ /$verbose_step/ ) { + elsif ( $description =~ /$args->{verbose_step}/ ) { $self->options->set( verbose => 2 ); } } # XXX cc_build uses this verbose setting, why? - $self->data->set( verbose => $verbose ) if $n > 2; + $self->data->set( verbose => $args->{verbose} ) if $args->{n} > 2; print "\n", $description, '...'; - print "\n" if $verbose && $verbose == 2; + print "\n" if $args->{verbose} && $args->{verbose} == 2; my $ret; # step return value eval { @@ -283,12 +295,12 @@ my $result = $step->result || 'done'; - print "..." if $verbose && $verbose == 2; + print "..." if $args->{verbose} && $args->{verbose} == 2; print "." x ( 71 - length($description) - length($result) ); - print "$result." unless $step =~ m{^inter/} && $ask; + print "$result." unless $step =~ m{^inter/} && $args->{ask}; # reset verbose value for the next step - $self->options->set( verbose => $verbose ); + $self->options->set( verbose => $args->{verbose} ); } =back Index: t/configure/004-configure.t =================================================================== --- t/configure/004-configure.t (revision 19698) +++ t/configure/004-configure.t (working copy) @@ -67,7 +67,7 @@ can_ok("Parrot::Configure", qw| add_steps |); can_ok("Parrot::Configure", qw| runstep |); can_ok("Parrot::Configure", qw| runsteps |); -can_ok("Parrot::Configure", qw| _runstep |); +can_ok("Parrot::Configure", qw| _run_this_step |); $conf->add_steps(get_steps_list()); my @confsteps = @{$conf->steps}; Index: t/postconfigure/01-data_slurp.t =================================================================== --- t/postconfigure/01-data_slurp.t (revision 19698) +++ t/postconfigure/01-data_slurp.t (working copy) @@ -67,7 +67,7 @@ can_ok("Parrot::Configure", qw| add_steps |); can_ok("Parrot::Configure", qw| runstep |); can_ok("Parrot::Configure", qw| runsteps |); -can_ok("Parrot::Configure", qw| _runstep |); +can_ok("Parrot::Configure", qw| _run_this_step |); $conf->add_steps(get_steps_list()); my @confsteps = @{$conf->steps}; --- Summary of my parrot 0.4.13 (r19698) configuration: configdate='Sun Jul 8 18:35:47 2007 GMT' Platform: osname=linux, archname=i686-linux jitcapable=1, jitarchname=i386-linux, jitosname=LINUX, jitcpuarch=i386 execcapable=1 perl=/usr/local/bin/perl Compiler: cc='cc', ccflags=' -pipe -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -D_GNU_SOURCE', Linker and Libraries: ld='cc', ldflags=' -L/usr/local/lib', cc_ldflags='', libs='-lnsl -ldl -lm -lcrypt -lutil -lpthread -lrt' Dynamic Linking: share_ext='.so', ld_share_flags='-shared -L/usr/local/lib -fPIC', load_ext='.so', ld_load_flags='-shared -L/usr/local/lib -fPIC' Types: iv=long, intvalsize=4, intsize=4, opcode_t=long, opcode_t_size=4, ptrsize=4, ptr_alignment=1 byteorder=1234, nv=double, numvalsize=8, doublesize=8 --- Environment: HOME =/home/jimk LANG (unset) LANGUAGE (unset) LD_LIBRARY_PATH (unset) LOGDIR (unset) PATH =/usr/local/bin:/usr/local/bin:/usr/bin:/bin:/usr/bin/X11:/usr/games:/usr/local/mysql/bin:/home/jimk/bin:/home/jimk/bin/perl SHELL =/bin/bash