Bug 723575 - Review Request: libpki - Easy-to-use PKI library
Summary: Review Request: libpki - Easy-to-use PKI library
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
Target Milestone: ---
Assignee: Nobody's working on this, feel free to take it
QA Contact: Fedora Extras Quality Assurance
Depends On:
TreeView+ depends on / blocked
Reported: 2011-07-20 15:43 UTC by Patrick Monnerat
Modified: 2012-11-28 12:36 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Last Closed: 2012-11-28 12:36:07 UTC
Type: ---

Attachments (Terms of Use)

Description Patrick Monnerat 2011-07-20 15:43:01 UTC
Spec URL: http://monnerat.fedorapeople.org/libpki.spec
SRPM URL: http://monnerat.fedorapeople.org/libpki-0.6.5-1.fc14.src.rpm
  This library provides the developer with all the needed features to
manage certificates, from generation to validation. The LibPKI Project
enables developers with the possibility to implement complex cryptographic
operations with a few simple function calls by implementing an high-level
cryptographic API.

The last version of the OCSP responder (ocspd) uses this library: thus I need libpki to upgrade ocspd.

rpmlint output:
$ rpmlint SPECS/libpki.spec 
0 packages and 1 specfiles checked; 0 errors, 0 warnings.
$ rpmlint SRPMS/libpki-0.6.5-1.fc14.src.rpm 
1 packages and 0 specfiles checked; 0 errors, 0 warnings.
$ rpmlint RPMS/x86_64/libpki-0.6.5-1.fc14.x86_64.rpm 
libpki.x86_64: E: incorrect-fsf-address /usr/share/doc/libpki-0.6.5/COPYING
libpki.x86_64: W: no-manual-page-for-binary pki-xpair
libpki.x86_64: W: no-manual-page-for-binary pki-crl
libpki.x86_64: W: no-manual-page-for-binary pki-request
libpki.x86_64: W: no-manual-page-for-binary pki-query
libpki.x86_64: W: no-manual-page-for-binary pki-cert
libpki.x86_64: W: no-manual-page-for-binary pki-derenc
libpki.x86_64: W: no-manual-page-for-binary url-tool
libpki.x86_64: W: no-manual-page-for-binary pki-tool
libpki.x86_64: W: no-manual-page-for-binary pki-siginfo
1 packages and 0 specfiles checked; 1 errors, 9 warnings.
$ rpmlint RPMS/x86_64/libpki-devel-0.6.5-1.fc14.x86_64.rpm 
libpki-devel.x86_64: W: no-documentation
libpki-devel.x86_64: W: no-manual-page-for-binary libpki-config
1 packages and 0 specfiles checked; 0 errors, 2 warnings.
$ rpmlint RPMS/x86_64/libpki-static-0.6.5-1.fc14.x86_64.rpm 
libpki-static.x86_64: W: no-documentation
1 packages and 0 specfiles checked; 0 errors, 1 warnings.
$ rpmlint RPMS/x86_64/libpki-doc-0.6.5-1.fc14.x86_64.rpm 
libpki-doc.x86_64: E: incorrect-fsf-address /usr/share/doc/libpki-doc-0.6.5/COPYING
1 packages and 0 specfiles checked; 1 errors, 0 warnings.
$ rpmlint RPMS/x86_64/libpki-debuginfo-0.6.5-1.fc14.x86_64.rpm 
1 packages and 0 specfiles checked; 0 errors, 0 warnings.

