Bug 589504 - Review Request: apache-commons-compress - Java API for working with tar, zip and bzip2 files
Summary: Review Request: apache-commons-compress - Java API for working with tar, zip ...
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Nobody's working on this, feel free to take it
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
: 589502 (view as bug list)
Depends On:
Blocks: JakartaCommonsRename
TreeView+ depends on / blocked
 
Reported: 2010-05-06 10:34 UTC by Sandro Mathys
Modified: 2010-05-12 12:21 UTC (History)
5 users (show)

Fixed In Version: apache-commons-compress-1.0-4.fc13
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2010-05-11 19:38:54 UTC
Type: ---
Embargoed:
akurtako: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Sandro Mathys 2010-05-06 10:34:10 UTC
Spec URL: http://red.fedorapeople.org/SRPMS/apache-commons-compress.spec
SRPM URL: http://red.fedorapeople.org/SRPMS/apache-commons-compress-1.0-2.fc12.src.rpm
Description:
The code in this component came from Avalon's Excalibur, but originally
from Ant, as far as life in Apache goes. The tar package is originally
Tim Endres' public domain package. The bzip2 package is based on the
work done by Keiron Liddle. It has migrated via:
Ant -> Avalon-Excalibur -> Commons-IO -> Commons-Compress.

Note: this is a re-review request for a package rename as described here:
http://fedoraproject.org/wiki/Package_Renaming_Process

The old package name was jakarta-commons-compress but was changed upstream to apache-commons-compress. To see the difference between this and the old spec file more easily I created a diff:
http://red.fedorapeople.org/SRPMS/apache-commons-compress.diff

As you can see nothing changed that's not necessary for the rename. Therefore this review should be easy.

Let me also provide the rpmlint output:
$ rpmlint {SPECS,RPMS/noarch,SRPMS}/apache-commons-compress*
apache-commons-compress.noarch: W: spelling-error Summary(en_US) bzip -> zip, blip, b zip
apache-commons-compress.noarch: W: spelling-error %description -l en_US bzip -> zip, blip, b zip
apache-commons-compress-javadoc.noarch: W: spelling-error Summary(en_US) Javadocs -> Java docs, Java-docs, Javanese
apache-commons-compress.src: W: spelling-error Summary(en_US) bzip -> zip, blip, b zip
apache-commons-compress.src: W: spelling-error %description -l en_US bzip -> zip, blip, b zip
3 packages and 1 specfiles checked; 0 errors, 5 warnings.

I think all those warnings can be ignored as they're about technical terms only.

Comment 1 Alexander Kurtakov 2010-05-06 11:05:38 UTC
Hmm, why are there 2 bugs for this?
See https://bugzilla.redhat.com/show_bug.cgi?id=589502

Comment 2 Sandro Mathys 2010-05-06 11:11:00 UTC
*** Bug 589502 has been marked as a duplicate of this bug. ***

Comment 3 Sandro Mathys 2010-05-06 11:28:17 UTC
Because bugzilla seems to be a bit unstable today (I get proxy and mysql errors all day long) and While retrying to submit this review I accidentally created two bugs. Closed one as duplicate now.

Comment 4 Sandro Mathys 2010-05-06 11:31:00 UTC
Awww, sorry...blocked the wrong bug :/ Bugzilla's errors are confusing me a little :/

Comment 5 Alexander Kurtakov 2010-05-06 12:46:57 UTC
I would really like to see this package build with maven2 as it is supposed to be done by upstream developers. 
All the additional maven integration work should be done too. 
We need this so any maven project depending on commons-compress can find it without further work from the packager. Additionally building with maven should make the jar a valid OSGi bundle suitable to be used in Eclipse and other OSGi containers.

Comment 6 Sandro Mathys 2010-05-06 13:51:08 UTC
Spec URL: http://red.fedorapeople.org/SRPMS/apache-commons-compress.spec
SRPM URL:
http://red.fedorapeople.org/SRPMS/apache-commons-compress-1.0-3.fc12.src.rpm

