You are viewing a plain text version of this content. The canonical link for it is here.
Posted to modperl-cvs@perl.apache.org by st...@apache.org on 2015/05/11 15:22:03 UTC

svn commit: r1678751 - in /perl/modperl/trunk: Makefile.PL lib/Apache2/Build.pm lib/ModPerl/BuildOptions.pm

Author: stevehay
Date: Mon May 11 13:22:03 2015
New Revision: 1678751

URL: http://svn.apache.org/r1678751
Log:
Use more effective means of finding apxs if MP_APXS is not specified on command line

The current logic in ModPerl::BuildOptions->init() just looks through the PATH for an apxs, but there is better logic (checking for an MP_APXS environment variable, amongst other things) in Apache2::Build->find_apxs_util(), so use that instead. That requires saving the value found in $build->{MP_APXS} rather than a file-scope lexical. Also change Makefile.PL to display the MP_APXS or MP_AP_PREFIX that it is using so that it is clear what option has been picked up.

With this change, the MP_APXS command-line argument will take precedence over anything else (as is the case now); the MP_APXS environment variable will be used instead if that is present; next, we check for MP_AP_PREFIX/bin/apxs; and finally we keep the last ditch attempt to find apxs in the PATH (corrected so that it works on Windows too - use the correct PATH separator, and look for apxs.bat).

This should hopefully fix CPAN RT#84732.

I've also renamed the misleading $self variable used in various parse*() functions in BuildOptions.pm since they are not called as methods so the first argument is actually the $build (i.e. the Apache2::Build) object passed in, not the ModPerl::BuildOptions class from init() (which was itself a class method call, not an object method call).

Modified:
    perl/modperl/trunk/Makefile.PL
    perl/modperl/trunk/lib/Apache2/Build.pm
    perl/modperl/trunk/lib/ModPerl/BuildOptions.pm

