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

Do not try to check min_perl_version during prereq registration #9

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

autarch
Copy link

@autarch autarch commented Jun 9, 2018

The call to $self->min_perl_version in the default sub for eumm_version would
be unlikely to get something correct. This is because eumm_version ends up
being called during prereq registration. That means that whatever plugin might
set a perl version prereq may not have run yet.

The symptom of this is you end up losing the min_perl_version bits in the
generated Makefile.PL, because min_perl_version is called too early and ends
up being set to an empty string.

Specifically, I saw this with a number of distro where I have a line like "use
5.0100" in the code and I'm using AutoPrereqs.

@autarch autarch force-pushed the autarch/fix-min_perl_version branch from 7b8c671 to 49e2ff3 Compare June 9, 2018 18:37
The call to $self->min_perl_version in the default sub for eumm_version would
be unlikely to get something correct. This is because eumm_version ends up
being called during prereq registration. That means that whatever plugin might
set a perl version prereq may not have run yet.

The symptom of this is you end up losing the min_perl_version bits in the
generated Makefile.PL, because min_perl_version is called too early and ends
up being set to an empty string.

Specifically, I saw this with a number of distro where I have a line like "use
5.0100" in the code and I'm using AutoPrereqs.
@autarch autarch force-pushed the autarch/fix-min_perl_version branch from 49e2ff3 to e0340a4 Compare June 9, 2018 18:43
@autarch
Copy link
Author

autarch commented Jun 9, 2018

I note that this change means that this plugin will always end up setting an EUMM prereq if there are multiple authors. If this is really a big problem I think it might be better to just do what the core dzil MakeMaker plugin does and join the authors together as a string.

autarch added a commit to autarch/Dist-Zilla-PluginBundle-DROLSKY that referenced this pull request Jun 9, 2018
@karenetheridge
Copy link
Collaborator

This sounds like a simple plugin ordering issue. Your MakeMaker should come after any other prereq source plugin (e.g. MinimumPerl, AutoPrereqs) and then eumm_version will be correct.

The issue of prereqs being needed by MakeMaker is exactly why Makefile.PL content is generated in its own phase, InstallerTool, instead of in the FileMunger phase -- because FileMunger comes before PrereqSource, and InstallerTool comes after.

@autarch
Copy link
Author

autarch commented Jun 18, 2018

This seems like an awfully fragile solution. Saying "run this plugin" last doesn't scale very well. At the very least, this could use some documentation.

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

Successfully merging this pull request may close these issues.

2 participants