Spec URL: http://people.atrpms.net/~hdegoede/pinball.spec SRPM URL: http://people.atrpms.net/~hdegoede/pinball-0.3.1-3.src.rpm Description: The Emilia Pinball project is an open source pinball simulator for linux and other unix systems. The current release is a stable and mature alpha. There are only two pinball machines to play with, it is however very addictive.
Requires: %{name} = %{version}-%{release} Add pkgconfig to this line %files devel Isn't there an .so file for this directory?
(In reply to comment #1) > Requires: %{name} = %{version}-%{release} > > Add pkgconfig to this line > Erm, it doesn't install any pc files, so pkgconfig isn't needed. > %files devel > > Isn't there an .so file for this directory? > Notice that it uses %{_libdir}/%{name} not just %{_libdir}, %{_libdir}/%{name} contains plugins which get loaded by there full soname by ltdl, which finds this name by looking at the .la files, so in essence the .la files replace the .so files for the purpose of plugin loading. The .so files also are not needed to link against, since this are not libs but plugins. The -devel package is to develop new plugins, not to develop programs using the existing ones, hence no .so files.
Sorry, didn't spot those! I'll get on reviewing the package as soon as my buildsys stops playing silly buggers.
Fails to build : you need to remove the smp_flags as it causes a race condition LoaderModule.cpp has a lot of errors in - the majority of them are the likes of lt_dlinit not declared in this scope. I'm actually starting to thing this is gcc being overly strict as amaya has this problem as well and I'm unable to see why it should be by examining the source
(In reply to comment #4) > Fails to build : you need to remove the smp_flags as it causes a race condition Done, sorry I used to have it set to -j3 in my .rpmrc but I've changed it to -j1 because lately I've been packaging a few huge c++ apps and my machine gut real sluggish with -j3 (I don't have a seperate builder like you do). > LoaderModule.cpp has a lot of errors in - the majority of them > are the likes of lt_dlinit not declared in this scope. > > I'm actually starting to thing this is gcc being overly strict as amaya has this > problem as well and I'm unable to see why it should be by examining the source Are you sure it doesn't start with a missing header error before those, as |i forgot to add a BR for libtool-ltdl-devel, which IMHO is the likely cause of this. Here is a new version with smp_flags removed and the BR added: Spec URL: http://people.atrpms.net/~hdegoede/pinball.spec SRPM URL: http://people.atrpms.net/~hdegoede/pinball-0.3.1-4.src.rpm
rpmlint warns that there is no documentation in the devel package mock builds cleanly and rpm -qa --requires is not showing anything not caught in the mock build --- SPEC FILE --- Good Upstream version is the same, md5sums both check out No dupes in the BR No ownership problems Software actually runs Bad %{_libdir}/%{name}/lib*.la Why? There should be no .la or .a files in a package Should /sbin/ldconfig not be also executed?
(In reply to comment #6) > Bad > > %{_libdir}/%{name}/lib*.la > > Why? There should be no .la or .a files in a package > Quoting from the specfile: "# .la files are needed for ltdl" What is not clear about that? > Should /sbin/ldconfig not be also executed? Quoting myself from comment #3: "The .so files also are not needed to link against, since this are not libs but plugins. The -devel package is to develop new plugins, not to develop programs using the existing ones, hence no .so files." Since they are not used to link against directly calling ldconfig is not needed, actually since they are not in a dir searched by ldconfig, calling ldconfig is futile.
Okay. I'm happy with them. The software works fine Good Licence fine Includes update-icon-cache No permission problems No directory ownership problems Has documentation (devel doesn't, can be ignored) Software works and installs No missing R's when installed No missing BRs or Rs when compiling Bad Missing rm -rf %{buildroot} at the start of %install Fix the one bad and it's good to go.
Can you also check that if you use the fedora libtool if the .la files are still needed? Please just upload the new spec file (unless something happens in the src.rpm)
It may be worth actually patching the app to use the fedora libtool and seems pretty trivial to do... in LoaderModule.cpp, Line 86 change lt_dlopen() to lt_dlopenext() and remove the extension on the passed in filename There may be a couple more of those, so you'll need to grep the source.
Created attachment 133896 [details] Patch to allow ltdl to load from an so instead of an la file Hey Hans, Here's a patch to load the modules from the .so file if the .la file is not present. This patch would be a good starting point with upstream but may need a little work (I'm not sure whether the fallback lt_dlopen() is necessary and my C++ dates from before the STL so you can tell how rusty I am.) I'll submit my spec file as well so you can diff for the changes.
Created attachment 133897 [details] My spec file. Patch the source. Include the .so file and rm the .la file. P.S. I suck at pinball :-)
(In reply to comment #8) > Bad > Missing rm -rf %{buildroot} at the start of %install > Good catch! New version with this fixed (only specfile changed): Spec URL: http://people.atrpms.net/~hdegoede/pinball.spec SRPM URL: http://people.atrpms.net/~hdegoede/pinball-0.3.1-5.src.rpm (In reply to comment #9) > Can you also check that if you use the fedora libtool if the .la files are > still needed? Yes they are still needed. (In reply to comment #10) > It may be worth actually patching the app to use the fedora libtool and seems > pretty trivial to do... > It already is using the Fedora/system libtool, quoting from %setup: "rm -fr libltdl", so there is no way its using its own version :) > in LoaderModule.cpp, Line 86 change > > lt_dlopen() to lt_dlopenext() and remove the extension on the passed in filename > > There may be a couple more of those, so you'll need to grep the source. That is not using the Fedora libtool, that is patching arounds libltdl's behaviour / default use to open the .la files. All kde apps with plugings (as in Fedora) do this, and I see no reason why pinball shouldn't/couldn't there is no reason to deviate from upstream here / use ltdl in a non standard way here. .la files are evil / must be removed for normal libraries which are intended to link against, because when an application gets build using libtool and those .la files are there libtool will prefer the .la files above the .so files and will use the often broken dependencies in those .la files instead of the soname deps in the .so files (which get tracked by rpm). This is known as libtool dependency hell, but this only happens with libraries which are intented to link against by other apps and which thus are usually installed directly into %_libdir and not into a subdir of it as the *plugins* are. So there is no reason to start patching and deviating from upstream here, the .la files are harmless because they are for plugins and beside harmless also needed as this is the default mode of operation of ltdl. If you don't like this file a bug against ltdl and the zillion of kde packages doing the same. (In reply to comment #11) > Created an attachment (id=133896) [edit] > Patch to allow ltdl to load from an so instead of an la file > > Hey Hans, Here's a patch to load the modules from the .so file if the .la file > is not present. This patch would be a good starting point with upstream but > may need a little work (I'm not sure whether the fallback lt_dlopen() is > necessary and my C++ dates from before the STL so you can tell how rusty I am.) > I'll submit my spec file as well so you can diff for the changes. Thanks, but no thanks see above for the reasoning.
the .la files here should be ok as-is.
With the change to the spec and the explanations here (and on IRC), I'm happy. APPROVED
Not sure why you're removing lib*.so, but I'm sure you have a good reason. (:
(In reply to comment #15) > With the change to the spec and the explanations here (and on IRC), I'm happy. > > APPROVED Thanks! (In reply to comment #16) > Not sure why you're removing lib*.so, but I'm sure you have a good reason. (: The .la files which are used / openened by pinball refer to the .so.x files, not to the .so ones, so for loading the plugins they are not nescesarry, since these .so files are plugins and libs they are not meant to be linked against either, so the .so files aren't nescesarry for that either. Thus they are completly unused, hence I remove them. I must say I didn't look what other ltdl using packages do with the .so files.
Imported and build, closing.