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