search

Perl / perl5

🐪 The Perl programming language
https://dev.perl.org/perl5/
Other
1.98k stars 560 forks source link

CPAN.pm v1.59 chdirs before looking for perl #3332

Closed p5pRT closed 19 years ago

p5pRT commented 23 years ago

Migrated from rt.perl.org#5634 (status was 'resolved')

Searchable as RT5634$

p5pRT commented 23 years ago

From michael@shoebox.net

CPAN.pm v1.59 chdirs before looking for an appropriate perl binary\, causing an invocation such as​:

  bin/perl -MCPAN -e shell

to -not- use bin/perl to run Makefile.PL. As far as I can tell\, what's happening is that CPAN.pm chdirs to the build directory before looking for a valid perl binary. Naturally\, it doesn't see a bin/perl in the build
directory\, so it falls back on looking in the directories in $ENV{PATH}.

Perl Info ``` Site configuration information for perl 5.00503: Configured by michael at Mon Feb 7 05:03:08 AKST 2000. Summary of my perl5 (5.0 patchlevel 5 subversion 3) configuration: Platform: osname=linux, osvers=2.2.14, archname=i386-linux uname='linux beowulf 2.2.14 #1 wed jan 26 13:38:32 akst 2000 i586 unknown ' hint=recommended, useposix=true, d_sigaction=define usethreads=undef useperlio=undef d_sfio=undef Compiler: cc='cc', optimize='-O2', gccversion=2.95.2 20000116 (Debian GNU/Linux) cppflags='-Dbool=char -DHAS_BOOL -D_REENTRANT -DDEBIAN -I/usr/local/include' ccflags ='-Dbool=char -DHAS_BOOL -D_REENTRANT -DDEBIAN -I/usr/local/include' stdchar='char', d_stdstdio=undef, usevfork=false intsize=4, longsize=4, ptrsize=4, doublesize=8 d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=12 alignbytes=4, usemymalloc=n, prototype=define Linker and Libraries: ld='cc', ldflags =' -L/usr/local/lib' libpth=/usr/local/lib /lib /usr/lib libs=-lnsl -lndbm -lgdbm -ldbm -ldb -ldl -lm -lc -lposix -lcrypt libc=, so=so, useshrplib=false, libperl=libperl.a Dynamic Linking: dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-rdynamic' cccdlflags='-fPIC', lddlflags='-shared -L/usr/local/lib' Locally applied patches: @INC for perl 5.00503: /home/michael/perl5lib/i386-linux /home/michael/perl5lib /var/mp3/perl5lib /usr/lib/perl5/5.005/i386-linux /usr/lib/perl5/5.005 /usr/local/lib/site_perl/5.005/i386-linux /usr/local/lib/site_perl/5.005 /usr/lib/perl5 . Environment for perl 5.00503: HOME=/home/michael LANG (unset) LANGUAGE (unset) LD_LIBRARY_PATH (unset) LOGDIR (unset) PATH=/home/michael/bin:/usr/local/bin:/bin:/usr/bin:/usr/local/sbin:/sbin:/usr/sbin:/usr/bin/X11:/var/lib/postgres/bin:/home/oracle/bin:/usr/local/games:/usr/games:. PERL5LIB=/home/michael/perl5lib:/var/mp3/perl5lib PERL_BADLANG (unset) SHELL=/bin/bash ```
p5pRT commented 23 years ago

From @floatingatoll

On Wed\, 31 Jan 2001 michael@​shoebox.net wrote​:

CPAN.pm v1.59 chdirs before looking for an appropriate perl binary\, causing an invocation such as​:

bin/perl \-MCPAN \-e shell

to -not- use bin/perl to run Makefile.PL. As far as I can tell\, what's happening is that CPAN.pm chdirs to the build directory before looking for a valid perl binary. Naturally\, it doesn't see a bin/perl in the build
directory\, so it falls back on looking in the directories in $ENV{PATH}.

At line 4188\, it invokes CPAN​::Distribution​::perl to find a valid perl interpreter - but at line 4176\, it's already done a chdir $builddir.

And you can't do the test before\, because on MacOS\, we skip the whole find $perl step entirely. But we need the chdir.

