fix core-cpan-diff and sync-with-cpan to handle dist files with v prefixed versions

[haarg]

Allow a v prefix on versions expected by the core-cpan-diff and sync-with-cpan scripts. Also align the two scripts with each other, and handle the same file extensions expected by PAUSE.

[jkeenan]

Two points.

First, the p.r. has a porting test failure.

Second, yesterday, in response to the problem raised by Russ Albery on the P5P list, I began to explore how the core distribution's Porting/ programs would handle the case of a distribution in core moving from an N.NN version number to a vN.N.N number. I located a CPAN distro numbered v0.0.2, downloaded it, hacked it to have a 0.01 $VERSION, created a branch, added that version to the core distribution (which was a lot of work), then uploaded that branch to my github repo.

In my local checkout of that branch, I then ran the blead versions of the two programs your p.r. modifies:

$ perl Porting/core-cpan-diff -ax

Acme::Homer:
    Perl: DMUEY/Acme-Homer-0.01.tar.gz
    CPAN: DMUEY/Acme-Homer-v0.0.2.tar.gz

Archive::Tar:
    Perl: BINGOS/Archive-Tar-3.02_001.tar.gz
    CPAN: BINGOS/Archive-Tar-3.02.tar.gz

So core-cpan-diff correctly spotted the difference between the distro on CPAN and the dummy version in my branch.

$ perl Porting/sync-with-cpan Acme::Homer --yes
*** NOTE *** For speedups you can pass --jobs=N as an arg to this script.
About to clean the cpan/ directory, and ensure its contents is up to date.
Will also checkout -f on cpan/, MANIFEST and Porting/Maintainers.pl
*** WARNING *** - this may DELETE uncommitted changes. Hit ^C if you have ANY doubts!
Hit <return> to continue; ^C to abort 
--yes was used on command line, continuing.

Updated 0 paths from the index
the cpan/ directory is now clean and up to date
---
Fetching https://cpan.metacpan.org/authors/id/D/DM/DMUEY/Acme-Homer-v0.0.2.tar.gz
Cleaning out old directory
Unpacking Acme-Homer-v0.0.2.tar.gz
Renaming directories
Creating new package directory
Populating new package directory
No changes? at Porting/sync-with-cpan line 653.

So sync-with-cpan fetched the "new" version from CPAN but failed to synch it into core. At the point of failure, this was the git status:

$ git status
On branch semantic-version-upgrade-20240324
Your branch is up to date with 'jkeenan/semantic-version-upgrade-20240324'.

Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
    modified:   cpan/Acme-Homer/Homer.pm

Untracked files:
  (use "git add <file>..." to include in what will be committed)
    cpan/Acme-Homer-0.01-OLD/
    cpan/Acme-Homer-v0.0.2.tar.gz
    cpan/Acme-Homer/Changes
    cpan/Acme-Homer/MANIFEST
    cpan/Acme-Homer/META.yml
    cpan/Acme-Homer/README

no changes added to commit (use "git add" and/or "git commit -a")

@haarg, could you run your modified versions of these programs on my branch and report the results? Thanks.

[demerphq]

@jkeenan I have checked and @haarg's patch will not change this behavior.

The error message you are encountering is from some crappy panic-style code I did which was related to automatically extracting change file entries from CHANGES and other change files. I tested it against all the distributions that we have in perl, which is why I did not bother improving the error message, I assumed it would never happen with the modules we have. Bad me. So I will follow up with a better patch for this message so it is more self-explanatory what is going wrong, and a way to defang the exception from the command line so it doesn't break a flow like the one that you have created. After all it is preventing you from updating the module because it cant parse the Changes file which is not designed to be parsed anyway, so really it shouldnt be a fatal exception. I manually edited sync-with-cpan so it outputs the following:

panic: No changes in change file Acme-Homer/Changes for this module?
(This indicates an issue with sync-with-cpan)
old_version: 0.01
new_version: 0.000002

Now, for the actual issue you are seeing, what is happening is that sync-with-cpan uses the CPAN generated file 02packages.details.txt for version lookups (this is actually the reason why sometimes you need to wait a day to upgrade a module that has been uploaded to CPAN, the file is updated daily only). You can find this file in your /tmp directory after running the script.

This file contains version data in "normalized floating point form" (i dont know a better way to express this), so the dist you wanted to update is versioned as v0.0.2, BUT cpan normalizes that to 0.000002, you can see this here:

$ grep Acme::Homer /tmp/02packages.details.txt 
Acme::Homer                    0.000002  D/DM/DMUEY/Acme-Homer-v0.0.2.tar.gz

sync-with-cpan uses the version number in the second column in that line. It does NOT use the version number from the file. The logic for extracting the changes entry looks for something that looks like that value. If we check the changes file we see this:

