-
Notifications
You must be signed in to change notification settings - Fork 2
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
Fix to build on Perl 5.37.9 #5
Conversation
…rors In Perl/perl5#20357 the Perl dev team fixed some inconsistencies in how eval behaved with regard to compile errors. Prior that PR the exact outcome of a compile error varied with the type and number of errors that were encountered, sometimes calling $SIG{__DIE__}, sometimes not, sometimes outputing "Execution ... aborted" messages, sometimes not. With the merge of that PR the behavior is now consistent, and in particular for this module the "Execution ... aborted" message is now always output when there is a compile error. This patch strips this message off the error before rethrowing the exception with _croak(). This patch also includes some minor changes to the code to make it easier to debug the code being eval()ed which I left commented out. I figured if it was useful to debug this issue then it might useful again in the future.
…stinct var names Using "$FILE" in multiple tests causes the "not imported" warnings. Simply replacing them with distinct names causes the warnings to go away.
a0c1328
to
5dd2c2f
Compare
@@ -28,15 +16,11 @@ WriteMakefile( | |||
'Slay::Maker' => 0.04, | |||
'File::Path' => 0, | |||
'File::Copy::Recursive' => 0, | |||
'ExtUtils::MakeMaker' => 7.66, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why require such a recent MakeMaker?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because the makfile does a bunch of contortions to deal with older makefiles, a simpler solution is to just require a modern one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That doesn't explain why it would need more than the 6.54
that was previously suggested
I'm puzzled by it removing the |
Removing it stopped the complaint about the license. |
Shrug. Your imagination is faulty I guess. [ What an arrogant comment. Sheesh. ] All of the below is assuming that I have removed the commit which alters the Makefile.PL and the META.yml in this PR, that is: 5dd2c2f
Notice there is complaint about an invalid license.
Conclusion, META_MERGE is buggy (why is Mark added twice?), and keeping the current META.yml isn't helpful particularly. IMO @eserte would be better off removing it, regenerating a MYMETA.yml and then copying that in place. |
Note this was reported again as Perl/perl5#21026 which I have closed as we still have Perl/perl5#20864 The failed AppVeyor tests seem to be due to a misconfiguration and are irrelevant to this PR. |
But when you «make dist», it will recreate the META.yml in the actual tarball, so any issue would reappear. |
The META.yml just does the wrong thing so remove it. Remove the logic to handle older ExtUtls::MakeMaker and instead just require a modern EUMM to proceed.
851d749
to
61bca8e
Compare
@Leont My primary intention of this PR was to fix the issue caused by my changes to Perl 5.37.9 so we could clear the related BBC ticket. The change to the Makefile.PL is a secondary feature because I wanted to stop it from reporting "Invalid LICENSE value 'perl_5' ignored", but since you keep complaining about it I have created #6 without that commit and I will close this PR. Feel free to post your own patch to fix the "invalid license" issue if you want. |
Well, I just tried it, and no, the issue does not reappear in the tarball. I have not dug into why, but I think it is because in the current META.yaml contains:
but the new one (recreated as you stated above) contains:
|
Fixes the test failure, silences "not imported" warnings during test, and bumps version to v0.14.
This also includes changes to Makefile.PL so that it depends on EUMM 7.66 (the latest), and removes the META.yml which was resulting in bogus warnings about an unknown license and bizarre MYMETA.yml contents even when run against a modern EUMM. It also simplifies the Makefile.PL to just depend on a modern EUMM instead of doing different stuff for older ones. AFAIK, EUMM is backwards compatible to perl 5.8.0 so there is no reason not to expect it to be up to date.
The test failure is related to: Perl/perl5#20357
Fixes: Perl/perl5#20864