Perl-Toolchain-Gang / ExtUtils-MakeMaker

Perl module to make Makefiles and build modules (what backs Makefile.PL)
https://metacpan.org/release/ExtUtils-MakeMaker
64 stars 76 forks source link

Warnings from ExtUtils/MM_Win32.pm on windows #361

Open atoomic opened 3 years ago

atoomic commented 3 years ago

I noticed this issue while checking a windows CI runner

2020-08-03T03:11:49.2474341Z Use of uninitialized value in concatenation (.) or string at D:\a\perl\perl\lib/ExtUtils/MM_Win32.pm line 146.
2020-08-03T03:11:49.2475026Z Use of uninitialized value in concatenation (.) or string at D:\a\perl\perl\lib/ExtUtils/MM_Win32.pm line 146.

view raw log from https://pipelines.actions.githubusercontent.com/StwKAELrIIfJZjsjvFeJdim2u3AOp8ez6qvldOETh5vCDDWoz5/_apis/pipelines/1/runs/305/signedlogcontent/24?urlExpires=2020-08-03T04%3A56%3A23.5820001Z&urlSigningMethod=HMACV1&urlSignature=gbS4rY1tn8pCqAYy5pW3xLRld2r6pQOEIaKaAgW4PwQ%3D

Suggested patch:

diff --git a/cpan/ExtUtils-MakeMaker/lib/ExtUtils/MM_Win32.pm b/cpan/ExtUtils-MakeMaker/lib/ExtUtils/MM_Win32.pm
index 3db0f45260..0be33edcb6 100644
--- a/cpan/ExtUtils-MakeMaker/lib/ExtUtils/MM_Win32.pm
+++ b/cpan/ExtUtils-MakeMaker/lib/ExtUtils/MM_Win32.pm
@@ -142,8 +142,10 @@ sub init_tools {
     $self->{NOOP}     ||= 'rem';
     $self->{DEV_NULL} ||= '> NUL';

+    my $src = $self->{PERL_SRC} || '';
+
     $self->{FIXIN}    ||= $self->{PERL_CORE} ?
-      "\$(PERLRUN) -I$self->{PERL_SRC}\\cpan\\ExtUtils-PL2Bat\\lib $self->{PERL_SRC}\\win32\\bin\\pl2bat.pl" :
+      "\$(PERLRUN) -I$src\\cpan\\ExtUtils-PL2Bat\\lib $src\\win32\\bin\\pl2bat.pl" :
       'pl2bat.bat';

     $self->SUPER::init_tools;
Leont commented 3 years ago

This warning points to an actually issue. Trying to make the warning go away without fixing that issue is rather the wrong approach.

PERL_SRC is supposed to contain the path to the root of the perl source tree. It is always supposed to be set if PERL_CORE is set, and in fact MakeMaker warns if it isn't.

Making that path empty will only result in an invalid path, this does not solve anything.

atoomic commented 3 years ago

Good point, I would wait for a better patch to fix this issue.