Bug 214818 - Review Request: audacious - A GTK2 based media player similar to xmms
Review Request: audacious - A GTK2 based media player similar to xmms
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Parag AN(पराग)
Fedora Package Reviews List
:
Depends On:
Blocks: FE-ACCEPT 215165
  Show dependency treegraph
 
Reported: 2006-11-09 11:57 EST by Ralf Ertzinger
Modified: 2007-11-30 17:11 EST (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-11-26 15:12:47 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)

  None (edit)
Description Ralf Ertzinger 2006-11-09 11:57:10 EST
Spec URL: http://www.skytale.net/files/audacious/audacious.spec
SRPM URL: http://www.skytale.net/files/audacious/audacious-1.2.1-0.5.sky.src.rpm

Spec URL: http://www.skytale.net/files/audacious/audacious-plugins.spec
SRPM URL: http://www.skytale.net/files/audacious/audacious-plugins-1.2.2-0.5.sky.src.rpm

Description:
Audacious is a media player that currently uses a skinned
user interface based on Winamp 2.x skins. It is based on ("forked off")
BMP.


This is a re-review request for audacious and audacious-plugins. I already maintain audacious in FE. For this version upstream has split the original single package into two, one containing the player and one containing the plugins. Due to this largish change both packages are offered for rereview here before the new version is imported into FE.
Comment 1 Parag AN(पराग) 2006-11-11 05:46:24 EST
I think you should submit plugins as a different review request package instead
to  review both packages here.
Comment 2 Parag AN(पराग) 2006-11-11 11:39:51 EST
Ok. I will like to review this package.
Comment 3 Parag AN(पराग) 2006-11-11 12:09:02 EST
This package needs a lot to do still to go in FE.

rpmlint -iv audacious-1.2.1-0.5.src.rpm gave

W: audacious macro-in-%changelog 20
Macros are expanded in %changelog too, which can in unfortunate cases lead
to the package not building at all, or other subtle unexpected conditions that
affect the build.  Even when that doesn't happen, the expansion results in
possibly "rewriting history" on subsequent package revisions and generally
odd entries eg. in source rpms, which is rarely wanted.  Avoid use of macros
in %changelog altogether, or use two '%'s to escape them, like '%%foo'.

W: audacious macro-in-%changelog 20
Macros are expanded in %changelog too, which can in unfortunate cases lead
to the package not building at all, or other subtle unexpected conditions that
affect the build.  Even when that doesn't happen, the expansion results in
possibly "rewriting history" on subsequent package revisions and generally
odd entries eg. in source rpms, which is rarely wanted.  Avoid use of macros
in %changelog altogether, or use two '%'s to escape them, like '%%foo'.

W: audacious patch-not-applied Patch2: audacious-1.1.0-no-rpath.patch
A patch is included in your package but was not applied. Refer to the patches
documentation to see what's wrong.

W: audacious patch-not-applied Patch5: audacious-1.1.0-amidi-backend.patch
A patch is included in your package but was not applied. Refer to the patches
documentation to see what's wrong.

W: audacious patch-not-applied Patch8: audacious-1.1.1-playlist-twenty.patch
A patch is included in your package but was not applied. Refer to the patches
documentation to see what's wrong.


rpmlint on audacious-libs-1.2.1-0.5.i386.rpm
W: audacious-libs one-line-command-in-%post /sbin/ldconfig
You should use %post -p <command> instead of using:

%post
<command>

It will avoid the fork of a shell interpreter to execute your command as
well as allows rpm to automatically mark the dependency on your command.

W: audacious-libs one-line-command-in-%postun /sbin/ldconfig
You should use %postun -p <command> instead of using:

%postun
<command>

It will avoid the fork of a shell interpreter to execute your command as
well as allows rpm to automatically mark the dependency on your command.


rpmlint on audacious-1.2.1-0.5.i386.rpm
I: audacious checking
W: audacious incoherent-version-in-changelog 1.2.1-0.5.fc7 1.2.1-0.5
The last entry in %changelog contains a version identifier that is not
coherent with the epoch:version-release tuple of the package.

