From @schwern

Several places try to convert $^X to an absolute path. CPAN->perl,
ExtUtils​::MM_Unix->find_perl and Module​::Build all do this. CPANPLUS is
generally not affected as it lets either Module​::Build or the shell figure
it out.

The search order is typically some variant on​:

  my @​perls = ($^X, 'perl', 'perl5', "perl$]");
  my @​paths = (File​::Spec->path, $Config{binexp});

  foreach my $perl (@​perls) {
  foreach my $path (@​paths) {
  ...
  }
  }

In the case when $^X cannot be found this algorithm is likely to find not
just the wrong perl but the wrong version of Perl. This is because it
will look for "perl" before "perl5" and "perl5" before "perl5.00504".
I've had it accidentally pick up perl1 before! The order should be
reversed, looking for the more version specific names before the generic
ones.

Additionally, since 5.6 perl binaries have been named "perl5.6.1" and not
"perl5.00601". This algorithm does not search for that.

So the @​perls list should be​:

  @​perls = ($^X);
  push @​perls, sprintf "perl%vd", $^V if defined $^V;
  push @​perls, ("perl$]", "perl5", "perl");

Module​::Build and CPANPLUS do not appear to be vulnerable to this as they
only check for $^X and do not try any fallback filenames. CPANPLUS doesn't
even try to make $^X absolute and leaves that up to the shell or
Module​::Build. This is probably safer as if it cannot find your perl it
will yelp rather than silently risk running the wrong version. Changing
CPAN and MakeMaker's behaviors at this point isn't worth it, but a warning
wouldn't hurt.

Additionally, CPAN looks for $^X in the cwd but does it wrong. The
algorithm should be something like this​:

  # $^X is absolute
  if( File​::Spec->file_name_is_absolute($^X) ) {
  push @​perls, $^X;
  }
  else {
  my $first_dir = File​::Spec->splitdir($^X))[0];

  # $^X is ./path/to/perl or ../path/to/perl. Make it
  # absolute using the cwd.
  if( $first_dir eq File​::Spec->curdir or
  $first_dir eq File​::Spec->updir )
  {
  push @​perls, File​::Spec->rel2abs($^X);
  }
  }
  # else leave $^X alone and do a PATH search

This most closely simulates how a shell finds perl. CPAN.pm makes the
mistake of always looking for $^X in the cwd. This means it can get the
following wrong​:

  $ ls ./perl
  ./perl
  $ perl -MCPAN -e shell

$^X will be "perl". It will look for "$cwd/perl" and find the one in the
cwd rather than perform a PATH search.

MakeMaker, CPANPLUS and Module​::Build do not handle this case at all, they do
not look in the cwd. In this case MakeMaker and CPANPLUS will use the
relative $^X, which is dangerous because it will go wrong as soon as
something chdirs.

Module​::Build has a heuristic which checks to see if the perl it has found
is the same as the perl it was run with so though it will not find the
perl in the cwd it will not be fooled by ones later in the search. The
heuristic is pretty simple​: it compares the output of Config​::myconfig.

To summarize​:

  Change the filename search order to look for the most specific
  versions first. (Module​::Build and CPANPLUS not affected)

  Add perlX.Y.Z to the filename search. (MB and CP not affected)

  Add a warning when we cannot find $^X and must fall back to
  another filename. (MB and CP not affected)

  Look for "$cwd/$^X" only when $^X is ./perl or ../perl.
  (all affected)

  Compare myconfig of the found perl and the perl we were run with
  to better ensure we found the right Perl. (MB already does this.
  CP not affected as it does not search for Perl)

I'll patch up MakeMaker to do this and provide a patch for CPAN.pm. If I'm
feeling gung-ho I might do CPANPLUS and Module​::Build, too.

--
Michael G Schwern schwern@​pobox.com http​://www.pobox.com/~schwern
Reality is that which, when you stop believing in it, doesn't go away.
  -- Phillip K. Dick

From @kenahoo

Hi Schwern,

That's good analysis, but I don't actually think anything should be
changed in Module​::Build's method until we can demonstrate a case in
which it fails, and right now I know of none. Since it checks
Config​::myconfig(), it ensures that the version and any compilation
options are correct.

Perhaps it could also check a variable like $Config{installarchlib} or
something, to make sure we've got the right library path, in case the
crazy user has two exactly identically-configured perls installed in
two different places we happen to find with the search algorithm.

On Jul 13, 2005, at 5​:41 PM, Michael G Schwern wrote​:

CPAN.pm makes the
mistake of always looking for $^X in the cwd. This means it can get
the
following wrong​:

