Bug 561477 - Review Request: jnr-posix - Java Posix layer
Summary: Review Request: jnr-posix - Java Posix layer
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Stanislav Ochotnicky
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On: 561462 561466
Blocks: 561484
TreeView+ depends on / blocked
 
Reported: 2010-02-03 18:23 UTC by Mo Morsi
Modified: 2013-01-13 14:06 UTC (History)
6 users (show)

Fixed In Version: jnr-posix-1.1.4-3.fc13
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2010-12-13 20:01:32 UTC
sochotni: fedora-review+
tibbs: fedora-cvs+


Attachments (Terms of Use)

Description Mo Morsi 2010-02-03 18:23:37 UTC
Spec URL: http://mo.morsi.org/files/jruby/jnr-posix.spec
SRPM URL: http://mo.morsi.org/files/jruby/jnr-posix-1.0.8-1.fc12.src.rpm
Description: 
jnr-posix is a lightweight cross-platform POSIX emulation layer for Java, 
written in Java and is part of the JNR project 

Formerly orphaned package jna-posix:
http://admin.fedoraproject.org/pkgdb/packages/name/jna-posix (renamed to jnr-posix upstream)

Required by JRuby.

No koji build yet as jnr-posix depends on jnr-constants and jffi which are both pending Fedora approval as well.

$ rpmlint rpmbuild/RPMS/i386/jnr-posix-1.0.8-1.fc12.i386.rpm 
jnr-posix.i386: W: uncompressed-zip /usr/share/java/jnr-posix-1.0.8.jar
1 packages and 0 specfiles checked; 0 errors, 1 warnings.

Not sure what is causing this warning this is a jar file and acts as such when the package is installed.

$ rpmlint rpmbuild/SRPMS/jnr-posix-1.0.8-1.fc12.src.rpm 
jnr-posix.src:88: W: libdir-macro-in-noarch-package (main package) %attr(-,root,root) %{_libdir}/gcj/%{name}
1 packages and 0 specfiles checked; 0 errors, 1 warnings.


Ignoring this last warning as when package is noarch (eg if using java-openjdk)
the libdir file will not be included, and when the package is architecture
specific (eg when using gcj) it will be. See spec file for conditionals.

Comment 1 Mo Morsi 2010-09-09 20:26:00 UTC
Updated jnr-posix to current upstream release 1.1.4

Spec URL: http://mo.morsi.org/files/jruby/jnr-posix.spec
SRPM URL: http://mo.morsi.org/files/jruby/jnr-posix-1.1.4-1.fc13.src.rpm

Unfortunately jruby 1.4.0 (which I submitted to Fedora) doesn't seem to work with this version, whereas jruby 1.5.0, the current upstream stable release, does. I will start looking into updating the submitted jruby package to 1.5.0. Some of its dependencies will most likely also need to be updated, but those should be simpler to do as they are already in Fedora.

Comment 2 Alexander Kurtakov 2010-11-10 22:11:11 UTC
Java packages should ship javadoc subpackages. Please add one.

Comment 3 Mo Morsi 2010-11-15 18:40:12 UTC
Updated

Spec URL: http://mo.morsi.org/files/jruby/jnr-posix.spec
SRPM URL: http://mo.morsi.org/files/jruby/jnr-posix-1.1.4-2.fc13.src.rpm

$ rpmlint /home/mmorsi/rpmbuild/SRPMS/jnr-posix-1.1.4-2.fc13.src.rpm \
          /home/mmorsi/rpmbuild/RPMS/noarch/jnr-posix-1.1.4-2.fc13.noarch.rpm\
          /home/mmorsi/rpmbuild/RPMS/noarch/jnr-posix-javadoc-1.1.4-2.fc13.noarch.rpm

3 packages and 0 specfiles checked; 0 errors, 0 warnings.

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

Comment 4 Stanislav Ochotnicky 2010-11-18 12:38:08 UTC
I can do the review.

Comment 5 Stanislav Ochotnicky 2010-11-19 09:27:45 UTC
Package Review
==============

Key:
- = N/A
x = Check
! = Problem
? = Not evaluated

