[Bug 2037] Review request: acoustid-fingerprinter - Music AcoustID fingerprinting application
https://bugzilla.rpmfusion.org/show_bug.cgi?id=2037
--- Comment #7 from Ismael Olea <ismael(a)olea.org> 2012-01-04 13:51:45 CET ---
(In reply to comment #6)
=== Issues ===
1. Package doesn't consistently use macros:
%{_bindir}/acoustid-fingerprinter
done
2. Buildroot, rm -rf %{buildroot} in %install, are not needed unless
you want
to build for EL5:
http://fedoraproject.org/wiki/Packaging:Guidelines#BuildRoot_tag
done
4. %{_datadir}/icons/hicolor is owned by
"hicolor-icon-theme". You must not own
it too.
done
Update your %files section with:
%{_datadir}/icons/hicolor/*/apps/%{name}.png
I've created and svg file and opted using it instead bitmaps
5. You must have a 'Requires: hicolor-icon-theme', which owns the
%{_datadir}/icons/hicolor directory.
Added, but I don't understand why it could need the application owning it, I
mean, it can cause any technical trouble I can remember now?
6. %defattr in %files are not needed:
https://fedoraproject.org/wiki/Packaging:Guidelines#File_Permissions
done
7. Why are you using an external desktop instead of the one provided
upstream
included in the tarball? They are identical.
I contributed it to upstream and forgot to remove. Done too.
Verifying the desktop file will show the following warning:
key "Encoding" in group "Desktop Entry" is deprecated
done and contributed patch to upstream
Your scriptlet is not the one provided in the wiki page. It misses
the
%posttrans section
ups! done
10. Icons could be installed with a for loop (really NOT mandatory at
all but
the spec file would be more readable though):
as said, substituted by svg file.
11. The spec file should not use an hard-coded directory name and it
must not
mix hard-coded and macro paths:
https://fedoraproject.org/wiki/Packaging:Guidelines#Macros
Therefore:
-DCMAKE_INSTALL_PREFIX=/usr/
Should be:
-DCMAKE_INSTALL_PREFIX=%{_prefix}
fixed
12. Anyway -DCMAKE_INSTALL_PREFIX=/usr/ is useless. It's already
the default,
just strip it.
striping it doesn't work, so I left the _prefix as suggested
13. The sources contain two upstream ffmpeg header files in the /ffmpeg
directory. Please patch the source to use system header files.
both files are strongly modified from their upstream so I kept them.
14. The package doesn't work properly on F-16+ (with qt 4.8). You
cannot submit
fingerprints. An upstream patch is here:
https://github.com/lalinsky/acoustid-fingerprinter/commit/7577c13c02bcbf4...
I missed that. Added.
15. %{optflags} are partly overwritten. On x86_64 I can see:
"-O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector
--param=ssp-buffer-size=4 -m64 -mtune=generic -O3"
Please notice the -O3 at the end that overwrites the -O2 at the beginning.
Yep, but after examining CMakeLists.txt seems to be in some way intentional and
as I can't imagine any collateral efect I left it pristine.
Updated files:
http://olea.org/paquetes-rpm/fedora-16/acoustid-fingerprinter-0.5-2.fc16....
http://olea.org/tmp/acoustid-fingerprinter.spec
--
Configure bugmail: https://bugzilla.rpmfusion.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.