Bug 190040 - Review Request: hydrogen - Advanced drum machine
Review Request: hydrogen - Advanced drum machine
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Hans de Goede
Fedora Package Reviews List
:
Depends On: 183912 189313
Blocks:
  Show dependency treegraph
 
Reported: 2006-04-26 17:45 EDT by Anthony Green
Modified: 2008-04-12 16:41 EDT (History)
12 users (show)

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


Attachments (Terms of Use)

  None (edit)
Description Anthony Green 2006-04-26 17:45:14 EDT
Spec URL: http://people.redhat.com/green/FE/FC5/hydrogen.spec
SRPM URL: http://people.redhat.com/green/FE/FC5/hydrogen-0.9.3-1.src.rpm
Description: 
Hydrogen is an advanced drum machine for GNU/Linux. It's main goal is
to bring professional yet simple and intuitive pattern-based drum
programming.

This package depends on jack-audio-connection-kit and liblrdf, both of which have been submitted to Extras recently.
Comment 1 Anthony Green 2006-05-13 12:33:56 EDT
I've fixed an x86-64 install issue.  Updated bits are here...

Spec URL: http://people.redhat.com/green/FE/FC5/hydrogen.spec
SRPM URL: http://people.redhat.com/green/FE/FC5/hydrogen-0.9.3-2.src.rpm

Comment 2 Anthony Green 2006-05-13 17:17:14 EDT
On x86-64, read ladspa plugins from lib64 dirs...

Spec URL: http://people.redhat.com/green/FE/FC5/hydrogen.spec
SRPM URL: http://people.redhat.com/green/FE/FC5/hydrogen-0.9.3-3.src.rpm
Comment 3 Michael Schwendt 2006-05-13 17:41:47 EDT
[wrt 0.9.3-2]

* FE-NEEDSPONSOR blocker bug is for new contributors who don't have CVS
access yet. You don't belong to that group anymore.

* Fails to build (FC5), due to a hardcoded -lxml2 in configure{.in}.

/usr/bin/ld: cannot find -lxml2
collect2: ld returned 1 exit status

Missing:  BuildRequires: libxml2-devel

* From the build log:

| --- Checking for PortAudio -------------------------------------
| checking whether PORTAUDIOPATH environment variable is set... 
| PORTAUDIOPATH is not set. No PortAudio support.
| -----------------------------------------------------------------

PortAudio is available in Extras. Should it be enabled?

* rpmlint hydrogen-0.9.3-2.i386.rpm 
E: hydrogen shared-lib-without-dependency-information
/usr/lib/hydrogen/plugins/wasp_noisifier.so
E: hydrogen shared-lib-without-dependency-information
/usr/lib/hydrogen/plugins/wasp_booster.so
E: hydrogen shared-lib-without-dependency-information
/usr/lib/hydrogen/plugins/wasp_xshaper.so
E: hydrogen script-without-shellbang /usr/share/applications/hydrogen.desktop

The first three are safe to ignore. The latter is not. .desktop file is
executable. The review guidelines say it must be installed with
desktop-file-install.

* The explicit Requires on package names are not good. Kill them and
rely on rpmbuild's automatic dependencies on SONAMES.

* The hardcoded Qt version is not good. Source /etc/profile.d/qt.sh
which sets QTDIR correctly. Further, since many Qt configure checks
fail due to multilib locations, set QTLIB/QTINC like this:

--- hydrogen.spec.orig  2006-05-13 18:31:34.000000000 +0200
+++ hydrogen.spec       2006-05-13 22:58:46.000000000 +0200
@@ -11,8 +11,6 @@
 
 BuildRequires: flac-devel jack-audio-connection-kit-devel liblrdf-devel
 BuildRequires: qt-devel libsndfile-devel alsa-lib-devel
-Requires:      flac jack-audio-connection-kit liblrdf
-Requires:      qt libsndfile alsa-lib
 
 %description
 Hydrogen is an advanced drum machine for GNU/Linux. It's main goal is
@@ -25,8 +23,10 @@
 sed --in-place -e 's:$(prefix)/lib:%{_libdir}:g' plugins/wasp/Makefile.in
 
 %build
