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

Explicit mention of major version in code/tests #363

Open toddr opened 3 years ago

toddr commented 3 years ago

@bingos I see you've been doing some cleanup. Thank you!

I wanted to make sure we shared this with you so you were aware. we noticed that perl5 is mentioned explicitly in some places. Maybe these places could use $^V->{version}->[0] or even int $] to get the major?

Thanks for everything you do.

Todd


$>git grep /perl5
lib/ExtUtils/MM_Any.pm:    my $libstyle = $Config{installstyle} || 'lib/perl5';
lib/ExtUtils/MM_Any.pm:        $manstyle = $self->{LIBSTYLE} eq 'lib/perl5' ? 'lib/perl5' : '';
lib/ExtUtils/MakeMaker.pm:    INSTALLARCHLIB     INSTALL_BASE/lib/perl5/$Config{archname}
lib/ExtUtils/MakeMaker.pm:    INSTALLPRIVLIB     INSTALL_BASE/lib/perl5
lib/ExtUtils/MakeMaker/FAQ.pod:This will put modules into F<~/lib/perl5>, man pages into F<~/man> and
lib/ExtUtils/MakeMaker/FAQ.pod:set your C<PERL5LIB> environment variable to F<~/lib/perl5> or tell
lib/ExtUtils/MakeMaker/FAQ.pod:    use lib "$ENV{HOME}/lib/perl5";
lib/ExtUtils/MakeMaker/FAQ.pod:    use lib "/path/to/your/home/dir/lib/perl5";
lib/ExtUtils/MakeMaker/FAQ.pod:And then set PERL5LIB to F<~/tmp/lib/perl5>.  This works well when you
lib/ExtUtils/MakeMaker/FAQ.pod:=item "No rule to make target `/usr/lib/perl5/CORE/config.h', needed by `Makefile'"
t/INSTALL_BASE.t:    ("$instdir/lib/perl5/Big/Dummy.pm",
t/INSTALL_BASE.t:     "$instdir/lib/perl5/Big/Liar.pm",
t/INSTALL_BASE.t:     "$instdir/lib/perl5/$Config{archname}/perllocal.pod",
t/INSTALL_BASE.t:     "$instdir/lib/perl5/$Config{archname}/auto/Big/Dummy/.packlist"
t/fixin.t:#!/foo/bar/perl5.8.8 -w
t/fixin.t:        unlike $lines[$shb_line_num], qr[/foo/bar/perl5.8.8], "#! replaced";
``
Leont commented 3 years ago

Changing some of these paths would require coordination between the various modules that use them.

toddr commented 3 years ago

I would assume they'd be 5 for now so no coordination would be needed? But if other code is also assuming 5 then totally we need to fix those up regardless.

haarg commented 3 years ago

The things that come to mind immediately that this will impact includes: local::lib, Module::Build, Module::Build::Tiny, ExtUtils::InstallPaths, https://github.com/Perl-Toolchain-Gang/cpan-api-buildpl/blob/master/lib/CPAN/API/BuildPL.pm#L275, CPAN.pm, cpanm, cpm, Carton

bingos commented 3 years ago

The stuff in lib/ExtUtils/MakeMaker.pm and lib/ExtUtils/MakeMaker/FAQ.pod is just documentation, not functional.

The real stuff is in in MM_Any.pm and MM_Unix.pm

lib/ExtUtils/MM_Any.pm:    my $libstyle = $Config{installstyle} || 'lib/perl5';
lib/ExtUtils/MM_Any.pm:        $manstyle = $self->{LIBSTYLE} eq 'lib/perl5' ? 'lib/perl5' : '';
lib/ExtUtils/MM_Any.pm:           lib      => [qw(lib perl5)],
lib/ExtUtils/MM_Any.pm:           arch     => [('lib', 'perl5', $Config{archname})],
lib/ExtUtils/MM_Unix.pm:                     ("perl$Config{version}", 'perl5', 'perl');

and relates to INSTALL_BASE

https://github.com/Perl-Toolchain-Gang/ExtUtils-MakeMaker/blob/master/lib/ExtUtils/MakeMaker.pm#L1568

https://github.com/Perl-Toolchain-Gang/ExtUtils-MakeMaker/blob/master/lib/ExtUtils/MM_Any.pm#L2196

It affects where the installed module will end up so the downstream tools that @haarg mentioned will need to be similarly updated.