Bug 452251 - Review Request: libmatthew-java - collection of java libraries
Review Request: libmatthew-java - collection of java libraries
Status: CLOSED CURRENTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Thomas Fitzsimmons
Fedora Extras Quality Assurance
:
Depends On:
Blocks: 452688
  Show dependency treegraph
 
Reported: 2008-06-20 11:12 EDT by Omair Majid
Modified: 2008-06-28 18:16 EDT (History)
2 users (show)

See Also:
Fixed In Version: 0.7.1-1.fc9
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-06-28 18:16:08 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
fitzsim: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Omair Majid 2008-06-20 11:12:08 EDT
Spec URL: http://omajid.fedorapeople.org/libmatthew-java.spec
SRPM URL: http://omajid.fedorapeople.org/libmatthew-java-0.7.1-1.fc9.src.rpm

Description:

A colleciton of Java libraries

- Unix Sockets Library
This is a collection of classes and native code to allow you to read
and write Unix sockets in Java.
 
- Debug Library
This is a comprehensive logging and debugging solution.
 
- CGI Library
This is a collection of classes and native code to allow you to write
CGI applications in Java.

- I/O Library
This provides a few much needed extensions to the Java I/O subsystem.

- Hexdump
This class formats byte-arrays in hex and ascii for display.
Comment 1 Omair Majid 2008-06-20 11:15:53 EDT
Sorry, but I forgot to mention that this is my first package and I am looking
for a sponsor. Thanks
Comment 2 Omair Majid 2008-06-23 17:10:07 EDT
After talking to Thomas Fitzsimmons, modified spec file (and added a patch to
srpm) to build and run using openjdk 
Comment 3 Thomas Fitzsimmons 2008-06-24 11:41:33 EDT
- MUST: rpmlint must be run on every package. The output should be posted...

$ rpmlint SRPMS/libmatthew-java-0.7.1-1.fc9.src.rpm
$ rpmlint RPMS/i386/libmatthew-java-0.7.1-1.fc9.i386.rpm
-soname /usr/lib/libcgi-java.so
-soname /usr/lib/libunix-java.so
$ rpmlint RPMS/i386/libmatthew-java-javadoc-0.7.1-1.fc9.i386.rpm
$ rpmlint RPMS/i386/libmatthew-java-debuginfo-0.7.1-1.fc9.i386.rpm

- MUST: The package must be named according to the Package Naming Guideli...

OK

- MUST: The spec file name must match the base package %{name}, in the fo...

OK

- MUST: The package must meet the Packaging Guidelines .

OK

- MUST: The package must be licensed with a Fedora approved license and m...

OK

- MUST: The License field in the package spec file must match the actual ...

OK

- MUST: If (and only if) the source package includes the text of the lice...

OK

- MUST: The spec file must be written in American English.

OK

- MUST: The spec file for the package MUST be legible. If the reviewer is...

OK

- MUST: The sources used to build the package must match the upstream sou...

I'm not sure what this comment means:

# debian version is newer than upstream

Source must come from the actual upstream, not from another
distribution.  If Debian's version is better than upstream then add
another patch with those changes.

- MUST: The package must successfully compile and build into binary rpms ...

OK

- MUST: If the package does not successfully compile, build or work on an...

OK

- MUST: All build dependencies must be listed in BuildRequires, except fo...

OK

- MUST: The spec file MUST handle locales properly. This is done by using...

OK

