Bug 536944

Summary: Review Request: jai-imageio-core - Core Java Advanced Imaging Image I/O Tools API
Product: [Fedora] Fedora Reporter: Adam Goode <adam>
Component: Package ReviewAssignee: Mary Ellen Foster <mefoster>
Status: CLOSED NEXTRELEASE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: fedora-package-review, mefoster, notting
Target Milestone: ---Flags: mefoster: fedora-review+
kevin: fedora-cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2010-05-19 15:12:30 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Attachments:
Description Flags
Possible patch to generate-tarball.sh none

Description Adam Goode 2009-11-11 21:16:14 UTC
Spec URL: http://www.cs.cmu.edu/~agoode/tmp/jai-imageio-core.spec
SRPM URL: http://www.cs.cmu.edu/~agoode/tmp/jai-imageio-core-1.2-0.1.20091111cvs.fc11.src.rpm
Description: This package contains the core Java Advanced Imaging Image I/O Tools API, minus JPEG 2000, JAI Image I/O operations, and the C-based codecLib, giving Java programs the ability to read and write BMP, GIF, PCX, PNM, "Raw" (not digital camera RAW), TIFF, PCX, WBMP.

Note that only BSD-licensed code is included in the SRPM, the other stuff has been cleaned out.

Comment 1 Mary Ellen Foster 2010-01-11 11:44:24 UTC
I'll take this ... full review pending, but it would be useful to document the process of creating the tarball -- both the exact cvs checkout command, as well as precisely what files/directories to remove after the fact.

See https://fedoraproject.org/wiki/Packaging:SourceURL for some suggestions

Comment 3 Mary Ellen Foster 2010-02-03 16:14:40 UTC
Ack! Sorry I took so long to get back to this ... only one thing really to change (the issue with generate-tarball.sh) and it otherwise looks good

Review:
OK: rpmlint must be run on every package.
Output:
jai-imageio-core.src: W: strange-permission generate-tarball.sh 0775

===> False positive from the source package

jai-imageio-core.src: W: spelling-error %description -l en_US codecLib -> codeine, decliner, code's
jai-imageio-core.src:140: W: libdir-macro-in-noarch-package (main package) %{_libdir}/gcj/%{name}
jai-imageio-core.src: W: invalid-url Source0: jai-imageio-core-cvs20100111-CLEANED.tar.xz
jai-imageio-core.x86_64: W: spelling-error %description -l en_US codecLib -> codeine, decliner, code's
jai-imageio-core-javadoc.noarch: W: spelling-error Summary(en_US) Javadocs -> Java docs, Java-docs, Javanese
===> False positives

jai-imageio-core-debuginfo.x86_64: E: script-without-shebang /usr/src/debug/jai-imageio-core-cvs20100111-CLEANED/src/share/classes/com/sun/media/imageioimpl/plugins/gif/GIFImageMetadataFormat.java
jai-imageio-core-debuginfo.x86_64: E: script-without-shebang /usr/src/debug/jai-imageio-core-cvs20100111-CLEANED/src/share/classes/com/sun/media/imageioimpl/plugins/gif/GIFImageMetadata.java
jai-imageio-core-debuginfo.x86_64: E: script-without-shebang /usr/src/debug/jai-imageio-core-cvs20100111-CLEANED/src/share/classes/com/sun/media/imageioimpl/plugins/gif/GIFStreamMetadataFormatResources.java
jai-imageio-core-debuginfo.x86_64: E: script-without-shebang /usr/src/debug/jai-imageio-core-cvs20100111-CLEANED/src/share/classes/com/sun/media/imageioimpl/plugins/gif/GIFStreamMetadata.java
jai-imageio-core-debuginfo.x86_64: E: script-without-shebang /usr/src/debug/jai-imageio-core-cvs20100111-CLEANED/src/share/classes/com/sun/media/imageioimpl/plugins/gif/GIFStreamMetadataFormat.java
jai-imageio-core-debuginfo.x86_64: E: script-without-shebang /usr/src/debug/jai-imageio-core-cvs20100111-CLEANED/src/share/classes/com/sun/media/imageioimpl/plugins/gif/GIFImageMetadataFormatResources.java
jai-imageio-core-debuginfo.x86_64: E: script-without-shebang /usr/src/debug/jai-imageio-core-cvs20100111-CLEANED/src/share/classes/com/sun/media/imageioimpl/common/PaletteBuilder.java
===> All false positives from the debuginfo package