=== REQUIRED ITEMS ===
[x]  Rpmlint output:
rpmlint /var/lib/mock/fedora-rawhide-x86_64/result/jnr-posix-*
jnr-posix.noarch: W: uncompressed-zip /usr/share/java/jnr-posix-1.1.4.jar
3 packages and 0 specfiles checked; 0 errors, 1 warnings.

The warning is from new rpmlint. I'll have to contact rpmlint maintainer to add filter for jars (no reason to compress them).

[x]  Package is named according to the Package Naming Guidelines[1].
[x]  Spec file name must match the base package name, in the format %{name}.spec.
[x]  Package meets the Packaging Guidelines[2].
[x]  Package successfully compiles and builds into binary rpms.
[!]  Buildroot definition is not present

Buildroot is no longer necessary for supported Fedoras.

[x]  Package is licensed with an open-source compatible license and meets other legal requirements as defined in the legal section of Packaging Guidelines[3,4].
[x]  License field in the package spec file matches the actual license.
License type: CPL or GPLv2+ or LGPLv2+
[x]  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 is included in %doc.
[x]  All independent sub-packages have license of their own
[x]  Spec file is legible and written in American English.
[x]  Sources used to build the package matches the upstream source, as provided in the spec URL.
MD5SUM this package    : 318e10a1021b53e8513210adf88ad010
[x]  All build dependencies are listed in BuildRequires, except for any that are listed in the exceptions section of Packaging Guidelines[5].
[x]  Package must own all directories that it creates.
[x]  Package requires other packages for directories it uses.
[x]  Package does not contain duplicates in %files.
[x]  Permissions on files are set properly.
[!]  Package does NOT have a %clean section which contains rm -rf %{buildroot} (or $RPM_BUILD_ROOT). (not needed anymore)
%clean section with rm -rf %{buildroot} and rm -rf at the beginning of %install is not needed anymore on supported Fedoraas

[!]  Package consistently uses macros (no %{buildroot} and $RPM_BUILD_ROOT mixing)
You mix buildroot macro and RPM_BUILD_ROOT. Pick one

[x]  Package contains code, or permissable content.
[x]  Fully versioned dependency in subpackages, if present.
[-]  Package contains a properly installed %{name}.desktop file if it is a GUI application.
[x]  Package does not own files or directories owned by other packages.
[x]  Javadoc documentation files are generated and included in -javadoc subpackage in %{_javadocdir}/%{name} directory
[x]  Packages have proper BuildRequires/Requires on jpackage-utils
[x]  Javadoc subpackages have Require: jpackage-utils
Through main package. There has been some discussion about having javadoc depend on main package. My opinion is that javadocs should be independent. They don't really require main package. I'll leave it up to you, but if you decide to make it independent, make javadoc require jpackage-utils.

[x]  Package uses %global not %define
[-]  If package uses tarball from VCS include comment how to re-create that tarball (svn export URL, git clone URL, ...)
[x]  If source tarball includes bundled jar/class files these need to be removed prior to building
[x]  All filenames in rpm packages must be valid UTF-8.
[!]  Jar files are installed to %{_javadir}/%{name}.jar (see [6] for details)

Recent changes in Java packaging guidelines made all %{name}-%{version} things unwanted. So please install just %{name}.jar (no versioned symlinks). Same goes for javadoc subdirectory.

[!]  If package contains pom.xml files install it (including depmaps) even when building with ant
Source tarball contains pom.xml (working) but you don't install this. Please install it, and call add_to_maven_depmap with appropriate arguments. This will make this project usable from other maven projects.

=== Maven ===
[-]  Use %{_mavenpomdir} macro for placing pom files instead of %{_datadir}/maven2/poms
[-]  If package uses "-Dmaven.test.skip=true" explain why it was needed in a comment
[-]  If package uses custom depmap "-Dmaven2.jpp.depmap.file=*" explain why it's needed in a comment
[-]  Package uses %update_maven_depmap in %post/%postun
[-]  Packages have Requires(post) and Requires(postun) on jpackage-utils (for %update_maven_depmap macro)

^^ when you add maven support look at this to guide you

=== Other suggestions ===
[?]  If possible use upstream build method (maven/ant/javac)
Well, I checked and package (with small pom.xml modifications) is buildable with maven. Since both ways are possible I'd advise to use maven instead of ant. It's more error prone for the future (and no need to play with build-jar-repository)

