Return hint file loading to quoted eval to make strict vars more useful #367
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The use of 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: c177d68 |
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.
mohawk2
approved these changes
Sep 17, 2020
|
This broke User-Utmp: https://rt.cpan.org/Ticket/Display.html?id=133492 |
That wasn't the intent to require strict on hints |
ntyni
added a commit
to ntyni/perl5
that referenced
this pull request
Sep 7, 2022
The idiom of do './hints/linux.pl' or die $@; broke with Perl-Toolchain-Gang/ExtUtils-MakeMaker#367 which made $self a lexical variable so it is no longer visible in a file loaded with 'do'. This silently makes the loaded Linux hints a no-op, leading to missing symbols, broken modules and test failures in at least NDBM_File and ODBM_File on GNU/Hurd, as reported by Samuel Thibault in https://bugs.debian.org/1018289 . The replacement of eval `cat hints/linux.pl` is not very sophisticated but should be enough for this, and alternatives seem overly verbose. Bug-Debian: https://bugs.debian.org/1018289
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.