I never used maven2 in a package before but by looking at another spec file I was able to come up with something that seems to work. Not sure if it's normal that pretty some checksums fail, tho.

Also, rpmlint now shows a new warning but I think that can be ignored:
apache-commons-compress.noarch: W: non-conffile-in-etc /etc/maven/fragments/apache-commons-compress

Oh, that OSGi stuff is done automatically, isn't it? Or do I need to cp/install something?

Comment 7 Alexander Kurtakov 2010-05-06 14:12:03 UTC
Thanks,
I'm taking this one.

Maven2 part looks good.
Yes the OSGi stuff are just additional headers added automatically to the MANIFEST.MF file when you built with maven.

Comment 8 Peter Lemenkov 2010-05-07 09:59:42 UTC
Hello All!
I'm not a java-expert, but I'm in doubts whether *-javadoc part shold *require* main package. Could someone clarify this - is *-javadoc sub-package useless w/o main package?

Comment 9 Sandro Mathys 2010-05-07 10:19:31 UTC
Hi Peter, thanks for our input.

I agree that it's probably not technically necessary. But the template from the guidelines do it as well and so I applied the same:
https://fedoraproject.org/wiki/Packaging/Java#maven_2

The guidelines itself don't seem to mention it, so I think I could remove it. Does someone know more about that?

