Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Explicit mention of major version in code/tests #363

Open
toddr opened this issue Aug 4, 2020 · 4 comments
Open

Explicit mention of major version in code/tests #363

toddr opened this issue Aug 4, 2020 · 4 comments

Comments

@toddr
Copy link
Contributor

toddr commented Aug 4, 2020

@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
Copy link
Member

Leont commented Aug 4, 2020

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

@toddr
Copy link
Contributor Author

toddr commented Aug 4, 2020

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
Copy link
Member

haarg commented Aug 4, 2020

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
Copy link
Member

bingos commented Aug 5, 2020

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants