Perl-Toolchain-Gang / CPAN-Meta-Requirements

a set of version requirements for a CPAN dist
5 stars 15 forks source link

Version string normalization destroys alpha version info #32

Closed SineSwiper closed 7 years ago

SineSwiper commented 7 years ago

Since _version_object runs $vobj = version->new($vobj->normal);, a version string like 2.3.0.0_01 will get warped into v2.3.0.1 after _version_object returns. Even with 2 or 3 version parts, like 2.3.0_01 or 2.3_01, the final string is corrupted.

This may be a better solution:

if ( _is_qv($vobj) && !$vobj->is_alpha ) {
    $vobj = version->new($vobj->normal);
}
haarg commented 7 years ago

2.3.0.0_01 and v2.3.0.1 are equal, so it shouldn't really matter that the result is the latter. Alpha versions also aren't generally useful for dependency resolution.

SineSwiper commented 7 years ago

They aren't quite equal when trying to resolve that to a version to download via cpanm. At least the non-normalized version retains the original string and alpha flag:

version->new: bless( {
  alpha => 1,
  original => "v2.3.0.0_01",
  qv => 1,
  version => [
    2,
    3,
    0,
    1
  ]
}, 'version' )

CMR->_version_object: bless( {
  original => "v2.3.0.1",
  qv => 1,
  version => [
    2,
    3,
    0,
    1
  ]
}, 'version' )

Also note that this only broke with a fairly recent version of version on 0.9913.

I realize that alpha versions on CPAN requirements are a rather extreme edge case. Although, we use it for identifying non-prod internally shared modules within our internal PAN.

Leont commented 7 years ago

This is sounding like an XY problem, I'm not sure what you're trying to do and what is feeding a vstring to CPAN::Meta::Requirements. The solution here is probably to not use a vstring but a normal string started with a v.

perl -Mversion -E 'for my $v (v2.3.0.0_01, "v2.3.0.0_01"){ say qv($v)->stringify }'
SineSwiper commented 7 years ago

In this case, we are interacting with a CMR object directly to get a list of modules to upgrade. That includes parsing through different internal and CPAN requirements to get a common list to send to cpanm in Module~$requirement_string syntax.

If we are upgrading to a non-prod module, we would pass something similar to the following:

$module_reqs->add_string_requirement( "Internal::Module" => "== v2.3.0.0_01" );

After all of the requirements gathering is done, we would get a list of module/req strings in the form of:

my @module_strings =
    map { $_.'~'.$module_reqs->requirements_for_module($_) }
    sort $module_reqs->required_modules
;

Unfortunately, at that point, the version string has been warped to the non-existent v2.3.0.1. We do start out with a v$ver string, but the normalization within CMR removes the alpha information.

I have a few workarounds, like saving exact versions separately or adding it directly via __modify_entry_for. But, I figured this might be best fixed either here or in version.

karenetheridge commented 7 years ago

But v2.3.0.1 does exist -- it's the same as v2.3.0.0_01... cpan clients should be able to find one just as well as the other.

On Wed, Mar 29, 2017 at 9:31 AM, Brendan Byrd notifications@github.com wrote:

In this case, we are interacting with a CMR object directly to get a list of modules to upgrade. That includes parsing through different internal and CPAN requirements to get a common list to send to cpanm in Module~$requirement_string syntax.

If we are upgrading to a non-prod module, we would pass something similar to the following:

$module_reqs->add_string_requirement( "Internal::Module" => "== v2.3.0.0_01" );

After all of the requirements gathering is done, we would get a list of module/req strings in the form of:

my @modulestrings = map { $.'~'.$module_reqs->requirements_formodule($) } sort $module_reqs->required_modules ;

Unfortunately, at that point, the version string has been warped to the non-existent v2.3.0.1. We do start out with a v$ver string, but the normalization within CMR removes the alpha information.

I have a few workarounds, like saving exact versions separately or adding it directly via __modify_entry_for. But, I figured this might be best fixed either here or in version.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/Perl-Toolchain-Gang/CPAN-Meta-Requirements/issues/32#issuecomment-290145968, or mute the thread https://github.com/notifications/unsubscribe-auth/AASfyxZ5bQ02uzAQYyrzJxb9doL7qo98ks5rqodKgaJpZM4MsTWg .

SineSwiper commented 7 years ago

It doesn't work for cpanm:

