Bug 668588 - Review Request: python26-imaging - Python's own image processing library
Review Request: python26-imaging - Python's own image processing library
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Andrew Elwell
Fedora Extras Quality Assurance
:
Depends On:
Blocks: 662755
  Show dependency treegraph
 
Reported: 2011-01-10 15:43 EST by Steve Traylen
Modified: 2011-02-12 19:20 EST (History)
3 users (show)

See Also:
Fixed In Version: python26-imaging-1.1.7-4.el5
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2011-02-12 19:20:48 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
andrew.elwell: fedora‑review+
tibbs: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Steve Traylen 2011-01-10 15:43:58 EST
Spec URL: http://cern.ch/straylen/rpms/python26-imaging/python26-imaging.spec
SRPM URL: http://cern.ch/straylen/rpms/python26-imaging/python26-imaging-1.1.7-3.el5.src.rpm
Description: 
The Python Imaging Library (PIL) adds image processing capabilities
to your Python interpreter.

This is an EPEL5 only package for use with the python26 packages.
Comment 2 Andrew Elwell 2011-01-12 09:15:54 EST
Informal review (as I'm still awaiting sponsorship):

MUST Items:
* rpmlint
[aelwell@pcitgtelwell pil]$ rpmlint python26-imaging*
python26-imaging.src: W: spelling-error %description -l en_US devel -> delve, devil, revel
python26-imaging.src: W: spelling-error %description -l en_US tk -> kt, t, k
1 packages and 1 specfiles checked; 0 errors, 2 warnings.
-- false warning. OK

OK - Package named according to the Package Naming Guidelines.
-- hard coded python version is there for an explained reason
OK - Spec file in the format %{name}.spec unless your package has an exemption.
WARN - The package must meet the Packaging Guidelines.
-- "All patches should have an upstream bug link or comment" -- they don't (but it's only a SHOULD)
OK - Package licensed with a Fedora approved license.
OK - License field in the package spec file must match the actual license.
OK (it's in the README) - If source includes the text of the license(s) package it in %{doc}.
OK - The spec file must be written in American English.
OK - The spec file for the package MUST be legible.
OK (fc14a54e1ce02a0225be8854bfba478e) - The sources must match the upstream source URL md5sum.
OK (tested on RHEL5 + EPEL python26) - Package MUST successfully compile and build into binary rpms
 - unsucessful compile, build or work on an architecture should be listed ExcludeArch.
    -- each ExcludeArch has corresponding bugzilla no in comment adjacent.
OK - Build dependencies listed in BuildRequires.
OK - Spec file using the %find_lang macro for locales (not using %{_datadir}/locale/*).
OK - If shared library files (not just symlinks), call ldconfig in %post and %postun. 
OK - Packages must NOT bundle copies of system libraries.
OK - if relocatable, must be stared with rationalization
   -- without this use of Prefix: /usr is considered a blocker. 
OK - A package must own all directories that it creates, or require a package which creates that directory.
OK - No duplicates in the spec file's %files listings. (Notable exception: license texts in specific situations)[14]
OK - Permissions on files must be set properly.
OK   -- Executables should be set with executable permissions, for example. 
OK   -- Every %files section must include a %defattr(...) line. 
OK - Each package must consistently use macros.
OK - The package must contain code, or permissable content.
OK - Large documentation files must go in a -doc subpackage.
OK - things in %doc must not affect the runtime of the application.
OK - Header files must be in a -devel package.
N/A - Static libraries must be in a -static package.
N/A - If library files with a suffix (libfoo.so.1.1), then plain .so in a -devel package.
OK - -devel packages need fully versioned dependency: Requires: %{name} = %{version}-%{release}
OK - Packages must NOT contain any .la libtool archives, 
   -- these must be removed in the spec if they are built.
OK - GUI applications must include a %{name}.desktop file,
   --  installed with desktop-file-install in the %install section.
   -- or explain why exempt in a comment in the spec file.
OK - Packages must not own files or directories already owned by other packages.
OK - All filenames in rpm packages must be valid UTF-8.

SHOULD Items:
Items below are things that the package (or reviewer) SHOULD do, but is not required to do.
N/A - query upstream if the source does not include license text(s) as a separate file.
NO - description and summary in spec file should contain translations, if available.
N/A (awaiting sponsorship) - The reviewer should test that the package builds in mock.
Not Tested - The package should compile and build into binary rpms on all supported architectures.
PARTIALLY DONE - The reviewer should test that the package functions as described.
OK - If scriptlets are used, those scriptlets must be sane. 
YES - Usually, subpackages other than devel should require the base package using a fully versioned dependency.
 - The placement of pkgconfig(.pc) files depends on their usecase, and this is usually for development purposes, so should be placed in a -devel pkg. A reasonable exception is that the main pkg itself is a devel tool not installed in a user runtime, e.g. gcc or gdb. [30]
 - If the package has file dependencies outside of /etc, /bin, /sbin, /usr/bin, or /usr/sbin consider requiring the package which provides the file instead of the file itself.
 - your package should contain man pages for binaries/scripts. If it doesn't, work with upstream to add them where they make sense.


Tested on a RHEL5 (clone) box + EPEL for dependencies -- Built OK and basic functionality OK.
Comment 3 Jason Tibbitts 2011-01-21 18:14:53 EST
A couple of other things you might have missed:

Need to run rpmlint on the built rpms, too:
python26-imaging.x86_64: W: private-shared-object-provides /usr/lib64/python2.6/site-packages/PIL/_imagingmath.so _imagingmath.so()(64bit)
python26-imaging.x86_64: W: private-shared-object-provides /usr/lib64/python2.6/site-packages/PIL/_imagingcms.so _imagingcms.so()(64bit)
python26-imaging.x86_64: W: private-shared-object-provides /usr/lib64/python2.6/site-packages/PIL/_imagingft.so _imagingft.so()(64bit)
python26-imaging.x86_64: W: private-shared-object-provides /usr/lib64/python2.6/site-packages/PIL/_imaging.so _imaging.so()(64bit)
python26-imaging-sane.x86_64: W: private-shared-object-provides /usr/lib64/python2.6/site-packages/_sane.so _sane.so()(64bit)
python26-imaging-tk.x86_64: W: private-shared-object-provides /usr/lib64/python2.6/site-packages/PIL/_imagingtk.so _imagingtk.so()(64bit)
python26-imaging-tk.x86_64: W: no-documentation

Not that you can use the filtering functionality on ancient releases like EL5 anyway, so these aren't really problematic.

EPEL isn't built for s390, so all of the s390 conditional stuff is mostly pointless.  (Though I could see value in trying to keep the spec as close as possible to the Fedora one.)

Otherwise this pre-review gets my ACK.
Comment 4 Steve Traylen 2011-01-26 07:28:27 EST
Thanks for the comments, as you say the filtering of .so was never done anywhere much in RHEL/EPEL5
though it is possible to 'sed' them away.

I'll leave the package as is.

Steve.
Comment 5 Andrew Elwell 2011-01-26 08:59:50 EST
OK - contents of original review (comment #2) still apply. Builds ok in mock for 
both 32 and 64 bit epel5:

INFO: Done(python26-imaging-1.1.7-4.el5.src.rpm) Config(epel-5-x86_64)

$ rpmlint /var/lib/mock/epel-5-x86_64/result/python26-imaging-*64.rpm
python26-imaging.x86_64: W: spelling-error %description -l en_US devel -> delve, devil, revel
python26-imaging.x86_64: W: spelling-error %description -l en_US tk -> kt, t, k
python26-imaging-tk.x86_64: W: no-documentation
5 packages and 0 specfiles checked; 0 errors, 3 warnings.

(can be safely ignored)


INFO: Done(python26-imaging-1.1.7-4.el5.src.rpm) Config(epel-5-i386) 
$ rpmlint /var/lib/mock/epel-5-i386/result/python26-imaging-*6.rpm
python26-imaging.i386: W: spelling-error %description -l en_US devel -> delve, devil, revel
python26-imaging.i386: W: spelling-error %description -l en_US tk -> kt, t, k
python26-imaging-tk.i386: W: no-documentation
5 packages and 0 specfiles checked; 0 errors, 3 warnings.

(ditto)

so, review OK.
Comment 6 Steve Traylen 2011-01-26 10:01:00 EST
New Package SCM Request
=======================
Package Name: python26-imaging
Short Description: Python's own image processing library
Owners: stevetraylen
Branches: el5


This is an EPEL5 only package so the devel branch can be closed immediately.

Steve
Comment 7 Jason Tibbitts 2011-01-26 12:57:12 EST
Git done (by process-git-requests).
Comment 8 Fedora Update System 2011-01-26 13:22:28 EST
python26-imaging-1.1.7-4.el5 has been submitted as an update for Fedora EPEL 5.
https://admin.fedoraproject.org/updates/python26-imaging-1.1.7-4.el5
Comment 9 Fedora Update System 2011-01-27 13:23:43 EST
python26-imaging-1.1.7-4.el5 has been pushed to the Fedora EPEL 5 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update python26-imaging'.  You can provide feedback for this update here: https://admin.fedoraproject.org/updates/python26-imaging-1.1.7-4.el5
Comment 10 Fedora Update System 2011-02-12 19:20:43 EST
python26-imaging-1.1.7-4.el5 has been pushed to the Fedora EPEL 5 stable repository.  If problems still persist, please make note of it in this bug report.

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