-%configure QTDIR=%{_libdir}/qt-3.3
-%{__make} %{?_smp_mflags} QTDIR=%{_libdir}/qt-3.3
+unset QTDIR || : ; . /etc/profile.d/qt.sh
+export QTLIB=${QTDIR}/lib QTINC=${QTDIR}/include
+%configure
+%{__make} %{?_smp_mflags}
 
 %install
 %{__rm} -rf $RPM_BUILD_ROOT
Comment 4 Anthony Green 2006-05-13 19:50:58 EDT
Thanks for the review.  Updated bits here...

Spec URL: http://people.redhat.com/green/FE/FC5/hydrogen.spec
SRPM URL: http://people.redhat.com/green/FE/FC5/hydrogen-0.9.3-4.src.rpm

(In reply to comment #3)
> [wrt 0.9.3-2]
> 
> * FE-NEEDSPONSOR blocker bug is for new contributors who don't have CVS
> access yet. You don't belong to that group anymore.

Ah.. I thought sponsorship applied to packages, not people.  Thanks for clearing
this up.

> * Fails to build (FC5), due to a hardcoded -lxml2 in configure{.in}.
> 
> /usr/bin/ld: cannot find -lxml2
> collect2: ld returned 1 exit status
> 
> Missing:  BuildRequires: libxml2-devel

Fixed. 

> * From the build log:
> 
> | --- Checking for PortAudio -------------------------------------
> | checking whether PORTAUDIOPATH environment variable is set... 
> | PORTAUDIOPATH is not set. No PortAudio support.
> | -----------------------------------------------------------------
> 
> PortAudio is available in Extras. Should it be enabled?

No, I don't think so.  jack is the audio driver of choice for these kinds of apps.
 
> * rpmlint hydrogen-0.9.3-2.i386.rpm 
> E: hydrogen shared-lib-without-dependency-information
> /usr/lib/hydrogen/plugins/wasp_noisifier.so
> E: hydrogen shared-lib-without-dependency-information
> /usr/lib/hydrogen/plugins/wasp_booster.so
> E: hydrogen shared-lib-without-dependency-information
> /usr/lib/hydrogen/plugins/wasp_xshaper.so
> E: hydrogen script-without-shellbang /usr/share/applications/hydrogen.desktop
> 
> The first three are safe to ignore. The latter is not. .desktop file is
> executable. The review guidelines say it must be installed with
> desktop-file-install.

Fixed.  I've also added related post and postun scripts.

> * The explicit Requires on package names are not good. Kill them and
> rely on rpmbuild's automatic dependencies on SONAMES.

Fixed.

> * The hardcoded Qt version is not good. Source /etc/profile.d/qt.sh
> which sets QTDIR correctly. Further, since many Qt configure checks
> fail due to multilib locations, set QTLIB/QTINC like this:

Cool.  Fixed.

Thanks for spending time on this!

AG
Comment 5 Callum Lerwick 2006-05-15 15:24:15 EDT
Yeah, portaudio is redundant, and OSS is deprecated. One thing I've noticed
about hydrogen is it eats quite a bit of RAM, and then jack wants to lock it. If
the memlock rlimit isn't set to around 128mb, it will crash when used with jack
with mem locking enabled.
Comment 6 Callum Lerwick 2006-05-15 16:07:37 EDT
(noted in more detail on the jack review)
Comment 7 Callum Lerwick 2006-05-21 04:34:17 EDT
Michael: You have dibs on this review, but you hadn't assigned it to yourself. I
do want to work on this tommorow. Sorry if I'm stepping on your toes. Reassign
it if you want it?
Comment 8 Callum Lerwick 2006-05-23 05:25:54 EDT
MUST items:

- rpmlint: Ok?

$ rpmlint hydrogen-0.9.3-4.fc5.i386.rpm
E: hydrogen shared-lib-without-dependency-information
/usr/lib/hydrogen/plugins/wasp_xshaper.so
E: hydrogen shared-lib-without-dependency-information
/usr/lib/hydrogen/plugins/wasp_noisifier.so
E: hydrogen shared-lib-without-dependency-information
/usr/lib/hydrogen/plugins/wasp_booster.so

$ ldd /usr/lib/hydrogen/plugins/*
/usr/lib/hydrogen/plugins/wasp_booster.so:
        statically linked
/usr/lib/hydrogen/plugins/wasp_noisifier.so:
        statically linked
/usr/lib/hydrogen/plugins/wasp_xshaper.so:
        statically linked

Statically linked dynamic libraries? That's a new one to me. These are
apparently LADSPA plugins. Perhaps these should go in %{_libdir}/ladspa instead.
And even go in a "hydrogen-wasp-plugins" subpackage.

- Package name: Ok
- Spec name: Ok
- Meets packaging guidelines: Ok
- License: Ok
- Spec in American English: Ok
- Spec legible: Ok
- Sources match upstream: Ok
- Builds: Ok
- BuildRequires: Ok
- Locales: Ok
- ldconfig: NEEDSWORK
- Relocation: Ok
- Directory ownership: Ok
- %files: Ok
- %clean: Ok
- Macros: Ok
- Code vs. Content: Ok
- Documentation: Ok
- devel package: Ok
- .desktop file: Ok

SHOULD:

- Includes license text: Ok
- Mock build: Yes
- Builds on all archs: Built on i386, x86_64
- Package functional: Yes! http://www.haxxed.com/music/909fun.ogg
- Scriptlets: NEEDSWORK
- Subpackages: Ok

NEEDSWORK:

Source0 should be a full URL. It should be
http://dl.sf.net/sourceforge/hydrogen/hydrogen-0.9.3.tar.gz

You have a lingering buildreq on portaudio-devel.

I would recommend disabling OSS support. (%configure --disable-oss-support)
AFAIK OSS has been deprecated for some time now.

I don't think you need all that QTDIR stuff. It seems to build just fine without
it. Mock sources profile.d properly.

I don't think you need to update the icon cache, its not installing any into
/usr/share/icons

Don't need ldconfig, its not installing systemwide libraries.

The update-desktop-database doesn't match what's in ScriptletSnippets, and
according to ScriptletSnippets, you only need it if there's a MimeType key.
Comment 9 Hugo Cisneiros 2006-05-23 10:21:37 EDT
(In reply to comment #8)
> Source0 should be a full URL. It should be
> http://dl.sf.net/sourceforge/hydrogen/hydrogen-0.9.3.tar.gz

Discussing this in IRC some people realized that dl.sf.net is not always 
available (as it is a dns round-robin load-balance). In experience, some said 
that the easynews mirror never gets trouble so using it is better in general. 
I'm using it on some specs, so maybe you should consider using it too.
Comment 10 Trever Adams 2006-06-26 12:50:13 EDT
So, is this going to be included any time soon?
Comment 11 Anthony Green 2006-06-26 14:22:41 EDT
(In reply to comment #10)
> So, is this going to be included any time soon?

I'll work on it this week.  Thanks for the reminder.  I forgot that I had
received feedback.
Comment 12 Anthony Green 2006-07-02 21:41:15 EDT
(In reply to comment #8) 
> NEEDSWORK:
> 
> Source0 should be a full URL. It should be
> http://dl.sf.net/sourceforge/hydrogen/hydrogen-0.9.3.tar.gz

I've updated this to the easynews link.
 
> You have a lingering buildreq on portaudio-devel.

Fixed.
 
> I would recommend disabling OSS support. (%configure --disable-oss-support)
> AFAIK OSS has been deprecated for some time now.

Fixed.
 
> I don't think you need all that QTDIR stuff. It seems to build just fine without
> it. Mock sources profile.d properly.

I think all that stuff is a good idea in order to ensure a reproducable build
outside of mock.
 
> I don't think you need to update the icon cache, its not installing any into
> /usr/share/icons

Fixed.

> Don't need ldconfig, its not installing systemwide libraries.
 
Fixed.
 
> The update-desktop-database doesn't match what's in ScriptletSnippets, and
> according to ScriptletSnippets, you only need it if there's a MimeType key.

Fixed (it does have a MimeType key).

Here are the updated bits:

Spec URL: http://people.redhat.com/green/FE/FC5/hydrogen.spec
SRPM URL: http://people.redhat.com/green/FE/FC5/hydrogen-0.9.3-5.src.rpm
Comment 13 Anthony Green 2006-07-22 20:02:38 EDT
Updated bits...

Spec URL: http://people.redhat.com/green/FE/FC5/hydrogen.spec
SRPM URL: http://people.redhat.com/green/FE/FC5/hydrogen-0.9.3-6.src.rpm

This just adds a patch for a crasher that I will submit upstream now as well.

Comment 14 Till Maas 2006-09-12 16:25:33 EDT
You should not convert the icon in your spec file. The different sizes are 
only needed if they are manually edited in the other sizes to look better. 
This simple resizing is done automatically by a window manager if needed.
Comment 15 Till Maas 2006-09-12 16:27:26 EDT
Please ignore my last comment it was addressed to another review request.
Comment 16 Anthony Green 2007-03-17 16:51:06 EDT
Hi seg - I thought we were very close to wrapping this one up.  Could you please
review the last package?  Thanks!  AG
Comment 17 Till Maas 2007-03-26 00:19:37 EDT


(In reply to comment #12)
> (In reply to comment #8) 
> > NEEDSWORK:
> > 
> > Source0 should be a full URL. It should be
> > http://dl.sf.net/sourceforge/hydrogen/hydrogen-0.9.3.tar.gz
> 
> I've updated this to the easynews link.

Recommended is:
http://downloads.sourceforge.net/%{name}/%{name}-%{version}.tar.gz
see:
http://fedoraproject.org/wiki/Packaging/SourceURL#head-e27982f18a3bfd26b5b6ecbee113d2d8f3f006f2

> > The update-desktop-database doesn't match what's in ScriptletSnippets, and
> > according to ScriptletSnippets, you only need it if there's a MimeType key.
> 
> Fixed (it does have a MimeType key).
> 
> Here are the updated bits:
> 
> Spec URL: http://people.redhat.com/green/FE/FC5/hydrogen.spec
> SRPM URL: http://people.redhat.com/green/FE/FC5/hydrogen-0.9.3-5.src.rpm

The changelog says:
- Remove post/postun scriptlets.

Because the .desktop files MimeType entry you need the scriptlets from:
http://fedoraproject.org/wiki/Packaging/ScriptletSnippets#head-de6770dd9867fcd085a73a4700f6bcd0d10294ef
Comment 18 Anthony Green 2007-03-26 12:25:20 EDT
(In reply to comment #17)
> Recommended is:
> http://downloads.sourceforge.net/%{name}/%{name}-%{version}.tar.gz
> see:
>
http://fedoraproject.org/wiki/Packaging/SourceURL#head-e27982f18a3bfd26b5b6ecbee113d2d8f3f006f2

Fixed.

> The changelog says:
> - Remove post/postun scriptlets.

I don't know why I did this.  I've added these with update-desktop-database and 
gtk-update-icon-cache,
 
> Because the .desktop files MimeType entry you need the scriptlets from:
>
http://fedoraproject.org/wiki/Packaging/ScriptletSnippets#head-de6770dd9867fcd085a73a4700f6bcd0d10294ef

Done.

New bits here:
http://people.redhat.com/green/FE/devel/hydrogen-0.9.3-7.src.rpm
http://people.redhat.com/green/FE/devel/hydrogen.spec

Thanks!
Comment 19 Till Maas 2007-03-26 16:36:04 EDT
(In reply to comment #18)

> > The changelog says:
> > - Remove post/postun scriptlets.
> 
> I don't know why I did this.  I've added these with update-desktop-database and 
> gtk-update-icon-cache,

Maybe you removed the gtk-update-icon-cache stuff, because you need only
update-desktop-database, because the icons are not in the cached directories.
Comment 20 Luke Macken 2007-06-30 21:03:47 EDT
Trying to build this on F7...

src/lib/FLACFile.cpp: In member function 'void FLACFile_real::load(std::string)':
src/lib/FLACFile.cpp:167: error: 'set_filename' was not declared in this scope
src/lib/FLACFile.cpp:169: error: no matching function for call to
'FLACFile_real::init()'
/usr/include/FLAC++/decoder.h:226: note: candidates are: virtual
FLAC__StreamDecoderInitStatus FLAC::Decoder::File::init(FILE*)
/usr/include/FLAC++/decoder.h:227: note:                 virtual
FLAC__StreamDecoderInitStatus FLAC::Decoder::File::init(const char*)
/usr/include/FLAC++/decoder.h:228: note:                 virtual
FLAC__StreamDecoderInitStatus FLAC::Decoder::File::init(const std::string&)
src/lib/FLACFile.cpp:170: error: 'FLAC__FILE_DECODER_OK' was not declared in
this scope
src/lib/FLACFile.cpp:174: error: 'process_until_end_of_file' was not declared in
this scope
make[1]: *** [src/FLACFile.o] Error 1
make[1]: Leaving directory `/home/lmacken/rpmbuild/BUILD/hydrogen-0.9.3'
make: *** [hydrogenPlayer] Error 2
make: *** Waiting for unfinished jobs....
src/lib/FLACFile.cpp: In member function 'void FLACFile_real::load(std::string)':
src/lib/FLACFile.cpp:167: error: 'set_filename' was not declared in this scope
src/lib/FLACFile.cpp:169: error: no matching function for call to
'FLACFile_real::init()'
/usr/include/FLAC++/decoder.h:226: note: candidates are: virtual
FLAC__StreamDecoderInitStatus FLAC::Decoder::File::init(FILE*)
/usr/include/FLAC++/decoder.h:227: note:                 virtual
FLAC__StreamDecoderInitStatus FLAC::Decoder::File::init(const char*)
/usr/include/FLAC++/decoder.h:228: note:                 virtual
FLAC__StreamDecoderInitStatus FLAC::Decoder::File::init(const std::string&)
src/lib/FLACFile.cpp:170: error: 'FLAC__FILE_DECODER_OK' was not declared in
this scope
src/lib/FLACFile.cpp:174: error: 'process_until_end_of_file' was not declared in
this scope
make[1]: *** [src/FLACFile.o] Error 1
make[1]: Leaving directory `/home/lmacken/rpmbuild/BUILD/hydrogen-0.9.3'
make: *** [hydrogen] Error 2
error: Bad exit status from /var/tmp/rpm-tmp.93730 (%build)
Comment 21 Chris Mohler 2007-07-10 02:13:33 EDT
I hit the bump in #20 also.  I did not know this was up for review and was
building from source - I wandered in from google.  The meat of this patch will
get past the FLAC error:
http://cvs.archlinux.org/cgi-bin/viewcvs.cgi/multimedia/hydrogen/hydrogen-0.9.3-flac113.patch?rev=1.1&cvsroot=Extra&only_with_tag=CURRENT&content-type

It needs some work :)
Comment 22 Luke Macken 2007-08-22 04:12:57 EDT
I'm getting this build error with that patch:

src/lib/FLACFile.cpp: In member function 'void FLACFile_real::load(std::string)':
src/lib/FLACFile.cpp:167: error: 'set_filename' was not declared in this scope
src/lib/FLACFile.cpp:169: error: no matching function for call to
'FLACFile_real::init()'
/usr/include/FLAC++/decoder.h:226: note: candidates are: virtual
FLAC__StreamDecoderInitStatus FLAC::Decoder::File::init(FILE*)
/usr/include/FLAC++/decoder.h:227: note:                 virtual
FLAC__StreamDecoderInitStatus FLAC::Decoder::File::init(const char*)
/usr/include/FLAC++/decoder.h:228: note:                 virtual
FLAC__StreamDecoderInitStatus FLAC::Decoder::File::init(const std::string&)
src/lib/FLACFile.cpp:170: error: 'FLAC__FILE_DECODER_OK' was not declared in
this scope
src/lib/FLACFile.cpp:174: error: 'process_until_end_of_file' was not declared in
this scope
make[1]: *** [src/FLACFile.o] Error 1
Comment 23 Matthias Saou 2007-09-01 12:06:18 EDT
Removing old FE-REVIEW blocker and changed from NEW to ASSIGNED since the review
flag is set.
Comment 24 Hans de Goede 2007-10-03 18:06:42 EDT
I noticed that this seems kinda stuck and I would like to offer to help.

The patch from comment #21 looks good, I haven't tried it, but it should work,
If it doesn't then the used define probably isn't getting set correctly.
Comment 25 Lubomir Kundrak 2007-10-06 04:34:42 EDT
Luke: for me the patch works. What version of flac and flac-devel packages do
you have?
Comment 26 Lubomir Kundrak 2007-10-06 04:43:59 EDT
Another thing:

# Conditionally apply patch to read ladspa plugins from lib64 dir.
%ifarch x86_64
%patch1 -p0
%patch2 -p0
%endif

Please never do this. Never make arch-specific patches:
http://www.redhat.com/archives/fedora-devel-list/2007-September/msg01518.html
Comment 27 Lubomir Kundrak 2007-10-06 04:48:31 EDT
The License: tag is also not correct. GPLv2+ is the correct license here.
Comment 28 Lubomir Kundrak 2007-10-06 04:49:30 EDT
The patch names should mention the version they were created against, like
name-version-description.patch
Comment 29 Lubomir Kundrak 2007-10-06 05:30:58 EDT
This happens in hydrogen-0.9.3/plugins/wasp during the build

ld -shared -o wasp_booster.so plugins/booster.o
ld -shared -o wasp_noisifier.so plugins/noisifier.o
ld -shared -o wasp_xshaper.so plugins/xshaper.o

linking this way doesn't honor the Build id
http://fedoraproject.org/wiki/Releases/FeatureBuildId
Comment 30 Lubomir Kundrak 2007-10-06 05:32:11 EDT
[root@gopher SPECS]# rpm -Uvh
/home/lkundrak/rpmbuild/RPMS/i386/hydrogen-0.9.3-7.1.fc8.i386.rpm
Preparing...                    ########################################### [100%]
   1:hydrogen               ##########################                  ( 60%)
########################################### [100%]
/var/tmp/rpm-tmp.60215: line 1: 29061 Segmentation fault      (core dumped)
update-desktop-database >&/dev/null
[root@gopher SPECS]# 

Ayeeee :)
Comment 31 Lubomir Kundrak 2007-10-06 05:40:42 EDT
The above is most likely not hydrogen's fault, I'll investigate that later...
after the weekend. Below are patches I had to apply to hydrogen to get it built:

http://people.redhat.com/lkundrak/patches/hydrogen/

I did not address all the issues mentioned above; just enough to get that built.
Here's the SPEC file:

http://people.redhat.com/lkundrak/SPECS/hydrogen.spec

Anthony: Are you still interested in finishing the package? Apart from the
problems I made fixes for, are there any other outstanding problems?
Comment 32 Lubomir Kundrak 2007-10-06 08:03:09 EDT
g++ -c -pipe -g -w -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions
-fstack-protector --param=ssp-buffer-size=4 -m32 -march=i386 -mtune=generic
-fasynchronous-unwind-tables -D_REENTRANT  -DQT_NO_DEBUG -DQT_THREAD_SUPPORT
-DQT_SHARED -DQT_TABLET_SUPPORT -I/usr/lib/qt-3.3/mkspecs/default -I. -I. -Isrc
-I/usr/lib/qt-3.3/include -o src/Button.o src/gui/widgets/Button.cpp
src/AlsaMidiDriver.o: In function `alsaMidiDriver_thread(void*)':
/builddir/build/BUILD/hydrogen-0.9.3/src/lib/drivers/AlsaMidiDriver.cpp:80:
undefined reference to `Preferences::getInstance()'
src/DiskWriterDriver.o: In function `DiskWriterDriver::getSampleRate()':
/builddir/build/BUILD/hydrogen-0.9.3/src/lib/drivers/DiskWriterDriver.cpp:162:
undefined reference to `Preferences::getInstance()'
src/DiskWriterDriver.o: In function `diskWriterDriver_thread(void*)':
/builddir/build/BUILD/hydrogen-0.9.3/src/lib/drivers/DiskWriterDriver.cpp:40:
undefined reference to `Preferences::getInstance()'
src/JackDriver.o: In function `JackDriver::init(unsigned int)':
/builddir/build/BUILD/hydrogen-0.9.3/src/lib/drivers/JackDriver.cpp:242:
undefined reference to `Preferences::getInstance()'
/builddir/build/BUILD/hydrogen-0.9.3/src/lib/drivers/JackDriver.cpp:243:
undefined reference to `Preferences::getInstance()'
src/JackDriver.o:/builddir/build/BUILD/hydrogen-0.9.3/src/lib/drivers/JackDriver.cpp:392:
more undefined references to `Preferences::getInstance()' follow
collect2: ld returned 1 exit status
make[1]: *** [hydrogenPlayer] Error 1
make[1]: Leaving directory `/builddir/build/BUILD/hydrogen-0.9.3'
make: *** [hydrogenPlayer] Error 2
make: *** Waiting for unfinished jobs....

In mock for FC7
Comment 33 Anthony Green 2007-10-06 09:25:13 EDT
(In reply to comment #31)
> Anthony: Are you still interested in finishing the package? Apart from the
> problems I made fixes for, are there any other outstanding problems?

I'm interested, but don't have time now.  Could somebody else pick this up?

Thanks!
Comment 34 Lubomir Kundrak 2007-10-07 15:35:39 EDT
I've made a couple more fixes. Here are the new files:

Spec URL: http://people.redhat.com/lkundrak/SPECS/hydrogen.spec
SRPM URL:
http://people.redhat.com/lkundrak/mock-results/hydrogen-0.9.3-8.fc8.i386/hydrogen-0.9.3-8.fc8.src.rpm

The mock result with binary package and build log is here:
http://people.redhat.com/lkundrak/mock-results/hydrogen-0.9.3-8.fc8.i386/

The package builds correctly in mock in Fedora 7 and Rawhide on i386. rpmlint is
all silent and I am not aware of any outstanding issues. Callum: could you
please review the new packages?
Comment 35 Hans de Goede 2007-10-08 10:45:11 EDT
Taking over as reviewer, since Callum is very busy with other stuff ATM AFAIK. I
hope you don't mind Callum.

Full review done (sources match upstream, license ok, everything else also
checked), I've found a few items to fix:

MUST FIX
--------
- You currently pass LIBDIR=%{_libdir} to make but not to make install!
  I couldn't test this at a 64 bit machine atm, but either you should use it in
  both places or not all
- Remove these obsolete or unneeded options when installing the .desktop file:
  --add-category X-Fedora                         \
  --add-category AudioVideo                       \
  --add-category Application                      \
- Add the following to remove the obsolete Application category that is in
  upstreams .desktop file:
  --remove-category Application                   \
- Remove "MimeType=text/xml" from hydrogen.desktop, thats way too generic!
- Remove obsolete "Version=1.0" from hydrogen.desktop
- No longer run update-desktop-database from %post[un] now the .desktop file no
  longer defines a MimeType
- The update icon cache scriptlets should check the existence of
  gtk-update-icon-cache before calling it, see:
http://fedoraproject.org/wiki/Packaging/ScriptletSnippets#head-7103f6c38d1b5735e8477bdd569ad73ea2c49bda


SHOULD FIX
----------
- Add the following Categories from:
  http://standards.freedesktop.org/menu-spec/latest/apa.html  :
  Midi
- Add the following X-foo categories, to mathc what CCRMA has so that this
  package will work with CCRMA's nested audio utilities menus:
  X-Drumming X-MIDI X-Jack
Comment 36 Hans de Goede 2007-10-08 10:46:02 EDT
Fernando, this is moving to Fedora too, I guess you want to know, and maybe take
a quick peek for any errors.
Comment 37 Lubomir Kundrak 2007-10-08 11:44:21 EDT
Much thanks for the comments!

> - You currently pass LIBDIR=%{_libdir} to make but not to make install!
>   I couldn't test this at a 64 bit machine atm, but either you should use it in
>   both places or not all

That is no longer needed, I just forgot it there. Makefiles use ${libdir} from
autoconf and set -DLIBDIR for the ladspa plugins path via .pro files.

> - Remove these obsolete or unneeded options when installing the .desktop file:
>   --add-category X-Fedora                         \
>   --add-category AudioVideo                       \
>   --add-category Application                      \
> - Add the following to remove the obsolete Application category that is in
>   upstreams .desktop file:
>   --remove-category Application                   \
> - Remove "MimeType=text/xml" from hydrogen.desktop, thats way too generic!
> - Remove obsolete "Version=1.0" from hydrogen.desktop
> - No longer run update-desktop-database from %post[un] now the .desktop file no
>   longer defines a MimeType

> - Add the following Categories from:
>   http://standards.freedesktop.org/menu-spec/latest/apa.html  :
>   Midi
> - Add the following X-foo categories, to mathc what CCRMA has so that this
>   package will work with CCRMA's nested audio utilities menus:
>   X-Drumming X-MIDI X-Jack

Changed the options to desktop-file-install, removed
hydrogen-0.9.3-sound-category.patch patch.

> - The update icon cache scriptlets should check the existence of
>   gtk-update-icon-cache before calling it, see:
>
http://fedoraproject.org/wiki/Packaging/ScriptletSnippets#head-7103f6c38d1b5735e8477bdd569ad73ea2c49bda

I've modified the scriptlets accordingly.

Here's the new package:

Spec URL: http://people.redhat.com/lkundrak/SPECS/hydrogen.spec
SRPM URL:
http://people.redhat.com/lkundrak/mock-results/hydrogen-0.9.3-9.fc8.i386/hydrogen-0.9.3-9.fc8.src.rpm
The mock build results:
http://people.redhat.com/lkundrak/mock-results/hydrogen-0.9.3-9.fc8.i386/
Comment 38 Fernando Lopez-Lezcano 2007-10-08 13:16:17 EDT
(In reply to comment #36)
> Fernando, this is moving to Fedora too, I guess you want to know, and maybe take
> a quick peek for any errors.

_Thanks_ for the heads up. Looks fine so far (just took a quick look at the spec
file), thanks for including the Planet CCRMA menu categories so that it meshes
with the existing menus... one more for the migration!
Comment 39 Hans de Goede 2007-10-08 15:47:41 EDT
Latest version looks fine: Approved!

Which brings us to the interesting question who will maintain it, may I suggest
having both Anthony and Lubomir as owners / co-maintainers? And I think it would
be good to have Fernando from CCRMA as co-maintainer too, for a grand total of 3
:) Note Fernando's account system name is nando.
Comment 40 Lubomir Kundrak 2007-10-08 16:28:07 EDT
Hans: This will be CVS extras writable, so who'll be a formal maintainer is not
an issue at all. Anyways, I do not think I would want to maintain this, but as
Anthony says he doesn't have time at the time I'd be glad to help with package
mainteance tasks for now.

New Package CVS Request
=======================
Package Name: hydrogen
Short Description: Advanced drum machine
Owners:  green,lkundrak,nando
Branches: FC-6 F-7
Cvsextras Commits: yes
Comment 41 Kevin Fenzi 2007-10-09 00:18:26 EDT
cvs done.
Comment 42 Lubomir Kundrak 2007-10-09 02:16:31 EDT
Thanks Antony, Michael, Callum, Hugo, Till, Hans, Fernando and Kevin. The
package was imported into CVS and built.
Comment 43 Lubomir Kundrak 2008-04-11 17:06:42 EDT
Package Change Request
=======================
Package Name: hydrogen
Branches: EL-5
Cvsextras Commits: yes
Comment 44 Kevin Fenzi 2008-04-12 16:41:52 EDT
cvs done.

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