$ perl -Mversion -E "say version->new( version->new('1.013_03')->normal );"
v1.13.30
$ cpanm version@0.9912
--> Working on version
Fetching https://pinto-gsgpan.grantstreet.com/authors/id/J/JP/JPEACOCK/version-0.9912.tar.gz ... NOT FOUND
Fetching http://cpan.metacpan.org/authors/id/J/JP/JPEACOCK/version-0.9912.tar.gz ... OK
Configuring version-0.9912 ... OK
Building and testing version-0.9912 ... OK
Successfully installed version-0.9912 (downgraded from 0.9917)
1 distribution installed
$ perl -Mversion -E "say version->new( version->new('1.013_03')->normal );"
v1.13_30
$ cpanm Perl::Version~"== v1.13.30"
! Finding Perl::Version on metacpan failed.
! Finding Perl::Version (== v1.13.30) on apis failed.
! Couldn't find module or a distribution Perl::Version (== v1.13.30)
$ cpanm Perl::Version~"== v1.13_30"
! Finding Perl::Version on metacpan failed.
! Finding Perl::Version (== v1.13_30) on apis failed.
! Couldn't find module or a distribution Perl::Version (== v1.13_30)
$ cpanm Perl::Version~"== 1.013_03"
--> Working on Perl::Version
Fetching https://pinto-gsgpan.grantstreet.com/authors/id/B/BD/BDFOY/Perl-Version-1.013_03.tar.gz ... NOT FOUND
Fetching http://cpan.metacpan.org/authors/id/B/BD/BDFOY/Perl-Version-1.013_03.tar.gz ... OK
Configuring Perl-Version-1.013_03 ... OK
Building and testing Perl-Version-1.013_03 ... OK
Successfully installed Perl-Version-1.013_03 (upgraded from 1.013)
1 distribution installed

It doesn't seem to be able to find even the old version of the normalized string. If this is a cpanm bug, I'll open one there. Can somebody confirm that's the right course of action for this?

Leont commented 7 years ago

cpanm fatpacks its own dependencies, I wouldn't be surprised if what you obvserve above is because cpanm's version and the installed one disagree on how to interpret versions.

Leont commented 7 years ago

Also, why are you using normalized strings in the first place? It sounds like all your trouble will go away once you don't.

miyagawa commented 7 years ago

(i was posting an irrelevant comment earlier by using a wrong command)

I would say the issue is more likely on metacpan search, assuming they mean the identical version.

> cpanm Perl::Version~"== v1.13.30" -v
cpanm (App::cpanminus) 1.7042 on perl 5.014002 built for x86_64-linux
Work directory is /home/vagrant/.cpanm/work/1490825743.698
You have make /usr/bin/make
You have LWP 6.15
You have /bin/tar: tar (GNU tar) 1.27.1
Copyright (C) 2013 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>.
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.

Written by John Gilmore and Jay Fenlason.
You have /usr/bin/unzip
Searching Perl::Version (== v1.13.30) on cpanmetadb ...
! Finding Perl::Version (== v1.13.30) on cpanmetadb failed.
Searching Perl::Version (== v1.13.30) on metacpan ...
! Could not find a release matching Perl::Version (== v1.13.30) on MetaCPAN.
Searching Perl::Version on mirror http://www.cpan.org ...
Downloading index file http://www.cpan.org/modules/02packages.details.txt.gz ...
Found Perl::Version 1.013 which doesn't satisfy == v1.13.30.

it's been a while since i last touched the metacpan search API client, but i think it does some version normalization, but it does not necessarily match what's done on the server side.

SineSwiper commented 7 years ago

Leon, I'm using normalized strings because that's how it comes out of CMR. It then comes out the other end (from requirements_for_module) as the normalized string.

Should cpanm try to use module.version_numified from the MetaCPAN API here?

miyagawa commented 7 years ago

Yes, cpanm does use version_numified. https://github.com/miyagawa/cpanminus/blob/devel/lib/App/cpanminus/script.pm#L499

miyagawa commented 7 years ago

AH! It's not using the numified if it's an exact match. Maybe worth trying to change that?

SineSwiper commented 7 years ago

Ahhh, good find. So, something like?

    return {
        term => { 'module.version_numified' => $self->numify_ver_metacpan($req) },
    };
SineSwiper commented 7 years ago

I opened the cpanminus issue above.