Bug 174866 - Review Request: polypaudio: Improved Linux sound server
Review Request: polypaudio: Improved Linux sound server
Status: CLOSED NOTABUG
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Greg DeKoenigsberg
Fedora Package Reviews List
http://0pointer.de/lennart/projects/p...
:
: 174944 (view as bug list)
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2005-12-02 18:19 EST by Tom "spot" Callaway
Modified: 2007-11-30 17:11 EST (History)
4 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-06-14 10:54:58 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description Tom "spot" Callaway 2005-12-02 18:19:05 EST
Spec Name or Url: http://auroralinux.org/people/spot/review/polypaudio.spec
SRPM Name or Url: http://auroralinux.org/people/spot/review/polypaudio-0.7-1.src.rpm
Description:
polypaudio is a sound server for Linux and other Unix like operating
systems. It is intended to be an improved drop-in replacement for the
Enlightened Sound Daemon (ESOUND).
Comment 1 Rex Dieter 2005-12-04 14:50:32 EST
*** Bug 174944 has been marked as a duplicate of this bug. ***
Comment 2 Brian Pepple 2005-12-08 00:23:45 EST
NEEDSWORK

MD5Sums:
1c3693ab9c6904dbed6dfa7656778de4  polypaudio-0.7.tar.gz

Good:
* Source URL is canonical
* Upstream source tarball verified
* Package name conforms to the Fedora Naming Guidelines
* Group Tag is from the official list
* Buildroot has all required elements
* All paths begin with macros
* All directories are owned by this or other packages
* No deprecated fields used
* All desired features are enabled
* Package rebuilds as non-root user
* Package installs and uninstalls cleanly on FC4. (After adding missing BR)

