-
Notifications
You must be signed in to change notification settings - Fork 78
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
Add supports_param function #310
Conversation
So that, at some hoped-for time in the future, we can stop filling Makefiles.PL with silly version checks!
This API would be even neater:
Returning an empty list if unsupported. This achieves some DRY. |
Given that one would need to configure require 7.32 to use this, I don't think this is particularly useful. It's literally useless for any past feature, and for any future feature (I don't think we've added one in about 2 years, they're not particularly common anymore) one can easily use configure-requires as well. |
I agree with @Leont. |
It would probably be a better use of time to create a text file somewhere that people can consult, similar to CHANGES, but from the aspect of "I need syntax Y, what version does that need". Then that text file can be referred to when generating compatibility code in Makefile.PL, and truely support a broad range of versions. How one wishes to make that happen in Makefile.PL is up to them, but adding a feature to detect the presence of features added before it seems .... well.... putting the cart before the horse, so to speak. ( But for instance, somebody who was keen could make a 3rd party CPAN module that was bundleable in inc/ which could then be loaded on even old EUMMs and then do something with magic syntax to do feature detection, and this wouldn't suffer the same issue of needing a newer feature of EUMM ) |
That "text file" can mostly be gleaned from the current pod of EUMM. A good next step would be a CPAN module that has a simple API to determine feature support, either what features are available at a certain version or what version is required for a certain feature. |
And I am volunteering to write such a module, but I'd leave it up to others to determine how they want to use it for inc/ bundling or whatever other magic. |
Here is a start for the data of what is supported and when, with a suggestion of a format (CSV of key, supportedfrom, actionifnot, actionarg):
And I humbly submit that the move_to function on this hypothetical module should be based on this: https://github.com/ilmari/dbicx-autodoc/blob/7e09bbbaafbe37a3096924a675445d277a482596/Makefile.PL#L50-L65 |
There are already boilerplate Makefile.PLs; taking one and making it a module would be the approach of least resistance. |
Problem is you’d need to use it from within Makefile.PL, so you can’t simply depend on it; you need to pull it in via My own view of it is that EU::MM is in a position akin to autoconf – you should ship something full of autogenerated boilerplate that optimises for users’ ability to install the package without dependencies on anything on the user’s machine. Trying to fashion DRY APIs for use inside Makefile.PL itself optimises for author comfort in hand-maintaining that file instead. If you want author comfort, write a tool to generate the boilerplate for you at authoring time. (That was the first reason I jumped on the Dist::Zilla bandwagon early on, and it’s the last reason that will be keeping me on it in the end, even as I’m recognising Dist::Zilla as mostly a mistake and steadily reducing my reliance on it. (The choice of problems it tries to solve is sound, mind you; just the particular solution and implementation is deeply problematic.)) This knowledge about what feature is in what version of EU::MM should be machine-encoded into an authoring tool – or just a Makefile.PL template – used at authoring time by the author. Not a module that runs at installation time on the user’s machine. |
@ap - I agree with all of that, the module I am considering is simply a tool for tools such as you propose, which would be the 'further magic'. It can be bundled in inc and used that way, or it can be used to generate appropriate boilerplate on the author side and never be used in the dist. Or it can be configure_required if one so chooses. I would probably be inclined to include some form of that boilerplate generation (but maybe not at first). |
Why? I understand feature probing in the context of e.g. browsers where you have multiple competing implementations that support different selections of features at different versions. I also understand it in the context of software with optional features, e.g. perl itself which can be compiled with threads enabled or disabled etc.… or, say, vim, which can be compiled with or without several language interpreters embedded etc. But in the case of a single implementation that installs with a fixed set of features, where the presence/absence of a feature is fully described by which version you’re running…? What is the rationale for feature detection over version checks in that case? To me, it’s the magic in your Makefile.PL that’s crazy, given that it does not appear to be warranted. |
It is fairly trivial to monkey-patch such a feature into EUMM, but very fragile unless the list of versions that need monkey-patching is finite. Currently, the list is open-ended. My patch would solve that problem (monkey-patching being required only up to 7.30).
configure_requires does not work in 5.10, which all my modules support, and I do not believe in forcing users to install a version that is not actually needed for installation. |
But such an approach is inherently fragile, because in my experience things tend to get out of sync, unless the synchronization is automated. If new versions of EUMM would provide an API to provide a list of supported parameters (a more generally API than my proposed patch), then said automation could use it. |
Lists of version checks result in code that is hard to read and maintain. If all optional features are just listed in a hash (or hashes) that the code processes, maintenance and code clarity both improve. |
Yes, but writing that saved me having to go hunting for lists of versions and risking copying and pasting the wrong version into the wrong spot—something I have a tendency to do. |
That wouldn't be enough to be generally useful. One would also need to know what to do when that feature is not supported. You appear to be assuming unsupported features can always be safely ignored, this is not the case. |
Yes, but you need to know that the feature is not supported before you can even decide what to do instead.
I am not assuming that. Is Module::CoreList not useful? I’m sure it is used in many ways that were not anticipated when it was first written. All I am asking is that EUMM make available the list of known parameters that it already has. Maybe my proposed patch is not the best solution, since it is not versatile. But EUMM could still help when it comes to the alternatives proposed; e.g., by providing a function that lists all known parameters. Then callers can do what they want with it, from looking for specific items in the list, to generating modules that provide that list in different formats. |
I would posit that the best first place to do that is in documentation (including what the Makefile.PL should do if the feature is not available (e.g. when there is no TEST_REQUIRES, fold those prereqs into BUILD_REQUIRES, etc..). Even that alone has significant value to everyone out there today writing their own fallback routines. (Maybe the next step after that is presenting the data in a structured fashion that code can access; that will let authors experiment with various ways of using it.) |
That might be nice.
This does not have to be a ‘next step’. It can be done first, or at the same time. |
What’s interesting is that EUMM actually used to have such a feature (until 6.42), but it was undocumented. Examples of its use: It’s a pity it stopped working. |
I'm a big fan of introspection and would support an EUMM method that provided such. |
I agree that regression is unfortunate. It appears it wasn't noticed back then (or at least I'm unable to find a bug report about it). Overly aggressive modernization can easily cause this sort of issue. |
Ok, so what patch would be more likely to be accepted? |
FWIW I have released https://metacpan.org/release/DBOOK/ExtUtils-MakeMaker-Attributes-0.001 which could potentially be used to help solve this problem. It doesn't do anything on its own but is intended to be a tool for building whichever type of implement one wishes to use. I intend to include a Makefile.PL boilerplate generator but haven't decided how to correctly merge arbitrary prereq lists without requiring that CPAN::Meta::Requirements is installed. |
I’m not in a position to accept or reject patches but I’d strongly argue in favour of restoring and documenting That would make code that works with the new MakeMaker release also automatically work with older releases – except for a defined stretch of releases in the middle. Adding support for those via Also, this route only requires a single form of monkeypatch. If you were to add a new function, then it would have to be monkeypatched into all old MakeMakers, and that opens up the question of whether to have two forms depending on whether the given MakeMaker has Overall, restoring |
I don't see any argument against restoring previously existing functionality, even if I'm not convinced this sort of functionality is a good solution for this sort of problem. |
I have just opened pull request #312, which reinstates %Recognized_Att_Keys. |
It is getting quite common these days to fill Makefiles.PL with version checks, because ExtUtils::MakeMaker provides no other way to probe its functionality. This is something I will not do, because I think it’s crazy. My approaches tend to be more draconian: https://metacpan.org/source/SPROUT/Acme-Eatemup-0.02/Makefile.PL.
Of course, ideally, instead of working around the lack of the feature, I should be a good citizen and propose a new feature to fill the gap. So here is a patch that implements
supports_param
, which can be used by future Makefiles.PL (when EUMM 7.32 is standard) to probe for features, instead of looking at the version number.