Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 199021 - Review Request: zynaddsubfx - Real-time software synthesizer
Summary: Review Request: zynaddsubfx - Real-time software synthesizer
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Jason Tibbitts
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On: 199016
Blocks: FE-ACCEPT
TreeView+ depends on / blocked
 
Reported: 2006-07-15 20:25 UTC by Anthony Green
Modified: 2007-11-30 22:11 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2006-09-02 07:44:18 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)

Description Anthony Green 2006-07-15 20:25:36 UTC
Spec URL: http://people.redhat.com/~green/FE/FC5/zynaddsubfx.spec
SRPM URL: http://people.redhat.com/~green/FE/FC5/zynaddsubfx-2.2.1-5.src.rpm
Description: ZynAddSubFX is a opensource software synthesizer capable of making a
countless number of instruments, from some common heared from
expensive hardware to interesting sounds that you'll boost to an
amazing universe of sounds.

This package was derived from the latest planetccrma package.
It depends on mxml, which I've also submitted for review.

Comment 1 Jason Tibbitts 2006-07-21 00:36:34 UTC
Looks like there's the dreaded rpath problem:
  E: zynaddsubfx binary-or-shlib-defines-rpath /usr/bin/controller ['/usr/lib64']
  E: zynaddsubfx binary-or-shlib-defines-rpath /usr/bin/zynaddsubfx ['/usr/lib64']
  E: zynaddsubfx binary-or-shlib-defines-rpath /usr/bin/spliter ['/usr/lib64']

Unfortunately I can't figure out where it's coming from.

Also, the recommended Fedora compilation flags don't seem to be used at all;
everything seems to be compiled with -O6.  Unless I'm missing the flags that are
coming out of the various -config programs.

Comment 2 Anthony Green 2006-07-21 01:13:29 UTC
Thanks.  I had a quick look and I can fix these tonight.



Comment 3 Anthony Green 2006-07-21 06:53:25 UTC
(In reply to comment #1)
> Looks like there's the dreaded rpath problem:
>   E: zynaddsubfx binary-or-shlib-defines-rpath /usr/bin/controller ['/usr/lib64']
>   E: zynaddsubfx binary-or-shlib-defines-rpath /usr/bin/zynaddsubfx ['/usr/lib64']
>   E: zynaddsubfx binary-or-shlib-defines-rpath /usr/bin/spliter ['/usr/lib64']
> 
> Unfortunately I can't figure out where it's coming from.

This was a bug in "fltk-config --ldflags" output.
 
> Also, the recommended Fedora compilation flags don't seem to be used at all;
> everything seems to be compiled with -O6.  

Fixed.  I found a "fltk-config --cflags" output as well.  Two bugs were filed
against fltk.

Updated bits here:
Spec URL: http://people.redhat.com/~green/FE/FC5/zynaddsubfx.spec
SRPM URL: http://people.redhat.com/~green/FE/FC5/zynaddsubfx-2.2.1-6.src.rpm

Thanks.

Comment 4 Jason Tibbitts 2006-07-22 04:58:14 UTC
Cool, looks good now and builds fine; rpmlint is silent.

I note that you don't use a dist tag.  It's not an absolute requirement but it
does simplify your maintenance overhead a bit because it allows you to use the
same spec for multiple distro releases.  I just want to make sure you intended
to leave it out.

The %description leaves a bit to be desired in the grammar department, which is
understandable given that the author is not a native speaker.  Plus "that you'll
boost to an amazing universe of sounds" does put a smile on my face.  I'm not
really sure what to suggest; how about just:

ZynAddSubFX is an open source software synthesizer capable of making a
countless number of instrument sounds.

or somesuch.

I'm not sure that anything you depend on owns /usr/share/icons or the
directories under it.  (At least in FC5.)

Your scriptlets are slightly different from those in 
http://fedoraproject.org/wiki/ScriptletSnippets:
You don't call touch with --no-create; you don't use "||:" on the touch line, 
and you use /usr/bin instead of %{_bindir}.
I'm not sure what difference the first two make in practise.  The latter is a
stylistic issue; the macro is generally preferred over hardcoded paths, but the
suggested scriptlets are not consistent in this.

Review:
* source files match upstream:
   fca8560e37d799bd20d17e22b11674d6  ZynAddSubFX-2.2.1.tar.bz2
* package meets naming and packaging guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
X dist tag is present.
* build root is correct.
* 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).
* debuginfo package looks complete.
* rpmlint is silent.
* final provides and requires are sane:
   zynaddsubfx = 2.2.1-6
  =
   /bin/sh
   desktop-file-utils
   fltk >= 1.1.3
   jack-audio-connection-kit >= 0.101.1
   libX11.so.6()(64bit)
   libXext.so.6()(64bit)
   libXft.so.2()(64bit)
   libXrender.so.1()(64bit)
   libasound.so.2()(64bit)
   libasound.so.2(ALSA_0.9)(64bit)
   libfftw3.so.3()(64bit)
   libfltk.so.1.1()(64bit)
   libfontconfig.so.1()(64bit)
   libfreetype.so.6()(64bit)
   libjack.so.0()(64bit)
   liblash.so.2()(64bit)
   libmxml.so.1()(64bit)
   libuuid.so.1()(64bit)
   libz.so.1()(64bit)
   mxml >= 2.2
