Bug 222648 - Review Request: audacious-plugin-fc - Future Composer plugin for Audacious
Review Request: audacious-plugin-fc - Future Composer plugin for Audacious
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
All Linux
medium Severity medium
: ---
: ---
Assigned To: Jason Tibbitts
Fedora Package Reviews List
Depends On:
  Show dependency treegraph
Reported: 2007-01-15 09:46 EST by Michael Schwendt
Modified: 2007-11-30 17:11 EST (History)
1 user (show)

See Also:
Fixed In Version: 0.2-1
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Last Closed: 2007-07-05 15:24:36 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
tibbs: fedora‑review+
kevin: fedora‑cvs+

Attachments (Terms of Use)

  None (edit)
Description Michael Schwendt 2007-01-15 09:46:22 EST
Spec URL: http://home.arcor.de/ms2002sep/tmp/audacious-plugin-fc.spec
SRPM URL: http://home.arcor.de/ms2002sep/tmp/audacious-plugin-fc-0.1-1.src.rpm
Description: This is an input plugin for Audacious which can
play back Future Composer music files from AMIGA. Song-length
detection and seek are implemented, too.

Note: May take a while till source tarball is available at SF.net
everywhere. As I'm upstream for the old source code, here's a detached
sig: http://home.arcor.de/ms2002sep/tmp/audacious-plugin-fc-0.1.tar.bz2.asc

Input files for testing:
Comment 1 Martin Sourada 2007-01-17 16:57:14 EST
* rpmlint not silent:
  E: audacious-plugin-fc no-changelogname-tag
  E: audacious-plugin-fc unknown-key GPG#b8af1c54
- package naming guidelines met
- spec file name matches %%{name}
* packaging guidelines not met:
  - missing %%changelog entries in spec file
  - (slightly different build root from the preferred one)
- Package licensed with GPL compatible license
- %%doc section OK
- spec written in American English
- spec file legible (only you should remove in the %%install section "mkdir -p
$RPM_BUILD_ROOT" - it's not necessary; and as mentioned above, %%changelog missing)
- source match upstream (613c456b525d0f5ebce912a981e57264 
- builds successfully on at least i386 FC6
- all build dependences listed
- locales handled properly (none present)
- no shared libraries
- package is not relocatable
- package owns all its directories
- dirs not owned is owned by required package(s)
- no duplicates
- file permissions set properly
- contains proper %%clean section
- macros used consistently
- package contains code
- no large docs
- no -devel subpackage
- no pkgconfig files
- no libraries with suffix, no static libraries
- not a GUI app
- package does not own dirs or files owned by other packages
- builds in mock

====Things need to be done====
Add %%changelog section to your specfile:

You should change build root to the prefered one:

As mentioned above, you should remove from the %%install section "mkdir -p
$RPM_BUILD_ROOT". It is not necessary.

I am not sponsored. This pre-review is to help me get sponsored.
Comment 2 Till Maas 2007-01-17 18:03:55 EST
Source:	http://download.sourceforge.net/xmms-fc/audacious-plugin-fc-0.1.tar.bz2
So you do not need to change the Source with every new version.

Consider changing the release-tag to:
Release:        1%{?dist}

See: http://fedoraproject.org/wiki/Packaging/DistTag

And I noticed, that the other audacious plugins in extras have audacious-plugins
(plural) as prefix not only audacious-plugin (singular), e.g.
Comment 3 Michael Schwendt 2007-01-17 18:32:49 EST
All valid points.

* changed "Buildroot"
* changed "Source"
* added %changelog for this revision
* changed %install

Spec URL: http://home.arcor.de/ms2002sep/tmp/audacious-plugin-fc.spec
SRPM URL: http://home.arcor.de/ms2002sep/tmp/audacious-plugin-fc-0.1-2.src.rpm

* "audacious-plugins" is a collection of multiple plug-ins, where
the sub-package names are inherited from the main package. Instead,
the packager could have named the sub-packages "audacious-plugin-foo"
explicitly. This package contains a single plugin, and its upstream
name is audacious-plugin-fc, too.

* %{?dist} can be added in CVS.
Comment 4 Michael Schwendt 2007-03-28 05:35:19 EDT
An update only when Audacious 1.3 in FE devel cvs is releaased:

Spec URL: http://home.arcor.de/ms2002sep/tmp/audacious-plugin-fc.spec
SRPM URL: http://home.arcor.de/ms2002sep/tmp/audacious-plugin-fc-0.2-1.src.rpm
Comment 5 Jason Tibbitts 2007-06-28 21:10:47 EDT
Well, 1.3 has been in rawhide for ages now, so....
Comment 6 Jason Tibbitts 2007-06-28 22:11:16 EDT
You shouldn't need a build dependency on pkgconfig; audacious-devel should have
its own dependency (and indeed it does).  But of course it doesn't hurt anything.

It seems there's directory ownership problem; /usr/lib/audacious/Input is owned
by audacious-plugins, but they're not required by this package.  This seems like
the kind of thing the base audacious package should own.

* source files match upstream:
* package meets naming and versioning guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
* summary is OK.
* description is OK.
* build root is OK.
* license field matches the actual license.
* license is open source-compatible.
* license text included in package.
* latest version is being packaged.
* BuildRequires are proper.
* compiler flags are appropriate.
* %clean is present.
* package builds in mock (development, x86_64).
* package installs properly
* debuginfo package looks complete.
* rpmlint is silent.
* final provides and requires are sane:
   audacious-plugin-fc = 0.2-1
   audacious >= 1.3
* %check is not present; no upstream test suite.  I've no ability to actually 
   test this as I have no speakers on this machine.
* no shared libraries are added to the regular linker search paths.
X Ownership issues with /usr/lib/audacious/Input.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
* no scriptlets present.
* code, not content.
* documentation is small, so no -docs subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.
* no headers.
* no pkgconfig files.
* no static libraries.
* no libtool .la files.
Comment 7 Michael Schwendt 2007-06-29 03:43:48 EDT
> X Ownership issues with /usr/lib/audacious/Input

Cannot confirm.
The base audacious package requires audacious-plugins.
This plugin package requires the base audacious package.

*However*, in general, any other multimedia software could
implement and use Audacious' plugins, so directory ownership is
not trivial to decide on. Probably it could only be fixed with
an audacious-base package or similar "bloat".

> BR pkgconfig

This package directly buildrequires pkgconfig for its m4 code.

Eliminating BR based on what some arbitrary package in the dep-chain
pulls in cannot be recommended, because e.g. audacious-devel may stop
using pkgconfig any time.
Comment 8 Jason Tibbitts 2007-06-29 10:24:52 EDT
> The base audacious package requires audacious-plugins.

Of course; I didn't chase the dependencies that far.

> This package directly buildrequires pkgconfig for its m4 code.

OK, that's not the usual usage, and I didn't read the actual code.

Comment 9 Michael Schwendt 2007-06-29 18:13:06 EDT
New Package CVS Request
Package Name: audacious-plugin-fc
Short Description: Future Composer input plugin for Audacious
Owners: bugs.michael@gmx.net
Branches: F-7
Comment 10 Kevin Fenzi 2007-07-02 14:39:23 EDT
cvs done.
Comment 11 Fedora Update System 2007-07-03 12:20:54 EDT
audacious-plugin-fc-0.2-1 has been pushed to the Fedora 7 testing repository.  If problems still persist, please make note of it in this bug report.
Comment 12 Fedora Update System 2007-07-05 15:24:33 EDT
audacious-plugin-fc-0.2-1 has been pushed to the Fedora 7 stable repository.  If problems still persist, please make note of it in this bug report.

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