Bug 527996 - Review Request: jempbox - Java library for working with XMP metadata
Summary: Review Request: jempbox - Java library for working with XMP metadata
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Steve Traylen
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
: 472792 (view as bug list)
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2009-10-08 14:45 UTC by Orion Poplawski
Modified: 2009-10-19 15:27 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2009-10-19 15:27:59 UTC
Type: ---
Embargoed:
steve.traylen: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Orion Poplawski 2009-10-08 14:45:00 UTC
Spec URL: http://www.cora.nwra.com/~orion/fedora/jempbox.spec
SRPM URL: http://www.cora.nwra.com/~orion/fedora/jempbox-0.8.0-1.fc11.src.rpm
Description: 
Apache JempBox is an open source Java library for working with XMP metadata.

Comment 1 Orion Poplawski 2009-10-08 14:45:18 UTC
*** Bug 472792 has been marked as a duplicate of this bug. ***

Comment 2 Steve Traylen 2009-10-11 21:32:38 UTC
Hi,
Looks to be missing a buildrequires? I guess you need


Also you have a .jar of source files. - Its new to me
but I guess fine. Is there a precedent?

Build Error:

Executing(%prep): /bin/sh -e /var/tmp/rpm-tmp.SCGe2E
+ umask 022
+ cd /home/steve/rpmbuild/BUILD
+ LANG=C
+ export LANG
+ unset DISPLAY
+ cd /home/steve/rpmbuild/BUILD
+ rm -rf jempbox-0.8.0-incubating
+ /usr/bin/unzip -qq /home/steve/rpmbuild/SOURCES/jempbox-0.8.0-incubating-src.jar
+ STATUS=0
+ '[' 0 -ne 0 ']'
+ cd jempbox-0.8.0-incubating
+ /bin/chmod -Rf a+rX,u+w,g-w,o-w .
+ echo 'Patch #0 (jempbox-0.8.0-local.patch):'
Patch #0 (jempbox-0.8.0-local.patch):
+ /bin/cat /home/steve/rpmbuild/SOURCES/jempbox-0.8.0-local.patch
+ /usr/bin/patch -s -p1 -b --suffix .local --fuzz=0
+ find -name '*.class' -exec rm -f '{}' ';'
+ find -name '*.jar' -exec rm -f '{}' ';'
+ exit 0
Executing(%build): /bin/sh -e /var/tmp/rpm-tmp.91h68E
+ umask 022
+ cd /home/steve/rpmbuild/BUILD
+ cd jempbox-0.8.0-incubating
+ LANG=C
+ export LANG
+ unset DISPLAY
++ build-classpath junit4
+ export CLASSPATH=/usr/share/java/junit4.jar
+ CLASSPATH=/usr/share/java/junit4.jar
+ ant
Buildfile: build.xml

compile:
    [mkdir] Created dir: /home/steve/rpmbuild/BUILD/jempbox-0.8.0-incubating/target/classes
    [javac] Compiling 24 source files to /home/steve/rpmbuild/BUILD/jempbox-0.8.0-incubating/target/classes

junit:
    [mkdir] Created dir: /home/steve/rpmbuild/BUILD/jempbox-0.8.0-incubating/target/test-classes
    [javac] Compiling 2 source files to /home/steve/rpmbuild/BUILD/jempbox-0.8.0-incubating/target/test-classes

BUILD FAILED
/home/steve/rpmbuild/BUILD/jempbox-0.8.0-incubating/build.xml:145: Problem: failed to create task or type junit
Cause: the class org.apache.tools.ant.taskdefs.optional.junit.JUnitTask was not found.
        This looks like one of Ant's optional components.
Action: Check that the appropriate optional JAR exists in
        -/usr/share/ant/lib
        -/home/steve/.ant/lib
        -a directory added on the command line with the -lib argument

Do not panic, this is a common problem.
The commonest cause is a missing JAR.

This is not a bug; it is a configuration problem


Total time: 6 seconds
error: Bad exit status from /var/tmp/rpm-tmp.91h68E (%build)


