Bug 707132 - Review Request: java-service-wrapper - Java service wrapper
Summary: Review Request: java-service-wrapper - Java service wrapper
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Ville Skyttä
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2011-05-24 07:33 UTC by Miroslav Suchý
Modified: 2011-09-14 00:29 UTC (History)
3 users (show)

Fixed In Version: java-service-wrapper-3.2.5-5.el5
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2011-08-01 17:39:46 UTC
Type: ---
ville.skytta: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Description Miroslav Suchý 2011-05-24 07:33:16 UTC
SPEC:
http://miroslav.suchy.cz/fedora/java-service-wrapper/java-service-wrapper.spec
SRC.RPM:
http://miroslav.suchy.cz/fedora/java-service-wrapper/java-service-wrapper-3.2.4-2.20100928hg637f7cc19e33.fc15.src.rpm

Description:
The Java Service Wrapper enables a Java application to be run as a
Unix daemon.  It also monitors the health of your application and JVM.

Scratch build:
http://koji.fedoraproject.org/koji/taskinfo?taskID=3088648

rpmlint output:
$ rpmlint /home/msuchy/rpmbuild/SRPMS/java-service-wrapper-3.2.4-2.20100928hg637f7cc19e33.fc15.src.rpm /home/msuchy/rpmbuild/RPMS/x86_64/java-service-wrapper-3.2.4-2.20100928hg637f7cc19e33.fc15.x86_64.rpm /home/msuchy/rpmbuild/RPMS/noarch/java-service-wrapper-javadoc-3.2.4-2.20100928hg637f7cc19e33.fc15.noarch.rpm /home/msuchy/rpmbuild/RPMS/x86_64/java-service-wrapper-debuginfo-3.2.4-2.20100928hg637f7cc19e33.fc15.x86_64.rpm
java-service-wrapper.src: W: invalid-url Source2: java-service-wrapper-3.2.4-docs.tar.bz2
java-service-wrapper.x86_64: W: no-manual-page-for-binary java-service-wrapper
4 packages and 0 specfiles checked; 0 errors, 2 warnings.


First is because Cocoon (which is used for building docs) is not in Fedora. You can create this tar if you build rpm with --with-docs
Second is low impact bug, which I have in plan to address after the review (unless reviewer will have different opinion).

Comment 1 Ville Skyttä 2011-05-24 16:24:55 UTC
> +- add Provides

This is not correct, because the package is not compatible with the previous tanukiwrapper package.
http://fedoraproject.org/wiki/Packaging/NamingGuidelines#Renaming.2Freplacing_existing_packages

> +- removed epoch from BR

This is not correct either, because java-javadoc, java, and java-devel-openjdk do have an Epoch set:

$ rpm -q --provides java-1.6.0-openjdk java-1.6.0-openjdk-devel java-1.6.0-openjdk-javadoc | grep -E 'java(-javadoc|-devel-openjdk)? ='
java = 1:1.6.0
java-devel-openjdk = 1:1.6.0.0
java-javadoc = 1:1.6.0.0-52.1.9.7.fc14

> +- apply patch1 in way, which does not confuse rpmlint

This results in modifying files in the build root, which is not okay.  The previous way was fine, the rpmlint warning can just be ignored.  Note that this has already broken the patch included in the package, comparing my previous package revision with yours (with rpmdev-diff) highlights it:

-+ System.load( new File( "@LIBPATH@", file ).toString() );
++ System.load( new File( "/usr/lib64/java-service-wrapper", file ).toString() );

...so the /usr/lib64 patch got hardwired in the patch.

Comment 2 Ville Skyttä 2011-05-24 16:29:20 UTC
(In reply to comment #1)
> This results in modifying files in the build root, which is not okay.

s/build root/rpm source dir/

Comment 3 Miroslav Suchý 2011-05-24 18:52:51 UTC
> This is not correct, because the package is not compatible with the previous
tanukiwrapper package.

True. I will remove it.

>> +- removed epoch from BR
> This is not correct either, because java-javadoc, java, and java-devel-openjdk
do have an Epoch set:
That is true for openjdk, but not for gjc, which does not have epoch set. With epoch set we effectively enforce use of openjdk.

> This results in modifying files in the rpm source dir root, which is not okay.
No. It is modified in build root. 
a) I see nothing wrong on modifying files in buildroot. You can modify what you want there.
b) it is build root of binary package, not src.rpm. The build root is removed immediately after building package, so I see no way how it can affect other builds (on different platforms/archs).

