Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 665331 - Review Request: perl-Imager - Perl extension for Generating 24 bit Images
Summary: Review Request: perl-Imager - Perl extension for Generating 24 bit Images
Keywords:
Status: CLOSED DUPLICATE of bug 166254
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Robin Lee
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: rt4-dependencies-tracker
TreeView+ depends on / blocked
 
Reported: 2010-12-23 09:45 UTC by Ralf Corsepius
Modified: 2011-01-21 15:59 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2011-01-21 15:44:51 UTC
Type: ---
Embargoed:
robinlee.sysu: fedora-review+


Attachments (Terms of Use)

Description Ralf Corsepius 2010-12-23 09:45:06 UTC
Spec URL:  http://corsepiu.fedorapeople.org/packages/perl-Imager.spec
SRPM URL:  http://corsepiu.fedorapeople.org/packages/perl-Imager-0.79-1.fc15.src.rpm

Description:
Imager is a module for creating and altering images. It can read and
write various image formats, draw primitive shapes like lines,and
polygons, blend multiple images together in various ways, scale, crop,
render text and more.

Comment 1 Robin Lee 2011-01-05 09:58:47 UTC
MUST Items:
[+] MUST: rpmlint must be run on every package.
$ rpmlint ./RPMS/i686/perl-Imager-0.79-1.fc14.i686.rpm ./RPMS/i686/perl-Imager-debuginfo-0.79-1.fc14.i686.rpm SPECS/perl-Imager.spec SRPMS/perl-Imager-0.79-1.fc15.src.rpm 
perl-Imager.i686: W: devel-file-in-non-devel-package /usr/lib/perl5/Imager/include/imext.h
perl-Imager.i686: W: devel-file-in-non-devel-package /usr/lib/perl5/Imager/include/imrender.h
perl-Imager.i686: W: devel-file-in-non-devel-package /usr/lib/perl5/Imager/include/imperl.h
perl-Imager.i686: W: devel-file-in-non-devel-package /usr/lib/perl5/Imager/include/rendert.h
perl-Imager.i686: W: devel-file-in-non-devel-package /usr/lib/perl5/Imager/include/immacros.h
perl-Imager.i686: W: devel-file-in-non-devel-package /usr/lib/perl5/Imager/include/ext.h
perl-Imager.i686: W: devel-file-in-non-devel-package /usr/lib/perl5/Imager/include/imextdef.h
perl-Imager.i686: W: devel-file-in-non-devel-package /usr/lib/perl5/Imager/include/dynaload.h
perl-Imager.i686: W: devel-file-in-non-devel-package /usr/lib/perl5/Imager/include/stackmach.h
perl-Imager.i686: W: devel-file-in-non-devel-package /usr/lib/perl5/Imager/include/imextpltypes.h
perl-Imager.i686: W: devel-file-in-non-devel-package /usr/lib/perl5/Imager/include/iolayer.h
perl-Imager.i686: W: devel-file-in-non-devel-package /usr/lib/perl5/Imager/include/imexttypes.h
perl-Imager.i686: W: devel-file-in-non-devel-package /usr/lib/perl5/Imager/include/ppport.h
perl-Imager.i686: W: devel-file-in-non-devel-package /usr/lib/perl5/Imager/include/draw.h
perl-Imager.i686: W: devel-file-in-non-devel-package /usr/lib/perl5/Imager/include/imextpl.h
perl-Imager.i686: W: devel-file-in-non-devel-package /usr/lib/perl5/Imager/include/imerror.h
perl-Imager.i686: W: devel-file-in-non-devel-package /usr/lib/perl5/Imager/include/imio.h
perl-Imager.i686: W: devel-file-in-non-devel-package /usr/lib/perl5/Imager/include/imconfig.h
perl-Imager.i686: W: devel-file-in-non-devel-package /usr/lib/perl5/Imager/include/iolayert.h
perl-Imager.i686: W: devel-file-in-non-devel-package /usr/lib/perl5/Imager/include/imager.h
perl-Imager.i686: W: devel-file-in-non-devel-package /usr/lib/perl5/Imager/include/log.h
perl-Imager.i686: W: devel-file-in-non-devel-package /usr/lib/perl5/Imager/include/feat.h
perl-Imager.i686: W: devel-file-in-non-devel-package /usr/lib/perl5/Imager/include/imageri.h
perl-Imager.i686: W: devel-file-in-non-devel-package /usr/lib/perl5/Imager/include/regmach.h
perl-Imager.i686: W: devel-file-in-non-devel-package /usr/lib/perl5/Imager/include/plug.h
perl-Imager.i686: W: devel-file-in-non-devel-package /usr/lib/perl5/Imager/include/imdatatypes.h
3 packages and 1 specfiles checked; 0 errors, 26 warnings.

