https://fedoraproject.org/wiki/Packaging:Guidelines#Packaging_Static_Libraries_2 Build: fedfs-utils-0.8.0-9.fc19.src.rpm fedfs-utils-devel contains only static libraries, but no virtual -static package is provided
Manually verifying what the checker has found here, there is a packaging mistake: Information for RPM fedfs-utils-lib-0.8.0-9.fc19.x86_64.rpm http://koji.fedoraproject.org/koji/rpminfo?rpmID=3319856 contains: /usr/lib64/libnfsjunct.so This file belongs into the -devel package instead: https://fedoraproject.org/wiki/Packaging:Guidelines#Devel_Packages After fixing that, the checker would find that you still need to adhere to the Static Library Packaging guidelines. Shipping the static lib in the -devel package is not permitted, if there also is a shared lib: https://fedoraproject.org/wiki/Packaging:Guidelines#Packaging_Static_Libraries | 1. Static libraries and shared libraries. In this case, the static | libraries must be placed in a *-static subpackage. Separating the | static libraries from the other development files in *-devel allow | us to track this usage by checking which packages BuildRequire the | *-static package. The intent is that whenever possible, packages | will move away from using these static libraries, to the shared libraries.
I'll investigate and do what I can. I may need to apply for an exception for some of this but I'm not yet sure what, if anything.
No need to apply for an exception. The package contains a few mistakes that can be fixed: * https://fedoraproject.org/wiki/Packaging:Guidelines#Packaging_Static_Libraries | Libtool archives, foo.la files, should not be included. | Packages using libtool will install these by default even | if you configure with --disable-static, so they may need | to be removed before packaging. [...] Visit the link for the longer explanation. https://fedoraproject.org/wiki/Packaging:ReviewGuidelines MUST: Packages must NOT contain any .la libtool archives, these must be removed in the spec if they are built. fedfs-utils-devel /usr/include/nfs-plugin.h 2345 /usr/lib64/libnfsjunct.a 243598 /usr/lib64/libnfsjunct.la 983 <-- (!) fedfs-utils-lib /usr/lib64/libnfsjunct.so 20 /usr/lib64/libnfsjunct.so.0 20 /usr/lib64/libnfsjunct.so.0.0.0 132448 The *.so symlink belongs into the -devel package instead. It can simply be moved. Keep the *.so.0* files/symlinks in this -lib package, however. MUST: Static libraries must be in a -static package. So, since fedfs-utils contains shared _and_ static libs, the following is relevant: https://fedoraproject.org/wiki/Packaging:Guidelines#Packaging_Static_Libraries_2 | In this case, the static libraries must be placed in a *-static | subpackage. Separating the static libraries from the other development | files in *-devel allow us to track this usage by checking which packages | BuildRequire the *-static package. The intent is that whenever possible, | packages will move away from using these static libraries, to the shared | libraries. Which means, you would only need to move /usr/lib64/libnfsjunct.a from fedfs-utils-devel into a new subpackage fedfs-utils-static. Unless the static libs is not used, in that case you could simply delete it. | Packages including libraries should exclude static libs as far as possible | (eg by configuring with --disable-static). Static libraries should only be | included in exceptional circumstances.
(In reply to comment #3) > No need to apply for an exception. The package contains a few mistakes that > can be fixed: Sure, I will fix the obvious violations. But it is the location of libnfsjunct.so that I wanted to check. snip ... > > fedfs-utils-devel > > /usr/include/nfs-plugin.h 2345 > /usr/lib64/libnfsjunct.a 243598 > /usr/lib64/libnfsjunct.la 983 <-- (!) > > > fedfs-utils-lib > > /usr/lib64/libnfsjunct.so 20 > /usr/lib64/libnfsjunct.so.0 20 > /usr/lib64/libnfsjunct.so.0.0.0 132448 > > > The *.so symlink belongs into the -devel package instead. It can simply be > moved. Keep the *.so.0* files/symlinks in this -lib package, however. The libnfsjunct.so library is a plugin that applications may use. It's used by nfs-utils and upstream uses dlopen() to use the library and it's up to the plugin user to ensure version compatibility. Looks like the packaging guidelines say that it is ok to keep libnfsjunct.so in the -lib package in this case. Will that be ok then or would you like more information? Ian
The .so file is a non-issue. The spec file already gives the rationale why it is included in the -lib package and not the -devel package. %files lib # We need to include this in the lib package because it is # dlopen()ed by the junction support code in nfs-utils. %{_libdir}/libnfsjunct.so That is entirely okay.
On a 2nd thought, at the distribution level, theoretically one could patch nfs-utils to dlopen the versioned "libnfsjunct.so.0" instead and even add a strict dependency in the RPM packages, but I cannot tell where else the non-versioned "libnfsjunct.so" might be used at run-time.
(In reply to comment #6) > On a 2nd thought, at the distribution level, theoretically one could patch > nfs-utils to dlopen the versioned "libnfsjunct.so.0" instead and even add a > strict dependency in the RPM packages, but I cannot tell where else the > non-versioned "libnfsjunct.so" might be used at run-time. Why would upstream consider doing that since it would add an additional level of version check and require re-compile if either mechanism version check didn't match. It would be a hard sell and even harder to sincerely request since I don't believe the .so should be separated from the versioned libraries myself.
Why would upstream consider doing that? - Because libnfsjunct.so is just a symlink to an arbitrary version of libnfsjunct. - Because it would be smarter to look up the specific API/ABI version nfs-utils is made for, before possibly trying to load the non-versioned .so symlink as a fallback. The single dlsym check may not be sufficient in either case. - Because multiple versions of libnfsjunct can be installed in parallel, but there can be only one .so symlink, and it would need to point at either one. - Because there could even be multiple .so symlinks, but outside runtime linker's search path. > I don't believe the .so should be separated from the > versioned libraries myself. In general? Or just because a package has been found that wants to dlopen libnfsjunct.so instead of the versioned one? That's a corner-case. Discuss with the Fedora Packaging Committee if you disagree with the guidelines in general: https://fedoraproject.org/wiki/Packaging:Guidelines#Devel_Packages [...] Anyway, this ticket is about the violation of the Static Library Packaging Guidelines, not the shared libs.
(In reply to comment #8) > Why would upstream consider doing that? > - Because libnfsjunct.so is just a symlink to an arbitrary version of > libnfsjunct. > - Because it would be smarter to look up the specific API/ABI version > nfs-utils is made for, before possibly trying to load the non-versioned .so > symlink as a fallback. The single dlsym check may not be sufficient in > either case. > - Because multiple versions of libnfsjunct can be installed in parallel, > but there can be only one .so symlink, and it would need to point at either > one. > - Because there could even be multiple .so symlinks, but outside runtime > linker's search path. Sound points, but as you say here isn't the place. > > > I don't believe the .so should be separated from the > > versioned libraries myself. > > In general? Or just because a package has been found that wants to dlopen > libnfsjunct.so instead of the versioned one? In general, yes, but also because of potential segv in threaded applications. If an application uses an internal api implemented as shared libraries to make the interface independent of ways the service may need to be done, such as multiple map sources in autofs (LDAP, nis, etc.) then shared libraries that make the mistake of using thread specific data with callback functions within their library can cause the application to segv. There is nothing the application can do about it other than to dlopen() the errant library at startup and dlclose() it at exit. Sure, it happens because the thread specific data instance isn't destroyed before the library is unloaded and that is a bug but it can also be very hard to fix, such as is the case for libxml2 (or was when I looked seriously at it), and it's hard to get upstream interested as well since as far as they are concerned it works! Maybe they don't understand the issue or they are shy of the amount of work needed to fix it. The only library I found that worked in this case (when I looked) was the Kerberos libraries. The point being that needing to know the versioned name is an unnecessary complication in this case. > > That's a corner-case. Discuss with the Fedora Packaging Committee if you > disagree with the guidelines in general: > https://fedoraproject.org/wiki/Packaging:Guidelines#Devel_Packages > > [...] > > Anyway, this ticket is about the violation of the Static Library Packaging > Guidelines, not the shared libs. Indeed, yes, and I have gone way off topic, apologies. I'll get on and make the needed changes. Ian
Created attachment 687176 [details] Patch - fix several packaging mistakes in the spec file
Package fedfs-utils-0.8.0-10 has been built. I'll wait a couple of days for attached patch review before closing.
Dropping the .a/.la files is fine, of course. > patch review Also off-topic, but you could add %{?_isa} to the %{name}-nsdbparams Requires. It helps package resolvers (when there are problems in a dependency-chain, such as the several shared libs the -nsdbparams binary depends on): https://fedoraproject.org/wiki/Packaging:Guidelines#Requiring_Base_Package
(In reply to comment #12) > Dropping the .a/.la files is fine, of course. > > > patch review > > Also off-topic, but you could add %{?_isa} to the %{name}-nsdbparams > Requires. It helps package resolvers (when there are problems in a > dependency-chain, such as the several shared libs the -nsdbparams binary > depends on): > https://fedoraproject.org/wiki/Packaging:Guidelines#Requiring_Base_Package Sure, missed that in reviewing the guidelines as I did this.
Created attachment 696373 [details] Patch - change nsdbparams requires to include arch in requires This change adds the needed %{_isa} to nsdbparams requires, revision 11 includes this and has been built. Hopefully this is all that's needed and we can close this bug.
Sure, the originally reported issue is fixed.