RPM build errors:
    Bad exit status from /var/tmp/rpm-tmp.91h68E (%build)


Steve

Comment 3 Orion Poplawski 2009-10-12 16:40:48 UTC
Spec URL: http://www.cora.nwra.com/~orion/fedora/jempbox.spec
SRPM URL: http://www.cora.nwra.com/~orion/fedora/jempbox-0.8.0-2.fc11.src.rpm

Sorry about that, too many packages at once...

* Mon Oct 12 2009 Orion Poplawski <orion.com> - 0.8.0-2
- Fix BuildRequires

http://koji.fedoraproject.org/koji/taskinfo?taskID=1742372

jar is simply a zip file with a special directory structure.  No big deal.

Comment 4 Steve Traylen 2009-10-14 21:14:19 UTC
Hi,
A few line endings to be altered.

[steve@globus SPECS]$ rpmlint jempbox.spec ../SRPMS/jempbox-0.8.0-2.fc11.src.rpm ../RPMS/noarch/jempbox-0.8.0-2.fc11.noarch.rpm 
jempbox.noarch: W: wrong-file-end-of-line-encoding /usr/share/doc/jempbox-0.8.0/RELEASE-NOTES.txt
jempbox.noarch: W: wrong-file-end-of-line-encoding /usr/share/doc/jempbox-0.8.0/README.txt


Is there a reason not to the whole gcj thing?

https://fedoraproject.org/wiki/Packaging:Java#GCJ

There are no comments on what the patch is for? Should there
be a bug somewhere?

Steve

Comment 5 Orion Poplawski 2009-10-14 21:33:17 UTC
Good catches.  The patch is to disable downloading of required jar files from upstream, so it's going to be a packaging only kind of patch.

Spec URL: http://www.cora.nwra.com/~orion/fedora/jempbox.spec
SRPM URL: http://www.cora.nwra.com/~orion/fedora/jempbox-0.8.0-3.fc12.src.rpm

* Wed Oct 14 2009 Orion Poplawski <orion.com> - 0.8.0-3
- Add comment for patch
- Fix line endings
- Add GCJ support

Comment 6 Steve Traylen 2009-10-15 07:43:52 UTC
Hi Orion,

Review of jempbox:

MUST:

YES:
$ rpmlint jempbox.spec ../SRPMS/jempbox-0.8.0-3.fc11.src.rpm ../RPMS/x86_64/jempbox-*
jempbox.spec:85: W: libdir-macro-in-noarch-package (main package) %attr(-,root,root) %{_libdir}/gcj/%{name}
jempbox.src:85: W: libdir-macro-in-noarch-package (main package) %attr(-,root,root) %{_libdir}/gcj/%{name}
3 packages and 1 specfiles checked; 0 errors, 2 warnings.

which is expected because of the %if case with gcj.

NO:
Package is not named correctly.
The package contains /usr/share/java/jempbox-0.8.0-incubating.jar only
so in principal should be called jempbox-0.8.0-incubating as per

https://fedoraproject.org/wiki/Packaging:Java#Package_naming

but I think really a jempbox.jar symlink should be made with a 

(cd %{buildroot}%{_javadir} && for jar in *-%{version}*; do ln -sf ${jar} `echo $jar| sed  "s|-%{version}||g"`; done)

or similar.

I'll stop the review now since depending on how this is fixed changes
other elements of the review.

For an example package that does the symlinking to a generic
version of the jar see bug #521895 and if you have time to review
this that would be great.

Steve

Comment 7 Orion Poplawski 2009-10-15 14:59:25 UTC
Spec URL: http://www.cora.nwra.com/~orion/fedora/jempbox.spec
SRPM URL: http://www.cora.nwra.com/~orion/fedora/jempbox-0.8.0-4.fc12.src.rpm

* Thu Oct 15 2009 Orion Poplawski <orion.com> - 0.8.0-4
- Fix jar naming and add symbolic links

Comment 8 Steve Traylen 2009-10-15 20:44:02 UTC
Sorry, I realize this is probably rather irrelevant

