On Sat Apr 12 07:57:11 2008, particle wrote:
> On Fri, Apr 11, 2008 at 12:15 PM, via RT Bernhard Schmalhofer
> <[EMAIL PROTECTED]> wrote:
> > # New Ticket Created by  Bernhard Schmalhofer
> >  # Please include the string:  [perl #52776]
> >  # in the subject line of all future correspondence about this issue.
> >  # <URL: http://rt.perl.org/rt3/Ticket/Display.html?id=52776 >
> >
> >
> >  In the Makefile templates the line
> >
> >  RM_F     = @rm_f@
> >
> >  is frequently used.
> >  This usually expands to
> >
> >  RM_F     = $(PERL) -MExtUtils::Command -e rm_f
> >
> >  This utterly fails when $(PERL) isn't set up. So a saner expansion of
> >  $(RM_F) and friends is neede.
> >
> every parrot makefile depends on perl being available on the system.
> we should make sure every makefile template matches /[EMAIL PROTECTED]@$/
> 
> ~jerry
> 

Instead of forcing us to write more identical config information, let's just 
use the data we 
already have at config time:


 Let's borrow the syntax from the makefile generation itself, and just have 
RM_F's definition 
be:

+        rm_f      => '@perl@ -MExtUtils::Command -e rm_f',

We already know what @perl@ is at compile time (even if we don't necessarily 
know it when 
define rm_f, but that's ok), and there's no problem having it show up multiple 
times in the 
generated Makefile. By going this route, the generated Makefile lines would 
look like this for 
me on feather, e.g.:

RM_F             = /home/coke/bin/perl -MExtUtils::Command -e rm_f

Attached is a proof of concept patch that allows the configure data object to 
treat values 
passed to set with @foo@ in them specially, just like the makefile generator 
does. It mentions 
but doesn't actually implement the callback necessary if one were to do:

$conf->set('rm_f' =>'@perl@ -MExtUtils::Command -e rm_f');
$conf->set('perl' =>'/home/coke/bin/perl');

But this is a SMOP. Parrot builds for me with the patch as is, but only because 
we happen to 
call the sets in the right order at the moment. I suppose I could call YAGNI 
and ignore the 
other path for now, or replace with a die "unimplemented until we need it.".

Additionally (though it won't help the original problem, it will help remove a 
ton of 
boilerplate), we can also eliminate all the RM_F = @rm_f@ boilerplate in the 
makefiles, and 
simply use @rm_f@ where we now use $(RM_F).

The patch above and the strategy here would let us simplify the makefile 
templates 
considerably.

Comments?


Index: lib/Parrot/Configure/Data.pm

===================================================================

--- lib/Parrot/Configure/Data.pm        (revision 28149)

+++ lib/Parrot/Configure/Data.pm        (working copy)

@@ -132,7 +132,10 @@

 =item * Purpose

 

 Modifies or creates new values in the main part of the Parrot::Configure

-object's data structure..

+object's data structure. If the value contains a string like @foo@, the

+value for the key foo is interpolated in its place. If the key for

+foo hasn't been set yet, a callback is registered to do the interpolation

+later.

 

 =item * Arguments

 

@@ -156,8 +159,29 @@

     while ( my ( $key, $val ) = splice @_, 0, 2 ) {

         print "\t$key => ", defined($val) ? "'$val'" : 'undef', ",\n"

             if $verbose;

+

+

+        if (defined $val) {

+            while ($val =~ m/\@(\w+)\@/) {

+                my $proxykey = $1;

+                if (exists $self->{c}{$proxykey}) {

+                    $val =~ s/\@(\w+)\@/$self->{c}{$proxykey}/;

+                } else {

+                    warn "WHOOPS: needs a callback\n";

+                }

+            }

+        }

+

         $self->{c}{$key} = $val;

 

         foreach my $trigger ( $self->gettriggers($key) ) {

             print "\tcalling trigger $trigger for $key\n" if $verbose;

             my $cb = $self->gettrigger( $key, $trigger );

Index: config/init/defaults.pm

===================================================================

--- config/init/defaults.pm     (revision 28149)

+++ config/init/defaults.pm     (working copy)

@@ -178,11 +178,11 @@

         perl      => $^X,

         perl_inc  => $self->find_perl_headers(),

         test_prog => 'parrot',

-        rm_f      => '$(PERL) -MExtUtils::Command -e rm_f',

-        rm_rf     => '$(PERL) -MExtUtils::Command -e rm_rf',

-        mkpath    => '$(PERL) -MExtUtils::Command -e mkpath',

-        touch     => '$(PERL) -MExtUtils::Command -e touch',

-        chmod     => '$(PERL) -MExtUtils::Command -e ExtUtils::Command::chmod',

+        rm_f      => '@perl@ -MExtUtils::Command -e rm_f',

+        rm_rf     => '@perl@ -MExtUtils::Command -e rm_rf',

+        mkpath    => '@perl@ -MExtUtils::Command -e mkpath',

+        touch     => '@perl@ -MExtUtils::Command -e touch',

+        chmod     => '@perl@ -MExtUtils::Command -e ExtUtils::Command::chmod',

         ar        => $Config{ar},

         ar_flags  => 'cr',

 

@@ -202,14 +202,14 @@

 

         # make_c: Command to emulate GNU make's C<-C directory> option:  chdir

         # to C<directory> before executing $(MAKE)

-        make_c => '$(PERL) -e \'chdir shift @ARGV; system q{$(MAKE)}, @ARGV; 
exit $$? >> 8;\'',

+        make_c => '@perl@ -e \'chdir shift @ARGV; system q{$(MAKE)}, @ARGV; 
exit $$? >> 8;\'',

 

         # if platform has a .s file that needs to be assembled

         platform_asm => 0,

         as           => 'as',    # assembler

 

-        cp    => '$(PERL) -MExtUtils::Command -e cp',

-        mv    => '$(PERL) -MExtUtils::Command -e mv',

+        cp    => '@perl@ -MExtUtils::Command -e cp',

+        mv    => '@perl@ -MExtUtils::Command -e mv',

         lns   => $Config{lns},                          # soft link

         slash => '/',



Reply via email to