Comment 4 Ville Skyttä 2011-05-24 19:56:54 UTC
(In reply to comment #3)
> With epoch set we effectively enforce use of openjdk.

In my package that was actually the intention.  But I have no problem if you want to support gcj and this stuff works with it.

> > This results in modifying files in the rpm source dir root, which is not okay.
> No. It is modified in build root. 

No it isn't, you're using "sed -i -e ... %{PATCH1}" which modifies Patch1 in-place in the source dir (the -i argument).

> a) I see nothing wrong on modifying files in buildroot. You can modify what you
> want there.

That's true.  But the file is not modified in the build root, it is modified in the _source dir_, and this is why a modified patch (already sed'd) ended up in your source rpm.

Comment 6 Ville Skyttä 2011-06-08 19:19:57 UTC
Looks otherwise fine, but the jnilibpath patch still has /usr/lib64/java-service-wrapper path hardwired (probably due to earlier in-place sed'ing) instead of @LIBPATH@, which quite likely means stuff is broken on non-lib64 archs:

$ rpmdev-extract -q java-service-wrapper-3.2.5-1.fc15.src.rpm
$ grep usr/lib64 java-service-wrapper-3.2.5-1.fc15.src/java-service-wrapper-3.2.4-jnilibpath.patch 
+            System.load( new File( "/usr/lib64/java-service-wrapper", file ).toString() );

Comment 8 Ville Skyttä 2011-07-30 10:20:38 UTC
Looks good now, thanks.

Comment 9 Miroslav Suchý 2011-08-01 08:20:24 UTC
New Package CVS Request
=======================
Package Name: java-service-wrapper
Short Description: Java service wrapper
Owners: msuchy
Branches: F-15, EL-5, EL-6
InitialCC:

Comment 10 Gwyn Ciesla 2011-08-01 09:59:05 UTC
Git done (by process-git-requests).

Comment 11 Miroslav Suchý 2011-08-01 13:57:32 UTC
Package Change Request
======================
Package Name: java-service-wrapper
New Branches: F-16
Owners: msuchy
InitialCC: 

Aghh, F-16 has been already branched! I missed that. So beside F15 and F17, I would need F16.

Comment 12 Gwyn Ciesla 2011-08-01 14:00:00 UTC
Git done (by process-git-requests).

Comment 13 Fedora Update System 2011-08-01 14:13:59 UTC
java-service-wrapper-3.2.5-2.fc15 has been submitted as an update for Fedora 15.
https://admin.fedoraproject.org/updates/java-service-wrapper-3.2.5-2.fc15

Comment 14 Fedora Update System 2011-08-01 14:18:56 UTC
java-service-wrapper-3.2.5-2.fc16 has been submitted as an update for Fedora 16.
https://admin.fedoraproject.org/updates/java-service-wrapper-3.2.5-2.fc16

Comment 15 Fedora Update System 2011-08-01 17:26:15 UTC
java-service-wrapper-3.2.5-3.el5 has been submitted as an update for Fedora EPEL 5.
https://admin.fedoraproject.org/updates/java-service-wrapper-3.2.5-3.el5

Comment 16 Fedora Update System 2011-08-01 17:38:38 UTC
java-service-wrapper-3.2.5-4.el6 has been submitted as an update for Fedora EPEL 6.
https://admin.fedoraproject.org/updates/java-service-wrapper-3.2.5-4.el6

Comment 17 Fedora Update System 2011-08-10 03:21:58 UTC
java-service-wrapper-3.2.5-2.fc15 has been pushed to the Fedora 15 stable repository.

Comment 18 Fedora Update System 2011-08-12 08:09:58 UTC
java-service-wrapper-3.2.5-5.el6 has been submitted as an update for Fedora EPEL 6.
https://admin.fedoraproject.org/updates/java-service-wrapper-3.2.5-5.el6

Comment 19 Fedora Update System 2011-08-12 08:11:03 UTC
java-service-wrapper-3.2.5-5.el5 has been submitted as an update for Fedora EPEL 5.
https://admin.fedoraproject.org/updates/java-service-wrapper-3.2.5-5.el5

Comment 20 Fedora Update System 2011-08-22 15:00:42 UTC
java-service-wrapper-3.2.5-2.fc16 has been pushed to the Fedora 16 stable repository.

Comment 21 Fedora Update System 2011-09-14 00:29:06 UTC
java-service-wrapper-3.2.5-5.el6 has been pushed to the Fedora EPEL 6 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 22 Fedora Update System 2011-09-14 00:29:27 UTC
java-service-wrapper-3.2.5-5.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.