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.
I think you should submit plugins as a different review request package instead to review both packages here.
Ok. I will like to review this package.
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.
(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.
So you can resubmit new package audacious-1.2.1-0.6
I am not exactly with you here. The revised SRPM and spec files are available at the URL I gave in comment #4.
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.
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.
(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"
Ok then. I am off now will review would be modified state of this package/packages once reporter submits those changes.
I have added bug 215165 which deals with audacious-plugins
Ralf, Are you interested in splitting plugings package? Are you working on it?
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.
(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.
(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).
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.
Dominik, If you sre still following this bug review then i would like to see your comment in bug 215165
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.
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)
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.
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.
Thanks for the review.
Hi that massive memory leak still exists on the FC6 version, any chance you can update FC6 to 1.2.1?
Nope. FC6 will get 1.2.2. I'm currently waiting for someone with the appropriate privileges to branch the plugins for FC6.
hehehe fooled me there for a second! Thx. :)