Bug 191590

Summary:	Review Request: fluidsynth-dssi - a FluidSynth DSSI plugin
Product:	[Fedora] Fedora	Reporter:	Anthony Green <green>
Component:	Package Review	Assignee:	Till Maas <opensource>
Status:	CLOSED NEXTRELEASE	QA Contact:	Fedora Package Reviews List <fedora-package-review>
Severity:	medium	Docs Contact:
Priority:	medium
Version:	rawhide	CC:	opensource
Target Milestone:	---
Target Release:	---
Hardware:	All
OS:	Linux
Whiteboard:
Fixed In Version:		Doc Type:	Bug Fix
Doc Text:		Story Points:	---
Clone Of:		Environment:
Last Closed:	2006-09-16 23:25:55 UTC	Type:	---
Regression:	---	Mount Type:	---
Documentation:	---	CRM:
Verified Versions:		Category:	---
oVirt Team:	---	RHEL 7.3 requirements from Atomic Host:
Cloudforms Team:	---	Target Upstream Version:
Embargoed:
Bug Depends On:
Bug Blocks:	163779

Description Anthony Green 2006-05-13 15:12:23 UTC

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 19:56:03 UTC

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 20:00:03 UTC

The specfile uses "make" instead of "make %{?_smp_mflags}"

Comment 3 Anthony Green 2006-09-04 20:52:36 UTC

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 18:44:34 UTC

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 20:41:21 UTC

(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 22:49:56 UTC

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 23:25:55 UTC

(In reply to comment #6)
> Change the buildrequires and the package is
> 
> APPROVED

Done, thanks!