Bug 794715 - Review Request: apache-commons-ognl - Object Graph Navigation Library
Summary: Review Request: apache-commons-ognl - Object Graph Navigation Library
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Juan Hernández
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On: 803241
Blocks:
TreeView+ depends on / blocked
 
Reported: 2012-02-17 11:40 UTC by Andy Grimm
Modified: 2016-11-08 03:46 UTC (History)
6 users (show)

Fixed In Version: apache-commons-ognl-3.0.2-1.20120313svn1102435.fc17
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2012-04-12 03:07:20 UTC
Type: ---
Embargoed:
juan.hernandez: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Description Andy Grimm 2012-02-17 11:40:28 UTC
Name        : ognl
Version     : 3.0.4
License     : ASL 2.0
URL         : https://github.com/jkuhnert/ognl
Summary     : Object Graph Navigation Library
Description :
OGNL is an expression language for getting and setting properties of
Java objects, plus other extras such as list projection and selection
and lambda expressions.


SPEC:
http://downloads.eucalyptus.com/devel/packages/fedora-17/SPECS/ognl.spec

SRPM:
http://downloads.eucalyptus.com/devel/packages/fedora-17/sources/ognl-3.0.4-1.fc17.src.rpm


NOTE:  I'm actually a bit uncertain about how to deal with the license in this package.  Currently there is a discrepancy because the 3.0.x versions come from github (there are no tagged versions in the ASF repository yet) and still have an OpenSymphony (ASL 1.x-style) license, but OpenSymphony closed down last summer, and the code was donated to the ASF.

The license was updated in the ASF svn repo in May of 2011:

$ svn log -r 1102049
------------------------------------------------------------------------
r1102049 | simonetripodi | 2011-05-11 15:36:51 -0400 (Wed, 11 May 2011) | 1 line

updated LICENSE to ASF License 2.0
------------------------------------------------------------------------

I investigated moving to the ognl from apache commons, but they have made significant changes and still appear to be in the midst of a major release cycle, so sticking with 3.0.x seems prudent for now.

Comment 1 Alexander Kurtakov 2012-02-17 12:48:44 UTC
The license field in the rpm should match the content. In this case if the sources you build from are with the OpenSymphony license this is the license. You should also consider legal about opensymphony license because sometimes adding a single word to license can make it non-free anymore.
So for the package as is the license can not be ASL 2.0 unless this is what the sources state for.

Blocking FE-LEGAL to get their opinion on the OpenSymphony license.

Comment 2 Andy Grimm 2012-02-17 13:01:18 UTC
I understand your point.  One alternative here would be to take a subversion revision from apache.org after they updated the license.  I've already investigated this possibility.  I found a revision in the ASF repo from May where the license has been updated to ASL 2.0 and all of the packages have been moved into the org.apache.commons namespace.  They had already updated the version to "4.0-SNAPSHOT" at that point, but the code is essentially ognl-3.0.2.  I'm happy to package this instead, as it would simplify the legal situation.  I'm not interested in make spot / Fedora Legal review a license for a dead company.

Comment 3 Alexander Kurtakov 2012-02-17 13:04:56 UTC
Andy, 
This sounds like a solution. Once you upload the new package we can drop the block.

Comment 4 Andy Grimm 2012-02-17 20:36:12 UTC
Name        : commons-ognl
Version     : 3.0.2
License     : ASL 2.0
URL         : http://commons.apache.org/ognl/
Summary     : Object Graph Navigation Library
Description :
OGNL is an expression language for getting and setting properties of
Java objects, plus other extras such as list projection and selection
and lambda expressions.

SPEC:
http://downloads.eucalyptus.com/devel/packages/fedora-17/SPECS/commons-ognl.spec

SRPM:
http://downloads.eucalyptus.com/devel/packages/fedora-17/sources/commons-ognl-3.0.2-1.fc17.src.rpm

Again, note that I've versioned this package as "3.0.2" because it is essentially the 3.0.2 code with an ASL 2.0 license and a move to the org.apache.commons namespace.  I think that calling it by the in-tree version at the time, i.e. "4.0-incubating-SNAPSHOT", would be less clear.  I'm open to discussion on this, though.

Comment 5 Alexander Kurtakov 2012-03-13 20:04:31 UTC
I'm dropping the FE-LEGAL block as the codebase seems to be properly licensed with already approved license we no longer need the legal team opinion.

FWIW, all other apache commons packages are named apache-commons-smth so it would be good if this one follows the same name scheme at least for consistency.
About versioning the guidelines are pretty clear see https://fedoraproject.org/wiki/Packaging:NamingGuidelines#Snapshot_packages

Comment 6 Juan Hernández 2012-03-13 20:07:16 UTC
I am taking this for review.

Comment 7 Juan Hernández 2012-03-13 20:53:41 UTC
Package Review
==============

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

=== REQUIRED ITEMS ===
[!]  Rpmlint output:

$ rpmlint commons-ognl-3.0.2-1.fc17.src.rpm
commons-ognl.src: W: invalid-url Source0: commons-ognl-3.0.2.tar.xz
1 packages and 0 specfiles checked; 0 errors, 1 warnings.

Can't check binary packages as build fails.

[!]  Package is named according to the Package Naming Guidelines[1].

As pointed out by Alexander Kurtakov other packages from Apache commons are named apache-commons-whatever, so this one should be apache-commons-ognl.

Being a post release the version and release tags should be:

3.0.2-1.20120313svn1102435

[x]  Spec file name must match the base package name, in the format %{name}.spec.
[x]  Package meets the Packaging Guidelines[2].
[!]  Package successfully compiles and builds into binary rpms.