Needswork:
* Package needs additional BuildRequires:
Missing BR for xorg-x11-devel on FC4
Missing BR for libXt-devel & xorg-x11-proto-devel on FC5 (You might want to
verify that I'm correct on the necessary BR for FC5).

* Rpmlint errors:
W: polypaudio unstripped-binary-or-object /usr/bin/polypaudio
E: polypaudio setuid-binary /usr/bin/polypaudio root 04755
E: polypaudio non-standard-executable-perm /usr/bin/polypaudio 04755
E: polypaudio non-executable-script /etc/polypaudio/default.pa 0644
Comment 3 Christian Iseli 2005-12-08 09:12:24 EST
(In reply to comment #2)
> * Rpmlint errors:
> W: polypaudio unstripped-binary-or-object /usr/bin/polypaudio

Out of curiosity: did you build in mock ?
In my home setup, I have in .rpmmacros:
%debug_package  %{nil}
%_enable_debug_packages 0
and I noticed that it (apparently) causes the unstripped warnings in rpmlint...
Comment 4 Brian Pepple 2005-12-08 09:42:24 EST
(In reply to comment #3)
> Out of curiosity: did you build in mock ?

Yes. 

Comment 5 Paul Howarth 2005-12-08 09:46:16 EST
(In reply to comment #3)
> (In reply to comment #2)
> > * Rpmlint errors:
> > W: polypaudio unstripped-binary-or-object /usr/bin/polypaudio
> 
> Out of curiosity: did you build in mock ?
> In my home setup, I have in .rpmmacros:
> %debug_package  %{nil}
> %_enable_debug_packages 0
> and I noticed that it (apparently) causes the unstripped warnings in rpmlint...

That's to be expected. Turning off the separate debug package means that the
debug information is left in the main package, i.e. not stripped.
Comment 6 Ville Skyttä 2005-12-08 10:21:56 EST
setuid/setgid executables are not stripped by rpmbuild, see bug 117858.
Workaround: remove those bits in %install and restore in %files using %attr.
Comment 7 Christian Iseli 2005-12-08 11:45:02 EST
(In reply to comment #6)
> setuid/setgid executables are not stripped by rpmbuild, see bug 117858.

Ah, didn't know that.  Thanks!

> Workaround: remove those bits in %install and restore in %files using %attr.

Yup.  IMHO, this should be a SHOULD or even a MUST in the guidelines...
Comment 8 Michael A. Peters 2005-12-08 14:48:51 EST
(In reply to comment #7)
> (In reply to comment #6)
> > setuid/setgid executables are not stripped by rpmbuild, see bug 117858.
> 
> Ah, didn't know that.  Thanks!
> 
> > Workaround: remove those bits in %install and restore in %files using %attr.
> 
> Yup.  IMHO, this should be a SHOULD or even a MUST in the guidelines...
> 

Agreed - an rpm should be buildable by unprivileged users, which means those
bits have to be set with %attr

You may also need to patch the Makefile to not set them (to get it to build as a
non root user)
Comment 9 Ville Skyttä 2005-12-08 15:44:02 EST
Note that packages that don't insist doing chown/chgrp but do chmod
setuid/setgid are unaffected wrt. being buildable by unprivileged users, and the
'-'s in eg. %defattr(-,root,root,-) will do the right thing regarding the files'
ownership.  rpmbuild not stripping setuid/setgid binaries is a different,
orthogonal issue.
Comment 10 Christian Iseli 2005-12-08 17:45:13 EST
(In reply to comment #9)
> Note that packages that don't insist doing chown/chgrp but do chmod
> setuid/setgid are unaffected wrt. being buildable by unprivileged users, and the
> '-'s in eg. %defattr(-,root,root,-) will do the right thing regarding the files'
> ownership.

After building and installing the package, I get (on my FC4, x86_64 box):
root: ls -al /usr/bin/polypaudio
-rwsr-xr-x  1 root root 632416 Dec  8 23:35 /usr/bin/polypaudio

So, I stand by my (humble) opinion that the guidelines should mandate no
chown/chmod in %install, and use of %attr in %files.  This way, it is much
clearer which packages introduce suid binaries.

> rpmbuild not stripping setuid/setgid binaries is a different,
> orthogonal issue.

Right.  And quite useful as a warning :-)

BTW, rpmlint on x86_64 is noisy about rpath:
E: polypaudio binary-or-shlib-defines-rpath /usr/bin/polypaudio ['/usr/lib64']
E: polypaudio binary-or-shlib-defines-rpath /usr/bin/paplay ['/usr/lib64']
E: polypaudio binary-or-shlib-defines-rpath /usr/bin/pacat ['/usr/lib64']
E: polypaudio binary-or-shlib-defines-rpath /usr/bin/pactl ['/usr/lib64']
Comment 11 Pierre Ossman 2006-02-16 15:06:22 EST
(shameless plug)

This might be of interest: :)

http://article.gmane.org/gmane.linux.alsa.devel/31738
Comment 12 Pierre Ossman 2006-02-24 11:49:02 EST
What's the status of this? The links in the initial comment are dead. I'd really
like to see this included in Fedora.
Comment 13 Tom "spot" Callaway 2006-02-25 10:22:01 EST
Spec Name or Url: http://auroralinux.org/people/spot/review/polypaudio.spec
SRPM Name or Url: http://auroralinux.org/people/spot/review/polypaudio-0.7-2.src.rpm

Fixed packages resolve the unstripped binary. The rpath issues on x86_64 are
much harder to resolve, as removing the rpath calls causes it to explode on that
architecture, and an rpath of /usr/lib64 is not fatal, just a bit redundant.
Comment 14 Pierre Ossman 2006-02-25 10:28:11 EST
This multiple architecture business is a bit new to me, so I'm unclear to what
the problem is. Does library paths get stored inside the resulting binaries?

If you can point me in the right direction I can try to remedy the situation. :)
Comment 15 Tom "spot" Callaway 2006-02-25 10:35:27 EST
Yep, that's exactly what it is. Google for rpath, and grep through the
polypaudio source tree to see it in action.
Comment 16 Pierre Ossman 2006-02-25 15:41:53 EST
This seems to be a libtool issue since only x86_64 gets the rpath set. I also
examined the command sent to libtool and it contained no rpath. The resulting
gcc command does though. I tried googling the issue but didn't see anything to
help me solve it. Do you have any pointers?
Comment 17 Pierre Ossman 2006-03-03 06:57:09 EST
Further investigation shows that this is indeed a libtool problem (in
combination with gcc not giving a complete picture on multilib systems). It is
currently unresolved, so unless we do a dirty hack the RPATH is here to stay.

I'd also like to do some suggestions for packaging. There should be at least
four packages:

 * libpolyp - Client libraries.
 * libpolyp-devel - Headers and such for building clients
 * polypaudio - The daemon.
 * polypaudio-devel - Headers and such for building daemon modules.

Possibly one could also package the modules in their own packages to minimise
the dependencies.

As for separating libpolyp-devel files from polypaudio-devel files you have to
look in the Makefile for the 0.7 release. The 0.8 release will install these in
separate locations so it will be much more clear there.
Comment 18 Pierre Ossman 2006-05-09 14:10:38 EDT
Spec Name or Url: http://homes.hades.drzeus.cx/~drzeus/polypaudio/polypaudio.spec
SRPM Name or Url:
http://homes.hades.drzeus.cx/~drzeus/polypaudio/polypaudio-0.8.1-1.src.rpm

Updated to the current version (0.8.1). I've also created a whole bunch of
sub-packages to minimise the dependencies.
Comment 19 Pierre Ossman 2006-05-29 06:41:48 EDT
Spec Name or Url: http://homes.hades.drzeus.cx/~drzeus/polypaudio/polypaudio.spec
SRPM Name or Url:
http://homes.hades.drzeus.cx/~drzeus/polypaudio/polypaudio-0.9.0-1.src.rpm

Updated again. Anyone but me still interested in this?
Comment 20 Pierre Ossman 2006-05-29 06:42:30 EDT
Spec Name or Url: http://homes.drzeus.cx/~drzeus/polypaudio/polypaudio.spec
SRPM Name or Url:
http://homes.drzeus.cx/~drzeus/polypaudio/polypaudio-0.9.0-1.src.rpm

Bad URLs, sorry about that.
Comment 21 Pierre Ossman 2006-05-30 02:53:07 EDT
Spec Name or Url: http://homes.drzeus.cx/~drzeus/polypaudio/polypaudio.spec
SRPM Name or Url:
http://homes.drzeus.cx/~drzeus/polypaudio/polypaudio-0.9.0-2.src.rpm

Fixed some rpmlint problems. Remaining warnings:

polypaudio-0.9.0-2.i386.rpm:
E: polypaudio setuid-binary /usr/bin/polypaudio root 04755
E: polypaudio non-standard-executable-perm /usr/bin/polypaudio 04755
E: polypaudio non-executable-script /etc/polypaudio/default.pa 0644

polypaudio-devel-0.9.0-2.i386.rpm:
W: polypaudio-devel no-documentation

polypaudio-lib-glib-0.9.0-2.i386.rpm:
W: polypaudio-lib-glib no-documentation

polypaudio-lib-glib2-0.9.0-2.i386.rpm:
W: polypaudio-lib-glib2 no-documentation

polypaudio-module-alsa-0.9.0-2.i386.rpm:
W: polypaudio-module-alsa no-documentation

polypaudio-module-lirc-0.9.0-2.i386.rpm:
W: polypaudio-module-lirc no-documentation

polypaudio-module-x11-0.9.0-2.i386.rpm:
W: polypaudio-module-x11 no-documentation

polypaudio-utils-0.9.0-2.i386.rpm:
E: polypaudio-utils invalid-soname /usr/lib/libpolypdsp.so libpolypdsp.so
W: polypaudio-utils no-documentation
Comment 22 Jason Tibbitts 2006-06-14 10:54:58 EDT
Adding back the comments and status changes that were lost in the crash.  Note
that I might not have copies of the new review tickets that were opened since I
wasn't CC'd; could whoever did those add them back?

------- Additional Comments From tibbs@math.uh.edu  2006-06-10 16:29 EST -------
I am interested in this simply because I'd like to see some action on all of the
old review tickets. However, I'm not sure if this is spot's ticket or Pierre's
ticket.  Perhaps one of you could review the other's package, or if spot wishes
to drop his review request, this could be closed NOTABUG and Pierre could open a
new review request with his package.

BTW, 0.9.1 seems to be out now.

------- Additional Comments From drzeus-bugzilla@drzeus.cx  2006-06-10 20:34 EST
-------
I'm fine either way. The packages I've been putting up are based on spot's anyway.

I have a new RPM ready for 0.9.1 as well as for some Polypaudio utils. I've been
holding off on them until I saw some activity here.

------- Additional Comments From tcallawa@redhat.com  2006-06-11 15:19 EST -------
Pierre: Go ahead and open a new review request for this package. 

------- Additional Comments From drzeus-bugzilla@drzeus.cx  2006-06-13 09:27 EST
-------
New request opened as bug 194957.

(Note that the new request bug number is incorrect due to being lost in the crash.)
Comment 23 Pierre Ossman 2006-06-14 11:02:32 EDT
I'll readd the new requests. Should I add you as cc for them directly?
Comment 24 Pierre Ossman 2006-06-14 11:21:56 EDT
New request opened as bug 195221.

Note You need to log in before you can comment on or make changes to this bug.