# 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

Reply via email to