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

Pull Request for Bug #533 #537

Open
wants to merge 3 commits into
base: devel
Choose a base branch
from
Open

Conversation

SineSwiper
Copy link

This is the PR for #533.

Brendan Byrd added 3 commits May 17, 2017 17:58
Fixes miyagawa#533.  This also removes the numify_ver_metacpan method, since alpha
version matching is now in sync between 'version' and MetaCPAN's API.
@haarg
Copy link
Contributor

haarg commented May 18, 2017

I don't quite understand the point of this PR. Now that cpanminus queries are going through a shim, there is little point in changing the query sent to metacpan. Changing it only means more work in the shim to accommodate additional forms of the query.

The shim and the download_url endpoint that it uses will accept versions in any form and handle them appropriately. If there is a problem in that code, it will need to be handled in metacpan, not cpanm.

The only changes that would be sensible to make to cpanm's usage of metacpan is to convert to the /download_url/ endpoint, like #522.

@SineSwiper
Copy link
Author

Ultimately, the point of this PR is to fix the root problem of #533, or more specifically Perl-Toolchain-Gang/CPAN-Meta-Requirements#32, which is illustrated in xt/version_exact_alpha.t. If you can get a version of cpanm that passes that test, that would be great.

The TravisCI failures above appear to be related to the older version of CMR in the fatpack. I was about to push those changes up, but if you have a different approach, we can close this and move the conversation there.

@haarg
Copy link
Contributor

haarg commented May 22, 2017

To clarify the situation with the shim: MetaCPAN is going to be decommissioning the v0 API soon, so we are doing what we can for backwards compatibility. We've worked with current users of the v0 API to migrate to v1. We can't fix deployed versions of cpanm though, so we've developed a shim to allow it to work even after the v0 API goes away.

Part of what the v1 API provides is a /download_url/ end point. Given a module, a version, and a flag to include development versions, it will provide a download URL. The query used for that internally is actually based on the one in cpanm.

The shim layer takes the query cpanm sends and converts it back into the module, version, and dev flag. It then uses the /download_url/ end point to find the correct dist information. It is rather strict about the query forms it accepts: it only accepts the exact forms that cpanm uses (or has used). Modifications to the form of the query cpanm sends will result in the shim refusing to parse it, and wouldn't change the output even if they were handled by the shim.

If there are issues with the handling of the version or dev flag, those are in the MetaCPAN API. I've already changed the API to numify the versions when doing an exact match. I believe there may still be an issue with the handling of the dev flag though.

@SineSwiper
Copy link
Author

Checking the new test against the original 'devel' branch, there's still this case:

# REQUEST: bless( {
#   _content => "",
#   _headers => bless( {
#     "user-agent" => "cpanminus/1.7043 perl/5.018002"
#   }, 'HTTP::Headers' ),
#   _method => "GET",
#   _uri => bless( do{\(my $o = "http://api.metacpan.org/v0/release/_search?source=%7B%22fields%22%3A%5B%22download_url%22%2C%22stat%22%2C%22status%22%5D%2C%22filter%22%3A%7B%22and%22%3A%5B%7B%22term%22%3A%7B%22release.name%22%3A%22Perl-Version-1.013_03%22%7D%7D%2C%7B%22term%22%3A%7B%22release.author%22%3A%22BDFOY%22%7D%7D%5D%7D%7D")}, 'URI::http' )
# }, 'HTTP::Request' )
#

#   Failed test 'Normalized alpha version matched'
#   at xt/version_exact_alpha.t line 7.
#                   'cpanm (App::cpanminus) 1.7043 on perl 5.018002 built for x86_64-linux-thread-multi
# Work directory is /tmp/Guz6PsYmfI/work/1495493207.16325
# You have make /usr/bin/make
# You have LWP 6.13
# You have /bin/tar: tar (GNU tar) 1.23
# Copyright (C) 2010 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 ...
# -> FAIL Finding Perl::Version (==v1.13.30) on cpanmetadb failed.
# Searching Perl::Version (==v1.13.30) on metacpan ...
# Found Perl::Version 1.013_03 which doesn't satisfy ==v1.13.30.
# '
#     doesn't match '(?^:Successfully (?:re)?installed)'

MetaCPAN did find the match, but cpanm didn't believe the result, because its own comparison failed. This may just be a case of cpanm needing a CMR+version upgrade within its fatpacked libs. However, it looks like the upgrader needs some work to get it to install version correctly, so I can't test that scenario at the moment.

@SineSwiper
Copy link
Author

Not sure how to proceed here. We can't get away with a shim-only fix, since cpanm does its own satisfy_version check, and you end up with the error above.

@haarg
Copy link
Contributor

haarg commented Sep 14, 2017

The changes to the metacpan query used should be removed from this PR. Then it would probably be suitable for merging.

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