- MUST: Every binary RPM package which stores shared library files (not j...

The JNI objects should go in %{_libdir}/%{name}, not %{_libdir}.  See:

http://fedoraproject.org/wiki/Packaging/Java#Packaging_JAR_files_that_use_JNI

- MUST: If the package is designed to be relocatable, the packager must s...

OK

- MUST: A package must own all directories that it creates. If it does no...

OK

- MUST: A package must not contain any duplicate files in the %files list...

OK

- MUST: Permissions on files must be set properly. Executables should be ...

OK

- MUST: Each package must have a %clean section, which contains rm -rf %{...

OK

- MUST: Each package must consistently use macros, as described in the [w...

OK

- MUST: The package must contain code, or permissable content. This is de...

OK

- MUST: Large documentation files should go in a -doc subpackage. (The de...

OK

- MUST: If a package includes something as %doc, it must not affect the r...

OK

- MUST: Header files must be in a -devel package.

OK

- MUST: Static libraries must be in a -static package.

OK

- MUST: Packages containing pkgconfig(.pc) files must 'Requires: pkgconfi...

OK

- MUST: If a package contains library files with a suffix (e.g. libfoo.so...

See JNI comment above.

- MUST: In the vast majority of cases, devel packages must require the ba...

OK

- MUST: Packages must NOT contain any .la libtool archives, these should ...

OK

- MUST: Packages containing GUI applications must include a %{name}.deskt...

OK

- MUST: Packages must not own files or directories already owned by other...

OK

- MUST: At the beginning of %install, each package MUST run rm -rf %{buil...

OK

- MUST: All filenames in rpm packages must be valid UTF-8.

OK

- SHOULD: If the source package does not include license text(s) as a sep...

OK

- SHOULD: The description and summary sections in the package spec file s...

s/java/Java/ in the summary tag.

Usually description is in paragraph form, not bullet points.  I'll
leave this up to you though.

- SHOULD: The reviewer should test that the package builds in mock. See [...

A Rawhide i386 mock build fails:

+ /usr/lib/rpm/find-debuginfo.sh --strict-build-id
/builddir/build/BUILD/libmatthew-java-0.7.1
extracting debug info from
/var/tmp/libmatthew-java-0.7.1-1.fc10-root-mockbuild/usr/lib/libcgi-java.so
*** ERROR: No build ID note found in
/var/tmp/libmatthew-java-0.7.1-1.fc10-root-mockbuild/usr/lib/libcgi-java.so
RPM build errors:
error: Bad exit status from /var/tmp/rpm-tmp.14107 (%install)
    Bad exit status from /var/tmp/rpm-tmp.14107 (%install)
Child returncode was: 1

- SHOULD: The package should compile and build into binary rpms on all su...

rpmbuild works on i386.

- SHOULD: The reviewer should test that the package functions as describe...

No demos to try.

- SHOULD: If scriptlets are used, those scriptlets must be sane. This is ...

The if ... fi formatting is a little strange, but OK.

- SHOULD: Usually, subpackages other than devel should require the base p...

Does the javadoc subpackage actually need the base package to be
installed?  Usually javadoc subpackages are an exception to this
guideline.

- SHOULD: The placement of pkgconfig(.pc) files depends on their usecase,...

OK

- SHOULD: If the package has file dependencies outside of /etc, /bin, /sb...

OK
Comment 4 Thomas Fitzsimmons 2008-06-24 11:43:30 EDT
Clipping error on rpmlint output:

$ rpmlint RPMS/i386/libmatthew-java-0.7.1-1.fc9.i386.rpm 
libmatthew-java.i386: W: no-soname /usr/lib/libcgi-java.so
libmatthew-java.i386: W: no-soname /usr/lib/libunix-java.so

This warning must be fixed.
Comment 5 Omair Majid 2008-06-24 15:00:14 EDT
Updated:

Spec URL: http://omajid.fedorapeople.org/libmatthew-java/libmatthew-java.spec
SRPM URL:
http://omajid.fedorapeople.org/libmatthew-java/libmatthew-java-0.7.1-1.fc9.src.rpm

fixed soname warning, jni object locations, added --build-id flag to build on
rawhide and removed explicit dependency of javadoc on base package
Comment 6 Thomas Fitzsimmons 2008-06-24 17:30:20 EDT
$ rpmlint RPMS/i386/libmatthew-java-0.7.1-1.fc9.i386.rpm

mock build succeeds on fedora-rawhide-i386.

The upstream project shouldn't be using ld directly to link the JNI libraries. 
Can you patch it to use gcc instead?  This may allow you to not specify
--build-id.  Also, the contents of RPM_OPT_FLAGS (see rpm --eval %{optflags})
are not being passed to cpp, cc and ld.  They need to be.
Comment 7 Omair Majid 2008-06-25 10:33:23 EDT
Updated:

Spec URL: http://omajid.fedorapeople.org/libmatthew-java/libmatthew-java.spec
SRPM URL:
http://omajid.fedorapeople.org/libmatthew-java/libmatthew-java-0.7.1-1.fc9.src.rpm

replaced ld with gcc; passed %{optflags} to cflags, ppflags, ldflags and gcjflags
Comment 8 Thomas Fitzsimmons 2008-06-25 11:06:50 EDT
Just one more change request.  It may produce the same result, but the more
conventional way to link a shared object is to pass -shared to gcc.  So I'd
recommend replacing:

-Wl,-soname=$@

in soname.patch with

-shared

You should work with upstream to clean up these build scripts.  Most importantly
they should not be invoking the linker directly, but there's also the make
variables: specifying CFLAGS on the make line should add to, not override, the
tools options.  So if upstream is fixed you should only need:

make %{?_smp_mflags} \
    CFLAGS='%{optflags}'\
    GCJFLAGS='%{optflags}' \
    LDFLAGS='%{optflags}' \
    PPFLAGS='%{optflags}'

Please make the -shared change when you commit the package.

Approved.
Comment 9 Omair Majid 2008-06-25 16:35:21 EDT
Updated

Spec URL: http://omajid.fedorapeople.org/libmatthew-java/libmatthew-java.spec
SRPM URL:
http://omajid.fedorapeople.org/libmatthew-java/libmatthew-java-0.7.1-1.fc9.src.rpm

also includes a patch (sent it upstream too) to match 
make %{?_smp_mflags} \
    CFLAGS='%{optflags}'\
    GCJFLAGS='%{optflags}' \
    LDFLAGS='%{optflags}' \
    PPFLAGS='%{optflags}'
Comment 10 Omair Majid 2008-06-26 11:58:58 EDT
New Package CVS Request
=======================
Package Name: libmatthew-java
Short Description: collection of java libraries
Owners: omajid
Branches: F-9
InitialCC:
Cvsextras Commits: yes
Comment 11 Kevin Fenzi 2008-06-27 13:00:17 EDT
cvs done.
Comment 12 Fedora Update System 2008-06-27 16:25:42 EDT
libmatthew-java-0.7.1-1.fc9 has been submitted as an update for Fedora 9
Comment 13 Fedora Update System 2008-06-28 18:16:05 EDT
libmatthew-java-0.7.1-1.fc9 has been pushed to the Fedora 9 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.