Bug 191590 - Review Request: fluidsynth-dssi - a FluidSynth DSSI plugin
Review Request: fluidsynth-dssi - a FluidSynth DSSI plugin
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Till Maas
Fedora Package Reviews List
:
Depends On:
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2006-05-13 11:12 EDT by Anthony Green
Modified: 2007-11-30 17:11 EST (History)
1 user (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-09-16 19:25:55 EDT
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 Anthony Green 2006-05-13 11:12:23 EDT
Spec URL: http://people.redhat.com/green/FE/FC5/fluidsynth-dssi.spec
SRPM URL: http://people.redhat.com/green/FE/FC5/fluidsynth-dssi-0.9.1-2.src.rpm
Description: 
This is an implementation of the FluidSynth soundfont-playing software
synthesizer as a DSSI plugin. It makes use of DSSI's
run_multiple_synths() interface to allow sharing of resources between
multiple plugin instances -- soundfont data is shared between
instances, and FluidSynth's usual voice allocation methods are applied
across multiple instances as if each were a FluidSynth channel.
Comment 1 Till Maas 2006-08-24 15:56:03 EDT
I guess to avoid rpmlint symlink errors/warnings you can change
ln -s %{_bindir}/jack-dssi-host $RPM_BUILD_ROOT%{_bindir}/fluidsynth-dssi
to
cd %{_bindir}
ln -s jack-dssi-host fluidsynth-dssi
cd -
because there is no need to make the symlinks across the filesystem if source
and target are in the same directory. This concerns also your other dssi plugin(s).
Comment 2 Till Maas 2006-08-24 16:00:03 EDT
The specfile uses "make" instead of "make %{?_smp_mflags}"
Comment 3 Anthony Green 2006-09-04 16:52:36 EDT
Thanks - I've updated the SRPM here:

Spec URL: http://people.redhat.com/green/FE/FC5/fluidsynth-dssi.spec
SRPM URL: http://people.redhat.com/green/FE/FC5/fluidsynth-dssi-0.9.1-3.src.rpm

Comment 4 Till Maas 2006-09-15 14:44:34 EDT
The .desktop file should be in a separate file and not inside the spec.

You do not need:
Requires(post):   desktop-file-utils
Requires(postun): desktop-file-utils

make install DESTDIR="$RPM_BUILD_ROOT"
instead of %makeinstall should work, so you must not use %makeinstall
see:
http://fedoraproject.org/wiki/Packaging/Guidelines#head-fcaf3e6fcbd51194a5d0dbcfbdd2fcb7791dd002

You do not package COPYING with contains the license.

The license seems to be GPL but you wrote LGPL in the spec.


Comment 5 Anthony Green 2006-09-16 16:41:21 EDT
(In reply to comment #4)
> The .desktop file should be in a separate file and not inside the spec.

I decided to delete the .desktop file, as well as the fluidsynth-dssi binary, 
since it is redundant.  FE already has many other ways to run fluidsynth
(qsynth, etC).

> make install DESTDIR="$RPM_BUILD_ROOT"
> instead of %makeinstall should work, so you must not use %makeinstall

Fixed.

> You do not package COPYING with contains the license.

Fixed.
 
> The license seems to be GPL but you wrote LGPL in the spec.

Fixed.

Updated bits here:

Spec URL: http://people.redhat.com/green/FE/FC5/fluidsynth-dssi.spec
SRPM URL: http://people.redhat.com/green/FE/FC5/fluidsynth-dssi-0.9.1-4.src.rpm
Comment 6 Till Maas 2006-09-16 18:49:56 EDT
BuildRequires: alsa-lib-devel liblo-devel ladspa-devel is not needed since 
dssi-devel already depends on them and is in BuildRequires

rpmlint: ok 
naming: ok
packaging guidelines: ok
license: ok
sources: ok
06bce40ec6c86545d0587e9364bba116  fluidsynth-dssi-0.9.1.tar.gz
06bce40ec6c86545d0587e9364bba116  fluidsynth-dssi-0.9.1.tar.gz.1

files and directories: ok
mock build: ok

Change the buildrequires and the package is

APPROVED
Comment 7 Anthony Green 2006-09-16 19:25:55 EDT
(In reply to comment #6)
> Change the buildrequires and the package is
> 
> APPROVED

Done, thanks!


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