Re: [perl #44353] [BUG] Configure.pl: verbose-step option not working with named step

2007-10-05 Thread Allison Randal

James E Keenan wrote:


But I didn't actually implement this.  What I actually implemented was 
specifying the configuration step name *as an alternative to* specifying 
a portion of the step's description.  While this satisfied particle's 
original request, it led to some ugly code in 
Parrot::Configure::_run_this_step:

[...]
Code this ugly is difficult to test thoroughly.  In fact, as I've been 
working on this code in the reconfigure/ branch, this code excerpt is 
the only code in lib/Parrot/Configure.pm which is not 100% covered by 
the tests with respect to statements, branches and conditions.


Simplification is a good idea. I suspect most people would rather type 
some short unique value for the option than the text of the description 
anyway.


In the reconfigure/ branch, I have figured out how to do this with the 
--fatal-step option 
(https://rt.perl.org/rt3/Ticket/Display.html?id=45525).  I will soon be 
proposing a patch to trunk for --fatal-step, and it would be great if 
--verbose-step and --fatal-step could work in the same manner.


Agreed on wanting consistency here.

I should note that currently you can also specify --verbose-step=53 and 
have step # 53, and only #53, execute verbosely.  How that would be 
affected I haven't yet thought through?


Given a choice between the two, I'd rather see the step number in the 
non-verbose output:


 53)  Loading platform and local hints files...done.

than see the step name:

 inter::progs:  Loading platform and local hints files...done.

So, how about we make both --fatal-step and --verbose-step accept either 
a step number or step name. Show only the step number in the non-verbose 
output, but show both name and number in the verbose output. The comma 
delimited list could also work with numbers:


  perl Configure.pl --verbose-step=53,gen::makefiles,42

Selecting options by step name is an important feature, because the step 
numbers will change between releases. If someone has a script set up to 
run the config and compile with certain default options, they'll need to 
go by name instead of number.


Allison


[perl #44353] [BUG] Configure.pl: verbose-step option not working with named step

2007-08-05 Thread James Keenan via RT
particle:

I think this patch should get verbose-step to work the way you wanted it
to.  Please review.  This was developed in the 'step_verbosity_by_name'
branch; the attached is a diff between trunk and that branch.

kid51
Index: lib/Parrot/Configure/Options.pm
===
--- lib/Parrot/Configure/Options.pm (.../trunk) (revision 20496)
+++ lib/Parrot/Configure/Options.pm (.../branches/step_verbosity_by_name)   
(revision 20496)
@@ -138,6 +138,7 @@
--verboseOutput extra information
--verbose=2  Output every setting change
--verbose-step=N Set verbose for step N only
+   --verbose-step=name  Set verbose for step some::step only
--verbose-step=regex Set verbose for step matching description
--nomanicheckDon't check the MANIFEST
--step=(gen::languages)
Index: lib/Parrot/Configure.pm
===
--- lib/Parrot/Configure.pm (.../trunk) (revision 20496)
+++ lib/Parrot/Configure.pm (.../branches/step_verbosity_by_name)   
(revision 20496)
@@ -168,8 +168,11 @@
 sub add_steps {
 my ( $conf, @new_steps ) = @_;
 
-foreach my $step (@new_steps) {
-$conf-add_step($step);
+$conf-{list_of_steps} = [ @new_steps ];
+
+for (my $i = 0; $i = $#new_steps; $i++) {
+$conf-add_step($new_steps[$i]);
+$conf-{hash_of_steps}-{$new_steps[$i]} = $i + 1;
 }
 
 return 1;
@@ -260,20 +263,28 @@
 
 # set per step verbosity
 if ( defined $args-{verbose_step} ) {
-
-# by step number
 if (
-$args-{verbose_step} =~ /^\d+$/
-
-$args-{n} == $args-{verbose_step}
+(
+# by step number
+( $args-{verbose_step} =~ /^\d+$/ )
+and
+( $args-{n} == $args-{verbose_step} )
+)
+or
+(
+# by step name
+( ${$conf-{hash_of_steps}}{$args-{verbose_step}} )
+and
+( $args-{verbose_step} eq $step_name )
+)
+or
+(
+# by description
+$description =~ /$args-{verbose_step}/
+)
 ) {
 $conf-options-set( verbose = 2 );
 }
-
-# by description
-elsif ( $description =~ /$args-{verbose_step}/ ) {
-$conf-options-set( verbose = 2 );
-}
 }
 
 # RT#43673 cc_build uses this verbose setting, why?
@@ -312,6 +323,8 @@
 print ... if $args-{verbose}  $args-{verbose} == 2;
 print . x ( 71 - length($description) - length($result) );
 print $result. unless $step =~ m{^inter/}  $args-{ask};
+# reset verbose value for the next step
+$conf-options-set( verbose = $args-{verbose} );
 
 if ($conf-options-get(q{configure_trace}) ) {
 if (! defined $conftrace-[0]) {
@@ -325,8 +338,6 @@
 push @{$conftrace}, $evolved_data;
 nstore($conftrace, $sto);
 }
-# reset verbose value for the next step
-$conf-options-set( verbose = $args-{verbose} );
 }
 
 =item * Coption_or_data($arg)
Index: MANIFEST
===
--- MANIFEST(.../trunk) (revision 20496)
+++ MANIFEST(.../branches/step_verbosity_by_name)   (revision 20496)
@@ -1,7 +1,7 @@
 # ex: set ro:
 # $Id$
 #
-# generated by ./tools/dev/mk_manifest_and_skip.pl Fri Aug  3 15:09:40 2007 UT
+# generated by tools/dev/mk_manifest_and_skip.pl Sun Aug  5 14:47:24 2007 UT
 #
 # See tools/dev/install_files.pl for documentation on the
 # format of this file.
@@ -2891,6 +2891,7 @@
 t/configure/023-version.t   []
 t/configure/024-version.t   []
 t/configure/025-options_test.t  []
+t/configure/026-verbose_step_name.t []
 t/configure/101-init_manifest.01.t  []
 t/configure/101-init_manifest.02.t  []
 t/configure/102-init_defaults.01.t  []
Index: MANIFEST.SKIP
===
--- MANIFEST.SKIP   (.../trunk) (revision 20496)
+++ MANIFEST.SKIP   (.../branches/step_verbosity_by_name)   (revision 20496)
@@ -1,6 +1,6 @@
 # ex: set ro:
 # $Id$
-# generated by tools/dev/mk_manifest_and_skip.pl Thu Aug  2 01:41:36 2007 UT
+# generated by tools/dev/mk_manifest_and_skip.pl Sun Aug  5 14:47:24 2007 UT
 #
 # This file should contain a transcript of the svn:ignore properties
 # of the directories in the Parrot subversion repository. (Needed for
@@ -1740,6 +1740,8 @@
 ^t/tools/.*\.pbc/
 ^t/tools/.*\.pir$
 ^t/tools/.*\.pir/
+^t/tools/pdb\.t\..*$
+^t/tools/pdb\.t\..*/
 ^t/tools/pmc2c\..*\.c$
 ^t/tools/pmc2c\..*\.c/
 ^t/tools/pmc2c\..*\.dump$
Index: Configure.pl

Re: [perl #44353] [BUG] Configure.pl: verbose-step option not working with named step

2007-08-03 Thread James E Keenan

Joshua Hoblitt wrote:

The issue is here is that there is nothing in Configure.pl's output to
correlate the package name of the step with the output seen from a
typical run.

For example, say the test that outputs Determining architecture, OS and
JIT capability... is failing for some reason so you
want to run it verbosely?  Unless you are one of the PI number of people
in the world that has fiddled with the configure steps your not going to
know that steps package name is 'auto::jit'.  


I suspect, however, that the number of people in the world who at any 
moment *do* fiddle with the configure steps is a low multiple of PI -- 
and that they know where to look up the names of the steps 
(Parrot::Configure::Step::List).


The current system lets

you run that step verbosely with just `perl Configure.pl
--verbose-step=jit` and it just works.



But what if you say:  'perl Configure.pl --verbose-step=Determining' ? 
Then you get more than you want.  There's no requirement that the 
author/maintainer of a config/*/*.pm package provide a $description that 
unambiguously distinguishes that step from all others.



Possible ways forward:

1) Do nothing and document the behavior

2) add an additional test for a step with a package name matching the
string passed to --verbose-step



This is what particle was expecting to happen.  It's what I thought 
should have been happening -- even though upon re-reading the output of 
--help I can see that your interpretation is not unreasonable.



3) remove the description matching functionality

4) 2  3

5) remove --verbose-step altogether

6) remove the --verbose* options and just write a 'config.log'
on every run that can be searched with your text editor for choice.
This:
- removes code
- removes complexity
- is helpful for user support, please send me your config.log file



This, of course, is not so much a fix for the current problem as a 
request for a new feature, which should go into a separate RT.  It 
sounds both desirable and feasible.  (My recent addition of 
Parrot::Configure::Trace works somewhat the same way, logging the 
internal development of the P::C object over the course of the steps.)



7) have ever[y] configure step print its package name as part of its
output



I'm thinking that this is a happy compromise between the 'provide the 
step::name only' and 'provide something that appears in the description' 
position.  What the user would type to request verbosity would be fairly 
self-evident.



kid51


[perl #44353] [BUG] Configure.pl: verbose-step option not working with named step

2007-08-02 Thread via RT
# New Ticket Created by  James Keenan 
# Please include the string:  [perl #44353]
# in the subject line of all future correspondence about this issue. 
# URL: http://rt.perl.org/rt3/Ticket/Display.html?id=44353 


---
osname= linux
osvers= 2.6.18.3
arch=   i486-linux-gnu-thread-multi
cc= cc
---
Flags:
category=core
severity=medium
ack=no
---
particle reported tonight that the --verbose-step option to Configure.pl
doesn't DWIM.  It appears to work okay if you say:  perl Configure.pl
--verbose-step=2, but not if you name the step:

[li11-226:parrot] 515 $ perl Configure.pl --verbose-step=init::manifest
Parrot Version 0.4.14 Configure 2.0
Copyright (C) 2001-2007, The Perl Foundation.

Hello, I'm Configure. My job is to poke and prod your system to figure
out
how to build Parrot. The process is completely automated, unless you
passed in
the `--ask' flag on the command line, in which case I'll prompt you for
a few
pieces of info.

Since you're running this program, you obviously have Perl 5--I'll be
pulling
some defaults from its configuration.

Checking
MANIFEST.done.
Setting up Configure's default
values.done.
Setting up installation
paths.done.

I think the suspect code is here.  Start at line 274 of lib/Parrot/Configure.pm:

# by description
elsif ( $description =~ /$args-{verbose_step}/ ) {
$conf-options-set( verbose = 2 );
}

I feel that the regex test in the elsif makes no sense.  There's no
fundamental reason why a particular step's $description should match the
contents of $args-{verbose_step} -- which at this point should be
something like init::manifest.

Amazingly, this bad code has been in this package since before I began
to maintain it ... and because (a) no one complained until now and (b) I
didn't write a test, no one ever noticed.

Assignment:  Get the --verbose-step option working with a named step.

---
Summary of my parrot 0.4.14 (r20439) configuration:
  configdate='Fri Aug  3 02:54:55 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


Re: [perl #44353] [BUG] Configure.pl: verbose-step option not working with named step

2007-08-02 Thread Joshua Hoblitt
The issue is here is that there is nothing in Configure.pl's output to
correlate the package name of the step with the output seen from a
typical run.

For example, say the test that outputs Determining architecture, OS and
JIT capability... is failing for some reason so you
want to run it verbosely?  Unless you are one of the PI number of people
in the world that has fiddled with the configure steps your not going to
know that steps package name is 'auto::jit'.  The current system lets
you run that step verbosely with just `perl Configure.pl
--verbose-step=jit` and it just works.

Possible ways forward:

1) Do nothing and document the behavior

2) add an additional test for a step with a package name matching the
string passed to --verbose-step

3) remove the description matching functionality

4) 2  3

5) remove --verbose-step altogether

6) remove the --verbose* options and just write a 'config.log'
on every run that can be searched with your text editor for choice.
This:
- removes code
- removes complexity
- is helpful for user support, please send me your config.log file

7) have ever configure step print it's package name as part of it's
output

My vote would be for (in order of preference) 6, 2, 1.

Cheers,

-J

--
On Thu, Aug 02, 2007 at 07:59:53PM -0700, James Keenan wrote:
 # New Ticket Created by  James Keenan 
 # Please include the string:  [perl #44353]
 # in the subject line of all future correspondence about this issue. 
 # URL: http://rt.perl.org/rt3/Ticket/Display.html?id=44353 
 
 
 ---
 osname= linux
 osvers= 2.6.18.3
 arch=   i486-linux-gnu-thread-multi
 cc= cc
 ---
 Flags:
 category=core
 severity=medium
 ack=no
 ---
 particle reported tonight that the --verbose-step option to Configure.pl
 doesn't DWIM.  It appears to work okay if you say:  perl Configure.pl
 --verbose-step=2, but not if you name the step:
 
 [li11-226:parrot] 515 $ perl Configure.pl --verbose-step=init::manifest
 Parrot Version 0.4.14 Configure 2.0
 Copyright (C) 2001-2007, The Perl Foundation.
 
 Hello, I'm Configure. My job is to poke and prod your system to figure
 out
 how to build Parrot. The process is completely automated, unless you
 passed in
 the `--ask' flag on the command line, in which case I'll prompt you for
 a few
 pieces of info.
 
 Since you're running this program, you obviously have Perl 5--I'll be
 pulling
 some defaults from its configuration.
 
 Checking
 MANIFEST.done.
 Setting up Configure's default
 values.done.
 Setting up installation
 paths.done.
 
 I think the suspect code is here.  Start at line 274 of 
 lib/Parrot/Configure.pm:
 
 # by description
 elsif ( $description =~ /$args-{verbose_step}/ ) {
 $conf-options-set( verbose = 2 );
 }
 
 I feel that the regex test in the elsif makes no sense.  There's no
 fundamental reason why a particular step's $description should match the
 contents of $args-{verbose_step} -- which at this point should be
 something like init::manifest.
 
 Amazingly, this bad code has been in this package since before I began
 to maintain it ... and because (a) no one complained until now and (b) I
 didn't write a test, no one ever noticed.
 
 Assignment:  Get the --verbose-step option working with a named step.
 
 ---
 Summary of my parrot 0.4.14 (r20439) configuration:
   configdate='Fri Aug  3 02:54:55 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


pgpmawiJduTaG.pgp
Description: PGP signature