garu / App-cpanminus-reporter

stand-alone CPAN Testers client (for cpanminus and friends)
13 stars 13 forks source link

Bug when cpanm reports string missmatches between phases #11

Closed kentfredric closed 1 year ago

kentfredric commented 10 years ago

I was wondering why BOBW/X86-Udis86-1.7.2.3.tar.gz didn't send a report

Parsing /home/kent/.cpanm/build.log...
entering X86-Udis86-1.7.2.3, http://www.cpan.org/authors/id/B/BO/BOBW/X86-Udis86-1.7.2.3.tar.gz
left X86-Udis86-1.7.2.3, http://www.cpan.org/authors/id/B/BO/BOBW/X86-Udis86-1.7.2.3.tar.gz
Finished.

^^ Huh. Thats weird.

After looking through the parser, I find that its these lines having different strings that cause a failure:

Note: No v

Unpacking X86-Udis86-1.7.2.3.tar.gz
Entering X86-Udis86-1.7.2.3

... Note: v

Configuring X86-Udis86-v1.7.2.3
...
Building and testing X86-Udis86-v1.7.2.3
...
Successfully installed X86-Udis86-v1.7.2.3

But the code in reporter.pm is looking for lines without the v for the Building and testing parts, so it entirely misses the data :(

The cause is this:

 "version" : "v1.7.2.3"

This is correct per CPAN::Meta Specification.

However, cpanm reports the name of the tarball for the Entering statement, and uses the value of version from META.json from then on.

This means you're essentially relying the whole tarballs name exactly matching the distributions name + version, which is a somewhat flimsy assumption.

kentfredric commented 10 years ago

As a temporary fix, I've locally modified this line

-      elsif ( $dist and /^Building and testing $dist/) {
+     elsif ( $dist and /^Building and testing (\S+)/) {

Which gets around the no-report issue for me.

I just don't know how reliable that is long-term

karenetheridge commented 10 years ago

Here's another one that looks to be caused by the same root issue: http://www.cpantesters.org/cpan/report/7fecbb8e-a7df-11e3-a8dc-2015ec1cabe9

garu commented 10 years ago

Hi guys!

@kentfredric 19744becf6 should fix this issue. Could you please check? @karenetheridge I don't understand... what's wrong with the report you linked?

kentfredric commented 10 years ago

A quick glance says it should solve the /specific/ case above with the vstring. However, I doubt it will solve the generic case, which is that the metadata inside META.json can differ from the name of the tar.gz

Which means I think its quite legal to do:

    foo-bar.tar.gz : {
           'name': 'quux',
           'version': '1.500',
    }

Or similar.

And cpanm reports the string on the tar before it extracts it to get the metadata which gives it the name and version values.

Contrived I know, but there are probably examples.

garu commented 10 years ago

I see. Well, in that case I'm gonna make it more like your original patch. The reason I didn't was mainly because parsing build output text is very fragile, so I wanted to be as close to the example at hand as possible and double check data (like the dist name) whenever possible to avoid runaway logs.

That said, I'm fairly confident the 'Building and testing' string only occurs on cpanm for the innermost dist being installed, so it's safe to process the line without checking the dist name.

I'll add more tests anyway :)

kentfredric commented 10 years ago

I think the thing to lookout for with my change, is the case ether presented, which I believe was caused by my change.

ie: Its reporting HTML-FormHandler to the MooseX-Getopt test report queue

My suspicion is MooseX::Getopt failed somewhere before running HTML::FormHandler

Or some expected line wasn't emitted, which made it think $dist was still MooseX::Getopt... but how that is I don't know.

kentfredric commented 10 years ago

Ok, I've found one example case that really makes cpanm_reporter get confused:

https://metacpan.org/release/PLOCKABY/v1.0.0

$ cpan http://cpan.metacpan.org/authors/id/P/PL/PLOCKABY/v1.0.0.tar.gz
--> Working on http://cpan.metacpan.org/authors/id/P/PL/PLOCKABY/v1.0.0.tar.gz
--> Working on http://cpan.metacpan.org/authors/id/P/PL/PLOCKABY/v1.0.0.tar.gz
Fetching http://cpan.metacpan.org/authors/id/P/PL/PLOCKABY/v1.0.0.tar.gz
Fetching http://cpan.metacpan.org/authors/id/P/PL/PLOCKABY/v1.0.0.tar.gz ... OK
-> OK
Fetching http://cpan.metacpan.org/authors/id/P/PL/PLOCKABY/CHECKSUMS
Fetching http://cpan.metacpan.org/authors/id/P/PL/PLOCKABY/CHECKSUMS ... OK
-> OK
Verifying the signature of CHECKSUMS
random usage: poolsize=600 mixed=0 polls=0/0 added=0/0
              outmix=0 getlvl1=0/0 getlvl2=0/0
secmem usage: 0/32768 bytes in 0 blocks
Verified OK!
Verifying the SHA1 for v1.0.0.tar.gz
Checksum for v1.0.0.tar.gz: Verified!
Unpacking v1.0.0.tar.gz
Entering perl-thread-queue-priority-1.0.0
Checking configure dependencies from META.json
Checking if you have ExtUtils::MakeMaker 0 ... Yes (6.90)
Configuring Thread-Queue-Priority-v1.0.0
Running Makefile.PL
Configuring Thread-Queue-Priority-v1.0.0 ... Generating a Unix-style Makefile
Writing Makefile for Thread::Queue::Priority
Writing MYMETA.yml and MYMETA.json
OK
-> OK
Checking dependencies from MYMETA.json ...
Checking if you have ExtUtils::MakeMaker 0 ... Yes (6.90)
Checking if you have Scalar::Util 1.1 ... Yes (1.38)
Checking if you have threads::shared 1.21 ... Yes (1.46)
Checking if you have Test::More 0.5 ... Yes (1.001002)
Building and testing Thread-Queue-Priority-v1.0.0

Yes:

  1. tarball name doesn't match $distname-$version at all
  2. extracted path doesn't match ^$distname-$version, but instead, perl-$distname-$version
  3. configure produces a 3rd string not seen in either 1 or 2

cpanm-reporter doesn't even see this test and just skips over it, producing not even error messages =)

karenetheridge commented 10 years ago

@garu what's wrong with the report I linked is it was sent to me labelled as a report for MooseX-Getopt, but the build and test output is all HTML-FormHandler.

karenetheridge commented 10 years ago

..but now I see that @kentfredric covered that.

garu commented 1 year ago

I am pretty confident this was fixed but we forgot to close it, so I'm going to do it and if the error happens again with cpanm-reporter 0.19 please let me know and we'll work on it. Thanks!