OK: The package must be named according to the Package Naming Guidelines .
OK: The spec file name must match the base package %{name}, in the format
%{name}.spec unless your package has an exemption.
OK: The package must meet the Packaging Guidelines .
OK: The package must be licensed with a Fedora approved license and meet the
Licensing Guidelines .
OK: The License field in the package spec file must match the actual license.
OK: The spec file must be written in American English.
OK: The spec file for the package MUST be legible.
FIX: The sources used to build the package must match the upstream source, as
provided in the spec URL.
===> generate-tarball.sh uses the current date to check out the tarball and checks out the HEAD revision. I'd suggest changing it to take the date as a second parameter and to use that date as the checkout command. I'll attach the patch that worked for me; feel free to do this or something else.

Note that even with this change, I ended up with a file with a different md5sum than the one that included in your SRPM, and I'm not sure why -- maybe because I'm on a 64-bit computer?

OK: The package MUST successfully compile and build into binary rpms on at
least one primary architecture.
OK: All build dependencies must be listed in BuildRequires
OK: Packages must NOT bundle copies of system libraries.
OK: A package must own all directories that it creates.
OK: A Fedora package must not list a file more than once in the spec file's
%files listings.
OK: Permissions on files must be set properly.
OK: Each package must have a %clean section, which contains rm -rf %{buildroot}
(or $RPM_BUILD_ROOT).
OK: Each package must consistently use macros.
OK: The package must contain code, or permissable content.
OK: Packages must not own files or directories already owned by other packages.
OK: At the beginning of %install, each package MUST run rm -rf %{buildroot}
OK: All filenames in rpm packages must be valid UTF-8.

Comment 4 Mary Ellen Foster 2010-02-03 16:15:37 UTC
Created attachment 388559 [details]
Possible patch to generate-tarball.sh

Here's a possible patch to generate-tarball.sh that checks out a specific date-based revision instead of HEAD

Comment 5 Adam Goode 2010-02-03 16:53:42 UTC
Thanks. Do you happen to know if I should add some maven metadata?

Comment 6 Adam Goode 2010-02-03 17:00:07 UTC
Regarding MD5 of the tarball, xz is not guaranteed to generate identical files between runs.

Comment 7 Mary Ellen Foster 2010-02-03 17:08:09 UTC
Hmm. Don't know if that goes against the guidelines then -- I guess it's also possible to do diff -r on the directories to compare.

Regarding Maven stuff, there seems to be a POM for version 1.1 here:
    http://repo2.maven.org/maven2/com/sun/media/jai_imageio/1.1/
(the direct link to the POM doesn't work with wget etc.) You can probably use that as the basis for the metadata, and even edit the POM to say 1.2 instead of 1.1.

Comment 8 Adam Goode 2010-02-03 20:11:36 UTC
The timestamps and owner/group are different in the tarballs, even before xz. I have hopefully fixed this now. md5sum of the uncompressed data is the same now. xz still gives differences.

I'm skipping the maven stuff for now.

http://www.cs.cmu.edu/~agoode/tmp/jai-imageio-core.spec
http://www.cs.cmu.edu/~agoode/tmp/jai-imageio-core-1.2-0.3.20100202cvs.fc12.src.rpm

Comment 9 Mary Ellen Foster 2010-02-10 10:53:35 UTC
The md5sums still don't match, but I did a diff -r and the contents are identical, so I can live with that.

Sorry for the delay -- APPROVED.

Comment 10 Adam Goode 2010-02-15 02:13:36 UTC
New Package CVS Request
=======================
Package Name: jai-imageio-core
Short Description: Core Java Advanced Imaging Image I/O Tools API
Owners: agoode
Branches: F-11 F-12 EL-5
InitialCC:

Comment 11 Kevin Fenzi 2010-02-16 03:43:15 UTC
CVS done (by process-cvs-requests.py).