[+] MUST: The package must be named according to the Package Naming Guidelines.
[+] MUST: The spec file name must match the base package %{name}
[+] MUST: The package must be licensed with a Fedora approved license and meet the Licensing Guidelines.
[+] MUST: The License field in the package spec file must match the actual license.
[+] MUST: If (and only if) the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package must be included in %doc.
[+] MUST: The spec file must be written in American English.
[+] MUST: The spec file for the package MUST be legible.
[-] MUST: The sources used to build the package must match the upstream source, as provided in the spec URL.
Upstream: 
$ md5sum Imager-0.79.tar.gz 
ec73e34a0f6482a5a2d0178192f4837f  Imager-0.79.tar.gz

SRPM tarball:
$ md5sum perl-Imager-0.79-1.fc15.src/Imager-0.79.tar.gz 
eec0b53661ccce1fdb3b521037128a72  perl-Imager-0.79-1.fc15.src/Imager-0.79.tar.gz

[+] MUST: The package must successfully compile and build into binary rpms on at least one supported architecture.
[=] MUST: All build dependencies must be listed in BuildRequires
It is recommended to buildrequire core modules explicitly:

perl(Config)  
perl(constant)  
perl(Exporter)  
perl(File::Basename)  
perl(File::Spec)
perl(IO::File)  
perl(strict)  
perl(Test::Builder)  
perl(vars) 

[+] MUST: A package must own all directories that it creates. If it does not create a directory that it uses, then it should require a package which does create that directory.
[+] MUST: A package must not contain any duplicate files in the %files listing.
[+] MUST: Permissions on files must be set properly. Executables should be set with executable permissions, for example. Every %files section must include a %defattr(...) line.
[+] MUST: Each package must have a %clean section, which contains rm -rf %{buildroot} (or $RPM_BUILD_ROOT).
[+] MUST: Each package must consistently use macros, as described in the macros section of Packaging Guidelines.
[+] MUST: The package must contain code, or permissible content. This is described in detail in the code vs. content section of Packaging Guidelines.
[+] MUST: If a package includes something as %doc, it must not affect the runtime of the application.
[+] MUST: Packages must not own files or directories already owned by other packages.
[+] MUST: At the beginning of %install, each package MUST run rm -rf %{buildroot} (or $RPM_BUILD_ROOT).
[+] MUST: All filenames in rpm packages must be valid UTF-8.

SHOULD Items:
[+] SHOULD: The reviewer should test that the package builds in mock.
Rawhide scratch build:
http://koji.fedoraproject.org/koji/taskinfo?taskID=2701878
[+] SHOULD: The package should compile and build into binary rpms on all supported architectures.
[+] SHOULD: The reviewer should test that the package functions as described.
[+] SHOULD: Packages should try to preserve timestamps of original installed files.

Extra comments:
[-] URL should be http://imager.perl.org/
[=] Consider including Changes.old