$ ls \./perl
\./perl
$ perl \-MCPAN \-e shell

$^X will be "perl". It will look for "$cwd/perl" and find the one in
the
cwd rather than perform a PATH search.

MakeMaker, CPANPLUS and Module​::Build do not handle this case at all,
they do
not look in the cwd. In this case MakeMaker and CPANPLUS will use the
relative $^X, which is dangerous because it will go wrong as soon as
something chdirs.

Regarding the current-working directory inspection, it seems to me that
we shouldn't check there unless the CWD is in the PATH. Which M​::B
currently does.

  -Ken

From @schwern

On Wed, Jul 13, 2005 at 10​:15​:50PM -0500, Ken Williams wrote​:

That's good analysis, but I don't actually think anything should be
changed in Module​::Build's method until we can demonstrate a case in
which it fails, and right now I know of none. Since it checks
Config​::myconfig(), it ensures that the version and any compilation
options are correct.

Installing Module​::Build with an uninstalled Perl (ie. you build Perl and
run it from the source directory) will fail to find perl.

0 ~/devel/Module-Build$ ../../../../usr/local/src/bleadperl/perl -I/usr/local/src/bleadperl/lib -w Build.PL
Warning​: Can't locate your perl binary at lib/Module/Build/Base.pm line 139.

More common than you think, especially when working on bleadperl. MakeMaker
even has special code to deal with this case​:

0 ~/devel/ExtUtils-MakeMaker$ ../../../../usr/local/src/bleadperl/perl -I/usr/local/src/bleadperl/lib -w Makefile.PL
... Detected uninstalled Perl. Trying to continue.
Have /usr/local/src/bleadperl/lib/Config.pm expected /usr/local/perl/bleadperl/lib/5.9.3/darwin-thread-multi-2level/Config.pm
Writing Makefile for ExtUtils​::MakeMaker

The simple fix is to do the $cwd/$^X check if $^X is ./foo or ../foo.

Why wait until someone gets bit in the ass?

Perhaps it could also check a variable like $Config{installarchlib} or
something, to make sure we've got the right library path, in case the
crazy user has two exactly identically-configured perls installed in
two different places we happen to find with the search algorithm.

Or you could just compare the values of $INC{"Config.pm"}.

--
Michael G Schwern schwern@​pobox.com http​://www.pobox.com/~schwern
Ahh email, my old friend. Do you know that revenge is a dish that is best
served cold? And it is very cold on the Internet!

From @kenahoo

On Jul 14, 2005, at 2​:58 AM, Michael G Schwern wrote​:

On Wed, Jul 13, 2005 at 10​:15​:50PM -0500, Ken Williams wrote​:

That's good analysis, but I don't actually think anything should be
changed in Module​::Build's method until we can demonstrate a case in
which it fails, and right now I know of none. Since it checks
Config​::myconfig(), it ensures that the version and any compilation
options are correct.

Installing Module​::Build with an uninstalled Perl (ie. you build Perl
and
run it from the source directory) will fail to find perl.

0 ~/devel/Module-Build$ ../../../../usr/local/src/bleadperl/perl
-I/usr/local/src/bleadperl/lib -w Build.PL
Warning​: Can't locate your perl binary at lib/Module/Build/Base.pm
line 139.

If we do allow an uninstalled perl, then what happens when the user
does "Build install"? Unless we add code for adjusting the path, or
unless the user provides a suitable --destdir, it'll install to a
location that's not ripe yet.

The fact that you have to use a relative path rather than an absolute
one is a little bit weird, eh? It means you have to know this weird
fact about how perl will be searched for.

More common than you think, especially when working on bleadperl.
MakeMaker
even has special code to deal with this case​:

0 ~/devel/ExtUtils-MakeMaker$ ../../../../usr/local/src/bleadperl/perl
-I/usr/local/src/bleadperl/lib -w Makefile.PL
... Detected uninstalled Perl. Trying to continue.

How do you detect the fact that it's uninstalled? If this is truly the
main use-case for searching in CWD, perhaps we should just add such
detection code, and offer to set --destdir appropriately.

  -Ken

From @schwern

On Thu, Jul 14, 2005 at 07​:47​:49AM -0500, Ken Williams wrote​:

If we do allow an uninstalled perl, then what happens when the user
does "Build install"? Unless we add code for adjusting the path, or
unless the user provides a suitable --destdir, it'll install to a
location that's not ripe yet.

This is the user's problem, and presumably if they're installing modules with
an uninstalled Perl they know something of what they're doing. They have
presumably set a prefix or install_base or whatever. I'll often shove
stuff into /tmp/lib/ to test out bleadperl.