See http://koji.fedoraproject.org/koji/taskinfo?taskID=3891615. The relevant error is the following in build.log:

The repository system is offline but the artifact net.java.dev.jna:jna:jar:3.2.2 is not available in the local repository.

This can be resolved adding "BuildRequires: jna".

[x]  Buildroot definition is not present
[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: ASL 2.0
[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    : 33b6d3507a5fd8e952c6d156b891d022
MD5SUM upstream package: ee9905892d509ef4bf2cec826eaa2fa3

Compared the sources in this package with the upstream sources using diff and there is no difference.

[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 or must require other packages for directories it uses.
[x]  Package does not contain duplicates in %files.
[x]  File sections do not contain %defattr(-,root,root,-) unless changed with good reason
[x]  Permissions on files are set properly.
[x]  Package does NOT have a %clean section which contains rm -rf %{buildroot} (or $RPM_BUILD_ROOT). (not needed anymore)
[x]  Package consistently uses macros (no %{buildroot} and $RPM_BUILD_ROOT mixing)
[x]  Package contains code, or permissable content.
[-]  Fully versioned dependency in subpackages, if present.
[-]  Package contains a properly installed %{name}.desktop file if it is a GUI application.
[-]  Package does not own files or directories owned by other packages.
[x]  Javadoc documentation files are generated and included in -javadoc subpackage
[x]  Javadocs are placed in %{_javadocdir}/%{name} (no -%{version} symlinks)
[x]  Packages have proper BuildRequires/Requires on jpackage-utils
[x]  Javadoc subpackages have Require: jpackage-utils
[x]  Package uses %global not %define
[x]  If package uses tarball from VCS include comment how to re-create that tarball (svn export URL, git clone URL, ...)
[-]  If source tarball includes bundled jar/class files these need to be removed prior to building
[-]  All filenames in rpm packages must be valid UTF-8.
[x]  Jar files are installed to %{_javadir}/%{name}.jar (see [6] for details)
[x]  If package contains pom.xml files install it (including depmaps) even when building with ant
[x]  pom files has correct add_maven_depmap

=== Maven ===
[x]  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 "-Dmaven.local.depmap.file=*" explain why it's needed in a comment
[x]  Package DOES NOT use %update_maven_depmap in %post/%postun
[x]  Packages DOES NOT have Requires(post) and Requires(postun) on jpackage-utils for %update_maven_depmap macro

=== Other suggestions ===
[x]  If possible use upstream build method (maven/ant/javac)
[x]  Avoid having BuildRequires on exact NVR unless necessary
[x]  Package has BuildArch: noarch (if possible)
[!]  Latest version is packaged.

Version in the package is revision 1239219 and latest in the upstream repository is 1102435.

[!]  Reviewer should test that the package builds in mock.

Tested on: http://koji.fedoraproject.org/koji/taskinfo?taskID=3891615

=== Issues ===
1. The rpmlint warning about the URL is acceptable.
2. The name should be "apache-commons-ognl".
3. The "version-release" tags should be "3.0.2-1.20120313svn1102435".
4. The package does not build in Koji, it is missing "BuildRequires: jna".
5. This is not the latest version, but it has been already discusssed and agreed in the bug that it is acceptable and correct to package this version.

=== Final Notes ===
1. Issues #2, #3 and #4 need to be fixed. Once that is done I will continue the review.

Comment 9 Juan Hernández 2012-03-14 09:55:02 UTC
The package doesn't build in Koji. I think this is due to a bug in JNA.

Comment 10 Juan Hernández 2012-03-14 10:07:47 UTC
I was wrong regarding the jna dependency. It is not required as it is a transitive dependency. The problem is that there is currently an issue with the dependencies map in jna. See bug #803241.

This package looks nice now. I will approve once bug #803241 is fixed and I can build in Koji.

Comment 11 Juan Hernández 2012-03-14 13:17:56 UTC
Now that the issue in jna has been addressed the package builds correctly in Koji:

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

The binary packages are fine as well:

$ rpmlint apache-commons-ognl-3.0.2-1.20120313svn1102435.fc18.noarch.rpm apache-commons-ognl-javadoc-3.0.2-1.20120313svn1102435.fc18.noarch.rpm
2 packages and 0 specfiles checked; 0 errors, 0 warnings.http://koji.fedoraproject.org/koji/taskinfo?taskID=3893599

All the issues have been addressed, and #4 was in fact not needed. You may want to double check that and remove it before committing.

================
*** APPROVED ***
================

Comment 12 Andy Grimm 2012-03-14 14:49:34 UTC
New Package SCM Request
=======================
Package Name: apache-commons-ognl
Short Description: Object Graph Navigation Library
Owners: arg
Branches: f17
InitialCC:

Comment 13 Gwyn Ciesla 2012-03-14 15:05:35 UTC
Git done (by process-git-requests).

Comment 14 Fedora Update System 2012-03-14 18:59:26 UTC
apache-commons-ognl-3.0.2-1.20120313svn1102435.fc17 has been submitted as an update for Fedora 17.
https://admin.fedoraproject.org/updates/apache-commons-ognl-3.0.2-1.20120313svn1102435.fc17

Comment 15 Fedora Update System 2012-03-16 02:39:41 UTC
apache-commons-ognl-3.0.2-1.20120313svn1102435.fc17 has been pushed to the Fedora 17 testing repository.

Comment 16 Fedora Update System 2012-04-12 03:07:20 UTC
apache-commons-ognl-3.0.2-1.20120313svn1102435.fc17 has been pushed to the Fedora 17 stable repository.


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