On Mon, May 06 2019, Raphael Graf <r...@undefined.ch> wrote:
> On Sun, May 05, 2019 at 01:46:48PM +0200, Jeremie Courreges-Anglas wrote:
>> On Sun, May 05 2019, Raphael Graf <r...@undefined.ch> wrote:
>> > Here is an update to hydrogen-0.9.7.
>> >
>> > Notable changes:
>> > - Uses cmake instead of scons.
>> > - There is a shared library and three additional command-line binaries.
>> > - Enabled support for audio/ladspa plugins
>> >
>> > New features are listed here:
>> > http://hydrogen-music.org/
>> >
>> > Comments/tests welcome.
>> 
>> Here's some nitpicking about the Makefile only.
>> 
>> > Index: Makefile
>> > ===================================================================
>> > RCS file: /cvs/ports/audio/hydrogen/Makefile,v
>> > retrieving revision 1.25
>> > diff -u -p -u -p -r1.25 Makefile
>> > --- Makefile       8 Jan 2019 21:24:29 -0000       1.25
>> > +++ Makefile       5 May 2019 10:55:31 -0000
>> > @@ -1,58 +1,54 @@
>> >  # $OpenBSD: Makefile,v 1.25 2019/01/08 21:24:29 sebastia Exp $
>> >  
>> > -COMMENT=          software drum machine
>> > +COMMENT =         software drum machine
>> 
>> Please avoid gratuitous whitespace changes like this, it makes cvs blame
>> less useful for no real gain.  If you really care for consistency I'd
>> suggest to tweak the COMPILER line instead.
>
> Ok, I've removed the whitespace changes from the diff.

Thanks.

>> 
>> > -DISTNAME=         hydrogen-0.9.5
>> > -CATEGORIES=               audio
>> > +DISTNAME =                hydrogen-0.9.7
>> > +CATEGORIES =              audio
>> >  
>> > -HOMEPAGE=         http://www.hydrogen-music.org/
>> > +HOMEPAGE =                http://www.hydrogen-music.org/
>> > +
>> > +SHARED_LIBS =             hydrogen-core-0.9.7     0.0
>> 
>> I'm not saying it's a problem in practice, but this library name looks
>> suspicious...
>
> Yes, it is a strange name, I don't think it is a real problem though.
> Something like the following could be done to change the name:
>
> pre-configure:
>         sed -i 's,hydrogen-core-$${VERSION},hydrogen-core,g' \
>                 ${WRKSRC}/src/*/CMakeLists.txt
>
> Do you think this is worthwhile?

No, I just found the naming weird.

>> 
>> >  # GPLv2
>> > -PERMIT_PACKAGE_CDROM=     Yes
>> > +PERMIT_PACKAGE_CDROM =    Yes
>> >  
>> > -WANTLIB += ${COMPILER_LIBCXX} QtGui QtNetwork QtXml archive c
>> > -WANTLIB += jack lrdf m ogg sndfile sndio
>> > +WANTLIB += ${COMPILER_LIBCXX} QtGui QtNetwork QtXml QtXmlPatterns archive 
>> > c
>> > +WANTLIB += lrdf m sndfile sndio z
>> >  
>> > -MASTER_SITES=             ${MASTER_SITE_SOURCEFORGE:=hydrogen/}
>> > +MASTER_SITES =            ${MASTER_SITE_SOURCEFORGE:=hydrogen/}
>> >  
>> >  COMPILER =                base-clang ports-gcc base-gcc
>> >  
>> > -LIB_DEPENDS=              audio/libsndfile \
>> > -                  audio/flac \
>> > -                  audio/jack \
>> > +LIB_DEPENDS =             audio/libsndfile \
>> >                    archivers/libarchive \
>> >                    textproc/liblrdf
>> >  
>> > -RUN_DEPENDS=              devel/desktop-file-utils
>> > +BUILD_DEPENDS =           audio/ladspa
>> > +
>> > +RUN_DEPENDS =             devel/desktop-file-utils
>> >  
>> > -MODULES=          x11/qt4 devel/scons
>> > +MODULES =         devel/cmake x11/qt4
>> 
>> I would suggest splitting MODULES over multiple lines.
>
> done
>
>> 
>> Note: looks like hydrogen-1.0.0 will support Qt5.
>
> Yes, I already have a (working but incomplete) diff for 1.0.0-beta1.
> It will need some more work on the midi part to support the new
> midi feedback feature. 

ack.  Ports-wise this update looks good, except for a hidden dep on
cppunit, used to build src/tests/tests.

--8<--
russell /usr/ports/pobj/hydrogen-0.9.7/build-amd64$ src/tests/tests
QCoreApplication::applicationDirPath: Please instantiate the QApplication 
object first
assertion "__instance" failed: file 
"/usr/ports/pobj/hydrogen-0.9.7/hydrogen-0.9.7/src/core/include/hydrogen/Preferences.h",
 line 238, function "get_instance"
Abort trap
-->8--

So ok jca@ ports-wise if you add devel/cppunit to BUILD_DEPENDS (not
TEST_DEPENDS).  Good luck if you want to fix tests. :)

-- 
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE

Reply via email to