You'll have to patch out wagon dependency and use custom depmap file, since jnr-constants don't install pom.xml and depmap either. I'll be filing bug for that in a moment :-)

[!]  Javadocs are placed in %{_javadocdir}/%{name} (no -%{version} symlinks)
[x]  Avoid having BuildRequires on exact NVR unless necessary
[x]  Package has BuildArch: noarch (if possible)
[x]  Latest version is packaged.
[x]  Reviewer should test that the package builds in mock.
Tested on: fedora-rawhide-x86_64


=== Issues ===
1. present buildroot/clean sections
2. mixing of macros
3. (optional) javadoc Require on main package
4. naming of jar file
5. installing pom.xml and add_to_maven_depmap
6. (optional) use maven for building


[1] https://fedoraproject.org/wiki/Packaging:NamingGuidelines
[2] https://fedoraproject.org/wiki/Packaging:Guidelines
[3] https://fedoraproject.org/wiki/Packaging:LicensingGuidelines
[4] https://fedoraproject.org/wiki/Licensing:Main
[5] https://fedoraproject.org/wiki/Packaging/Guidelines#Exceptions_2
[6] https://fedoraproject.org/wiki/Packaging:Java#Filenames

Comment 6 Stanislav Ochotnicky 2010-11-19 09:47:28 UTC
Hmm, sorry...actually that warning about non-compressed file is actually valid. 

nbproject/project.properties contains "jar.compress=false" this has to be changed if you stay with ant.

Comment 7 Mo Morsi 2010-12-02 14:53:59 UTC
OK thanks for the feedback and srry for the delay (was off all last week).

Updated package to:

1. remove BuildRoot and clean section
2. Consistently use macros
3. jar and javadocs are now named %{name} and not %{name}-%{version}
4. installed pom.xml and used maven macros

As far as the javadoc subpackage, it already depends on the main package, so I think this isn't an issue (please elaborate if so). Also I'm just going to leave it as ant for now as thats the quickest way to get this is and is still standards compliant, we can update the package if this ever becomes an issue.

Updated SPEC and SRPM:

Spec URL: http://mo.morsi.org/files/jruby/jnr-posix.spec
SRPM URL: http://mo.morsi.org/files/jruby/jnr-posix-1.1.4-3.fc13.src.rpm

Again thanks for the review.

Comment 8 Mo Morsi 2010-12-02 14:55:22 UTC
Also changed patch0 to include the "jar.compress=true" fix

Comment 9 Stanislav Ochotnicky 2010-12-02 15:14:02 UTC
Nice, everything is in order now. APPROVED

Comment 10 Mo Morsi 2010-12-02 15:23:48 UTC
Sweet,thx alot.

New Package SCM Request
=======================
Package Name: jnr-posix
Short Description: Java Posix layer
Owners: mmorsi
Branches: f13 f14

Comment 11 Jason Tibbitts 2010-12-02 19:33:08 UTC
Git done (by process-git-requests).

Comment 12 Fedora Update System 2010-12-02 22:52:54 UTC
jnr-posix-1.1.4-3.fc13 has been submitted as an update for Fedora 13.
https://admin.fedoraproject.org/updates/jnr-posix-1.1.4-3.fc13

Comment 13 Fedora Update System 2010-12-02 22:53:40 UTC
jnr-posix-1.1.4-3.fc14 has been submitted as an update for Fedora 14.
https://admin.fedoraproject.org/updates/jnr-posix-1.1.4-3.fc14

Comment 14 Fedora Update System 2010-12-03 20:42:04 UTC
jnr-posix-1.1.4-3.fc13 has been pushed to the Fedora 13 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 jnr-posix'.  You can provide feedback for this update here: https://admin.fedoraproject.org/updates/jnr-posix-1.1.4-3.fc13

Comment 15 Fedora Update System 2010-12-13 20:01:22 UTC
jnr-posix-1.1.4-3.fc14 has been pushed to the Fedora 14 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 16 Fedora Update System 2010-12-15 08:58:28 UTC
jnr-posix-1.1.4-3.fc13 has been pushed to the Fedora 13 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.