Comment 10 Alexander Kurtakov 2010-05-10 09:25:25 UTC
(In reply to comment #9)
> Hi Peter, thanks for our input.
> 
> I agree that it's probably not technically necessary. But the template from the
> guidelines do it as well and so I applied the same:
> https://fedoraproject.org/wiki/Packaging/Java#maven_2
> 
> The guidelines itself don't seem to mention it, so I think I could remove it.
> Does someone know more about that?    

Javadoc packages are perfectly usable without the main package. They are a bunch of html files after all. As this is not mandatory according to the guidelines it should be up to the packager to decide.

Comment 11 Alexander Kurtakov 2010-05-10 10:19:23 UTC
Review:

OK: rpmlint must be run on every package. OUTPUT:

apache-commons-compress.noarch: W: spelling-error Summary(en_US) bzip -> zip, blip, b zip
apache-commons-compress.noarch: W: spelling-error %description -l en_US bzip -> zip, blip, b zip
apache-commons-compress.noarch: W: non-conffile-in-etc /etc/maven/fragments/apache-commons-compress
apache-commons-compress-javadoc.noarch: W: spelling-error Summary(en_US) Javadocs -> Java docs, Java-docs, Javanese


Not a problem. 

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. 
OK: The sources used to build the package must match the upstream source, as
provided in the spec URL.
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, except for any that
are listed in the exceptions section of the Packaging Guidelines ; inclusion of
those as BuildRequires is optional. Apply common sense.
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 consistently use macros.
OK: The package must contain code, or permissable content. 
OK: Large documentation files must go in a -doc subpackage. Javadocs
subpackage.
OK: If a package includes something as %doc, it must not affect the runtime of
the application. 
OK: Packages must not own files or directories already owned by other packages. 
OK: All filenames in rpm packages must be valid UTF-8. 

Other problems:
* You should not install in %build section. Please just remove the "install -d $MAVEN_REPO_LOCAL" line.
* All your symlinks are broken "ln -s %{name}-%{version}.jar" should be "ln -s %{shortname}-%{version}.jar
* You should not use %define but %global. See http://fedoraproject.org/wiki/Packaging:Guidelines#.25global_preferred_over_.25define
* You can make use of the %{name} macro on a few places (if you want to) - Javadoc subpackage Summary and Description.

Comment 12 Sandro Mathys 2010-05-10 10:37:14 UTC
Spec URL: http://red.fedorapeople.org/SRPMS/apache-commons-compress.spec
SRPM URL: http://red.fedorapeople.org/SRPMS/apache-commons-compress-1.0-4.fc12.src.rpm

Thanks for the review Alexander!

(In reply to comment #11)
> Other problems:
> * You should not install in %build section. Please just remove the "install -d
> $MAVEN_REPO_LOCAL" line.

This line looks important to me. Also, I see no reason not to install something into the unpacked sources if necessary for the build. After all, mvn-jpp will write to that dir as well. Also, the maven template from the guidelines does the same (with mkdir instead of install):
http://fedoraproject.org/wiki/Packaging:Java#maven_2

If you have a reason to believe mkdir would fit better I'm willing to replace the install by that, tho.

> * All your symlinks are broken "ln -s %{name}-%{version}.jar" should be "ln -s
> %{shortname}-%{version}.jar

Awww, very good catch! Should have paid more attention as I changed that symlinking part a bit recently :/ FIXED.

> * You should not use %define but %global. See
> http://fedoraproject.org/wiki/Packaging:Guidelines#.25global_preferred_over_.25define

Right, forgot to check that for the new renamed version - and the jakarta-* thingy is older than this rule :) FIXED.

> * You can make use of the %{name} macro on a few places (if you want to) -
> Javadoc subpackage Summary and Description.    

True, FIXED.

Comment 13 Alexander Kurtakov 2010-05-10 11:26:35 UTC
(In reply to comment #12)
> Spec URL: http://red.fedorapeople.org/SRPMS/apache-commons-compress.spec
> SRPM URL:
> http://red.fedorapeople.org/SRPMS/apache-commons-compress-1.0-4.fc12.src.rpm
> 
> Thanks for the review Alexander!
> 
> (In reply to comment #11)
> > Other problems:
> > * You should not install in %build section. Please just remove the "install -d
> > $MAVEN_REPO_LOCAL" line.
> 
> This line looks important to me. Also, I see no reason not to install something
> into the unpacked sources if necessary for the build. After all, mvn-jpp will
> write to that dir as well. Also, the maven template from the guidelines does
> the same (with mkdir instead of install):
> http://fedoraproject.org/wiki/Packaging:Java#maven_2
> 
> If you have a reason to believe mkdir would fit better I'm willing to replace
> the install by that, tho.

Well, using install in %build section can make someone believe that you are putting smth in the buildroot, which you are certainly not doing. Using mkdir is preferred to not confuse people.

> 
> > * All your symlinks are broken "ln -s %{name}-%{version}.jar" should be "ln -s
> > %{shortname}-%{version}.jar
> 
> Awww, very good catch! Should have paid more attention as I changed that
> symlinking part a bit recently :/ FIXED.
> 
> > * You should not use %define but %global. See
> > http://fedoraproject.org/wiki/Packaging:Guidelines#.25global_preferred_over_.25define
> 
> Right, forgot to check that for the new renamed version - and the jakarta-*
> thingy is older than this rule :) FIXED.
> 
> > * You can make use of the %{name} macro on a few places (if you want to) -
> > Javadoc subpackage Summary and Description.    
> 
> True, FIXED.    

Thanks,
This package is APPROVED.

Comment 14 Sandro Mathys 2010-05-10 11:32:52 UTC
Okay, will change this to mkdir instead of install before I add it to cvs. Thanks again for the review, Alexander.

New Package CVS Request
=======================
Package Name: apache-commons-compress
Short Description: Java API for working with tar, zip and bzip2 files
Owners: red
Branches: F-12 F13
InitialCC:

Comment 15 Kevin Fenzi 2010-05-11 04:44:02 UTC
CVS done (by process-cvs-requests.py).

Comment 16 Fedora Update System 2010-05-11 09:40:38 UTC
apache-commons-compress-1.0-4.fc12 has been submitted as an update for Fedora 12.
http://admin.fedoraproject.org/updates/apache-commons-compress-1.0-4.fc12

Comment 17 Fedora Update System 2010-05-11 09:41:20 UTC
apache-commons-compress-1.0-4.fc13 has been submitted as an update for Fedora 13.
http://admin.fedoraproject.org/updates/apache-commons-compress-1.0-4.fc13

Comment 18 Fedora Update System 2010-05-11 19:38:47 UTC
apache-commons-compress-1.0-4.fc12 has been pushed to the Fedora 12 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 19 Fedora Update System 2010-05-12 12:21:35 UTC
apache-commons-compress-1.0-4.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.