* %check is not present; no test suite upstream
* no shared libraries are present.
* package is not relocatable.
X owns the directories it creates. (/usr/share/icons)
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
? scriptlets present; differ from the suggested ones.
* 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 libtool .la droppings.
* GUI app; desktop file installed properly.  No MIME types defined, so no need
to update the desktop database.

Comment 5 Anthony Green 2006-08-27 03:17:02 UTC
(In reply to comment #4)
> Cool, looks good now and builds fine; rpmlint is silent.

Sorry for the long delay.

I've cleaned up everything you mentioned, with one exception:

> I'm not sure that anything you depend on owns /usr/share/icons or the
> directories under it.  (At least in FC5.)

Is this really a problem?  If so, why?  And what should I do about it?

Thanks!

Updated bits here:
Spec URL: http://people.redhat.com/~green/FE/FC5/zynaddsubfx.spec
SRPM URL: http://people.redhat.com/~green/FE/FC5/zynaddsubfx-2.2.1-7.src.rpm

AG


Comment 6 Jason Tibbitts 2006-08-30 05:30:28 UTC
Somehow I managed to miss your reply.  Sorry about that; too much bugzilla mail,
I guess.

About the unowned directory: here is the set of packages in FC5 (core+extras)
which own /usr/share/icons/hicolor/16x16/apps:

HelixPlayer-1:1.0.6-1.2.2.i386
hicolor-icon-theme-0:0.9-2.noarch
sound-juicer-0:2.14.4-1.fc5.1.i386
gdm-1:2.14.9-1.i386
fedora-logos-0:1.1.42-1.fc5.1.noarch
k3b-0:0.12.15-0.FC5.1.i386
openoffice.org-core-1:2.0.2-5.17.2.i386
rssowl-0:1.2.1-1.fc5.i386
pikdev-0:0.9.1-1.fc5.i386
banshee-0:0.10.8-1.i386
rssowl-0:1.2.1-2.fc5.i386
banshee-0:0.10.9-1..fc5.i386
koffice-core-0:1.5.1-1.fc5.i386
taskjuggler-0:2.2.0-1.fc5.i386
kdirstat-0:2.5.3-3.fc5.i386
amarok-0:1.4.1-2.fc5.i386
amarok-0:1.4.1-3.fc5.i386
koffice-core-0:1.5.2-1.fc5.i386
pikdev-0:0.9.1-2.fc5.i386
sound-juicer-0:2.14.4-1.fc5.1.i386
fedora-logos-0:1.1.42-1.fc5.1.noarch
gdm-1:2.14.9-1.i386
k3b-0:0.12.15-0.FC5.1.i386
openoffice.org-core-1:2.0.2-5.16.2.i386
openoffice.org-core-1:2.0.2-5.17.2.i386

Does this package depend, directly or indirectly, on any of those?  It doesn't
look like it to me.  And thus you could install this package and its
dependencies and /usr/share/icons/hicolor/16x16/apps would be unowned, and
that's problematic according to the guidelines.

I honestly don't know what the proper solution is.  Many packages own those
directories, but I guess it might also be reasonable to depend on
hicolor-icon-theme.

Everything else looks good.

Comment 7 Anthony Green 2006-08-30 07:14:16 UTC
(In reply to comment #6)
> I honestly don't know what the proper solution is.  Many packages own those
> directories, but I guess it might also be reasonable to depend on
> hicolor-icon-theme.

Done.

Updated bits here:
Spec URL: http://people.redhat.com/~green/FE/FC5/zynaddsubfx.spec
SRPM URL: http://people.redhat.com/~green/FE/FC5/zynaddsubfx-2.2.1-8.src.rpm


Comment 8 Jason Tibbitts 2006-09-02 04:15:43 UTC
OK, this looks good.  The /usr/share/icons/hicolor/* directories are owned by a
dependency, and /usr/share/pixmaps is owned by filesystem so there's no problem
there.  That was the last of the issues I could see.

Do note that your last change caused this rpmlint warning to pop up:

W: zynaddsubfx mixed-use-of-spaces-and-tabs

because you indented the one line you added with a tab instead of spaces as in
the rest of the specfile.  This is just rpmlint being picky and is not a blocker.

APPROVED

Comment 9 Anthony Green 2006-09-02 07:44:18 UTC
Thanks.


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