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

Return hint file loading to quoted eval to make strict vars more useful #367

Closed toddr closed 3 years ago

toddr commented 3 years ago

Return hint file loading to quoted eval to make strict vars more useful

This allows hints files to properly access $self without special magic. Note that no change to the documentation is required because the documentation has always stated that the code was 'evaled'.

An audit has already been done of the existing CPAN hint files. No files use utf8 or appear to have anything beyond latin1 bytes. No BOMs were seen either.

This change also reverts the behavior back to how it worked in perl 5.3. It's not clear what protection we were trying to put in place at the time. Before this change, hints have full access to the stash. Given that $self is a variable local to the subroutine, there shouldn't need to be any special protections. The only thing using do provides is protection from modifying lexically scoped package variables. This doesn't seem very valuable given that in exchange you have to say no strict 'vars' in hints.pl to get strict to be on.

The failure message is also more clear and hopefully easier to find and debug when it happens. Right now it is a little obscure in some make output when it fails.

toddr commented 3 years ago

The use of $self in hints files currently violates strict vars. This change avoids calling do which is essentially a require. strictures are reset for each require so this causes a strict problem with hints when blead perl is compiled with -Dusedefaultstrict.

I think this change moves things to work closer to the way they were supposed to to begin with.

For reference, this was the original change to use do: c177d6879f8580676bbb7949c275e3afd35d13b2

haarg commented 3 years ago

This broke User-Utmp: https://rt.cpan.org/Ticket/Display.html?id=133492

toddr commented 3 years ago

This broke User-Utmp: https://rt.cpan.org/Ticket/Display.html?id=133492

That wasn't the intent to require strict on hints