Bug 787738 - Review Request: wss4j - Apache WS-Security implementation
Summary: Review Request: wss4j - Apache WS-Security implementation
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Garrett Holmstrom
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2012-02-06 16:06 UTC by Andy Grimm
Modified: 2016-11-08 03:46 UTC (History)
5 users (show)

Fixed In Version: wss4j-1.5.12-2.fc17
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2012-03-01 20:28:20 UTC
Type: ---
Embargoed:
gholms: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Description Andy Grimm 2012-02-06 16:06:15 UTC
Name        : wss4j
Version     : 1.5.12
License     : ASL 2.0
URL         : http://ws.apache.org/wss4j/
Summary     : Apache WS-Security implementation
Description :
The Apache WSS4J™ project provides a Java implementation of the
primary security standards for Web Services.


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

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

Comment 1 Garrett Holmstrom 2012-02-17 23:34:13 UTC
I presume you chose version 1.5 over 1.6 so you could patch out the opensaml dependency.  Any idea how long 1.5 will have upstream support?

The spec file has only a few minor issues:
- ™ must not appear in package descriptions
- Patch entries in the spec file need descriptive comments
- The java dep must be versioned per the java guidelines
- You need to add post and postun deps on jpackage-utils per the java guidelines

Just fix those and you should be good to go.  Note that you won't be able to build for EPEL 5 with this spec file if that matters to you.  An exhaustive review follows.

Mandatory review guidelines:
ok - rpmlint output (none)
ok - Package meets naming guidelines
ok - Spec file name matches base package name
ok - License is acceptable (ASL 2.0)
ok - License field in spec is correct
ok - License files included in package %docs or not included in upstream source
ok - License files installed when any subpackage combination is installed
ok - Spec written in American English
ok - Spec is legible
ok - Sources match upstream unless altered to fix permissibility issues
     Upstream MD5:  7f0029d960a140b5054a3c339259daac  wss4j-src-1.5.12.zip
     Your MD5:      7f0029d960a140b5054a3c339259daac  wss4j-src-1.5.12.zip
ok - Build succeeds on at least one supported platform
-- - Build succeeds on all supported platforms or has ExcludeArch + bugs filed
ok - BuildRequires correct
-- - Package handles locales with %find_lang
-- - %post, %postun call ldconfig if package contains shared .so files
ok - No bundled system libs
-- - Relocatability is justified
ok - Package owns all directories it creates
ok - Package requires other packages for directories it uses but does not own
ok - No duplicate files in %files unless necessary for license files
ok - File permissions are sane
-- - Each %files section contains %defattr on EL4
ok - Consistent use of macros
ok - Sources contain only permissible code or content
-- - Large documentation files go in -doc package
ok - Missing %doc files do not affect runtime
-- - Headers go in -devel package
-- - Static libs go in -static package
-- - Unversioned .so files go in -devel package
-- - Devel packages require base with fully-versioned dependency
ok - Package contains no .la files
-- - GUI app uses desktop-file-install/desktop-file-validate for .desktop files
-- - Package's files and directories don't conflict with others' or justified
ok - File names are valid UTF-8

Optional review guidelines:
-- - Query upstream about including license files
no - Translations of description, Summary
ok - Builds in mock
ok - Builds on all supported platforms
-- - Scriptlets are sane
-- - Non-devel subpackage Requires are sane
-- - .pc files go in -devel unless main package is a development tool
ok - No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin
-- - Man pages included for all executables
-- - Package with test-suite executes it in %check section

Packaging guidelines:
ok - Has dist tag
ok - Useful without external bits
ok - Package obeys FHS, except libexecdir, /usr/target, /run
ok - No files in /bin, /sbin, /lib* on >= F17
-- - Programs launched before FS mounting use /run instead of /var/run
-- - Binaries in /bin, /sbin do not depend on files in /usr on < F17
ok - Changelog in prescribed format
ok - Spec file lacks Packager, Vendor, PreReq tags
-- - Correct BuildRoot tag on < F10/EL6
     Builds will not work on EPEL 5.
-- - Correct %clean section on < F13/EL6
     Builds will not work on EPEL 5.
NO - Requires correct, justified where necessary
     Java guideline violation; see below
NO - Summary, description do not use trademarks incorrectly
     Remove ™ from the package description.
ok - All relevant documentation is packaged, tagged appropriately
ok - Documentation files do not have executable permissions
-- - %build honors applicable compiler flags or justifies otherwise
-- - Package with .pc files Requires pkgconfig on < EL6
-- - Useful -debuginfo package or disabled and justified
ok - No static executables
ok - Rpath absent or only used for internal libs
-- - Config files marked with %config
-- - %config files marked noreplace or justified
ok - No %config files under /usr
-- - Systemd units/init scripts are sane
-- - Spec uses macros instead of hard-coded directory names where appropriate
ok - Spec uses macros for executables only when configurability is needed
-- - %makeinstall used only when ``make install DESTDIR=...'' doesn't work
-- - Macros in Summary, %description expandable at SRPM build time
-- - Spec uses %{SOURCE#} instead of $RPM_SOURCE_DIR or %{sourcedir}
-- - %global instead of %define where appropriate
-- - Package containing translations BuildRequires gettext
ok - File timestamps preserved by file ops
-- - Parallel make
ok - Spec does not use Requires(pre,post) notation
-- - User, group creation handled correctly (See Packaging:UsersAndGroups)
-- - Web app files go in /usr/share/%{name}, not /var/www
-- - Conflicts are justified
ok - No external kernel modules
ok - No files in /srv, /opt, /usr/local
ok - One project per package
NO - Patches link to upstream bugs/comments/lists or are otherwise justified
     Please add patch descriptions to the spec file.
-- - Packages needing dirs in /var/run or /var/lock use tmpfiles.d on >= F15
-- - Renamed packages migrate from old packages correctly
-- - Programs that support IPv4 and IPv6 without functionality loss enable both

Java guidelines:
ok - Javadocs go in javadoc subpackage
ok - Prefer split JARs over monolithic
ok - JAR file names correct
ok - JAR files go in %{_javadir} or %{_javadir}-$version
-- - Multiple JAR files go in a %{name} subdirectory
ok - Javadocs go in unversioned %{_javadocdir}/%{name}
ok - javadoc subpackage is noarch on > EL5
ok - BuildRequires java-devel, jpackage-utils
NO - Requires java >= $version, jpackage-utils
     The Java guidelines require a versioned dependency on java.
NO - Dependencies on java/java-devel >= 1.6.0 add epoch 1
     Should be fixed with the above
NO - Package requiring maven Requires jpackage-utils for post and postun
ok - Package requiring maven contains correct maven-specific code in spec
-- - Wrapper script in %{_bindir}
-- - GCJ AOT bits follow GCJ guidelines
ok - No devel package
ok - pom.xml files, if any, installed with %add_maven_depmap
-- - JNI shared objects, JARs that require them go in %{_libdir}/%{name}
-- - Calls to System.loadLibrary replaced w/ System.load w/ full .so path
ok - Bundled JAR files not included or used for build
ok - No Javadoc %post/%ghost
ok - No class-path elements in JAR manifests

Comment 2 Andy Grimm 2012-02-20 14:52:26 UTC
> I presume you chose version 1.5 over 1.6 so you could patch out the opensaml
> dependency.  Any idea how long 1.5 will have upstream support?

Version 1.5.x uses OpenSAML 1.x, and Version 1.6.x uses OpenSAML 2.x,and while that doesn't impact this build, it probably impacts upgrade for some people and will prolong the life of 1.5.x.  There is still a "1_5_x-fixes" branch, which has commits as recent as December.  I expect that we'll move to 1.6 in Fedora 18, as we have time to package more dependencies.

> The spec file has only a few minor issues:
> - ™ must not appear in package descriptions

Fixed.

> - Patch entries in the spec file need descriptive comments

Comments added.

> - The java dep must be versioned per the java guidelines

I don't see this in the current java guidelines, and I suspect that it would only mislead people.  Use of Fedora 17 implies JDK 7, but compiling with an older JDK for an older distro is certainly valid.

> - You need to add post and postun deps on jpackage-utils per the java
guidelines

Again, current java guidelines don't include this; in fact, it's forbidden:
https://fedoraproject.org/wiki/Java_review_template

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

SRPM:
http://downloads.eucalyptus.com/devel/packages/fedora-17/sources/wss4j-1.5.12-2.fc17.src.rpm


Thanks.

Comment 3 Garrett Holmstrom 2012-02-21 23:19:24 UTC
(In reply to comment #2)
> > - The java dep must be versioned per the java guidelines
> 
> I don't see this in the current java guidelines, and I suspect that it would
> only mislead people.  Use of Fedora 17 implies JDK 7, but compiling with an
> older JDK for an older distro is certainly valid.

It's right here:  http://fedoraproject.org/wiki/Packaging:Java#BuildRequires_and_Requires

At this point it is probably unnecessary cruft, but right now it is still a MUST.  I suggest asking the rest of the Java SIG if this is okay to drop from the guidelines.  If it is then this package should be good to go; everything else looks fine.

> > - You need to add post and postun deps on jpackage-utils per the java
> guidelines
> 
> Again, current java guidelines don't include this; in fact, it's forbidden:
> https://fedoraproject.org/wiki/Java_review_template

Bah, the rule only applies to maven 2 now.  If you aren't going to add this to EPEL 6 then feel free to ignore it.


Mandatory review guidelines:
ok - rpmlint output (none)
ok - Package meets naming guidelines
ok - Spec file name matches base package name
ok - License is acceptable (ASL 2.0)
ok - License field in spec is correct
ok - License files included in package %docs or not included in upstream source
ok - License files installed when any subpackage combination is installed
ok - Spec written in American English
ok - Spec is legible
ok - Sources match upstream unless altered to fix permissibility issues
     Upstream MD5:  7f0029d960a140b5054a3c339259daac  wss4j-src-1.5.12.zip
     Your MD5:      7f0029d960a140b5054a3c339259daac  wss4j-src-1.5.12.zip
ok - Build succeeds on at least one supported platform
-- - Build succeeds on all supported platforms or has ExcludeArch + bugs filed
ok - BuildRequires correct
-- - Package handles locales with %find_lang
-- - %post, %postun call ldconfig if package contains shared .so files
ok - No bundled system libs
-- - Relocatability is justified
ok - Package owns all directories it creates
ok - Package requires other packages for directories it uses but does not own
ok - No duplicate files in %files unless necessary for license files
ok - File permissions are sane
-- - Each %files section contains %defattr on EL4
ok - Consistent use of macros
ok - Sources contain only permissible code or content
-- - Large documentation files go in -doc package
ok - Missing %doc files do not affect runtime
-- - Headers go in -devel package
-- - Static libs go in -static package
-- - Unversioned .so files go in -devel package
-- - Devel packages require base with fully-versioned dependency
ok - Package contains no .la files
-- - GUI app uses desktop-file-install/desktop-file-validate for .desktop files
-- - Package's files and directories don't conflict with others' or justified
ok - File names are valid UTF-8

Optional review guidelines:
-- - Query upstream about including license files
no - Translations of description, Summary
ok - Builds in mock
ok - Builds on all supported platforms
-- - Scriptlets are sane
-- - Non-devel subpackage Requires are sane
-- - .pc files go in -devel unless main package is a development tool
ok - No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin
-- - Man pages included for all executables
-- - Package with test-suite executes it in %check section

Packaging guidelines:
ok - Has dist tag
ok - Useful without external bits
ok - Package obeys FHS, except libexecdir, /usr/target, /run
ok - No files in /bin, /sbin, /lib* on >= F17
-- - Programs launched before FS mounting use /run instead of /var/run
-- - Binaries in /bin, /sbin do not depend on files in /usr on < F17
ok - Changelog in prescribed format
ok - Spec file lacks Packager, Vendor, PreReq tags
-- - Correct BuildRoot tag on < F10/EL6
     Builds will not work on EPEL 5.
-- - Correct %clean section on < F13/EL6
     Builds will not work on EPEL 5.
ok - Requires correct, justified where necessary
     Lack of maven2 support will prevent builds on EPEL 6.
ok - Summary, description do not use trademarks incorrectly
ok - All relevant documentation is packaged, tagged appropriately
ok - Documentation files do not have executable permissions
-- - %build honors applicable compiler flags or justifies otherwise
-- - Package with .pc files Requires pkgconfig on < EL6
-- - Useful -debuginfo package or disabled and justified
ok - No static executables
ok - Rpath absent or only used for internal libs
-- - Config files marked with %config
-- - %config files marked noreplace or justified
ok - No %config files under /usr
-- - Systemd units/init scripts are sane
-- - Spec uses macros instead of hard-coded directory names where appropriate
ok - Spec uses macros for executables only when configurability is needed
-- - %makeinstall used only when ``make install DESTDIR=...'' doesn't work
-- - Macros in Summary, %description expandable at SRPM build time
-- - Spec uses %{SOURCE#} instead of $RPM_SOURCE_DIR or %{sourcedir}
-- - %global instead of %define where appropriate
-- - Package containing translations BuildRequires gettext
ok - File timestamps preserved by file ops
-- - Parallel make
ok - Spec does not use Requires(pre,post) notation
-- - User, group creation handled correctly (See Packaging:UsersAndGroups)
-- - Web app files go in /usr/share/%{name}, not /var/www
-- - Conflicts are justified
ok - No external kernel modules
ok - No files in /srv, /opt, /usr/local
ok - One project per package
ok - Patches link to upstream bugs/comments/lists or are otherwise justified
-- - Packages needing dirs in /var/run or /var/lock use tmpfiles.d on >= F15
-- - Renamed packages migrate from old packages correctly
-- - Programs that support IPv4 and IPv6 without functionality loss enable both

Java guidelines:
ok - Javadocs go in javadoc subpackage
ok - Prefer split JARs over monolithic
ok - JAR file names correct
ok - JAR files go in %{_javadir} or %{_javadir}-$version
-- - Multiple JAR files go in a %{name} subdirectory
ok - Javadocs go in unversioned %{_javadocdir}/%{name}
ok - javadoc subpackage is noarch on > EL5
ok - BuildRequires java-devel, jpackage-utils
NO - Requires java >= $version, jpackage-utils
     Check with the rest of the Java SIG if the version is okay to omit.
-- - Dependencies on java/java-devel >= 1.6.0 add epoch 1
-- - Package requiring maven2 Requires jpackage-utils for post and postun
ok - Package requiring maven contains correct maven-specific code in spec
-- - Wrapper script in %{_bindir}
-- - GCJ AOT bits follow GCJ guidelines
ok - No devel package
ok - pom.xml files, if any, installed with %add_maven_depmap
-- - JNI shared objects, JARs that require them go in %{_libdir}/%{name}
-- - Calls to System.loadLibrary replaced w/ System.load w/ full .so path
ok - Bundled JAR files not included or used for build
ok - No Javadoc %post/%ghost
ok - No class-path elements in JAR manifests

Comment 4 Alexander Kurtakov 2012-02-22 17:57:06 UTC
Requires: java can be without a version unless you know that the package requires minimal java version aka 1.6.0 or 1.7.0 even in which case you should add the version.

Comment 5 Garrett Holmstrom 2012-02-22 18:27:07 UTC
This seems to also be the case for several other recently-approved packages, so that's good enough for me.  The Java guidelines could use an update to reflect reality.  A proposal for that is here:  http://lists.fedoraproject.org/pipermail/java-devel/2012-February/004355.html

Comment 6 Andy Grimm 2012-02-22 19:14:40 UTC
New Package SCM Request
=======================
Package Name: wss4j
Short Description: Apache WS-Security implementation
Owners: arg
Branches: f17

Comment 7 Gwyn Ciesla 2012-02-22 19:56:55 UTC
Git done (by process-git-requests).

Comment 8 Fedora Update System 2012-03-16 01:04:41 UTC
wss4j-1.5.12-2.fc17 has been submitted as an update for Fedora 17.
https://admin.fedoraproject.org/updates/wss4j-1.5.12-2.fc17

Comment 9 Fedora Update System 2012-04-12 01:55:10 UTC
wss4j-1.5.12-2.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.