Spec URL: http://people.redhat.com/~dhowells/keyutils/keyutils-1.0-2.fc5/keyutils.spec
SRPM URL: http://people.redhat.com/~dhowells/keyutils/keyutils-1.0-2.fc5/keyutils-1.0-2.fc5.src.rpm
This package provides userspace utility programs to allow the user to manipulate the kernel key management facility. Libraries are provided so that other userspace programs may use the facility as well. The binaries need to go in /bin, /sbin and /lib so that they can be used to authorise/authenticate the mounting of /usr.
This package is included as part of RHEL-4 and has been tested there.
Before we can continue, please fix these issues:
# rpmlint keyutils-*.rpm W: keyutils no-url-tag
E: keyutils standard-dir-owned-by-package /usr/share/man/man5
E: keyutils standard-dir-owned-by-package /usr/share/man/man8
E: keyutils executable-marked-as-config-file /etc/request-key.conf
E: keyutils script-without-shellbang /etc/request-key.conf
E: keyutils standard-dir-owned-by-package /usr/share/man/man1
W: keyutils-debuginfo no-url-tag
W: keyutils-devel no-dependency-on keyutils
W: keyutils-devel no-url-tag
W: keyutils-devel no-documentation
E: keyutils-devel script-without-shellbang /usr/include/keyutils.h
W: keyutils-libs no-url-tag
Besides these, the follow issues caught my eye:
* Unowned directory:
* BuildRoot not conforming to FE guidelines
* Why this (IMO weird) library naming?
I've put a new SPEC and SRPM up for download:
This should fix everything barring the "weird" library naming - with that, I
followed the Ulrich Drepper guide to library naming. Btw, did you mean a
literal '*' in the name of a library?
(In reply to comment #2)
Looks *much* better now.
> This should fix everything barring the "weird" library naming - with that, I
> followed the Ulrich Drepper guide to library naming.
Well, how comes the rest of the world is not following this proposal?
The only thing that matters is the SONAME, the library's filename is largely
ignorable (c.f. info libtool, for why this naming is considered harmful).
> Btw, did you mean a
> literal '*' in the name of a library?
Nope, your are encoding the rpm's Release-tag into the library name. I am
building under fc4 and changed the spec's Release:-tag into FE conforming:
With this, I end up with
=> Bug in your Makefile: The spec's Release tag overrides the Makefile's RELEASE
Further (minor) issue:
# rpm -qvlp keyutils-libs-devel-1.1-1.fc4.i386.rpm
/usr/lib/libkeyutils.so -> //lib/libkeyutils.so.1
Note the double slash at the beginning. I am not aware about this being harmful
under Linux, but under other OS it is harmful.
Further general issue:
* Shipping a static library (Discouraged with FE).
> Well, how comes the rest of the world is not following this proposal?
glibc and binutils both use it.
> The only thing that matters is the SONAME, the library's filename is largely
> ignorable (c.f. info libtool, for why this naming is considered harmful).
Well, in my libtool info page, in section 6.4 (Managing release information),
it holds up `libbfd-2.9.0.so' as the example of naming.
> With this, I end up with
The library's filename is, as you said, largely ignorable; and the fact that
the library version number contains 'fc4' will not cause binary
incompatibility, since the SONAME is set to the interface symlink
What would you suggest? I want the release number in there since the library
may well be different between two compilations. I suppose I could edit out the
distribution tag, but that's messy.
Anyway, I've fixed the Makefile problem and the double-slash problem:
The static library should be there as this library wraps some system calls
that aren't available through glibc.
Btw, note that PAM and mount may both need to use the library in this package
at some point.
(In reply to comment #4)
> > Well, how comes the rest of the world is not following this proposal?
> glibc and binutils both use it.
> > The only thing that matters is the SONAME, the library's filename is largely
> > ignorable (c.f. info libtool, for why this naming is considered harmful).
> Well, in my libtool info page, in section 6.4 (Managing release information),
> it holds up `libbfd-2.9.0.so' as the example of naming.
Yes, binutils is a counter example of good practice and glibc is Drepper's
playground - Almost everybody else is not following this bad habits.
> > With this, I end up with
> > /lib/libkeyutils-1.1.1.fc4.so
> The library's filename is, as you said, largely ignorable; and the fact that
> the library version number contains 'fc4' will not cause binary
> incompatibility, since the SONAME is set to the interface symlink
> What would you suggest?
You should learn to destinguish library API-versioning from package versioning.
> Anyway, I've fixed the Makefile problem and the double-slash problem:
> SPEC URL:
> SRPM URL:
> The static library should be there as this library wraps some system calls
> that aren't available through glibc.
Hmm? I fail to understand this, because your binaries are dynamically linked
I am not going to approve this package in it's current shape.
Should be OK if you drop the static library, and drop the weirdness with
$(RELEASE) in the library filename.
I don't think that using Uli's (weird, IMO) suggested library naming should be a
barrier to Extras approval, but there's no need for you to make it even
_weirder_ by including $(RELEASE) in the filename.
That's removed the release number from the library filename and disabled the
> Hmm? I fail to understand this, because your binaries are dynamically linked
> against libkeyutils.so.
The main user of these libraries is not intended to be my utilities; the main
user is meant to be things like mount, PAM modules, NFS4, encrypted
filesystems. Some of these things may require static tools to be installed to
make a key available for an in-kernel filesystem or other service available,
so that you can mend your system when it's broken.
It's possible that I *should* include a static version of keyctl and make
request-key statically linked, but I'll defer that decision for now.
(In reply to comment #7)
> The main user of these libraries is not intended to be my utilities; the main
> user is meant to be things like mount, PAM modules, NFS4, encrypted
> filesystems. Some of these things may require static tools to be installed to
> make a key available for an in-kernel filesystem or other service available,
> so that you can mend your system when it's broken.
To my knowledge, none of those things are statically linked. We discourage
static linking in Fedora.
If the static library is dropped, this package should be fine. Ralf?
(In reply to comment #9)
> If the static library is dropped, this package should be fine. Ralf?
Well, I don't know.
The OP claimed there were technical requirements to ship the static libs. So,
from my (package review focused) perspective, a minimum requirement to proceed
would be him to either drop the static libs or to prove the necessity of
I.e. yes, dropping the static lib would remove one road blocker.
The other issue (shared library names) is my personal preference (I consider
"Drepper-style" library naming useless featuritis), which will cause me not
approve a package. But it's not a technical problem, which should prevent others
from approving a package.
As stated in comment #7, I've disabled the building of the static library for
the moment (I can always bring it back later if it becomes necessary), and
I've removed the release number from the library name.
Is that sufficient, Ralf?
David, do you have an updated SRPM? Ralf never committed to a review but from
from reading this ticket it looks like the package should be fine. I'll be
happy to do a formal review if I have the final SRPM to work with.
The SRPM linked to by comment #7 should be it:
The Source0 URL doesn't exist. You're the upstream so it doesn't seem to matter
much for the purposes of the review, but it would be good to make sure that the
source is always where the srpm says it is.
You install a shared libary into a system location but you don't call ldconfig.
I believe this is a blocker.
* package meets naming and packaging guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
* dist tag is present.
* build root is correct.
* license field matches the actual license.
* license is open source-compatible. License text included in package.
? source files match upstream (can't check)
? latest version is being packaged (I assume so since you're the upstream)
* BuildRequires are proper.
* package builds in mock (development, x86_64).
* rpmlint is silent.
* final provides and requires are sane:
config(keyutils) = 1.1-3.fc6
keyutils = 1.1-3.fc6
config(keyutils) = 1.1-3.fc6
keyutils-libs = 1.1-3.fc6
keyutils-libs-devel = 1.1-3.fc6
keyutils-libs = 1.1-3.fc6
X shared libraries are present (in -libs subpackage) but ldconfig is not called.
* package is not relocatable.
* owns the directories it creates.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
* %clean is present.
* %check is not present; no test suite upstream.
? no scriptlets present (but there should be)
* code, not content.
* documentation is small, so no -docs subpackage is necessary. (Most of the
documentation is in the -devel subpackage).
* %docs are not necessary for the proper functioning of the package.
* headers are in -devel subpackage.
* no pkgconfig files.
* no libtool .la droppings.
* not a GUI app.
Okay, I've made the source tarball available at the Source0 URL and I've fixed
the ldconfig lack.
Looks good; the two issues I had are fixed.
I didn't realize that you required sponsorship. I've gone ahead and sponsored
your cvsextras membership; you should be ready to check in once your key
percolates through the system.
There doesn't seem to be any way to avoid the requirement for sponsorship.
Anyway, thanks! I've now built my packages, though they don't seem to be
available to yum yet.