$ ls -l jempbox*
lrwxrwxrwx. 1 root root    11 2009-10-15 22:41 jempbox-0.8.0-incubating.jar -> jempbox.jar
lrwxrwxrwx. 1 root root    11 2009-10-15 22:41 jempbox-0.8.0.jar -> jempbox.jar
-rw-r--r--. 1 root root 51158 2009-10-15 22:38 jempbox.jar

I think it is meant to be other way around with

jempbox-0.8.0-incubating.jar

the real file and the others as symlinks to it.

Steve

Comment 9 Orion Poplawski 2009-10-15 20:48:12 UTC
Spec URL: http://www.cora.nwra.com/~orion/fedora/jempbox.spec
SRPM URL: http://www.cora.nwra.com/~orion/fedora/jempbox-0.8.0-5.fc12.src.rpm

* Thu Oct 15 2009 Orion Poplawski <orion.com> - 0.8.0-5
- Reverse symbolic links

Comment 10 Steve Traylen 2009-10-15 21:04:04 UTC
Hi Orion,
Speedy.

Koji Build: http://koji.fedoraproject.org/koji/taskinfo?taskID=1748214


YES: rpmlint
$ rpmlint SPECS/jempbox.spec RPMS/x86_64/jempbox-0.8.0-5.fc11.x86_64.rpm RPMS/x86_64/jempbox-debuginfo-0.8.0-5.fc11.x86_64.rpm SRPMS/jempbox-0.8.0-5.fc11.src.rpm 
SPECS/jempbox.spec:89: W: libdir-macro-in-noarch-package (main package) %attr(-,root,root) %{_libdir}/gcj/%{name}
jempbox.src:89: W: libdir-macro-in-noarch-package (main package) %attr(-,root,root) %{_libdir}/gcj/%{name}
3 packages and 1 specfiles checked; 0 errors, 2 warnings.

which is expected because of the %if case for gcj.

YES: Package name.
YES: Spec file name.
YES: Package guidelines for java.
YES: License clear and good.
YES: License in .spec correct.
YES: LICENSE.txt included.
YES: American English.
YES: Legible.
YES: 
$ md5sum jempbox-0.8.0-incubating-src.jar ../SOURCES/jempbox-0.8.0-incubating-src.jar 
b15d0837cc25b47bbdb0e2a49b8c6b65  jempbox-0.8.0-incubating-src.jar
b15d0837cc25b47bbdb0e2a49b8c6b65  ../SOURCES/jempbox-0.8.0-incubating-src.jar

YES: compiles, see koji above.
YES: compiels on all archs.
YES: BuildRequires good.
YES: no locales present.
YES: ldconfig not needed.
YES: no system libs.
YES: not relocatable.
YES: owns all directories.
YES: No double file entries.
YES: All %defattr
YES: buildroot removed in %clean.
YES: Yes there is code in there.
YES: no large docs.
YES: %doc not needed runtime.
YES: no header files present.
YES: No static libs.
YES: No pkgconfig files.
YES: No devel package.
YES: No .la files.
YES: No gui.
YES: buildroot removed at install.
YES: valid utf8

Voila, APPROVED

Thanks for the review.

Steve

Comment 11 Orion Poplawski 2009-10-15 21:07:42 UTC
Thanks, Steve.  Very thorough.

New Package CVS Request
=======================
Package Name: jempbox
Short Description: Java library for working with XMP metadata
Owners: orion
Branches: F-12 F-11 F-10
InitialCC:

Comment 12 Orion Poplawski 2009-10-15 21:42:59 UTC
Add EL-5

New Package CVS Request
=======================
Package Name: jempbox
Short Description: Java library for working with XMP metadata
Owners: orion
Branches: F-12 F-11 F-10 EL-5
InitialCC:

Comment 13 Kevin Fenzi 2009-10-17 20:44:46 UTC
cvs done.

Comment 14 Orion Poplawski 2009-10-19 15:27:59 UTC
Checked in and built.


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