Bug 292351 - Review Request: ladspa-blop-plugins - Bandlimited LADSPA Oscillator Plugins
Review Request: ladspa-blop-plugins - Bandlimited LADSPA Oscillator Plugins
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Parag AN(पराग)
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-09-16 04:27 EDT by Hans de Goede
Modified: 2007-11-30 17:12 EST (History)
4 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-09-21 14:55:19 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
panemade: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Hans de Goede 2007-09-16 04:27:53 EDT
Spec URL: http://people.atrpms.net/~hdegoede/ladspa-blop-plugins.spec
SRPM URL: http://people.atrpms.net/~hdegoede/ladspa-blop-plugins-0.2.8-3.fc8.src.rpm
Description:
BLOP comprises a set of LADSPA plugins that generate bandlimited
sawtooth, square, variable pulse and slope-variable triangle waves,
for use in LADSPA hosts, principally for use with one of the many
modular software synthesisers available.

They are wavetable based, and are designed to produce output with
harmonic content as high as possible over a wide pitch range.
Comment 1 Michael Schwendt 2007-09-18 11:06:33 EDT
* RPM %optflags are ignored completely. -g is missing, too.
Instead, -O3 -funroll-loops ... are used which don't guarantee
a significant performance gain.

* According to the Makefile some other files are built with
-O0 as -O1 would break some of the code. (interesting that the
authors haven't added a test suite for that)

* Hint: Deleting files from the extracted tarball in the
%install section breaks --short-circuit rpmbuild. Preferably,
this is avoided when it takes only minimal effort to do so.
Comment 2 Hans de Goede 2007-09-18 15:09:55 EDT
(In reply to comment #1)
> * RPM %optflags are ignored completely. -g is missing, too.
> Instead, -O3 -funroll-loops ... are used which don't guarantee
> a significant performance gain.
> 

Ah, my bad I assume that becaused the project used autotools that the optflags
would just be there, fixed now.

> * According to the Makefile some other files are built with
> -O0 as -O1 would break some of the code. (interesting that the
> authors haven't added a test suite for that)
> 

Hmm, very interesting. This was probably caused by there use of some obscure
-ffoo options, and this was determined long ago with a much older gcc. However
as you said, there is no test case, so I've just left this as is to be on the
safe side.

> * Hint: Deleting files from the extracted tarball in the
> %install section breaks --short-circuit rpmbuild. Preferably,
> this is avoided when it takes only minimal effort to do so.
> 

Fixed (and saved 100kb of installed space)

Here is a new version:
Spec URL: http://people.atrpms.net/~hdegoede/ladspa-blop-plugins.spec
SRPM URL: http://people.atrpms.net/~hdegoede/ladspa-blop-plugins-0.2.8-4.fc8.src.rpm
Comment 3 Parag AN(पराग) 2007-09-19 23:35:18 EDT
1)preserve timestamps in install command.
any reason for not using ldconfig scriptlet as I can see soem shared libraries
2)getting installed in /usr/lib?
Comment 4 Hans de Goede 2007-09-20 05:03:23 EDT
(In reply to comment #3)
> 1)preserve timestamps in install command.
Done:
Spec URL: http://people.atrpms.net/~hdegoede/ladspa-blop-plugins.spec
SRPM URL: http://people.atrpms.net/~hdegoede/ladspa-blop-plugins-0.2.8-5.fc8.src.rpm

> 2) any reason for not using ldconfig scriptlet as I can see soem shared libraries
> getting installed in /usr/lib?

Note the files are installed in a subdir of /usr/lib, which is not search by ld
(these are plugins not libs). so there is no need to run ldconfig

Comment 5 Parag AN(पराग) 2007-09-20 06:27:54 EDT
thanks will take this for review and post it by tomorrow.
Comment 6 Parag AN(पराग) 2007-09-21 00:53:02 EDT
Review:
+ package builds in mock (development i386).
+ rpmlint is silent for SRPM and RPM.
+ source files match upstream.
4baedbf1e7cacc7d1034c4bcd5556d6f  blop-0.2.8.tar.gz
+ package meets naming and packaging guidelines.
+ specfile is properly named, 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 is included in package.
+ %doc files present.
+ BuildRequires are proper.
+ %clean is present.
+ package installed properly.
+ Macro use appears rather consistent.
+ Package contains code.
+ no static libraries.
+ no .pc file present.
+ no -devel subpackage exists.
+ no .la files.
+ translations are available.
+ Does owns the directories it creates.
+ no duplicates in %files.
+ file permissions are appropriate.
+ no scriptlets are used.
+ Pacakge ladspa-blop-plugins-0.2.8-5.fc8 ->
   Provides: adsr_1653.so adsr_1680.so amp_1654.so blop = 0.2.8-4.fc8
branch_1673.so dahdsr_2021.so difference_2030.so fmod_1656.so
interpolator_1660.so lp4pole_1671.so parabola_1649_data.so product_1668.so
pulse_1645.so quantiser100_2029.so quantiser20_2027.so quantiser50_2028.so
random_1661.so ratio_2034.so sawtooth_1641.so sawtooth_1641_data.so
sequencer16_1677.so sequencer32_1676.so sequencer64_1675.so square_1643.so
square_1643_data.so sum_1665.so sync_pulse_2023.so sync_square_1678.so
tracker_2025.so triangle_1649.so
  Requires: ladspa libc.so.6 libc.so.6(GLIBC_2.0) libc.so.6(GLIBC_2.3.4)
libm.so.6 rtld(GNU_HASH)
+ Not a GUI app.

APPROVED.
Comment 7 Hans de Goede 2007-09-21 02:46:52 EDT
Thanks!

New Package CVS Request
=======================
Package Name:      ladspa-blop-plugins
Short Description: Bandlimited LADSPA Oscillator Plugins
Owners:            jwrdegoede nando
Branches:          F-7 devel
InitialCC:         <empty>
Cvsextras Commits: Yes

Comment 8 Kevin Fenzi 2007-09-21 12:37:01 EDT
cvs done.
Comment 9 Hans de Goede 2007-09-21 14:55:19 EDT
Imported and build, closing.

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