I HOPE ALL rpmlint warnings and errors are SELF-EXPLANATORY here.
Resubmit package solving above errors and warnings.
Comment 4 Ralf Ertzinger 2006-11-11 12:34:27 EST
(In reply to comment #3)

Most of 

> rpmlint on audacious-1.2.1-0.5.i386.rpm
> I: audacious checking
> W: audacious incoherent-version-in-changelog 1.2.1-0.5.fc7 1.2.1-0.5
> The last entry in %changelog contains a version identifier that is not
> coherent with the epoch:version-release tuple of the package.

That one will go away on it's own when built in the FE build system.
Set %dist to 'fc7' in your build environment to create e-v-r tags
like those created by the FE build system.

The rest of them have been fixed in audacious-1.2.1-0.6, available at
http://www.skytale.net/files/audacious.
Comment 5 Parag AN(पराग) 2006-11-11 12:56:02 EST
So you can resubmit new package audacious-1.2.1-0.6
Comment 6 Ralf Ertzinger 2006-11-11 13:00:25 EST
I am not exactly with you here. The revised SRPM and spec files are available at
the URL I gave in comment #4.
Comment 7 Parag AN(पराग) 2006-11-11 13:21:26 EST
I have not seen any review request that contains 2 packages in a single request.
So i think you an create a new review request for plugins package and make its
dependent on this package. 
Comment 8 Dominik 'Rathann' Mierzejewski 2006-11-11 13:53:15 EST
I would suggest splitting off the more exotic plugins (i.e. the ones that bring
in uncommon dependencies) into separate packages. Namely: fluidsynth amidi
backend, adplug input plugin, modplug input plugin, musepack input plugin,
timidity input plugin, and all output plugins except alsa/oss.

What's the point of splitting off -libs from audacious?

Also, I agree that there should be two separate review requests, one for the
player and one for the plugins.
Comment 9 Thorsten Leemhuis 2006-11-11 13:54:49 EST
(In reply to comment #7)
> I have not seen any review request that contains 2 packages in a single request.

There are some, and I always wondered if we should {dis,}allow this or if we
simply don't care and handle it on a case by cases basis.

/me votes for "encourage people to file one review bug per package"
Comment 10 Parag AN(पराग) 2006-11-11 14:05:00 EST
Ok then. I am off now will review would be modified state of this
package/packages  once reporter submits those changes.
Comment 11 Ralf Ertzinger 2006-11-11 14:38:34 EST
I have added bug 215165 which deals with audacious-plugins
Comment 12 Parag AN(पराग) 2006-11-13 06:42:12 EST
Ralf,
     Are you interested in splitting plugings package? Are you working on it?
Comment 13 Ralf Ertzinger 2006-11-13 06:50:43 EST
Hmm. It seems like bugzilla has eaten my reply to comment #8.

Short version:
Splitting off additional plugins in separate packages is hard because of the
upgrade path, so while I am interested in splitting of packages in general (and
have done so with the new pulseaudio output plugin in the new audacious-plugins)
splitting off plugins which are contained in the current audacious-1.1.x
packages will possibly not happen. Definitely not during this update.

The -libs subpackage was introduced to prevent circular dependencies between
audacious and audacious-plugins.
Comment 14 Dominik 'Rathann' Mierzejewski 2006-11-13 16:41:39 EST
(In reply to comment #13)
> Hmm. It seems like bugzilla has eaten my reply to comment #8.
> 
> Short version:
> Splitting off additional plugins in separate packages is hard because of the
> upgrade path, so while I am interested in splitting of packages in general
> (and have done so with the new pulseaudio output plugin in the new
> audacious-plugins) splitting off plugins which are contained in the current
> audacious-1.1.x packages will possibly not happen. Definitely not during this
> update.

Long version please. I'm not convinced it can't be done.

> The -libs subpackage was introduced to prevent circular dependencies between
> audacious and audacious-plugins.

Ok, but is it necessary for audacious to require audacious-plugins? Granted,
it's not usable without them, but it works.
Comment 15 Ralf Ertzinger 2006-11-14 03:37:46 EST
(In reply to comment #14)

> Long version please. I'm not convinced it can't be done.

I do not say that it can not be done. I say that I do not think that it is worth
all the trouble it will bring.

> Ok, but is it necessary for audacious to require audacious-plugins? Granted,
> it's not usable without them, but it works.

If users install a media player they expect it to play media. If users update an
existing version of a media player they expect it to play the same media it did
before.

Breaking this expectations will lead to bug reports (completely unnecessary
ones, in my opinion).
Comment 16 Parag AN(पराग) 2006-11-14 04:02:13 EST
anyway what i can say right now is that packaging of this package is ok and i
found no rpmlint errors and also mock build worked fine. But audacious plugins
package failed in mock build it needs libglade as BR.
Comment 17 Parag AN(पराग) 2006-11-16 04:31:38 EST
Dominik,
 If you sre still following this bug review then i would like to see your
comment in bug 215165
Comment 18 Christopher Stone 2006-11-24 11:45:45 EST
Thought I'd let you know that I have about 1000 ogg files in my directory, and
running an x86_64 FC6 system.  When I run audacious (the version that is now in
extras) I get *massive* memory leaks.  I have 2GB of memory, and about 20% is
used  normally, but after running audacious for a few minutes selecting several
songs my memory usage goes up to about 50%.  After even more usage my sytem
eventually starts thrashing to the point where I have to basically press the
reset button on my PC and reboot.
Comment 19 Ralf Ertzinger 2006-11-24 11:51:24 EST
Does it happen with the 386 version, too?

My current playlist contains roughly 3000 songs (ogg and mp3 mixed) and I can
run it for hours. If it did eat all my memory I'd have noticed, I think.

Can you try to rebuild the SRPM reviewed here on x86_64 (please remember that
you will need audacious-plugins, too)
Comment 20 Christopher Stone 2006-11-24 12:22:02 EST
I rebuilt the current rpms in this bug report and I can no longer reproduce the
memory leak. :)

One other problem I notice is that bmp used to remember the last song I selected
when I opened the file open dialog box.  audacious no longer remembers the last
song I selected and when the file open dialog box is opened it always starts at
the top of the directory list.
Comment 21 Parag AN(पराग) 2006-11-25 12:45:19 EST
Ok here comes Final Review.
Review:
+ package builds in mock (development i386) for FC6.
+ rpmlint is silent for RPM and SRPM.
+ source files match upstream.
f718f66ec0cab46bf6210d2243d12be1   audacious-1.2.1.tgz
+ package meets naming and packaging guidelines.
+ specfile is properly named and is cleanly written.
+ Spec file is written in American English.
+ Spec file is legible.
+ dist tag is present.
+ build root is correct.
+ license is open source-compatible.  License text included in package.
+ %doc is small; no -doc subpackage required.
+ %doc does not affect runtime.
+ COPYING included in %doc.
+ BuildRequires are proper.
+ %clean is present.
+ package installed properly.
+ Macro use appears rather consistent.
+ Package contains code, not content.
+ no headers or static libraries.
+ .pc files present in -devel
+ -devel and -libs subpackage exists
+ no .la files.
+ translations are available.
+ update-desktop-database is used at postun and post
+ called /sbin/ldconfig on libs package.
+ owns the directories it creates.
+ doesn't own any directories it shouldn't.
+ no duplicates in %files.
+ file permissions are appropriate.
+ Desktop file installed successfully
+ Desktop file is handled correctly in SPEC file.
+ GUI application
APPROVED.
Comment 22 Ralf Ertzinger 2006-11-25 14:59:09 EST
Thanks for the review.
Comment 23 Christopher Stone 2006-12-02 15:18:02 EST
Hi that massive memory leak still exists on the FC6 version, any chance you can
update FC6 to 1.2.1?
Comment 24 Ralf Ertzinger 2006-12-02 15:31:36 EST
Nope.








FC6 will get 1.2.2. I'm currently waiting for someone with the appropriate
privileges to branch the plugins for FC6.
Comment 25 Christopher Stone 2006-12-02 16:01:55 EST
hehehe fooled me there for a second! 

Thx. :)

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