Modified: perl/modperl/trunk/Makefile.PL
URL: http://svn.apache.org/viewvc/perl/modperl/trunk/Makefile.PL?rev=1678751&r1=1678750&r2=1678751&view=diff
==============================================================================
--- perl/modperl/trunk/Makefile.PL (original)
+++ perl/modperl/trunk/Makefile.PL Mon May 11 13:22:03 2015
@@ -227,14 +227,14 @@ EOI
         : MIN_HTTPD_VERSION_DYNAMIC;
 
     if ($build->{MP_APXS}) {
-        debug "Using APXS => $build->{MP_APXS}";
+        print "Using APXS => $build->{MP_APXS}\n";
     }
     elsif ($build->{MP_AP_PREFIX}) {
         if (my $reason = $build->ap_prefix_invalid) {
             error "invalid MP_AP_PREFIX: $reason";
             exit 1;
         }
-        debug "Using Apache prefix => $build->{MP_AP_PREFIX}";
+        print "Using Apache prefix => $build->{MP_AP_PREFIX}\n";
     }
     else {
         unless ($build->{MP_USE_STATIC}) {

Modified: perl/modperl/trunk/lib/Apache2/Build.pm
URL: http://svn.apache.org/viewvc/perl/modperl/trunk/lib/Apache2/Build.pm?rev=1678751&r1=1678750&r2=1678751&view=diff
==============================================================================
--- perl/modperl/trunk/lib/Apache2/Build.pm (original)
+++ perl/modperl/trunk/lib/Apache2/Build.pm Mon May 11 13:22:03 2015
@@ -143,13 +143,15 @@ sub httpd_is_source_tree {
         defined $prefix && -d $prefix && -e "$prefix/CHANGES";
 }
 
-# try to find the apxs utility, set $apxs to the path if found,
+# try to find the apxs utility, set $self->{MP_APXS} to the path if found,
 # otherwise to ''
-my $apxs; # undef so we know we haven't tried to set it yet
 sub find_apxs_util {
     my $self = shift;
 
-    $apxs = ''; # not found
+    if (not defined $self->{MP_APXS}) {
+        $self->{MP_APXS} = ''; # not found
+    }
+
     my @trys = ($Apache2::Build::APXS,
                 $self->{MP_APXS},
                 $ENV{MP_APXS});
@@ -179,7 +181,7 @@ sub find_apxs_util {
         next unless ($apxs_try = $_);
         chomp $apxs_try;
         if (-x $apxs_try) {
-            $apxs = $apxs_try;
+            $self->{MP_APXS} = $apxs_try;
             last;
         }
     }
@@ -209,7 +211,7 @@ sub ap_destdir {
 sub apxs {
     my $self = shift;
 
-    $self->find_apxs_util() unless defined $apxs;
+    $self->find_apxs_util() unless defined $self->{MP_APXS};
 
     my $is_query = (@_ == 2) && ($_[0] eq '-q');
 
@@ -223,7 +225,7 @@ sub apxs {
         }
     }
 
-    unless ($apxs) {
+    unless ($self->{MP_APXS}) {
         my $prefix = $self->{MP_AP_PREFIX} || "";
         return '' unless -d $prefix and $is_query;
         my $val = $apxs_query{$_[1]};
@@ -231,15 +233,15 @@ sub apxs {
     }
 
     my $devnull = devnull();
-    my $val = qx($apxs @_ 2>$devnull);
+    my $val = qx($self->{MP_APXS} @_ 2>$devnull);
     chomp $val if defined $val;
 
     unless ($val) {
         # do we have an error or is it just an empty value?
-        my $error = qx($apxs @_ 2>&1);
+        my $error = qx($self->{MP_APXS} @_ 2>&1);
         chomp $error if defined $error;
         if ($error) {
-            error "'$apxs @_' failed:";
+            error "'$self->{MP_APXS} @_' failed:";
             error $error;
         }
         else {
@@ -921,6 +923,7 @@ sub new {
     my $self = bless {
         cwd        => Cwd::fastcwd(),
         MP_LIBNAME => 'mod_perl',
+        MP_APXS    => undef, # so we know we haven't tried to set it yet
         @_,
     }, $class;
 

Modified: perl/modperl/trunk/lib/ModPerl/BuildOptions.pm
URL: http://svn.apache.org/viewvc/perl/modperl/trunk/lib/ModPerl/BuildOptions.pm?rev=1678751&r1=1678750&r2=1678751&view=diff
==============================================================================
--- perl/modperl/trunk/lib/ModPerl/BuildOptions.pm (original)
+++ perl/modperl/trunk/lib/ModPerl/BuildOptions.pm Mon May 11 13:22:03 2015
@@ -21,6 +21,7 @@ use warnings;
 
 use Apache2::Build ();
 use Apache::TestTrace;
+use Config ();
 my $param_qr = qr([\s=]+);
 
 use constant VERBOSE => 1;
@@ -29,7 +30,7 @@ use constant UNKNOWN_FATAL => 2;
 use File::Spec;
 
 sub init {
-    my ($self, $build) = @_;
+    my ($class, $build) = @_;
 
     #@ARGV should override what's in .makepl_args.mod_perl2
     #but @ARGV might also override the default MP_OPTS_FILE
@@ -67,28 +68,30 @@ sub init {
     $build->{MP_COMPAT_1X} = 1
         unless exists $build->{MP_COMPAT_1X} && !$build->{MP_COMPAT_1X};
 
-    # make a last ditch effort to find apxs in $ENV{PATH}
+    # try to find apxs
     if (!$build->{MP_AP_PREFIX} && !$build->{MP_APXS}) {
+        $build->find_apxs_util();
 
-        my @paths = split(/:/, $ENV{PATH});
-        my $potential_apxs;
-        while (!$potential_apxs) {
-
-            last if scalar(@paths) == 0; # don't loop endlessly
-            $potential_apxs = File::Spec->catfile(shift @paths, 'apxs');
-            if (-e $potential_apxs && -x $potential_apxs) {
-
-                $build->{MP_APXS} = $potential_apxs;
-                print "MP_APXS unspecified, using $potential_apxs\n\n";
-            } else {
-                undef $potential_apxs;
+        # make a last ditch effort to find apxs in $ENV{PATH}
+        if (!$build->{MP_APXS}) {
+            my @paths = split(/$Config::Config{path_sep}/, $ENV{PATH});
+            my $potential_apxs;
+            while (!$potential_apxs) {
+                last if scalar(@paths) == 0; # don't loop endlessly
+                $potential_apxs = File::Spec->catfile(shift @paths, 'apxs');
+                $potential_apxs .= '.bat' if Apache2::Build::WIN32();
+                if (-e $potential_apxs && -x $potential_apxs) {
+                    $build->{MP_APXS} = $potential_apxs;
+                } else {
+                    undef $potential_apxs;
+                }
             }
         }
     }
 }
 
 sub parse {
-    my ($self, $lines, $opts) = @_;
+    my ($build, $lines, $opts) = @_;
 
     $opts = VERBOSE|UNKNOWN_FATAL unless defined $opts;
     my $table = table();
@@ -149,10 +152,10 @@ sub parse {
             }
 
             if ($table->{$key}->{append}){
-                $self->{$key} = join " ", grep $_, $self->{$key}, $val;
+                $build->{$key} = join " ", grep $_, $build->{$key}, $val;
             }
             else {
-                $self->{$key} = $val;
+                $build->{$key} = $val;
             }
 
             print "   $key = $val\n" if $opts & VERBOSE;
@@ -166,17 +169,17 @@ sub parse {
 }
 
 sub parse_file {
-    my $self = shift;
+    my $build = shift;
 
     my $fh;
     my @dirs = qw(./ ../ ./. ../.);
     push @dirs, "$ENV{HOME}/." if exists $ENV{HOME};
     my @files = map { $_ . 'makepl_args.mod_perl2' } @dirs;
-    unshift @files, $self->{MP_OPTIONS_FILE} if $self->{MP_OPTIONS_FILE};
+    unshift @files, $build->{MP_OPTIONS_FILE} if $build->{MP_OPTIONS_FILE};
 
     for my $file (@files) {
         if (open $fh, $file) {
-            $self->{MP_OPTIONS_FILE} = $file;
+            $build->{MP_OPTIONS_FILE} = $file;
             last;
         }
         $fh = undef;
@@ -184,22 +187,22 @@ sub parse_file {
 
     return unless $fh;
 
-    print "Reading Makefile.PL args from $self->{MP_OPTIONS_FILE}\n";
-    my $unknowns = parse($self, [<$fh>]);
+    print "Reading Makefile.PL args from $build->{MP_OPTIONS_FILE}\n";
+    my $unknowns = parse($build, [<$fh>]);
     push @ARGV, @$unknowns if $unknowns;
 
     close $fh;
 }
 
 sub parse_argv {
-    my $self = shift;
+    my $build = shift;
     return unless @ARGV;
 
     my @args = @ARGV;
     @ARGV = ();
 
     print "Reading Makefile.PL args from \@ARGV\n";
-    my $unknowns = parse($self, \@args);
+    my $unknowns = parse($build, \@args);
     push @ARGV, @$unknowns if $unknowns;
 }