Comment 2 Ralf Corsepius 2011-01-05 10:46:55 UTC
(In reply to comment #1)
> MUST Items:
> [+] MUST: rpmlint must be run on every package.
> $ rpmlint ./RPMS/i686/perl-Imager-0.79-1.fc14.i686.rpm
> ./RPMS/i686/perl-Imager-debuginfo-0.79-1.fc14.i686.rpm SPECS/perl-Imager.spec
> SRPMS/perl-Imager-0.79-1.fc15.src.rpm 
> perl-Imager.i686: W: devel-file-in-non-devel-package
> /usr/lib/perl5/Imager/include/imext.h
> perl-Imager.i686: W: devel-file-in-non-devel-package
> /usr/lib/perl5/Imager/include/imrender.h
> perl-Imager.i686: W: devel-file-in-non-devel-package
> /usr/lib/perl5/Imager/include/imperl.h
> perl-Imager.i686: W: devel-file-in-non-devel-package
> /usr/lib/perl5/Imager/include/rendert.h
> perl-Imager.i686: W: devel-file-in-non-devel-package
> /usr/lib/perl5/Imager/include/immacros.h
> perl-Imager.i686: W: devel-file-in-non-devel-package
> /usr/lib/perl5/Imager/include/ext.h
> perl-Imager.i686: W: devel-file-in-non-devel-package
> /usr/lib/perl5/Imager/include/imextdef.h
> perl-Imager.i686: W: devel-file-in-non-devel-package
> /usr/lib/perl5/Imager/include/dynaload.h
> perl-Imager.i686: W: devel-file-in-non-devel-package
> /usr/lib/perl5/Imager/include/stackmach.h
> perl-Imager.i686: W: devel-file-in-non-devel-package
> /usr/lib/perl5/Imager/include/imextpltypes.h
> perl-Imager.i686: W: devel-file-in-non-devel-package
> /usr/lib/perl5/Imager/include/iolayer.h
> perl-Imager.i686: W: devel-file-in-non-devel-package
> /usr/lib/perl5/Imager/include/imexttypes.h
> perl-Imager.i686: W: devel-file-in-non-devel-package
> /usr/lib/perl5/Imager/include/ppport.h
> perl-Imager.i686: W: devel-file-in-non-devel-package
> /usr/lib/perl5/Imager/include/draw.h
> perl-Imager.i686: W: devel-file-in-non-devel-package
> /usr/lib/perl5/Imager/include/imextpl.h
> perl-Imager.i686: W: devel-file-in-non-devel-package
> /usr/lib/perl5/Imager/include/imerror.h
> perl-Imager.i686: W: devel-file-in-non-devel-package
> /usr/lib/perl5/Imager/include/imio.h
> perl-Imager.i686: W: devel-file-in-non-devel-package
> /usr/lib/perl5/Imager/include/imconfig.h
> perl-Imager.i686: W: devel-file-in-non-devel-package
> /usr/lib/perl5/Imager/include/iolayert.h
> perl-Imager.i686: W: devel-file-in-non-devel-package
> /usr/lib/perl5/Imager/include/imager.h
> perl-Imager.i686: W: devel-file-in-non-devel-package
> /usr/lib/perl5/Imager/include/log.h
> perl-Imager.i686: W: devel-file-in-non-devel-package
> /usr/lib/perl5/Imager/include/feat.h
> perl-Imager.i686: W: devel-file-in-non-devel-package
> /usr/lib/perl5/Imager/include/imageri.h
> perl-Imager.i686: W: devel-file-in-non-devel-package
> /usr/lib/perl5/Imager/include/regmach.h
> perl-Imager.i686: W: devel-file-in-non-devel-package
> /usr/lib/perl5/Imager/include/plug.h
> perl-Imager.i686: W: devel-file-in-non-devel-package
> /usr/lib/perl5/Imager/include/imdatatypes.h
The warning are all bogus (== rpmlint is wrong on these)-.


> [-] MUST: The sources used to build the package must match the upstream source,
> as provided in the spec URL.
> Upstream: 
> $ md5sum Imager-0.79.tar.gz 
> ec73e34a0f6482a5a2d0178192f4837f  Imager-0.79.tar.gz
> 
> SRPM tarball:
> $ md5sum perl-Imager-0.79-1.fc15.src/Imager-0.79.tar.gz 
> eec0b53661ccce1fdb3b521037128a72 
> perl-Imager-0.79-1.fc15.src/Imager-0.79.tar.gz
> 
> [+] MUST: The package must successfully compile and build into binary rpms on
> at least one supported architecture.
> [=] MUST: All build dependencies must be listed in BuildRequires
> It is recommended to buildrequire core modules explicitly:
> 
> perl(Config)  
> perl(constant)  
> perl(Exporter)  
> perl(File::Basename)  
> perl(File::Spec)
> perl(IO::File)  
> perl(strict)  
> perl(Test::Builder)  
> perl(vars) 
You are over-interpreting the guidelines.

It's common practice in perl package to only require explicitly required modules (typically those from META.yml) and those required to run the tests.

Explicit requiring perl-core modules NOT required and doesn't make sense.

> Extra comments:
> [-] URL should be http://imager.perl.org/
This is a CPAN package. The URL needs to point to CPAN.

> [=] Consider including Changes.old
I disagree. Changes.old only contains many years old infomation which hardly is relevant to users.

Comment 3 Robin Lee 2011-01-05 12:35:51 UTC
(In reply to comment #2)

And rebuild an SRPM since md5sum not matched.

Others are not blocking issues.

Comment 4 Ralf Corsepius 2011-01-21 04:32:40 UTC
(In reply to comment #3)
> (In reply to comment #2)
> 
> And rebuild an SRPM since md5sum not matched.
No idea why you are seeing this.

Whatever I try, I always get
eec0b53661ccce1fdb3b521037128a72  Imager-0.79.tar.gz

I can only guess, there might be something wrong with the cpan mirror you are accessing (not unlikely, I've seen the happening before) or with the tools you apply to download (This is just a wild guess).

Anyway, here is an updated rpm:
[md5sum: 2505a581d4ca57448fbff756e850eda5  Imager-0.80.tar.gz]

Spec URL:  http://corsepiu.fedorapeople.org/packages/perl-Imager.spec
SRPM URL: 
http://corsepiu.fedorapeople.org/packages/perl-Imager-0.80-1.fc15.src.rpm

Comment 5 Robin Lee 2011-01-21 06:09:32 UTC
[+] md5sum matches
$ md5sum Imager-0.80.tar.gz 
2505a581d4ca57448fbff756e850eda5  Imager-0.80.tar.gz

[+] Rawhide build succeeded
http://koji.fedoraproject.org/koji/taskinfo?taskID=2734361


This package is approved by 'cheeselee'.

Comment 6 Ralf Corsepius 2011-01-21 07:15:52 UTC
Thanks for the review.

New Package SCM Request
=======================
Package Name: perl-Imager
Short Description: Perl extension for Generating 24 bit Images
Owners: corsepiu
Branches: f13 f14
InitialCC: perl-sig

Comment 7 Jason Tibbitts 2011-01-21 14:03:47 UTC
This package already exists in Fedora; it's even in the current release.

Comment 8 Ralf Corsepius 2011-01-21 14:48:09 UTC
(In reply to comment #7)
> This package already exists in Fedora; it's even in the current release.
I am speachless ... once more nobody noticed (comprising me)

<rant> seems to me as if we are stuck in piles of bureaucracy which blocks the view to the most essential stuff </rant>

Comment 9 Jason Tibbitts 2011-01-21 14:57:28 UTC
I fail to understand your rant.  "yum info" or "yum search" will show you that the package exists.  I can understand problems with packages imported but never built (though it's pretty damn easy to find those as well if you bother to look at all), yet this isn't one of those.

Personally I use the convenient search box at:
  http://fedoraproject.org/PackageReviewStatus/
to look for pending reviews and 
  http://bugz.fedoraproject.org/package-name
to find information on packages that might have been created but aren't available in yum for some reason.  If you feel that's excessive bureaucracy then I'm afraid there's not much anyone can do for you.

Comment 10 Robin Lee 2011-01-21 15:36:52 UTC
I am sorry, too. I should have noticed that earlier.
After all, this issue should be closed.

Comment 11 Robin Lee 2011-01-21 15:44:51 UTC

*** This bug has been marked as a duplicate of bug 166254 ***

Comment 12 Ralf Corsepius 2011-01-21 15:59:08 UTC
(In reply to comment #9)
> I fail to understand your rant. 
OT for bugzilla - I will try to provide a more comprehensive answer on PM.

> "yum info" or "yum search" will show you that
> the package exists.  I can understand problems with packages imported but never
> built (though it's pretty damn easy to find those as well if you bother to look
> at all), yet this isn't one of those.
Only so far: This is the 3rd time in a row that this has happened to me in
recent weeks. IMO, this gives a nice picture about the shape and usability
of Fedora's package submission/review infrastructure.

> Personally I use the convenient search box at:
I normally use repoquery and build packages against all supported fedora in
mock. This usually catches all such cases - Makes me wonder why it didn't in these recent cases.

>   http://fedoraproject.org/PackageReviewStatus/
> to look for pending reviews and 
>   http://bugz.fedoraproject.org/package-name
> to find information on packages that might have been created but aren't
> available in yum for some reason.  If you feel that's excessive bureaucracy
Yes, very bluntly and somewhat exaggerated said: I feel it's a muddy swamp.

> then I'm afraid there's not much anyone can do for you.
As we say in German: "Only death is certain"


Note You need to log in before you can comment on or make changes to this bug.