The fact that you have to use a relative path rather than an absolute
one is a little bit weird, eh? It means you have to know this weird
fact about how perl will be searched for.

I think most folks don't even think about it, it should just work. Why
would there be weird searching going on at all? Normally one wouldn't even
think a program would have to search for the program which executed it.

More common than you think, especially when working on bleadperl.
MakeMaker
even has special code to deal with this case​:

0 ~/devel/ExtUtils-MakeMaker$ ../../../../usr/local/src/bleadperl/perl
-I/usr/local/src/bleadperl/lib -w Makefile.PL
... Detected uninstalled Perl. Trying to continue.

How do you detect the fact that it's uninstalled? If this is truly the
main use-case for searching in CWD, perhaps we should just add such
detection code, and offer to set --destdir appropriately.

Voodoo. There's a bunch of heuristics in MM_Unix around line 1604
which mostly involves sniffing around for perl.h. The UNINSTALLED_PERL
flag which gets set is used to make sure that perl can find its libraries
by setting up the appropriate -Is when its run.

  # Make sure perl can find itself before it's installed.
  $self->{$run} .= q{ "-I$(PERL_LIB)" "-I$(PERL_ARCHLIB)"}
  if $self->{UNINSTALLED_PERL} || $self->{PERL_CORE};

So that might be useful to put into MB.

I think the Perl build process takes advantage of all this when it asks
if you want to install any extra modules from CPAN. It would be interesting
to see what happened when you ask it for Module​::Build. I think it will
still work as the perl Makefile is smart enough to set PERL5LIB appropriately
rather than rely on the build system to do it.

--
Michael G Schwern schwern@​pobox.com http​://www.pobox.com/~schwern
Just call me 'Moron Sugar'.
  http​://www.somethingpositive.net/sp05182002.shtml

@jkeenan - Status changed from 'new' to 'open'

From @jkeenan

On Wed Jul 13 15​:41​:54 2005, schwern wrote​:

Several places try to convert $^X to an absolute path. CPAN->perl,
ExtUtils​::MM_Unix->find_perl and Module​::Build all do this. CPANPLUS
is
generally not affected as it lets either Module​::Build or the shell
figure
it out.

The search order is typically some variant on​:

my @​perls = \($^X\, 'perl'\, 'perl5'\, "perl$\]"\);
my @​paths = \(File​::Spec\->path\, $Config\{binexp\}\);

foreach my $perl \(@​perls\) \{
    foreach my $path \(@​paths\) \{
        \.\.\.
    \}
\}

In the case when $^X cannot be found this algorithm is likely to find
not
just the wrong perl but the wrong version of Perl. This is because it
will look for "perl" before "perl5" and "perl5" before "perl5.00504".
I've had it accidentally pick up perl1 before! The order should be
reversed, looking for the more version specific names before the
generic
ones.

Additionally, since 5.6 perl binaries have been named "perl5.6.1" and
not
"perl5.00601". This algorithm does not search for that.

So the @​perls list should be​:

@​perls = \($^X\);
push @​perls\, sprintf "perl%vd"\, $^V if defined $^V;
push @​perls\, \("perl$\]"\, "perl5"\, "perl"\);

Module​::Build and CPANPLUS do not appear to be vulnerable to this as
they
only check for $^X and do not try any fallback filenames. CPANPLUS
doesn't
even try to make $^X absolute and leaves that up to the shell or
Module​::Build. This is probably safer as if it cannot find your perl
it
will yelp rather than silently risk running the wrong version.
Changing
CPAN and MakeMaker's behaviors at this point isn't worth it, but a
warning
wouldn't hurt.

Additionally, CPAN looks for $^X in the cwd but does it wrong. The
algorithm should be something like this​:

\# $^X is absolute
if\( File​::Spec\->file\_name\_is\_absolute\($^X\) \) \{
    push @​perls\, $^X;
\}
else \{
    my $first\_dir = File​::Spec\->splitdir\($^X\)\)\[0\];

    \# $^X is \./path/to/perl or \.\./path/to/perl\.  Make it
    \# absolute using the cwd\.
    if\( $first\_dir eq File​::Spec\->curdir or
        $first\_dir eq File​::Spec\->updir \)
    \{
        push @​perls\, File​::Spec\->rel2abs\($^X\);
    \}
\}
\# else leave $^X alone and do a PATH search

This most closely simulates how a shell finds perl. CPAN.pm makes the
mistake of always looking for $^X in the cwd. This means it can get
the
following wrong​:

