[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 j.w.r.degoede at hhs.nl  2007-11-19 07:33 EST -------
(In reply to comment #12)
> (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?
> 

Ah, the third line in the guidelines is new (AFAIK) in the past only the first 2
were allowed, my bad.

> > * 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 
> 

Readability in a terminal based editor, AFAIK everyone wraps all lines in the
spec files at 80 chars.


> > * 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
> 

Nasty, I'll try to reproduce this and see if I can come up with a fix.

> > * put %doc directly under %defattr
> 
> done
> 
> btw, once again, where this requirement comes from?
> 

Readability / consistency, this is how everyone (almost everyone) else does it.

> > * 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
> 

Its not a broken machine %{?_smp_mflags} with badly written makefiles will break
builds depending on timing, its a race condition problem, which depends on
timing, which will change depeding on workload and speed of the machine.

> > * 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
> 

Normally for libraries you use:
%post -p /sbin/ldconfig

And then /sbin/ldconfig gets used as script interpreter (interpreting an empty
script), which will automatically add the Requires.

> 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 :-(

Good work sofar I'll see if I can recreate the permission problem and figure out
whats going on.


-- 
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.