- Upstream has been requested by e-mail to change the license file and to provide man1 pages.
- Documentation that can be provided in the -devel and -static subpackages is already present in the (required) main package.
- Although "make test" is recognized, no %check spec section is provided: the upstream-provided tests need a tty input and can only be executed by themselves without failing :-(

Thanks in advance for reviewing

Comment 1 Michael Schwendt 2011-07-22 10:28:33 UTC
Not a full review:

> %global with_doc	1

%bcond_without  doc  1

is more flexible and mighty as a default. With it you could simply toggle the %with_doc macro when building, e.g.  rpmbuild --without doc ...

> #	Remove unneeded files.
> rm -rf "${RPM_BUILD_ROOT}%{_datadir}/libpki"

Something like this asks for an explanation in a spec file comment. It is obvious that "rm -rf" removes an entire tree, but the _why_ isn't answered. What files are in there and why are they unneeded?

> #-------------------------------------------------------------------------------
> %post
> #-------------------------------------------------------------------------------
> /sbin/ldconfig
> #-------------------------------------------------------------------------------
> %postun
> #-------------------------------------------------------------------------------
> /sbin/ldconfig

Caution! Here you create shell scripts that contain your long '#----...' lines, because there is no way yet to decide which comment is part of the script or just the spec file. Take a look at e.g. the rpm -qp --scripts libpki-*rpm output of the built packages. If you really want to execute just ldconfig, prefer running it directly with an automatic dependency on its path instead of /bin/sh:

%post -p /sbin/ldconfig
%postun -p /sbin/ldconfig

> %{_bindir}/pki-cert
> %{_bindir}/pki-crl
> %{_bindir}/pki-derenc
> %{_bindir}/pki-query
> %{_bindir}/pki-request
> %{_bindir}/pki-siginfo
> %{_bindir}/pki-tool
> %{_bindir}/pki-xpair
> %{_bindir}/url-tool

Very generic names that bear a high risk of causing conflicts with other/future packages, especially if these tools are shipped within a library package.

Just the '-' in 'pki-' and 'url-' currently creates an own namespace. However, the 'pki' and 'url' prefix is in use already:

$ repoquery --whatprovides /usr/bin/pki\*

$ yum list pki-\*|wc -l

$ repoquery --whatprovides /usr/bin/url\*

Even if this library's tools plan to occupy the 'pki-' namespace for executables like this (and not the 'libpki-' namespace), it would be better to move these files into a libpki-tools or libpki-utils package, so the tools remain optional whereas the library package might be dragged in as a dependency of something.

$ libpki-config --cflags
-I/usr/include/libxml2 -DLINUX

The libpki-devel package is missing a dependency on libxml2-devel as pki.h and several other headers include libxml headers, but the pkgconfig file explicitly depends on libxml-2.0.pc.

Further, libpki.pc is broken (which also explains why the automatic .pc file dep has not been added by rpmbuild):

$ cat /usr/lib64/pkgconfig/libpki.pc |grep libdir

Comment 2 Michael Schwendt 2011-07-22 10:37:04 UTC
The deeper look at the pkgconfig problem:

$ pkg-config --cflags libpki
Package openssl-0.9.8 was not found in the pkg-config search path.
Perhaps you should add the directory containing `openssl-0.9.8.pc'
to the PKG_CONFIG_PATH environment variable
Package 'openssl-0.9.8', required by 'libpki', not found

$ cat /usr/lib64/pkgconfig/libpki.pc|grep ^Req
Requires: libxml-2.0 openssl-0.9.8
$ ls /usr/lib64/pkgconfig/openssl.pc 
$ rpm -q openssl-devel
$ pkg-config --print-requires libpki

That's why the pkgconfig dep generation fails.

Comment 3 Patrick Monnerat 2011-08-03 13:36:53 UTC
Thanks for the partial review. Here are the new links:
SPEC: http://monnerat.fedorapeople.org/libpki.spec
SRPMS: http://monnerat.fedorapeople.org/libpki-0.6.5-2.fc14.src.rpm

Last changelog:
* Fri Jul 22 2011 Patrick Monnerat <pm@datasphere.ch> 0.6.5-2
- Patch "pkgconfig" to fix pkg-config configuration file.
- Sub-package "tools" for binary utilities.
- libxml2-devel dependence added to devel sub-package.
- "rpmbuild ... --without doc" is now recognized.

rpmlint unchanged, except tools related messages issued by new tools sub-package instead of -devel.

Thanks for the %bcond_without trick: I did not know it.

%post and %postun scriptlets: WONTFIX. I agree with you it should be done this way, but this is an RPM problem. I tried "-p", but it does not work unless the next lines are empty until the next '%xxx' paragraph. I also tried %macros, but they are not substituted inside a scriptlet. Since the current solution works perfectly (though requiring /bin/sh which is always already present), I keep it unchanged to preserve my RPM comments: those improve a lot the readbility. RPM should provide a dummy "%endOfScript" paragraph or a comment escape scriptlet option to resolve this problem, but this is not the topic of this review request :-/

There are still upstream problems with this package (i.e.: soname): I'm in contact with the upstream project maintainer about them.

Thanks again Michael for your work on it.

Comment 4 Patrick Monnerat 2012-11-28 12:36:07 UTC
Too many problems (memory leaks) with this library: resigning.
Will live with old version of ocspd not using it.

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