$ ls \./perl
\./perl
$ perl \-MCPAN \-e shell

$^X will be "perl". It will look for "$cwd/perl" and find the one in
the
cwd rather than perform a PATH search.

MakeMaker, CPANPLUS and Module​::Build do not handle this case at all,
they do
not look in the cwd. In this case MakeMaker and CPANPLUS will use the
relative $^X, which is dangerous because it will go wrong as soon as
something chdirs.

Module​::Build has a heuristic which checks to see if the perl it has
found
is the same as the perl it was run with so though it will not find the
perl in the cwd it will not be fooled by ones later in the search.
The
heuristic is pretty simple​: it compares the output of
Config​::myconfig.

To summarize​:

Change the filename search order to look for the most specific
versions first\.  \(Module​::Build and CPANPLUS not affected\)

Add perlX\.Y\.Z to the filename search\.  \(MB and CP not affected\)

Add a warning when we cannot find $^X and must fall back to
another filename\.  \(MB and CP not affected\)

Look for "$cwd/$^X" only when $^X is \./perl or \.\./perl\.
\(all affected\)

Compare myconfig of the found perl and the perl we were run with
to better ensure we found the right Perl\.  \(MB already does this\.
CP not affected as it does not search for Perl\)

I'll patch up MakeMaker to do this and provide a patch for CPAN.pm.
If I'm
feeling gung-ho I might do CPANPLUS and Module​::Build, too.

This RT was originally filed by Schwern in 2005. There was some
back-and-forth with Ken Williams, original author of Module​::Build, but
then discussion petered out.

Is this an issue which P5P needs to address? Is it something we could
refer to the Toolchain Gang?

Thank you very much.
Jim Keenan

From @Leont

On Sat, Sep 7, 2013 at 4​:29 AM, James E Keenan via RT <
perlbug-followup@​perl.org> wrote​:

This RT was originally filed by Schwern in 2005. There was some
back-and-forth with Ken Williams, original author of Module​::Build, but
then discussion petered out.

Is this an issue which P5P needs to address? Is it something we could
refer to the Toolchain Gang?

Yeah, I think this is a discussion for the toolchain gang, not p5p. Though
I'm not sure there really is all that much to discuss, unless
Module​::Build's eviction makes life more difficult for downstream packagers.

Leon

From @jkeenan

On Sat Sep 07 00​:49​:27 2013, LeonT wrote​:

On Sat, Sep 7, 2013 at 4​:29 AM, James E Keenan via RT <
perlbug-followup@​perl.org> wrote​:

This RT was originally filed by Schwern in 2005. There was some
back-and-forth with Ken Williams, original author of Module​::Build, but
then discussion petered out.

Is this an issue which P5P needs to address? Is it something we could
refer to the Toolchain Gang?

Yeah, I think this is a discussion for the toolchain gang, not p5p. Though
I'm not sure there really is all that much to discuss, unless
Module​::Build's eviction makes life more difficult for downstream
packagers.

Leon

I forwarded the original post to the cpan-workers mailing list, where
the first reaction to the problem was skeptical. I will monitor that
list for a week, then close this ticket unless someone feels there is a
problem to be solved by the porters and proceeds to solve it.

Thank you very much.
Jim Keenan

From @jkeenan

On Sat Sep 07 14​:48​:59 2013, jkeenan wrote​:

On Sat Sep 07 00​:49​:27 2013, LeonT wrote​:

On Sat, Sep 7, 2013 at 4​:29 AM, James E Keenan via RT <
perlbug-followup@​perl.org> wrote​:

This RT was originally filed by Schwern in 2005. There was some
back-and-forth with Ken Williams, original author of
Module​::Build, but
then discussion petered out.

Is this an issue which P5P needs to address? Is it something we could
refer to the Toolchain Gang?

Yeah, I think this is a discussion for the toolchain gang, not p5p.
Though
I'm not sure there really is all that much to discuss, unless
Module​::Build's eviction makes life more difficult for downstream
packagers.

Leon

I forwarded the original post to the cpan-workers mailing list, where
the first reaction to the problem was skeptical. I will monitor that
list for a week, then close this ticket unless someone feels there is a
problem to be solved by the porters and proceeds to solve it.

There have been no further posts to the cpan-workers mailing list in the
past week
(http​://www.nntp.perl.org/group/perl.cpan.workers/2013/09/msg1062.html)
on this topic (or any other!). So I see no reason not to close this
ticket now. Doing so.

Thank you very much.
Jim Keenan

@jkeenan - Status changed from 'open' to 'rejected'