Re: [new] audio/zmusic-1.1.3

Omar Polo <op@openbsd.org> [2021-12-11, 12:56 +0100]:

> Timo Myyrä <timo.myyra@bittivirhe.fi> 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 <timo.myyra@bittivirhe.fi>
>
> -# 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