$ cat cpan/Acme-Homer/Changes 
Revision history for Perl extension Acme::Homer

0.0.1  Sat Dec 17 19:17:27 2005
    - original version; created by h2xs 1.22 with options
        -AXc -n Acme::Homer

0.0.2  Tue Dec 20 10:13:23 2005
    - POD fixes and addition of Flanders section

As you can see the file does not contain the normalized 0.000002 value, but rather contains an entry for 0.0.2 ..... Interestingly it also contains an entry for 0.0.1 but actually THAT version was 0.01, so in fact this modules module version migration has broken the versioning model, as 0.000002 < 0.01. So sync-with-cpan would not find version 0.01 in the changes file EITHER.

SO, does @haarg's patch fix sync-with-cpan with regard to v0.0.x style versions, as far as I can tell not completely, and in fact there are issues with the intrinsic structure of the CPAN data files that sync-with-cpan uses to handle this type of version, second, the test case you chose unfortunately is broken as well.

Ill take a look at improving some of this, but given that CPAN itself is part of the problem here we have to think carefully about how this works.

As far as I can tell @haarg's patch is fine, it just isnt a complete solution to the problem at hand.

NOTE: after i originally posted this I applied @haarg's patch, and as I expected it does not change anything regarding Acme::Homer.

[rra]

The three spots that @haarg found were the same three spots that I found in searching through all the Porting scripts, but I had been assuming that the CPAN list would keep the version number from META.yml. Alas, it looks like that's not the case. There are several places that compare version numbers from the CPAN listing to version numbers read from modules (mostly, it looked like, using ExtUtils::MakeMaker, which returns the v-string), so I suspect a canonicalized version comparator using version->parse will be needed for those cases.

[demerphq]

@haarg maybe you should leave this to me, I will update the code that tripped up @jkeenan and look into the related issues in sync-with-cpan.

[jkeenan]

@haarg maybe you should leave this to me, I will update the code that tripped up @jkeenan and look into the related issues in sync-with-cpan.

FWIW, I'm not overly worried about this. Since we rarely (and correctly, IMO) add a new CPAN distribution to the core, we've never, to the best of my knowledge, automated the process, which would have presumed that we had certain standards for how a cpan/ distribution is structured before it can be brought into core. So setting up this artificial test case was where I ended up spending most of my time. I was not at all surprised that Porting/sync-with-cpan stumbled here. In my experience, the program works well 95% of the time -- and that's good enough. The other 5% is weird cases that require judgment on the part of the person running the program -- but that's what we have core committers for!

The real test of @haarg's (or anyone's) patch is: Once a distro which has moved from N.N versioning to vN.N.N versioning has been initially adapted in core, does sync-with-cpan DWIM thereafter? If @haarg's p.r. accomplishes that, then that's fine by me.

[haarg]

Regarding 0.0000002 vs v0.0.2 in 02packages, that will depend on how the version is declared. The conversion to numeric form will only happen if there is no provides data in the META file, and if the version is declared as a version object. This is the case with Acme::Homer, but that type of version declaration has been discouraged for a long time. The new podlators declares its versions with a package statement, which is not subject to numeric conversion.

So I think this is a worthwhile change to merge even if further improvements could be made. These scripts don't need to work for all CPAN modules, and I think we're unlikely to care about the cases they don't handle. For core-cpan-diff, the version from the file isn't even used, so this is just a fix to the conversion to a dist name.

[Grinnz]

FWIW If you need to determine that 0.000002 and v0.0.2 and 0.0.2 are the same version, I would suggest passing each into version->parse and comparing the resulting version objects for equality (this doesn't help the misdeclared version in Changes but otherwise will handle any other normalization).

[demerphq]

@haarg said:

So I think this is a worthwhile change to merge even if further improvements could be made.

Oh, I didn't mean to imply that your patch shouldn't be applied. I meant following up on the issue that @jkeenan raised. I think your patch should be applied once you resolve the failing test. When i said I would take over I meant to keep your commit (but fix the exec bit) and take over the PR to add a few more. If you want to apply it as is (with the exec bit fix of course) that is fine as well, I can produce another PR to fix what i feel responsible for.

@jkeenan said:

I was not at all surprised that Porting/sync-with-cpan stumbled here

Well, I am. The exception message produce is unnecessarily confusing, and I regret leaving it like that. In hindsight I should have made that code throw a better error and it should be possible to bypass the exception. The changes file extraction should also handle cases like this at least somewhat more gracefully than it does right now. Note that before I added this code we used to have to manually copy the relevant entry from the Changes file into the commit that merged the update. So the fatal exception is a regression as well as confusing. Not that long ago this code would not have thrown an exception at all.

[demerphq]

@Grinnz wrote:

FWIW If you need to determine that 0.000002 and v0.0.2 and 0.0.2 are the same version

Cheers, noted.