Well\, here's the cleanest way I could figure to do it. Andreas\, will this work for you? This fixes the problem discussed above - works for me now.

R.

$ PATH=/usr/bin bin/perl -MCPAN -e 'die CPAN​::Distribution​::perl' /home/rs/bin/perl at -e line 1.

Inline Patch ```diff --- lib/CPAN.pm.orig Thu Feb 1 01:25:31 2001 +++ lib/CPAN.pm Thu Feb 1 01:31:19 2001 @@ -4173,10 +4173,10 @@ } $CPAN::Frontend->myprint("\n CPAN.pm: Going to build ".$self->id."\n\n"); my $builddir = $self->dir; - chdir $builddir or Carp::croak("Couldn't chdir $builddir: $!"); - $self->debug("Changed directory to $builddir") if $CPAN::DEBUG; if ($^O eq 'MacOS') { + chdir $builddir or Carp::croak("Couldn't chdir $builddir: $!"); + $self->debug("Changed directory to $builddir") if $CPAN::DEBUG; ExtUtils::MM_MacOS::make($self); return; } @@ -4186,6 +4186,8 @@ $system = $self->{'configure'}; } else { my($perl) = $self->perl or die "Couldn\'t find executable perl\n"; + chdir $builddir or Carp::croak("Couldn't chdir $builddir: $!"); + $self->debug("Changed directory to $builddir") if $CPAN::DEBUG; my $switch = ""; # This needs a handler that can be turned on or off: # $switch = "-MExtUtils::MakeMaker ". ```
p5pRT commented 23 years ago

From [Unknown Contact. See original ticket]

On Thu\, Feb 01\, 2001 at 02​:16​:45AM -0800\, Richard Soderberg wrote​:

At line 4188\, it invokes CPAN​::Distribution​::perl to find a valid perl interpreter - but at line 4176\, it's already done a chdir $builddir.

And you can't do the test before\, because on MacOS\, we skip the whole find $perl step entirely. But we need the chdir.

Well\, here's the cleanest way I could figure to do it. Andreas\, will this work for you? This fixes the problem discussed above - works for me now.

