Bug 530251

Some remarks:

* You install the shared libs to %{_libdir}/gearbox
Any particular reason for doing so rsp. why they should not be installed into %{_libdir}?


* The doxygen call belongs into %build not into %install


* The *.pc's don't acknowledge the cross-dependencies between the libraries this package consists of. 

For example: libhokuyo_aist.so* depends on libflexiport:
ldd usr/lib64/gearbox/libhokuyo_aist.so
...
	libflexiport.so.1.0.0 => not found
...
but hokuyo_aist.pc doesn't reflect this:
# cat usr/lib64/pkgconfig/hokuyo_aist.pc
...
Requires: 
Libs: -L/usr/lib64/gearbox  -lhokuyo_aist 
Cflags: -I/usr/include/gearbox

Thanks for taking a look.

The shared libs are only installed to %{_libdir}/gearbox because that's the way it's done upstream.  If there's a compelling reason not to create a seperate directory for them, i can change it rather easily.  I didn't see anything about it in the packaging guidelines, so I didn't worry about it.

The rest of the stuff i will check on and fix.

The compelling reason is dynamic linking (ld.so). %{_libdir} is in its standard search path, while %{_libdir}/gearbox isn't.

Would adding a file in /etc/ld.so.conf.d/ pointing to /usr/lib/gearbox mitigate this?  Or is it better to just put everything in /usr/lib?

Also, once everything is in /usr/lib, ldd finds the depends just fine without adding them to the .pc file explicitly.

