not properly comparing vstring versions.

[xenoterracide]

basically, if I use a decimal version and the upstream is a multipart it doesn't convert it properly for comparison.

perl Makefile.PL
Warning: prerequisite Business::CyberSource 0.004000 not found. We have v0.4.0.
Writing Makefile for MyLib::3rdParty::CyberSource
Writing MYMETA.yml and MYMETA.json

Business::CyberSource uses

 our $VERSION = 'v0.4.0';

which is a valid vstring, and passes version::is_strict tests. I can look into writing tests and a patch later.

[dagolden]

Business::CyberSource uses

    our $VERSION = 'v0.4.0';

which is a valid vstring, and passes version::is_strict tests. I can look into writing tests and a patch later.

Technically, putting 'v0.4.0' in a string isn't a legal v-string -- that would be C<v0.4.0> (i.e. without quotes). version.pm is smart enough to handle a string that looks like a v-string, but the Perl interpreter is not. Whether EU::MM should be similarly smart is open to interpretation. For sure, any Perl prior to 5.10 without version.pm installed will not be able to successfully check a version in a use line.

For example, given Foo.pm with the same $VERSION line you give above:

$ perl585 -wle 'use Foo; print $Foo::VERSION'
v0.4.0

$ perl585 -wle 'use Foo 0.004; print $Foo::VERSION'
Argument "v0.4.0" isn't numeric in subroutine entry at -e line 1.
Foo version 0.004 required--this is only version v0.4.0 at -e line 1.
BEGIN failed--compilation aborted at -e line 1.

Changing Foo.pm to have a proper v-string gives this:

$ perl585 -wle 'use Foo 0.004; print $Foo::VERSION'

For more on how complicated v-string versions are, read (reread?) http://www.dagolden.com/index.php/369/version-numbers-should-be-boring/

Nevertheless, for more diagnosis of your actual error message, what version of ExtUtils::MakeMaker? What version of Perl? What version of version.pm (if any) is installed?

-- David

[xenoterracide]

according to version.pm it is valid, doesn't seem to require that it's actually a version object.

http://search.cpan.org/~jpeacock/version-0.94/lib/version.pod#How_to_check_for_a_legal_version_string

Important Note: Even if you pass in what looks like a decimal number ("1.2"), a dotted-decimal will be created ("v1.200.0"). To avoid confusion or unintentional errors on older Perls, follow these guidelines:

Always use a dotted-decimal with (at least) three components Always use a leading-v Always quote the version

it doesn't particularly matter to me if what happens is it gets translated to 0.004000 before being compared, but in theory it should still work (either that or maybe dzil which wrote the makefile, should intelligently translate.

module_info ExtUtils::MakeMaker | psp4 thrall-i

Name:        ExtUtils::MakeMaker
Version:     6.59
Directory:   /home/ccushing/perl5/perlbrew/perls/perl-5.14.2/lib/5.14.2
File:        /home/ccushing/perl5/perlbrew/perls/perl-5.14.2/lib/5.14.2/ExtUtils/MakeMaker.pm
Core module: yes

Business-CyberSource [master] % module_info version | psp4 thrall-i

Name:        version
Version:     0.94
Directory:   /home/ccushing/perl5/perlbrew/perls/perl-5.14.2/lib/site_perl/5.14.2/i686-linux
File:        /home/ccushing/perl5/perlbrew/perls/perl-5.14.2/lib/site_perl/5.14.2/i686-linux/version.pm
Core module: no

[dagolden]

On Mon, Oct 10, 2011 at 9:38 PM, Caleb Cushing reply@reply.github.com wrote:

according to version.pm it is valid, doesn't seem to require that it's actually a version object.

http://search.cpan.org/~jpeacock/version-0.94/lib/version.pod#How_to_check_for_a_legal_version_string

"Legal according to version.pm" isn't quite "legal according to the Perl parser" (particularly since the latter is different for different versions of Perl).

Always use a dotted-decimal with (at least) three components Always use a leading-v Always quote the version

That's guidance for version->new/declare, not $VERSION, though the example is in the context of using version->new/declare in $VERSION.

There is a big difference between these lines:

our $VERSION = "v0.4.0";
our $VERSION = v0.4.0;
use version; our $VERSION = version->declare("v0.4.0");

The latter two will work. The former won't.

it doesn't particularly matter to me if what happens is it gets translated to 0.004000 before being compared, but in theory it should still work (either that or maybe dzil which wrote the makefile, should intelligently translate.

If you're using dzil, did you write the $VERSION line yourself or did you let dzil write the $VERSION line for you? I'm pretty sure that the PkgVersion plugin would insert it (correctly) without quotes.

--David

[xenoterracide]