This is why I had difficulty crafting a patch\, and finally gave up because I figured I needed to do more research. What happens in the case of $self->{'configure'} being defined in​:

  if ($self->{'configure'}) {   $system = $self->{'configure'};   } else {

doesn't the chdir still need to be done?

Michael

p5pRT commented 23 years ago

From @floatingatoll

On Thu\, 1 Feb 2001\, Michael Fowler wrote​:

On Thu\, Feb 01\, 2001 at 02​:16​:45AM -0800\, Richard Soderberg wrote​:

And you can't do the test before\, because on MacOS\, we skip the whole find $perl step entirely. But we need the chdir.

This is why I had difficulty crafting a patch\, and finally gave up because I figured I needed to do more research. What happens in the case of $self->{'configure'} being defined in​:

if \($self\->\{'configure'\}\) \{
  $system = $self\->\{'configure'\};
\} else \{

doesn't the chdir still need to be done?

Oh\, blast. You're right.

Andreas\, this is starting to get a bit weird. Can you finish this fix? I'm not quite sure how you'd like it done.

R.

p5pRT commented 21 years ago

From @schwern

The attached patch fixes this bug by the simple method of storing the Perl we started with as an absolute path before anything is done. Then perl() can reference this information. I can't see how this could go wrong on MacOS.

I've also taken the liberty of speeding up perl() by caching its result.

p5pRT commented 21 years ago

From @schwern

CPAN.pm.patch ```diff --- lib/CPAN.pm 2003/07/16 01:15:55 1.1 +++ lib/CPAN.pm 2003/07/16 01:25:38 @@ -53,6 +53,8 @@ $CPAN::Signal ||= 0; $CPAN::Frontend ||= "CPAN::Shell"; $CPAN::Defaultsite ||= "ftp://ftp.perl.org/pub/CPAN"; +$CPAN::StartingPerl ||= CPAN::starting_perl(); + package CPAN; use strict qw(vars); @@ -84,6 +86,21 @@ } } + +#-> sub CPAN::starting_perl +sub starting_perl { + my @candidates = (); + push @candidates, File::Spec->file_name_is_absolute($^X) ? $^X : ""; + my $pwd = CPAN::anycwd(); + push @candidates, File::Spec->catfile(CPAN::anycwd(), $^X); + + my($perl) = grep { MM->maybe_command($_) } @candidates; + + return $perl; +} + + + #-> sub CPAN::shell ; sub shell { my($self) = @_; @@ -4394,10 +4411,9 @@ #-> sub CPAN::Distribution::perl ; sub perl { my($self) = @_; - my($perl) = File::Spec->file_name_is_absolute($^X) ? $^X : ""; - my $pwd = CPAN::anycwd(); - my $candidate = File::Spec->catfile($pwd,$^X); - $perl ||= $candidate if MM->maybe_command($candidate); + return $self->{perl} if defined $self->{perl}; + + my $perl = $CPAN::StartingPerl; unless ($perl) { my ($component,$perl_name); DIST_PERLNAME: foreach $perl_name ($^X, 'perl', 'perl5', "perl$]") { @@ -4412,7 +4428,7 @@ } } } - $perl; + $self->{perl} = $perl; } #-> sub CPAN::Distribution::make ; ```
p5pRT commented 19 years ago

From @smpeters

[schwern - Tue Jul 15 18​:31​:18 2003]​:

The attached patch fixes this bug by the simple method of storing the Perl we started with as an absolute path before anything is done. Then perl() can reference this information. I can't see how this could go wrong on MacOS.

I've also taken the liberty of speeding up perl() by caching its result.

This is still a problem as of CPAN 1.76.

p5pRT commented 19 years ago

From @schwern

[schwern - Tue Jul 15 18​:31​:18 2003]​: The attached patch fixes this bug by the simple method of storing the Perl we started with as an absolute path before anything is done. Then perl() can reference this information. I can't see how this could go wrong on MacOS.

I've also taken the liberty of speeding up perl() by caching its result.

This patch was never applied.

I've attached a better patch which simply moves the find perl logic from individual CPAN​::Distribution objects to be a function of the CPAN. The location of Perl is simply cached in a global.

p5pRT commented 19 years ago

From @schwern

CPAN.pm.patch ```diff --- lib/CPAN.pm 2005/07/12 05:15:37 1.1 +++ lib/CPAN.pm 2005/07/12 05:37:26 @@ -54,6 +54,8 @@ $CPAN::Signal ||= 0; $CPAN::Frontend ||= "CPAN::Shell"; $CPAN::Defaultsite ||= "ftp://ftp.perl.org/pub/CPAN"; +$CPAN::Perl ||= CPAN::find_perl(); + package CPAN; use strict qw(vars); @@ -85,6 +87,7 @@ } } + #-> sub CPAN::shell ; sub shell { my($self) = @_; @@ -668,6 +671,32 @@ #-> sub CPAN::getcwd ; sub getcwd {Cwd::getcwd();} +#-> sub CPAN::find_perl ; +sub find_perl { + my($perl) = File::Spec->file_name_is_absolute($^X) ? $^X : ""; + my $pwd = CPAN::anycwd(); + my $candidate = File::Spec->catfile($pwd,$^X); + $perl ||= $candidate if MM->maybe_command($candidate); + + unless ($perl) { + my ($component,$perl_name); + DIST_PERLNAME: foreach $perl_name ($^X, 'perl', 'perl5', "perl$]") { + PATH_COMPONENT: foreach $component (File::Spec->path(), + $Config::Config{'binexp'}) { + next unless defined($component) && $component; + my($abs) = File::Spec->catfile($component,$perl_name); + if (MM->maybe_command($abs)) { + $perl = $abs; + last DIST_PERLNAME; + } + } + } + } + + return $perl; +} + + #-> sub CPAN::exists ; sub exists { my($mgr,$class,$id) = @_; @@ -4407,29 +4436,11 @@ } } -#-> sub CPAN::Distribution::perl ; + sub perl { - my($self) = @_; - my($perl) = File::Spec->file_name_is_absolute($^X) ? $^X : ""; - my $pwd = CPAN::anycwd(); - my $candidate = File::Spec->catfile($pwd,$^X); - $perl ||= $candidate if MM->maybe_command($candidate); - unless ($perl) { - my ($component,$perl_name); - DIST_PERLNAME: foreach $perl_name ($^X, 'perl', 'perl5', "perl$]") { - PATH_COMPONENT: foreach $component (File::Spec->path(), - $Config::Config{'binexp'}) { - next unless defined($component) && $component; - my($abs) = File::Spec->catfile($component,$perl_name); - if (MM->maybe_command($abs)) { - $perl = $abs; - last DIST_PERLNAME; - } - } - } - } - $perl; + return $CPAN::Perl; } + #-> sub CPAN::Distribution::make ; sub make { ```
p5pRT commented 19 years ago

From @steve-m-hay

Michael G Schwern via RT wrote​:

[schwern - Tue Jul 15 18​:31​:18 2003]​: The attached patch fixes this bug by the simple method of storing the Perl we started with as an absolute path before anything is done. Then perl() can reference this information. I can't see how this could go wrong on MacOS.

I've also taken the liberty of speeding up perl() by caching its result.

This patch was never applied.

I've attached a better patch which simply moves the find perl logic from individual CPAN​::Distribution objects to be a function of the CPAN. The location of Perl is simply cached in a global.

The (new) patch looks fine to me (not that I'm any CPAN.pm expert)\, but before I apply it I'm just curious to understand why the existing code doesn't work.

On my Win32 system\, if I set the PATH to be just "C​:\WINDOWS;C​:\WINDOWS\system32" and cd to C​:\perl then the command

bin\perl -MCPAN -e shell

followed by\, say\, "make Sort​::Versions" works fine. (My perl binary is installed as C​:\perl\bin\perl.exe)

CPAN​::Distribution​::make does indeed chdir to the build directory before calling CPAN​::Distribution​::perl\, but that succeeds in finding perl (C​:\perl\bin\perl.exe) even though there is no bin\perl in the build directory.

It continues to work even after I insert

$^X = 'bin\\perl'

at the start of CPAN​::Distribution​::perl to simulate an OS that doesn't set $^X to an absolute path.

It works because the

  DIST_PERLNAME​: foreach $perl_name ($^X\, 'perl'\, 'perl5'\, "perl$]") {   PATH_COMPONENT​: foreach $component (File​::Spec->path()\,   $Config​::Config{'binexp'}) {

loops succeed when $perl_name is 'perl' and $component is $Config​::Config{'binexp'} (in my case\, 'C​:\perl\bin').

Why doesn't this work out for you?

Radan Computational Ltd.

The information contained in this message and any files transmitted with it are confidential and intended for the addressee(s) only. If you have received this message in error or there are any problems\, please notify the sender immediately. The unauthorized use\, disclosure\, copying or alteration of this message is strictly forbidden. Note that any views or opinions presented in this email are solely those of the author and do not necessarily represent those of Radan Computational Ltd. The recipient(s) of this message should check it and any attached files for viruses​: Radan Computational will accept no liability for any damage caused by any virus transmitted by this email.

p5pRT commented 19 years ago

From @schwern

On Tue\, Jul 12\, 2005 at 05​:25​:07PM +0100\, Steve Hay wrote​:

Why doesn't this work out for you?

Because it should never do that search. In your example you got lucky because the relative Perl you ran CPAN.pm with happened to match the one in $Config{binexp}. What if that's not true? Consider the following.

$ cd ~/tmp/ $ which perl /sw/bin/perl $ perl -v

This is perl\, v5.8.6 built for darwin-thread-multi-2level

\

$ ln -s /usr/bin/perl5.8.1 perl $ ./perl -v

This is perl\, v5.8.1-RC3 built for darwin-thread-multi-2level (with 1 registered patch\, see perl -V for more detail)

\

$ ./perl -MCPAN -e shell

cpan shell -- CPAN exploration and modules installation (v1.76) ReadLine support enabled

cpan> test Text​::Metaphone CPAN​: Storable loaded ok Going to read /Users/schwern/.cpan/Metadata   Database was generated on Mon\, 11 Jul 2005 22​:03​:40 GMT Running test for module Text​::Metaphone Running make for M/MS/MSCHWERN/Text-Metaphone-1.96.tar.gz ...etc...   CPAN.pm​: Going to build M/MS/MSCHWERN/Text-Metaphone-1.96.tar.gz

Checking if your kit is complete... Looks good Writing Makefile for Text​::Metaphone cp Metaphone.pm blib/lib/Text/Metaphone.pm gcc-3.3 -c -I/sw/include -fno-common -DPERL_DARWIN -no-cpp-precomp -fno-strict-aliasing -pipe -Os -DVERSION=\"1.96\" -DXS_VERSION=\"1.96\" "-I/sw/lib/perl5-core/5.8.6/darwin-thread-multi-2level/CORE" my_memory.c   ^^^^^

Ding ding ding ding ding! I ran CPAN.pm with 5.8.1 but its building with 5.8.6. Because it chdir'd to the build directory before trying to resolve my relative $^X it could not find it. So in desperation it started looking through my PATH and found /sw/bin/perl.

-- Michael G Schwern schwern@​pobox.com http​://www.pobox.com/~schwern Don't try the paranormal until you know what's normal.   -- "Lords and Ladies" by Terry Prachett

p5pRT commented 19 years ago

From @steve-m-hay

Michael G Schwern wrote​:

On Tue\, Jul 12\, 2005 at 05​:25​:07PM +0100\, Steve Hay wrote​:

Why doesn't this work out for you?

Because it should never do that search. In your example you got lucky because the relative Perl you ran CPAN.pm with happened to match the one in $Config{binexp}. What if that's not true?

It wasn't quite true. The relative Perl (bin\perl) didn't match $Config{binexp} (C​:\perl\bin). It was the 'perl' bit that matched $Config{binexp}. And wouldn't that always be the case?

It wasn't so much me getting lucky as you getting unlucky in your example below. Unlucky\, because CPAN​::Distribution​::perl happens to trawl through your PATH (where it accidentally found the wrong perl) before trying $Config{binexp} (where it would have found the right perl).

If you just changed

  PATH_COMPONENT​: foreach $component (File​::Spec->path()\,   $Config​::Config{'binexp'}) {

to

  PATH_COMPONENT​: foreach $component ($Config​::Config{'binexp'}\,   File​::Spec->path()) {

then your example below would work wouldn't it?

And it may also make sense for

  DIST_PERLNAME​: foreach $perl_name ($^X\, 'perl'\, 'perl5'\, "perl$]") {

to be changed to

  DIST_PERLNAME​: foreach $perl_name ('perl'\, "perl$]"\, $^X\, 'perl5') {

since 'perl' is very likely to be found in $Config{binexp}.

I agree that it makes much more sense to look for perl sooner rather than later\, and also caching the result is clearly far better than repeatedly re-searching for something that can't have changed\, but I'm just wondering if there is anything that we can learn from why your examples fail that can be used to improve the search for perl\, given that the same search code is going to be kept.

Consider the following.

$ cd ~/tmp/ $ which perl /sw/bin/perl $ perl -v

This is perl\, v5.8.6 built for darwin-thread-multi-2level

\

$ ln -s /usr/bin/perl5.8.1 perl $ ./perl -v

This is perl\, v5.8.1-RC3 built for darwin-thread-multi-2level (with 1 registered patch\, see perl -V for more detail)

\

$ ./perl -MCPAN -e shell

cpan shell -- CPAN exploration and modules installation (v1.76) ReadLine support enabled

cpan> test Text​::Metaphone CPAN​: Storable loaded ok Going to read /Users/schwern/.cpan/Metadata Database was generated on Mon\, 11 Jul 2005 22​:03​:40 GMT Running test for module Text​::Metaphone Running make for M/MS/MSCHWERN/Text-Metaphone-1.96.tar.gz ...etc... CPAN.pm​: Going to build M/MS/MSCHWERN/Text-Metaphone-1.96.tar.gz

Checking if your kit is complete... Looks good Writing Makefile for Text​::Metaphone cp Metaphone.pm blib/lib/Text/Metaphone.pm gcc-3.3 -c -I/sw/include -fno-common -DPERL_DARWIN -no-cpp-precomp -fno-strict-aliasing -pipe -Os -DVERSION=\"1.96\" -DXS_VERSION=\"1.96\" "-I/sw/lib/perl5-core/5.8.6/darwin-thread-multi-2level/CORE" my_memory.c ^^^^^

Ding ding ding ding ding! I ran CPAN.pm with 5.8.1 but its building with 5.8.6. Because it chdir'd to the build directory before trying to resolve my relative $^X it could not find it. So in desperation it started looking through my PATH and found /sw/bin/perl.

Radan Computational Ltd.

The information contained in this message and any files transmitted with it are confidential and intended for the addressee(s) only. If you have received this message in error or there are any problems\, please notify the sender immediately. The unauthorized use\, disclosure\, copying or alteration of this message is strictly forbidden. Note that any views or opinions presented in this email are solely those of the author and do not necessarily represent those of Radan Computational Ltd. The recipient(s) of this message should check it and any attached files for viruses​: Radan Computational will accept no liability for any damage caused by any virus transmitted by this email.

p5pRT commented 19 years ago

From @schwern

On Wed\, Jul 13\, 2005 at 08​:41​:48AM +0100\, Steve Hay wrote​:

It wasn't quite true. The relative Perl (bin\perl) didn't match $Config{binexp} (C​:\perl\bin). It was the 'perl' bit that matched $Config{binexp}. And wouldn't that always be the case?

No\, not if we want a chance to find the perl they ran with.

It wasn't so much me getting lucky as you getting unlucky in your example below. Unlucky\, because CPAN​::Distribution​::perl happens to trawl through your PATH (where it accidentally found the wrong perl) before trying $Config{binexp} (where it would have found the right perl).

If you just changed

    PATH\_COMPONENT​: foreach $component \(File​::Spec\->path\(\)\,
                    $Config​::Config\{'binexp'\}\) \{

to

    PATH\_COMPONENT​: foreach $component \($Config​::Config\{'binexp'\}\,
                    File​::Spec\->path\(\)\) \{

then your example below would work wouldn't it?

No\, because the Perl I ran exists nowhere in $Config{binexp} nor PATH. Its sitting in my temp directory. CPAN.pm will never find it unless it looks in the directory I ran the CPAN shell from. Thus it cannot chdir before looking for perl if we hope to find the same Perl the user ran CPAN.pm with.

And it may also make sense for

  DIST\_PERLNAME​: foreach $perl\_name \($^X\, 'perl'\, 'perl5'\, "perl$\]"\) \{

to be changed to

  DIST\_PERLNAME​: foreach $perl\_name \('perl'\, "perl$\]"\, $^X\, 'perl5'\) \{

since 'perl' is very likely to be found in $Config{binexp}.

Ack! No! That means in this situation​:

  $ which perl5.8.1   /usr/bin/perl5.8.1   $ which perl   /sw/bin/perl   $ perl -v

  This is perl\, v5.8.6 built for darwin-thread-multi-2level

  $ perl5.8.1 -MCPAN -e shell

Its going to look for "perl" before "perl5.8.1" and thus probably pick up /sw/bin/perl instead of /usr/bin/perl5.8.1

However\, putting "perl$]" before "perl5" makes sense. Its better to check for a Perl of a specific version and then degrade to a more general check. These days C\<sprintf "perl%vd"\, $^V> is more likely to match (ie. perl5.8.6).

The ordering should probably be​: $^X\, $versioned_perl\, "perl$]"\, "perl5"\, "perl". But that's another bug.

-- Michael G Schwern schwern@​pobox.com http​://www.pobox.com/~schwern Don't try the paranormal until you know what's normal.   -- "Lords and Ladies" by Terry Prachett

p5pRT commented 19 years ago

From @steve-m-hay

Michael G Schwern wrote​:

No\, [...]

No\, [...]

Ack! No! [...]

OK\, I give up. And I see you've sensibly moved this issue to a new ticket (36539).

Patch applied as change 25142.

Radan Computational Ltd.

The information contained in this message and any files transmitted with it are confidential and intended for the addressee(s) only. If you have received this message in error or there are any problems\, please notify the sender immediately. The unauthorized use\, disclosure\, copying or alteration of this message is strictly forbidden. Note that any views or opinions presented in this email are solely those of the author and do not necessarily represent those of Radan Computational Ltd. The recipient(s) of this message should check it and any attached files for viruses​: Radan Computational will accept no liability for any damage caused by any virus transmitted by this email.

p5pRT commented 19 years ago

@schwern - Status changed from 'open' to 'resolved'