Skip Menu |
 

This queue is for tickets about the Module-CPANTS-Analyse CPAN distribution.

Report information
The Basics
Id: 28982
Status: resolved
Priority: 0/
Queue: Module-CPANTS-Analyse

People
Owner: Nobody in particular
Requestors: a.r.ferreira [...] gmail.com
Cc:
AdminCc:

Bug Information
Severity: (no value)
Broken in: (no value)
Fixed in: (no value)



Subject: Fwd: [PATCH] manifest checking
Date: Fri, 24 Aug 2007 20:19:49 -0300
To: bug-Module-CPANTS-Analyse [...] rt.cpan.org
From: "Adriano Ferreira" <a.r.ferreira [...] gmail.com>
Download (untitled) / with headers
text/plain 3.6k
Hi, Thomas. The following patch to Module-CPANTS-Analyse-0.72 brings two minor fixes: 1. makes manifest checking more compliant to the standard handling as done by ExtUtils::Manifest 2. avoid using chdir() in Module::CPANTS::Analyse->unpack which may introduce inconvenient mistakes The issue with the manifest solves the case of a MANIFEST which includes comments such as #commented-file-name In the current version (0.72), MCA would complain about a "#commented-file-name" in MANIFEST not found in the distribution. There was some coincidences in the 0.72 implementation that make it deal correctly with lines such as # comment or # comment (which made things very misleading until I understood the current behavior). That was because s/\s.*$//; was meant to strip the comments in lines like filename # comment but chomped all of lines like # comment and left only "#" in a line like # comment and then that was discarded because of the following line of code next unless /\w/; These have been solved according to the following diff: diff -u -r Module-CPANTS-Analyse-0.72/lib/Module/CPANTS/Kwalitee/Manifest.pm Module-CPANTS-Analyse/lib/Module/CPANTS/Kwalitee/Manifest.pm --- Module-CPANTS-Analyse-0.72/lib/Module/CPANTS/Kwalitee/Manifest.pm Sat Jun 30 18:15:25 2007 +++ Module-CPANTS-Analyse/lib/Module/CPANTS/Kwalitee/Manifest.pm Fri Aug 24 14:24:51 2007 @@ -24,8 +24,10 @@ my @manifest; while (<$fh>) { chomp; - s/\s.*$//; - next unless /\w/; + next if /^\s*#/; # discard pure comments + + s/\s.*$//; # strip file comments + next unless $_; # discard blank lines push(@manifest,$_); } close $fh; The corresponding code is a straightforward analog to the one found in sub maniread() from ExtUtils::Manifest. While I was fixing this, I wrote a test like this: { my $a=Module::CPANTS::Analyse->new({ dist => 't/eg/Good-Dist-0.01.tar.gz' }); $a->unpack; $a->analyse; is($a->d->{manifest_matches_dist}, 1, 'manifest matches dist'); } { my $a=Module::CPANTS::Analyse->new({ dist => 't/eg/no-manifest-0.01.tar.gz' }); $a->unpack; $a->analyse; is( $a->d->{manifest_matches_dist}, undef, 'manifest does not match dist' ); } and found that it crashed (not finding the second tarball) even though the objects were recreated. It worked alright if I run only the code in one brace or another. So it was a matter of global changes. The culprit was sub unpack() in Module::CPANTS::Analyse that did chdir($me->testdir); which changed the current directory and screwed the rest of the test which used relative paths. The solution was: --- Module-CPANTS-Analyse-0.72/lib/Module/CPANTS/Analyse.pm Sat Jun 30 18:15:25 2007 +++ Module-CPANTS-Analyse/lib/Module/CPANTS/Analyse.pm Fri Aug 24 19:44:39 2007 @@ -46,13 +46,12 @@ return 'cant find dist' unless $me->dist; copy($me->dist,$me->testfile); - chdir($me->testdir); $me->d->{size_packed}=-s $me->testfile; my $archive; eval { $archive=Archive::Any->new($me->testfile); - $archive->extract(); + $archive->extract($me->testdir); }; That is using Archive::Any->extract() with a directory as argument made the chdir() unnecessary and got rid of this side-effect. The patch is attached together with t/plugin_manifest.t - a new test with focus on Module::CPANTS::Kwalitee::Manifest and t/eg/Good-Dist.tar.gz - a distribution with a high kwalitee (including a good MANIFEST) t/eg/no-manifest.tar.gz - a distribution without a MANIFEST t/eg/bad-manifest.tar.gz - a distribution with a MANIFEST that did not match Best regards, Adriano Ferreira
Download mca.diff
text/x-diff 3.1k

Message body is not shown because sender requested not to inline it.

Download plugin_manifest.t
text/x-perl 995b

Message body is not shown because sender requested not to inline it.

Download Good-Dist-0.01.tar.gz
application/x-gzip 1.2k

Message body not shown because it is not plain text.

Download no-manifest-0.01.tar.gz
application/x-gzip 1k

Message body not shown because it is not plain text.

Download bad-manifest-0.01.tar.gz
application/x-gzip 1.2k

Message body not shown because it is not plain text.

Download (untitled) / with headers
text/plain 272b
Thanks for the spot & the patch! I finally applied it, after not answering for much too long... The new version is currently only availabe in SVN ( http://cpants.googlecode.com/svn/trunk/Module-CPANTS-Analyse/ ), but I will push a new release soon (tonight or tomorrow)


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

Please report any issues with rt.cpan.org to rt-cpan-admin@bestpractical.com.