[Bug 280751] Review Request: qmmp - Qt-based multimedia player

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: qmmp - Qt-based multimedia player


https://bugzilla.redhat.com/show_bug.cgi?id=280751





------- Additional Comments From kvolny at redhat.com  2007-11-19 06:22 EST -------
(In reply to comment #10)
> * buildroot does not match the buildroot mandated by the guidelines

there is:

BuildRoot:      %{_tmppath}/%{name}-%{version}-%{release}-root

reading the guidelines 
http://fedoraproject.org/wiki/Packaging/Guidelines#head-b4fdd45fa76cbf54c885ef0836361319ab962473

The recommended values for the BuildRoot tag are (in descending order of 
preference) : 
%(mktemp -ud %{_tmppath}/%{name}-%{version}-%{release}-XXXXXX)
%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
%{_tmppath}/%{name}-%{version}-%{release}-root

so it matches exactly the third line from the guidelines, what is the problem?

> * remove "* MPEG1 layer 1/2/3 support;" from %description

done

> * BuildRequires line longer then 80 chars, please split this up in multiple
>   lines each starting with BuildRequires:
> * desktop-file-install line longer then 80 chars, please split this up in
>   multiple lines.

done

btw, where comes this requirement from? - reading the guidelines, I see that 
only the Description cannot have lines longer than 80 chars 
(http://fedoraproject.org/wiki/Packaging/Guidelines#head-ef67b32cfe3903b0aaab1b3c920940769007da6a)

> * use %defattr(-,root,root,-)

done

... but this produces the following rpmlint error:
qmmp.x86_64: E: non-readable /usr/bin/qmmp 0601

I am confused where this comes from, since the compiled qmmp binary has 755 
and I see no change on install of that file

unfortunately, the executable not being readable means that the included 
resources are unusable, effectively disabling translations

> * put %doc directly under %defattr

done

btw, once again, where this requirement comes from?

> * drop %{?_smp_mflags}, if it fails on some systems it must be dropped, the 
fact
>   that it happens to work on others is not relevant, we don't want a 
lottery, we
>   want reproducable builds

ok ... but I thought that it would be better to fix one b0rk3n machine than 
crippling the build everywhere

> * add: "Requires(post): /sbin/ldconfig" 
and "Requires(postun): /sbin/ldconfig"

done

btw, is that documented somewhere? - I took inspiration from another packages 
providing libraries and did not see that, so I expect there is a _lot_ of 
packages broken this way in the repos

> * All these lines:
>   %dir %{_libdir}/qmmp
>   %dir %{_libdir}/qmmp/Input
>   %dir %{_libdir}/qmmp/Output
>   %{_libdir}/qmmp/Input/*.so
>   %{_libdir}/qmmp/Output/*.so
>   Can be written as just:
>   %{_libdir}/qmmp
>   Notice that you will still need:
>   %{_libdir}/libqmmp.so
>   Listed seperately

done

> * Drop the second %defattr line

done

> Possible improvements:
> ----------------------
> * Use single quotes instead of double quotes around your sed scripts so that
>   you do not have to use \ infront of " inside the scripts.

done, thanks

new files:
Spec URL: http://sn.bluehost.cz/tmp/no-mp3/qmmp.spec
SRPM URL: http://sn.bluehost.cz/tmp/no-mp3/qmmp-0.1.4-4.fc8.src.rpm

however, this is unusable because of the abovementioned problem with 
permissions :-(

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.