strip underscore from versions when considering "accepts module"

[karenetheridge]

As discussed on irc (#distzilla, 2015-07-15), the underscore in a module's $VERSION should be stripped before passing it to version.pm in accepts_module() (or a private submethod thereof). This will restore consistency with CPAN.pm and other tools.

(We also need to ensure cpanm and Module::Build do the same thing.)

cc @miyagawa @Leont @shadowcat-mst

[shadowcat-mst]

cpanm just uses CMR, I think? EUMM does the s///, then passes it to CMR. Not sure what CPAN.pm does (it definitely strips, but I dunno if it uses CMR or its own code still)

[miyagawa]

is this related to #26?

If so, stripping alpha-ness from $VERSION will make a false requirement that passes in CMR's accepts_module() but will fail on runtime. (See the issue linked above)

I usually don't lurk on #distzilla so a summary for that, or a specific problem that you need to address with an actual failing example, would be helpful for me to complete the context.

also - yes, cpanm uses CMR, hence miyagawa/cpanminus/issues/463 and #26

[karenetheridge]

Yes. I don't understand the (presumably failing?) example in #26 though; can someone clarify?

[miyagawa]

suppose you have installed a dev version of Foo 2.000_001 from CPAN.

package Foo;
our $VERSION = '2.000_001';

and your code has a requirement for Foo => 2.000001.

This issue is about making that requirement pass, by eliminating _ before comparison inside CMR.

That's fine until in the runtime you have a code that says:

use Foo 2.000001;

at this point perl will blow up: Foo version 2.000001 required--this is only version 2.000_001.

[karenetheridge]

That's fine until in the runtime you have a code that says:

use Foo 2.000001;

at this point perl will blow up: Foo version 2.000001 required--this is only version 2.000_001.

We covered that in the #distzilla discussion -- the conclusion was basically
"we shouldn't have to support authors who do that, because that's dreadfully
wrong -- therefore the authors can keep both halves."

That is -- any author that declares an underscore version but doesn't include a follow-up line $VERSION = eval $VERSION; is being naughty and wrong.

[miyagawa]

"we shouldn't have to support authors who do that, because that's dreadfully wrong -- therefore the authors can keep both halves.

Well, it's not just "authors" who does this - it could be an end user, or a tool (a la PrereqScanner, cpanfile, Module::Metadata) that programatically ends up calling module->VERSION(requirement).

I honestly don't have a strong preference, but just wanted to point out that this issue contains an incorrect assumption (which you've covered as a wrong usage, just fine) and will possibly lead to a technical debt.

[karenetheridge]

I think you misunderstood. It's only authors that are declaring $VERSIONs with an underscore but neglecting the eval line -- that's the bad bit. There's nothing wrong with calling ->VERSION on any module, whether or not it's a trial release.

[miyagawa]

There's nothing wrong with calling ->VERSION on any module, whether or not it's a trial release.

But it's not OK to call, if the installed version is a devel release and there's no eval by the author.

The end result of this is that, as an end user, even if you have requires => { Foo => "2.001" } in your requirement, you're not sure if you can safely call Foo->VERSION(2.001), depending on the dev/trial state.

[karenetheridge]

But it's not OK to call, if the installed version is a devel release and there's no eval by the author.

Why is that relevant for this discussion? It's not safe to call today if there's no eval by the author. The breakage isn't made worse, and the code was just as wrong yesterday as it is today.

[miyagawa]

Why is that relevant for this discussion? It's not safe to call today if there's no eval by the author.

Sure, but you do know that it's not safe, because tools (i.e. cpanm or CPAN::Meta::Requirements) tells you that the installed version doesn't satisfy your requirement before running that code.

[miyagawa]

I'm merely pointing out that, by accepting this change, there'll be a mismatch in a list of prereqs and runtime perl's UNIVERSAL:::VERSION behavior. i.e. if a user installs a dev version of the module, the user can't specifically require that version.

If we all think that is a use case that can be ignored, and using a module with use Module VER with the version number that only exists as a dev version is un-supported, I'm fine.

cc @dagolden because you pointed out that fact in #26

[miyagawa]

Here's a convoluted scenario:

• Author A uploads Foo-2.0_1 (dev) to CPAN
• User X installs Foo-2.0_1 with --dev
• Author A re-uploads Foo-2.01 (non-dev) to CPAN. It gets indexed
• Author B uploads Bar, with a prereq Foo => 2.01
• User X installs Bar

Today, because '2.0_1' doesn't satisfy the requirement '2.01' cpanm will try to upgrade to the "non-dev" version of Foo. With this change, cpanm will fail to upgrade it because '2.0_1' will now satisfy '2.01'. In addition a runtime call of use Foo 2.01 will fail.

Again, this is convoluted, and it must be an author A's fault of reusing the same version number in non-dev and dev. Just wanted to point out that, by accepting this change we're deprecating the currently supported scenario.

Look, I'm the one who requested this first in #26 :)

[Leont]

I'd rather fix this in version.pm

[karenetheridge]

I'd rather fix this in version.pm

Me too. But can we do both? I suspect the CMR fix will be faster to implement, so it can at least move the problem one layer lower while we fret with what to change in version.pm.