(In reply to comment #4)
> Would adding a file in /etc/ld.so.conf.d/ pointing to /usr/lib/gearbox mitigate
> this?
Technically it would, but this is not the way how "system wide installation" is supposed to work.

These libs are ordinary shared libs => They belong into %{_libdir}.

>  Or is it better to just put everything in /usr/lib?
Yes, putting them into %{_libdir} is the way to go.

> Also, once everything is in /usr/lib, ldd finds the depends just fine without
> adding them to the .pc file explicitly. 
*.pc's have nothing to do with ldd.

*.pc's are helper files to aid compile-time linkage (invoking ld/gcc).

ldd is a tool to check how ld.so* would treat them at run-time.

Spec URL: http://www.richmattes.com/rpms/gearbox/gearbox.spec
SRPM URL: http://www.richmattes.com/rpms/gearbox/gearbox-9.07-2.fc11.src.rpm

koji dist-f11:
http://koji.fedoraproject.org/koji/taskinfo?taskID=1764091
koji dist-f12:
http://koji.fedoraproject.org/koji/taskinfo?taskID=1764094

rpmlint:
$ rpmlint gearbox.spec ../RPMS/i586/gearbox*
3 packages and 1 specfiles checked; 0 errors, 0 warnings.

Fixes:
*I changed the .pc files to point to all of the required libraries explicitly.  

*I moved all of the libraries into %{_libdir}, and moved the accompanying cmake files to /usr/share/cmake/Modules.

* I put the doxygen generation step in %build instead of %install

Thanks for the feedback, let me know if there's any more issues!

New update: I've fixed the following issues

* Fixed a small bug to enable ppc support.  Now the only arch that isn't supported is ppc64, because of missing dependancies (ice)

* Fixed some static cmake files with wrong paths, so they get generated with the correct paths during the build process.

* Made it so ALL cmake files go into /usr/share/gearbox/cmake

* Fixed a typo that prevented pkg-config fixes from being applied.

* Upstream further clarified their licenses, so I patched the LICENSE file to their most recent svn revision.

Spec URL: http://www.richmattes.com/rpms/gearbox/gearbox.spec
SRPM URL: http://www.richmattes.com/rpms/gearbox/gearbox-9.07-3.fc11.src.rpm

koji dist-f11:
http://koji.fedoraproject.org/koji/taskinfo?taskID=1795294
koji dist-f12:
http://koji.fedoraproject.org/koji/taskinfo?taskID=1795298

rpmlint:
$ rpmlint gearbox.spec ../RPMS/i586/gearbox*
3 packages and 1 specfiles checked; 0 errors, 0 warnings.

News:
I've been working with upstream, they're ready to make a new release after cleaning up their buildsystem to support 32/64bit architectures.  They're concerned that other projects that rely on theirs won't build correctly the libraries libraries and cmake files are moved out of %{_libdir}/gearbox.  I've been emailing the fedora-packaging list for a little clarification, but haven't gotten a response yet.  I'm ready to submit a package with either configuration, just waiting on some more feedback from the expert packagers.

Update: upgraded to release 9.11.  This release includes fixes for the build issues 9.07 had.

Spec URL: http://www.richmattes.com/rpms/gearbox/gearbox.spec
SRPM URL: http://www.richmattes.com/rpms/gearbox/gearbox-9.11-4.fc12.src.rpm

$ rpmlint gearbox.spec ../RPMS/i686/gearbox*
3 packages and 1 specfiles checked; 0 errors, 0 warnings.

koji dist-f11:
http://koji.fedoraproject.org/koji/taskinfo?taskID=1823514

koji dist-f12:
http://koji.fedoraproject.org/koji/taskinfo?taskID=1823518

Another small update:

There's a few parts of this library that rely on the ICE package, which doesn't exist on ppc64 systems.  I split those libraries into a separate sub-package, gearbox-ice, and made it so those parts won't build on a ppc64 system.  That way, programs that don't rely on gearbox's ICE related components (i.e. the newly pushed player-3.0.0 package) can still be built on ppc64 systems.  

Spec URL: http://www.richmattes.com/rpms/gearbox/gearbox.spec
SRPM URL: http://www.richmattes.com/rpms/gearbox/gearbox-9.11-5.fc12.src.rpm


$ rpmlint gearbox.spec ../RPMS/i686/gearbox*
gearbox-ice.i686: W: no-documentation
4 packages and 1 specfiles checked; 0 errors, 1 warnings.

koji dist-f11:
http://koji.fedoraproject.org/koji/taskinfo?taskID=1911878

koji dist-f12:
http://koji.fedoraproject.org/koji/taskinfo?taskID=1911873

I goofed on the dist versions, I should have started at -1 again when I bumped the package to release 9.11.  I fixed this issue, which puts the package version at 9.11-3.  I also added %post and %postun ldconfig to gearbox and gearbox-ice

Spec URL: http://www.richmattes.com/rpms/gearbox/gearbox.spec
SRPM URL: http://www.richmattes.com/rpms/gearbox/gearbox-9.11-3.fc12.src.rpm

$ rpmlint gearbox.spec ../SRPMS/gearbox* ../RPMS/i686/gearbox*
gearbox-ice.i686: W: no-documentation
5 packages and 1 specfiles checked; 0 errors, 1 warnings.

koji dist-f11:
http://koji.fedoraproject.org/koji/taskinfo?taskID=1930860

koji dist-f12:
http://koji.fedoraproject.org/koji/taskinfo?taskID=1930865

Here is my quick review. I have asked Jeff Spaleta to go over this as well. He agreed to do so, and he can sponsor you if he deems the packages acceptable.

MUST
* OK: rpmlint
* OK: package name
* OK: package version and release
* OK: spec file name
* OK: package guideline-compliant
* OK: license complies with guidelines
* OK: license field accurate
* OK: license file not deleted
* OK: spec in US English
* OK: spec legible
* OK: source matches upstream
  sha256sum 12170be6b6c477926eb4e274574d877b4ccd7230978088a61b892e6f1dbb6e08
* OK: builds under >= 1 archs, others excluded
* OK: dependencies (requires)
* OK: build dependencies complete
* N/A: locales handled using %find_lang, no %{_datadir}/locale
* OK: library -> ldconfig
* N/A: relocatable: give reason
* OK: own all directories
* OK: no dupes in %files
* OK: permission
* OK: %clean RPM_BUILD_ROOT
* OK: macros used consistently
* OK: Package contains code
* N/A: large docs => -doc
* N/A: doc not runtime dependent
* OK: headers in -devel
* N/A: static in -static
* OK: if contains *.pc, req pkgconfig
* OK: if libfiles are suffixed, the non-suffixed goes to devel
* OK: devel requires versioned base package
* N/A: desktop file uses desktop-file-install
* OK: clean buildroot before install
* OK: filenames UTF-8

SHOULD
* OK: if license text missing, ask upstream to include it
* N/A: desc and summary contain translations if available
* OK: package build in mock on all architectures
  Koji URLs given by packager
* OK: package functioned as described
  Did tests with the Hokuyo laser range finder and worked just fine
* OK: scriplets are sane
* OK: other subpackages should require versioned base
* N/A: if main pkg is development-wise, pkgconfig can go in main package
* OK: require package not files

I'm happy with the package and the parts I could test work here.

Thanks for the review and the recommendation.  I just noticed that there's a small bug in how the pkg-config files are being generated.  I patched the build scripts and should have a new srpm and spec up later today.

Here's the new build with the fixed pkg-config build.  The only thing that I added was the patch file

Spec URL: http://www.richmattes.com/rpms/gearbox/gearbox.spec
SRPM URL: http://www.richmattes.com/rpms/gearbox/gearbox-9.11-4.fc12.src.rpm


$ rpmlint gearbox.spec ../SRPMS/gearbox* ../RPMS/i686/gearbox*
gearbox-ice.i686: W: no-documentation
5 packages and 1 specfiles checked; 0 errors, 1 warnings.

Some initial remarks for 9.11-4

* SourceURL
  - For sourceforge hosted tarball, please follow
    https://fedoraproject.org/wiki/Packaging/SourceURL#Sourceforge.net

* Build failure
  - Due to DSOlinkage change on F-13, your srpm does not build
    on F-13:
    http://koji.fedoraproject.org/koji/taskinfo?taskID=2046939
    http://fedoraproject.org/wiki/UnderstandingDSOLinkChange

    You can check this behavior on F-12 by adding
-----------------------------------------------------------
export LDFLAGS="-Wl,--no-add-needed"
-----------------------------------------------------------
    before %cmake line.

* Dependency for -devel subpackage
  - "Requires: pkgconfig" is no longer needed (now rpmbuild automatically
    detects this dependency)

  - Some header files have dependency for ice-devel
    For example, ./gbxsickacfr/gbxiceutilacfr/buffer.h has
-----------------------------------------------------------
    17  #include <IceUtil/Monitor.h>
    18  #include <IceUtil/Mutex.h>
    19  #include <IceUtil/Time.h>
-----------------------------------------------------------
    So gearbox-devel should have "Requires: ice-devel".

* ppc64 switch
  - Maybe following is simpler:
-----------------------------------------------------------
%cmake -DENABLE_LIB_BASICEXAMPLE=OFF -DENABLE_LIB_GBXUTILACFR=ON ... \
%ifarch ppc64
   -DENABLE_LIB_GBXSICKACFR=OFF \
%else
   -DENABLE_LIB_GBXSICKACFR=ON \
%endif
    .
-----------------------------------------------------------

* -ice subpackage splitting
  - Currently this makes no gain.
-----------------------------------------------------------
# env LANG=C rpm -ivh --test gearbox-9.11-4.1.fc13.i686.rpm 
error: Failed dependencies:
        libGbxIceUtilAcfr.so.1.0.0 is needed by gearbox-9.11-4.1.fc13.i686
-----------------------------------------------------------

* Directory ownership issue
  https://fedoraproject.org/wiki/Packaging/UnownedDirectories#Common_Mistakes
  - Currently the following directories are not owned by
    any packages.
-----------------------------------------------------------
%{_libdir}/%{name}
-----------------------------------------------------------

* rpath
  https://fedoraproject.org/wiki/Packaging/Guidelines#Beware_of_Rpath
-----------------------------------------------------------
$ rpmlint gearbox | grep rpath
gearbox.i686: E: binary-or-shlib-defines-rpath /usr/lib/libflexiport.so.1.0.0 ['/usr/lib']
gearbox.i686: E: binary-or-shlib-defines-rpath /usr/lib/libGbxGarminAcfr.so.1.0.0 ['/usr/lib']
gearbox.i686: E: binary-or-shlib-defines-rpath /usr/lib/libGbxUtilAcfr.so.1.0.0 ['/usr/lib']
gearbox.i686: E: binary-or-shlib-defines-rpath /usr/lib/libGbxNovatelAcfr.so.1.0.0 ['/usr/lib']
gearbox.i686: E: binary-or-shlib-defines-rpath /usr/lib/libGbxSmartBatteryAcfr.so.1.0.0 ['/usr/lib']
gearbox.i686: E: binary-or-shlib-defines-rpath /usr/lib/libhokuyo_aist.so.1.0.0 ['/usr/lib']
gearbox.i686: E: binary-or-shlib-defines-rpath /usr/lib/libGbxSerialAcfr.so.1.0.0 ['/usr/lib']
gearbox.i686: E: binary-or-shlib-defines-rpath /usr/lib/libGbxSerialDeviceAcfr.so.1.0.0 ['/usr/lib']
gearbox.i686: E: binary-or-shlib-defines-rpath /usr/lib/libGbxLockFileAcfr.so.1.0.0 ['/usr/lib']
gearbox.i686: E: binary-or-shlib-defines-rpath /usr/lib/libGbxNovatelUtilAcfr.so.1.0.0 ['/usr/lib']
-----------------------------------------------------------
  - Please remove these rpaths.

I fixed the DSO problems, corrected the Sourceforge URL, and added the missing ice-devel dependency.  I removed the superfluous java BuildRequires as well, it isn't needed.  I also got rid of the -ice subpackage, and applied the conditionals to the cmake line as you suggested.  The directory ownership of %{_libdir} has been fixed as well.  As far as rpath, it looks like the %cmake macro in F13 no longer includes "-DCMAKE_SKIP_RPATH:BOOL=ON" which is probably why I didn't run into that issue on my F12 machine.  I added it to my list of cmake switches in the spec file to ensure it's always included during the build.


Spec URL: http://www.richmattes.com/rpms/gearbox/gearbox.spec
SRPM URL: http://www.richmattes.com/rpms/gearbox/gearbox-9.11-5.fc12.src.rpm

koji:
dist-f12: http://koji.fedoraproject.org/koji/taskinfo?taskID=2048002
dist-f13: http://koji.fedoraproject.org/koji/taskinfo?taskID=2048013

rpmlint:
$ rpmlint ../SRPMS/gearbox-9.11-5.fc12.src.rpm ../RPMS/i686/gearbox* gearbox.spec
4 packages and 1 specfiles checked; 0 errors, 0 warnings.

Some additional notes:

- "Requires: ice" is not needed. rpmbuild automatically checks
  libraries-related dependency and adds these dependencies into
  binary rpms:
  https://fedoraproject.org/wiki/Packaging/Guidelines#Requires

- It is recommended to use %% instead of % in comment lines to avoid
  unexpected macro expansion.

------------------------------------------------------------------
    This package (gearbox) is APPROVED by mtasaka
------------------------------------------------------------------

Please follow the procedure written on:
http://fedoraproject.org/wiki/PackageMaintainers/Join
from "Install the Client Tools (Koji)".

Now I am sponsoring you.

If you want to import this package into Fedora 11/12/13, you also have
to look at
http://fedoraproject.org/wiki/Infrastructure/UpdatesSystem/Bodhi-info-DRAFT
(after once you rebuilt this package on koji Fedora rebuilding system).

If you have questions, please ask me.

Removing NEEDSPONSOR.

Thanks for sponsoring me!  And thanks for the quick review.  I'll fix the ice dependency, and look at which flags I need to add to change %cmake to %%cmake.

New Package CVS Request
=======================
Package Name: gearbox
Short Description: A collection of usable peer-reviewed robotics-related libraries
Owners: rmattes
Branches: F-11 F-12 F-13
InitialCC: timn

CVS done.

gearbox-9.11-5.fc12 has been submitted as an update for Fedora 12.
http://admin.fedoraproject.org/updates/gearbox-9.11-5.fc12

gearbox-9.11-5.fc12 has been pushed to the Fedora 12 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update gearbox'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/gearbox-9.11-5.fc12

Closing.

gearbox-9.11-5.fc12 has been pushed to the Fedora 12 stable repository.  If problems still persist, please make note of it in this bug report.

Package Change Request
======================
Package Name: gearbox
New Branches: el6
Owners: rmattes
InitialCC: 

I'd like to add the gearbox package to el6

Git done (by process-git-requests).