last I checked not quoting the string won't work at all. I let dzil write it for me, but I don't use PkgVersion I wrote OurPkgVersion as an alternative. Also multipart version strings have to be quoted or they don't work... they aren't even valid as strings that way.

#!/usr/bin/perl
use strict;
use warnings;
use version 0.86 qw( is_strict);

{
    package foo;

    our $VERSION = v0.1.0;

    1;
}
{
    package bar;

    our $VERSION = 'v0.1.0';

    1;
}

print "foo $foo::VERSION\n";
print "bar $bar::VERSION\n";
print "foo has a strict version\n" if is_strict( $foo::VERSION );
print "bar has a strict version\n" if is_strict( $bar::VERSION );

foo 
bar v0.1.0
bar has a strict version

I know that for a fact because I rewrote Test::Version due to this failure with one of my modules.

[xenoterracide]

and because of having rewritten Test::Version which uses version and Module::Extract::VERSION I'm quite confident that almost everything recognizes this as a version, though not everything can support the use v0.1.0; format which is exactly equivalent to use 0.001000;

module_info Business::CyberSource | psp4 thrall-i

Name:        Business::CyberSource
Version:     v0.4.0
Directory:   /home/ccushing/perl5/perlbrew/perls/perl-5.14.2/lib/site_perl/5.14.2
File:        /home/ccushing/perl5/perlbrew/perls/perl-5.14.2/lib/site_perl/5.14.2/Business/CyberSource.pm
Core module: no

[schwern]

I have a few, somewhat conflicting, thoughts.

Philosophically... while $VERSION = 'v1.2.3' might work, it destroys the idea that $VERSION should be a thing you compare with other things. It puts the load on the user to first convert $VERSION into something which can be compared, and I don't like that. It violates good encapsulation. It says "$VERSION will be a thing you can parse with version.pm" rather than "$VERSION will be a thing you can compare with other $VERSIONs". I don't like hard wiring $VERSION to version.pm nor shoving the work onto the individual user to parse whatever the hell you've put into $VERSION.

Pragmatically, $VERSION = 'v1.2.3' doesn't work very well in three common ways to compare versions...

# by variable
print $Foo::VERSION == 0.004000 ? "yes" : "no";

# by method
print Foo->VERSION == 0.004000 ? "yes" : "no";

# by method argument
print Foo->VERSION(0.004000) ? "yes" : "no";

$VERSION = 'v1.2.3' only works for the last one... but only for 5.10 and up. $VERSION = qv('v1.2.3') works for all of them (until 5.12 when VERSION appears to stringify before returning, I don't know why). So I don't know how much I want to be encouraging that sort of thing.

From MakeMaker's perspective, being lax in what we accept would say we should DWIM and just deal with it. Now that we bundle version.pm we can always run $VERSION through it. That also has the benefit of making version comparision A) somebody else's problem and B) consistent.

From a usability perspective... it would be nice if there was something simpler than the currently recommended use version; our $VERSION = qv('v1.2.3'); but while $VERSION = 'v1.2.3' is easier for the module author to write, it's more work for everyone down the chain. I don't think this is the solution.

I think the benefits from MakeMaker using version.pm to do version comparisons dominates the other more abstract bits. $VERSION = 'v1.2.3' would work as a side effect (we'd have to go through work to explicitly disallow it) but I would not encourage it nor document it and would consider it a bug waiting to happen.

[mohawk2]

Cross-ref to #28 and #42 - all need to be considered together.

[dagolden]

I think EUMM should use CPAN::Meta::Requirements for checking dependencies, since the whole point of it was to have one standard library for encapsulating all the necessary logic and conversions.

[mohawk2]

I tried this, but I feared chaos if I didn't remove the "v" at the start of versions (for META files). If that's not a problem, then MakeMaker::clean_versions can be updated accordingly and simplified, and I'll make another PR.

Another issue on which I'd like wisdom - I was careful to use version::stringify as it promises to give back very similar to what it's given. If user specifies "v1.2", will it be a problem if what ends up in META was "1.2.0"? I assumed yes, but seek wisdom.

[dagolden]

CPAN::Meta::Requirements pretty aggressively washing everything it gets through version.pm. Having a leading "v" is actually better than removing it because "v1.2" is not the same as "1.2".

CPAN::Meta will generate META files with properly formatted versions. If the user specifies "v1.2" it should render it as "v1.2.0" in accordance with the spec. See Version Formats.

It sounds like you might need to (a) assemble prereqs into a CPAN::Meta::Prereq object and (b) do something like @leont's CPAN::Meta::Check to check against installed.

[mohawk2]

I'll remove the line that was removing the initial "v" from the stringified version.pm output. Let the chips fall where they may!