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

Minimum perl version added #1

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
2 participants
@prateek88

prateek88 commented Feb 24, 2015

I hae added MIN_PERL_VERSION in Makefile.PL. I have got this minimum perl version from Perl::MinimumVersion module.

@mschilli

This comment has been minimized.

Owner

mschilli commented Feb 25, 2015

Hmm .... Simple.pm says 5.003, do you know why we would need 5.8.3?

@prateek88

This comment has been minimized.

prateek88 commented Feb 25, 2015

Hi Mike,

I used Perl::MinimumVersion module to get the minimum version required. This module checks from the the line "require v5.###.###" and also checks minimum version required for used syntax. I also used perldelta for getting the changes in perl v5.8.3.

Perldelta says and also documented in Perl::MinimumVersion:

"Reading $^E now preserves $!. Previously, the C code implementing $^E did not preserve errno , so reading $^E could cause errno and therefore $! to change unexpectedly."

And Simple.pm uses $^E in lines 544 and 632.
That's why I thought to make it 5.8.3.

@mschilli

This comment has been minimized.

Owner

mschilli commented Mar 1, 2015

Interesting, but I don't think this will cause problems with earlier perl versions because all Proc::Simple does is declare

local( $., $@, $!, $^E, $? );

to avoid waitpid() changing the exit status. It's not actually making use of $^E, so I think the warning of the MinimumVersion module is a false alert.

Thanks for reporting this, but I think we're okay leaving the minimum version number unchanged.

@mschilli mschilli closed this Mar 1, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment