Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
Summary: Review Request: p11-kit - Library for loading and sharing PKCS#11 modules
https://bugzilla.redhat.com/show_bug.cgi?id=725905
Summary: Review Request: p11-kit - Library for loading and sharing PKCS#11 modules Product: Fedora Version: rawhide Platform: All OS/Version: Linux Status: NEW Severity: medium Priority: medium Component: Package Review AssignedTo: nobody@fedoraproject.org ReportedBy: kalevlember@gmail.com QAContact: extras-qa@fedoraproject.org CC: notting@redhat.com, package-review@lists.fedoraproject.org Classification: Fedora Story Points: --- Type: ---
Spec URL: http://kalev.fedorapeople.org/p11-kit.spec SRPM URL: http://kalev.fedorapeople.org/p11-kit-0.2-1.fc15.src.rpm Description: p11-kit provides a way to load and enumerate PKCS#11 modules, as well as a standard configuration setup for installing PKCS#11 modules in such a way that they're discoverable.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=725905
--- Comment #1 from Kalev Lember kalevlember@gmail.com 2011-07-26 18:38:15 EDT --- Koji scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=3232586
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=725905
Kalev Lember kalevlember@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |stefw@collabora.co.uk
--- Comment #2 from Kalev Lember kalevlember@gmail.com 2011-07-26 18:45:43 EDT --- Stef, by what name are applications supposed to look up the library if they want to use it as a PKCS11 proxy module? Should we be installing the libp11-kit.so symlink in the main package so that applications could dlopen() it using that name?
Also, are you aware that using Apache License 2.0 code makes the whole library incompatible with GPLv2-only code?
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=725905
--- Comment #3 from Stef Walter stefw@collabora.co.uk 2011-07-27 03:28:47 EDT --- (In reply to comment #2)
Stef, by what name are applications supposed to look up the library if they want to use it as a PKCS11 proxy module? Should we be installing the libp11-kit.so symlink in the main package so that applications could dlopen() it using that name?
That sounds like a good plan. I look at putting this into p11-kit upstream.
Also, are you aware that using Apache License 2.0 code makes the whole library incompatible with GPLv2-only code?
Hmmm, that's true. I'll rewrite the bits that came from apr, and then we can get rid of this issue.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=725905
--- Comment #4 from Stef Walter stefw@collabora.co.uk 2011-07-27 05:33:10 EDT --- Fixed apache license issue in p11-kit master.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=725905
--- Comment #5 from Stef Walter stefw@collabora.co.uk 2011-07-27 06:15:42 EDT --- Added the symlink.
Kalev, could you please verify that this fixes the problems? Once you let me know, I'll package a new release.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=725905
--- Comment #6 from Kalev Lember kalevlember@gmail.com 2011-07-27 08:56:03 EDT --- +# Proxy module is actually same as library, so install a link +install-exec-hook: + $(LN_S) libp11-kit.so $(DESTDIR)$(libdir)/p11-kit-proxy.so
I think it would be better to create the symlink to libp11-kit.so.0 (note the .0), so that we could put the .so in -devel subpackage: p11-kit rpm: p11-kit-proxy.so libp11-kit.so.0 libp11-kit.so.0.0.0
p11-kit-devel rpm: libp11-kit.so
Otherwise looks very nice. Thanks for doing this, Stef!
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=725905
--- Comment #7 from Stef Walter stefw@collabora.co.uk 2011-07-27 09:43:39 EDT --- Could you provide a patch to do this reliably even when the libtool versioning gets updated?
As far as I can tell libtool versioning is calculated within libtool, and I'm not sure how to access it from the makefile.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=725905
--- Comment #8 from Kalev Lember kalevlember@gmail.com 2011-07-27 17:21:32 EDT --- I am not sure how to best do it either; anything I can come up with seems needlessly fragile. The following with readlink works at least:
install-exec-hook: - $(LN_S) libp11-kit.so $(DESTDIR)$(libdir)/p11-kit-proxy.so + $(LN_S) `readlink $(DESTDIR)$(libdir)/libp11-kit.so` $(DESTDIR)$(libdir)/p11-kit-proxy.so
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=725905
--- Comment #9 from Stef Walter stefw@collabora.co.uk 2011-07-28 08:36:47 EDT --- Good plan. Done.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=725905
--- Comment #10 from Kalev Lember kalevlember@gmail.com 2011-07-29 11:43:10 EDT --- * Fri Jul 29 2011 Kalev Lember kalevlember@gmail.com - 0.3-1 - Update to 0.3 - Upstream rewrote the ASL 2.0 bits, which makes the whole package BSD-licensed
Spec URL: http://kalev.fedorapeople.org/p11-kit.spec SRPM URL: http://kalev.fedorapeople.org/p11-kit-0.3-1.fc15.src.rpm
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=725905
Tomas Mraz tmraz@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED CC| |tmraz@redhat.com AssignedTo|nobody@fedoraproject.org |tmraz@redhat.com
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=725905
Tomas Mraz tmraz@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag| |fedora-review?
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=725905
Tomas Mraz tmraz@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag| |needinfo?(kalevlember@gmail | |.com)
--- Comment #11 from Tomas Mraz tmraz@redhat.com 2011-08-17 09:19:12 EDT --- There is a problem that the -devel subpackage puts doc files in /usr/share/gtk-doc/html but this directory is not owned by anything that is required by p11-kit-devel.
Also parts of the documentation document the configuration file format and that should be perhaps documented somehow in the main package, or even some kind of example config files included in the main package.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=725905
Kalev Lember kalevlember@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag|needinfo?(kalevlember@gmail | |.com) |
--- Comment #12 from Kalev Lember kalevlember@gmail.com 2011-08-17 09:47:52 EDT --- (In reply to comment #11)
There is a problem that the -devel subpackage puts doc files in /usr/share/gtk-doc/html but this directory is not owned by anything that is required by p11-kit-devel.
That directory is owned by the p11-kit-devel package itself to avoid a runtime dependency on the gtk-doc package: $ rpm -qlp p11-kit-devel-0.3-1.fc15.x86_64.rpm | grep gtk-doc /usr/share/gtk-doc /usr/share/gtk-doc/html /usr/share/gtk-doc/html/p11-kit /usr/share/gtk-doc/html/p11-kit/api-index-full.html /usr/share/gtk-doc/html/p11-kit/config-format.html ...
See the section in the packaging guidelines on File and Directory Ownership [1] that specifically explains how to deal with gtk-doc directory ownership: https://fedoraproject.org/wiki/Packaging/Guidelines#The_directory_is_owned_b...
Also parts of the documentation document the configuration file format and that should be perhaps documented somehow in the main package, or even some kind of example config files included in the main package.
I don't really want to put large html documentation in the main package. p11-kit is probably going to be included in the live CD and it's nice to keep the size down. However, instead of having the docs in the -devel package, it would be easy to split them out to a separate -doc package. In the future, when upstream has some example config files, we could also put these in the -doc subpackage. Would that be better than shipping all the docs in -devel?
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=725905
Tomas Mraz tmraz@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag|fedora-review? |fedora-review+
--- Comment #13 from Tomas Mraz tmraz@redhat.com 2011-08-17 10:48:56 EDT --- You're right, I somehow overlooked that in the file list.
As for the documentation - I don't think splitting it to a separate -doc subpackage is a solution. The configuration file format is really simple and the documentation or example would be pretty minimal and doing a separate subpackage for it would be an overkill.
I'd really like to see the global and example module config file directly in the main package. And/or manual page for the p11-kit binary. However I will not block the package on this.
rpmlint output: rpmlint -v SRPMS/p11-kit-0.3-1.fc14.src.rpm RPMS/x86_64/p11-kit* p11-kit.src: I: checking p11-kit.src: I: checking-url http://p11-glue.freedesktop.org/p11-kit.html (timeout 10 seconds) p11-kit.src: I: checking-url http://p11-glue.freedesktop.org/releases/p11-kit-0.3.tar.gz (timeout 10 seconds) p11-kit-debuginfo.x86_64: I: checking p11-kit-debuginfo.x86_64: I: checking-url http://p11-glue.freedesktop.org/p11-kit.html (timeout 10 seconds) p11-kit-devel.x86_64: I: checking p11-kit-devel.x86_64: I: checking-url http://p11-glue.freedesktop.org/p11-kit.html (timeout 10 seconds) p11-kit.x86_64: I: checking p11-kit.x86_64: I: checking-url http://p11-glue.freedesktop.org/p11-kit.html (timeout 10 seconds) p11-kit.x86_64: W: no-manual-page-for-binary p11-kit 4 packages and 0 specfiles checked; 0 errors, 1 warnings.
A manual page for the p11-kit would be nice however this is not a blocker.
The package is named according to the naming guidelines. The package meets licensing and packaging guidelines. The source tarball is the same as the upstream tarball.
Note for future - there will have to be probably added gettext if the localization of p11-kit is completed and configure also searches for the gtk-doc binaries. But they are both not needed to build the package now.
The requires on the base package should be changed to Requires: %{name}%{?_isa} = %{version}-%{release} But this is again a nitpick, but please change it before the import.
The package is ACCEPTed.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=725905
--- Comment #14 from Kalev Lember kalevlember@gmail.com 2011-08-17 11:00:15 EDT --- (In reply to comment #13)
The requires on the base package should be changed to Requires: %{name}%{?_isa} = %{version}-%{release} But this is again a nitpick, but please change it before the import.
Will do. Thanks for the review, Tomáš!
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=725905
Kalev Lember kalevlember@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag| |fedora-cvs?
--- Comment #15 from Kalev Lember kalevlember@gmail.com 2011-08-17 11:01:56 EDT --- New Package SCM Request ======================= Package Name: p11-kit Short Description: Library for loading and sharing PKCS#11 modules Owners: kalev mclasen Branches: InitialCC:
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=725905
--- Comment #16 from Tomas Mraz tmraz@redhat.com 2011-08-17 11:26:30 EDT --- Can you please add me as comaintainer?
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=725905
--- Comment #17 from Jon Ciesla limb@jcomserv.net 2011-08-17 11:28:49 EDT --- Git done (by process-git-requests).
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=725905
--- Comment #18 from Kalev Lember kalevlember@gmail.com 2011-08-17 11:38:17 EDT --- Grr, I forgot to request the f16 branch. Tomas, I think you'll need to request the ACLs for the master branch in pkgdb, but the SCM admin request should cover the ACLs in f16.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=725905
Kalev Lember kalevlember@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag|fedora-cvs+ |fedora-cvs?
--- Comment #19 from Kalev Lember kalevlember@gmail.com 2011-08-17 11:38:41 EDT --- Package Change Request ====================== Package Name: p11-kit New Branches: f16 Owners: kalev mclasen tmraz
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=725905
--- Comment #20 from Jon Ciesla limb@jcomserv.net 2011-08-17 11:40:36 EDT --- Git done (by process-git-requests).
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=725905
Kalev Lember kalevlember@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |CLOSED Fixed In Version| |p11-kit-0.3-2.fc16 Resolution| |NEXTRELEASE Last Closed| |2011-08-17 11:59:48
--- Comment #21 from Kalev Lember kalevlember@gmail.com 2011-08-17 11:59:48 EDT --- Package imported and built; closing the ticket.
package-review@lists.fedoraproject.org