Omar Polo <[email protected]> [2021-12-11, 12:56 +0100]: > Timo Myyrä <[email protected]> writes: > >> Hi, >> >> Here's an new port audio/zmusic. It is the music library which got >> extracted from the gzdoom as standalone part. This is dependency for >> upcoming games/gzdoom update. >> >> Timo > > Hello, > > the port looks fine, just some comments. -DZMUSIC_VERSION is not > needed, modcmake takes care by itself of the library name once you have > SHARED_LIBS defined. (usually one does a first build then sets > SHARED_LIBS and re-build again so the plist is correct) > > source/CMakeLists.txt unconditionally checks for FluidSynth too, so we > need to either patch that out or add a LDEP, I went with the latter. > > I'd also add a comment about the C++ version (C++11 in this case) before > the COMPILER line. > > Regarding the licensing, the only instance of 'Free Software Foundation' > in sources/ is a GPLv2+ header, while various stuff in thirdparties is > gpl2+ or 3+. In short, I'd set it to GPLv3+, should be fair enough. > > Then, some nitpicking: > > - we can avoid to define V and DISTNAME by setting > PKGNAME=${DISTNAME:L} > - don't indent WANTLIBs, taking the output from make > port-lib-depends-check is easier > - I think CONFIGURE_ARGS one per line are more readable. > > It'd be nice to use timidity from ports instead of the bundled one, but > it needs also tidmidityplus and I don't know how hard it could be. The > other stuff in thirdparty/ doesn't seem to be available in the port > tree, and cmake doesn't even try to look for a system version of those > libraries, so it shouldn't be a problem for bulk builds. > > btw, there was a previous submission for libgme[0] as deps for srb2, > zmusic could use libgme too instead of bundling it. (this note is more > for the archive if/when we'll import it than a comment on the submission > itself) > > [0]: https://marc.info/?l=openbsd-ports&m=163878475724147&w=2 > > > Here's a diff against your makefile and an updated tarball: > > --- Makefile.orig Sat Dec 11 12:51:25 2021 > +++ Makefile Sat Dec 11 13:17:51 2021 > @@ -2,11 +2,11 @@ > > COMMENT = gzdoom music library as standalone form for reuse > > -V = 1.1.3 > +PKGNAME = ${DISTNAME:L} > + > GH_ACCOUNT = coelckers > GH_PROJECT = ZMusic > -GH_TAGNAME = ${V} > -DISTNAME = zmusic-${V} > +GH_TAGNAME = 1.1.3 > > SHARED_LIBS += zmusic 0.0 # 1.1.0 > SHARED_LIBS += zmusiclite 0.0 # 1.1.0 > @@ -17,20 +17,22 @@ > > MAINTAINER = Timo Myyra <[email protected]> > > -# GPL3 > +# GPL3+ > PERMIT_PACKAGE = Yes > > -WANTLIB += ${COMPILER_LIBCXX} m mpg123 > -WANTLIB += sndfile z > +WANTLIB += ${COMPILER_LIBCXX} fluidsynth m mpg123 sndfile z > > +# C++11 > COMPILER = base-clang ports-gcc > > MODULES = devel/cmake > > -CONFIGURE_ARGS += -DZMUSIC_VERSION=${LIBzmusic_VERSION} > -CONFIGURE_ARGS += -DDYN_MPG123=NO -DDYN_SNDFILE=NO > +CONFIGURE_ARGS += -DDYN_FLUIDSYNTH=NO \ > + -DDYN_MPG123=NO \ > + -DDYN_SNDFILE=NO > > -LIB_DEPENDS = audio/libsndfile \ > +LIB_DEPENDS = audio/fluidsynth \ > + audio/libsndfile \ > audio/mpg123 > > NO_TEST = Yes
Good points and makes the diff better. Timo
