Bug #5386 for Module-Build: Module::Build can use the wrong Perl and install the module in the wrong location

Thu Feb 19 11:28:04 2004 Guest - Ticket created

Subject:

Module::Build can use the wrong Perl and install the module in the wrong location

Assuming the goal of Module::Build's find_perl_interpreter method is to determine the full path to the Perl interpreter that is currently running, then it gets it wrong. It relies heavily on $Config{perlpath} being correct, but there's no guarantee of that and it's often wrong in many common cases. One such case is if you install a version of Perl, say 5.6.1, and, then some number of months later, you install a new version of Perl, say 5.8.0, in the same directory, say /usr/local/. Your /usr/local/bin will have the following Perl executables: perl5.6.1, perl5.8.0, and perl, which perl being a hard link to perl5.8.0. Now let's say you have some application which works only with perl5.6.1 but not perl5.8.0. Perhaps there's a bug in perl5.8.0 or whatever. Now suppose this 5.6.1-only application needs some module that utilizes Module::Build installation. When the user invokes "perl5.6.1 Build.PL", Module::Build actually ends up using perl5.8.0 and installing the module in the 5.8.0 directory instead of the 5.6.1 directory that the user wanted. The workaround is to invoke it as "/usr/local/bin/perl5.6.1 Build.PL" instead, but, for someone who has been using MakeMaker to install modules for many years, this will come as a big surprise when you find out you've installed the module in the wrong location. ExtUtils::MM's init_PERL method (see also EU::MM's find_perl) gets it right, and I think in your attempt to simplify the implementation for Module::Build you may have oversimplified. If you want to keep things simple, File::Spec->canonpath($^X) works better if you require a recent enough version of File::Spec (doesn't work with the version of File::Spec that ships with 5.6.1, for example). In any event, please consider incorporating EU::MM's init_PERL algorithm for determining the full path to the currently running Perl into Module::Build. % perl5.6.1 -MModule::Build -e 'print Module::Build->new('dist_name' => 'foo', 'dist_version' => 1.0)->find_perl_interpreter,"\n"' /usr/local/bin/perl <-- actually perl5.8.0!!! % /usr/local/bin/perl5.6.1 -MModule::Build -e 'print Module::Build->new('dist_name' => 'foo', 'dist_version' => 1.0)->find_perl_interpreter,"\n"' /usr/local/bin/perl5.6.1 Compare this with the result you get from ExtUtils::MM: % perl5.6.1 -MExtUtils::MM -e 'print ExtUtils::MM->new({ 'NAME' => 'foo' })->{PERL},"\n"' /usr/local/bin/perl5.6.1

Thu Feb 19 14:14:04 2004 kwilliams [...] cpan.org - Status changed from 'new' to 'open'

Thu Feb 19 14:14:04 2004 kwilliams [...] cpan.org - Correspondence added

I'm aware that occasionally find_perl_interpreter() returns the wrong result, but I don't think it actually ever installs into the wrong location unless your Config.pm is totally out of whack. Could you provide an example? Using File::Spec->canonpath($^X) is fairly useless, because it will fail whenever $^X isn't an absolute path. Perhaps one way to safeguard against choosing the wrong path is to compare the result of `$fullperl -V` against Config->myconfig() in the current process. By the way, find_perl_interpreter() works as a class method in recent versions of M::B, so you can just do: perl -MModule::Build -le 'print Module::Build->find_perl_interpreter' -Ken

Thu Feb 19 15:22:14 2004 Guest - Correspondence added

From:

edwardjsabol [...] iname.com

[KWILLIAMS - Thu Feb 19 14:14:04 2004]: Show quoted text

> I'm aware that occasionally find_perl_interpreter() returns the wrong > result, but I don't think it actually ever installs into the wrong > location unless your Config.pm is totally out of whack. Could you > provide an example?

Not at the moment. I could be wrong about it installing in the wrong place. I was going from memory based on a CPAN-assisted installation I did recently in which the module (I forget which one) did get installed in the wrong location, but it's possible that was due to either user error or a CPAN configuration problem. Let's leave that aside for a second. In the scenario I described in my previous e-mail, would Module::Build use the correct version of Perl for the tests? That's just as important, I think. If it does, then you seem to be saying that find_perl_interpreter() only needs to find any Perl interpreter and not necessarily the Perl interpreter that is currently running in order for Module::Build to install and test correctly. Show quoted text

> Using File::Spec->canonpath($^X) is fairly useless, because it will > fail whenever $^X isn't an absolute path.

Oh, you're right. I was fooled by Perl. I guess Perl 5.8.x ensures that $^X is an absolute path, so when I tested File::Spec->canonpath($^X) with 5.8.0 and got an absolute path back, I thought it was because of some change to the functionality of File::Spec, but it's not. % perl5.8.0 -MFile::Spec -e 'print File::Spec->canonpath($^X),"\n"' /usr1/local/bin/perl5.8.0 % perl5.6.1 -MFile::Spec -e 'print File::Spec->canonpath($^X),"\n"' perl5.6.1 OK, so that was a dead-end. In any event, I think I'm advocating using the algorithm from ExtUtils::MM. Basically, if $^X isn't absolute, search the PATH for the executable in question and prepend the directory it was found in to make it absolute. ExtUtils::MM jumps through a few more hoops than that even, but that's the gist of it, I think. Show quoted text

> By the way, find_perl_interpreter() works as a class method in recent > versions of M::B, so you can just do: > > perl -MModule::Build -le 'print Module::Build->find_perl_interpreter'

Ooh, that's convenient. Let me tell you where I'm coming from... I'm investigating some Perl 5.6.1 build problems associated with mod_perl and Apache::Request. I'm one of the peripheral Apache::Request developers working on libapreq2 (Apache::Request) for mod_perl 2. Anyway, both mod_perl and Apache::Request currently use Apache::Build and/or Apache::Test which in turn use $Config{perlpath}. I've already described a common scenario in which $Config{perlpath} gives the wrong version of Perl. If you try to install and test Apache::Request in the scenario I described earlier, it would definitely build and test using the wrong version of Perl and, if the tests actually passed, it would install in the wrong location too. I was asked to look at how ExtUtils::MM and Module::Build do it to see if there was a correct method for determining the absolute path to the currently running Perl process. In my tests, ExtUtils::MM got it correct in every scenario and platform and Perl version that I tested and, well, Module::Build didn't, as seen in my previous e-mail.

Mon Feb 23 18:37:28 2004 kwilliams [...] cpan.org - Correspondence added

[guest - Thu Feb 19 15:22:14 2004]: Show quoted text

> then you seem to be saying that > find_perl_interpreter() only needs to find any Perl interpreter and > not necessarily the Perl > interpreter that is currently running in order for Module::Build to > install and test correctly.

No, to be completely correct, it does need to find the *correct* perl interpreter. What I'm saying is that even if it finds the wrong one though, there's only a small set of things that will be affected, and one of those things is running regression tests. But building and installing modules should not be affected, because they don't use an external perl interpreter. Show quoted text

> In any event, I think I'm advocating using > the algorithm from > ExtUtils::MM. Basically, if $^X isn't absolute, search the PATH for > the executable in question > and prepend the directory it was found in to make it absolute. > ExtUtils::MM jumps through a > few more hoops than that even, but that's the gist of it, I think.

I'll see what I can work out for it.

Mon Feb 23 19:22:17 2004 ken [...] mathforum.org - Correspondence added

Date:	Mon, 23 Feb 2004 18:22:13 -0600
Subject:	Re: [cpan #5386] Module::Build can use the wrong Perl and install the module in the wrong location
From:	Ken Williams <ken [...] mathforum.org>
To:	bug-Module-Build [...] rt.cpan.org
RT-Send-Cc:

Try this patch. Unfortunately it requires backticks... -Ken Index: lib/Module/Build/Base.pm =================================================================== RCS file: /cvsroot/module-build/Module-Build/lib/Module/Build/Base.pm,v retrieving revision 1.258 diff -u -r1.258 Base.pm --- lib/Module/Build/Base.pm 21 Feb 2004 19:31:19 -0000 1.258 +++ lib/Module/Build/Base.pm 24 Feb 2004 00:15:01 -0000 @@ -166,11 +166,27 @@ } sub find_perl_interpreter { - my $perl; - File::Spec->file_name_is_absolute($perl = $^X) - or -f ($perl = $Config::Config{perlpath}) - or ($perl = $^X); - return $perl; + return $^X if File::Spec->file_name_is_absolute($^X); + my $proto = shift; + my $c = ref($proto) ? $proto->{config} : \%Config::Config; + my $exe = $c->{exe_ext}; + + my $thisperl = $^X; + if ($proto->os_type eq 'VMS') { + # VMS might have a file version at the end + $thisperl .= $exe unless $thisperl =~ m/$exe(;\d+)?$/i; + } elsif (defined $exe) { + $thisperl .= $exe unless $thisperl =~ m/$exe$/i; + } + + my @candidates = ($c->{perlpath}); + push @candidates, map File::Spec->catfile($_, $thisperl), File::Spec->path(); + + my $myconfig = Config->myconfig; + foreach my $perl (@candidates) { + return $perl if `$perl -MConfig=myconfig -e print -e myconfig` eq $myconfig; + } + return; } sub base_dir { shift()->{properties}{base_dir} }

Mon Feb 23 20:06:59 2004 Guest - Correspondence added

From:

sabol [...] alderaan.gsfc.nasa.gov

[ken@mathforum.org - Mon Feb 23 19:22:17 2004]: Show quoted text

> Try this patch. Unfortunately it requires backticks...

Thanks, Ken. It worked with the 5.00503, 5.6.1, and 5.8.0 executables I have installed here. Unfortunately, I have no convenient way of testing it on VMS or Windows-like platforms though. I hope that's not a problem for you. One quibble: I would change my @candidates = ($c->{perlpath}); push @candidates, map File::Spec->catfile($_, $thisperl), File::Spec->path(); to my @candidates = map File::Spec->catfile($_, $thisperl), File::Spec->path(); push @candidates, $c->{perlpath}; IMHO, $Config{perlpath} should be the last resort, not the first one. On my system, perl is a link to perl5.00503. Using your version of the patch: % perl5.00503 -MModule::Build -e 'print Module::Build->find_perl_interpreter,"\n"' /usr1/local/bin/perl % perl5.00503 -MModule::Build -e 'print Module::Build->find_perl_interpreter,"\n"' /usr1/local/bin/perl5.00503 I guess it's a matter of taste, since they are functionally the same perl executable, but, if I specify perl5.00503 on the command line, I would prefer it if find_perl_interpreter() would return the full path to perl5.00503 and not what it is linked to. Also, I think you can make the loop over @candidates more efficient if you put a -f test in there to test for existence and to make sure it's a file before executing the backticked command. return $perl if -f $perl && `$perl -MConfig=myconfig -e print -e myconfig` eq $myconfig;

Mon Feb 23 23:26:40 2004 ken [...] mathforum.org - Correspondence added

Date:	Mon, 23 Feb 2004 22:26:38 -0600
From:	"Ken Williams" <ken [...] mathforum.org>
To:	bug-Module-Build [...] rt.cpan.org
Subject:	Re: [cpan #5386] Module::Build can use the wrong Perl and install the module in the wrong location
RT-Send-Cc:

Hi Edward, Both your suggestions sound good to me, I'll implement them. Thanks. -Ken

Tue Feb 24 21:25:46 2004 Guest - Correspondence added

From:

sabol [...] alderaan.gsfc.nasa.gov

[ken@mathforum.org - Mon Feb 23 19:22:17 2004]: Show quoted text

> Try this patch. Unfortunately it requires backticks...

Thanks, Ken. It worked with the 5.00503, 5.6.1, and 5.8.0 executables I have installed here. Unfortunately, I have no convenient way of testing it on VMS or Windows-like platforms though. I hope that's not a problem for you. One quibble: I would change my @candidates = ($c->{perlpath}); push @candidates, map File::Spec->catfile($_, $thisperl), File::Spec->path(); to my @candidates = map File::Spec->catfile($_, $thisperl), File::Spec->path(); push @candidates, $c->{perlpath}; IMHO, $Config{perlpath} should be the last resort, not the first one. On my system, perl is a link to perl5.00503. Using your version of the patch: % perl5.00503 -MModule::Build -e 'print Module::Build->find_perl_interpreter,"\n"' /usr1/local/bin/perl % perl5.00503 -MModule::Build -e 'print Module::Build->find_perl_interpreter,"\n"' /usr1/local/bin/perl5.00503 I guess it's a matter of taste, since they are functionally the same perl executable, but, if I specify perl5.00503 on the command line, I would prefer it if find_perl_interpreter() would return the full path to perl5.00503 and not what it is linked to. Also, I think you can make the loop over @candidates more efficient if you put a -f test in there to test for existence and to make sure it's a file before executing the backticked command. return $perl if -f $perl && `$perl -MConfig=myconfig -e print -e myconfig` eq $myconfig;

Sun Feb 29 09:07:31 2004 kwilliams [...] cpan.org - Status changed from 'open' to 'resolved'