Bug 258362 - audio/libsndfile: Unnecessary uses of CMake
Summary: audio/libsndfile: Unnecessary uses of CMake
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: freebsd-multimedia (Nobody)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-09-08 09:37 UTC by WHR
Modified: 2021-09-20 17:23 UTC (History)
4 users (show)

See Also:
tcberner: maintainer-feedback+


Attachments
audio/libsndfile/files/patch-programs_sndfile-play.c (410 bytes, text/plain)
2021-09-08 09:37 UTC, WHR
no flags Details
audio.libsndfile-without-cmake.diff (3.79 KB, patch)
2021-09-08 12:10 UTC, WHR
no flags Details | Diff
audio.libsndfile-without-cmake.diff (3.82 KB, patch)
2021-09-08 12:20 UTC, WHR
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description WHR 2021-09-08 09:37:56 UTC
Created attachment 227754 [details]
audio/libsndfile/files/patch-programs_sndfile-play.c

The build fails with following messages (I uses GCC 9):

programs/sndfile-play.c: In function 'alsa_write_float':
programs/sndfile-play.c:331:4: error: duplicate case value
  331 |    case -ESTRPIPE :
      |    ^~~~
programs/sndfile-play.c:293:4: note: previously used here
  293 |    case -EPIPE :
      |    ^~~~

This is due to tho following file used by libsndfile has defined `ESTRPIPE` as `EPIPE`:
> /usr/local/include/alsa/pcm.h:#define ESTRPIPE EPIPE

Since the error is in ALSA-specific code (inside '#if HAVE_ALSA_ASOUNDLIB_H'), it won't happen if ALSA library isn't installed.


Another issue in this port is it uses CMake, despite the source itself also supporting GNU autotools as bulid system: the released source tarball contains pregenerated GNU build scripts ready for use. For me it is terrible to have to install the huge crap CMake just to build this port; please consider supporting building this port without CMake.
Comment 1 WHR 2021-09-08 11:57:32 UTC
Comment on attachment 227754 [details]
audio/libsndfile/files/patch-programs_sndfile-play.c

Since papadave already submitted my patch upstream (https://github.com/libsndfile/libsndfile/pull/775), which successfully get merged, let's discuss the possibility of CMake-free building instead.
Comment 2 WHR 2021-09-08 12:10:54 UTC
Created attachment 227757 [details]
audio.libsndfile-without-cmake.diff

The GNU autoconf version.
Added new build option ALSA to control ALSA library dependency, which is previously always disabled.
Removed build option TEST, because I didn't find the matching option of the 'configure' script; may be it should be invoked as 'make test' instead??
Comment 3 WHR 2021-09-08 12:16:26 UTC
Comment on attachment 227757 [details]
audio.libsndfile-without-cmake.diff

>diff --git a/audio/libsndfile/Makefile b/audio/libsndfile/Makefile
>index 95cf2fe40b87..09eceb94b9d4 100644
>--- a/audio/libsndfile/Makefile
>+++ b/audio/libsndfile/Makefile
>@@ -11,7 +11,7 @@ COMMENT=	Reading and writing files containing sampled sound (like WAV or AIFF)
> LICENSE=	LGPL21+
> LICENSE_FILE=	${WRKSRC}/COPYING
> 
>-USES=		cmake cpe localbase pkgconfig python:build,test shebangfix \
>+USES=		cpe localbase pkgconfig python:build,test shebangfix \
> 		tar:bz2
> SHEBANG_FILES=	programs/test-sndfile-metadata-set.py \
> 		src/binheader_writef_check.py \
>@@ -21,33 +21,27 @@ CPE_VENDOR=	${CPE_PRODUCT}_project
> 
> USE_LDCONFIG=	yes
> 
>-CMAKE_ARGS=	-DCMAKE_DISABLE_FIND_PACKAGE_ALSA:BOOL=True \
>-		-DCMAKE_DISABLE_FIND_PACKAGE_Sndio:BOOL=True \
>-		-DCMAKE_DISABLE_FIND_PACKAGE_Speex:BOOL=True \
>-		-DCMAKE_DISABLE_FIND_PACKAGE_SQLite3:BOOL=True
>+GNU_CONFIGURE=	yes
>+CONFIGURE_ARGS=	--disable-sqlite
> 
>-OPTIONS_DEFINE=	DOCS EXTERNAL MANPAGES STATIC TEST
>+OPTIONS_DEFINE=	DOCS EXTERNAL MANPAGES STATIC ALSA
> OPTIONS_SUB=	yes
>-OPTIONS_DEFAULT=	EXTERNAL
>+OPTIONS_DEFAULT=	EXTERNAL ALSA
> EXTERNAL_DESC=	Enable FLAC, Ogg Vorbis, Opus support
>-TEST_DESC=	Build tests (forces static library only)
>-TEST_IMPLIES=	STATIC
>+ALSA_DESC=	Enable ALSA support for sndfile-play
> 
> EXTERNAL_LIB_DEPENDS=	libFLAC.so:audio/flac \
> 			libogg.so:audio/libogg \
> 			libopus.so:audio/opus \
> 			libvorbis.so:audio/libvorbis
>-
>-MANPAGES_CMAKE_BOOL=	INSTALL_MANPAGES
>-STATIC_CMAKE_OFF=	-DBUILD_SHARED_LIBS:BOOL=ON
>-TEST_CMAKE_BOOL=	BUILD_TESTING
>+ALSA_LIB_DEPENDS=	libasound.so:audio/alsa-lib
>+EXTERNAL_CONFIGURE_OFF=	--disable-external-libs
>+STATIC_CONFIGURE_ON=	--disable-shared --enable-static
>+ALSA_CONFIGURE_ON=	--enable-alsa
>+ALSA_CONFIGURE_OFF=	--disable-alsa
> 
> .include <bsd.port.options.mk>
> 
>-.if ! ${PORT_OPTIONS:MEXTERNAL}
>-EXTRA_PATCHES=	${FILESDIR}/extrapatch-cmake_SndFileChecks.cmake-disableexternallibs
>-.endif
>-
> do-test:
> 	(cd ${TEST_WRKSRC} && CTEST_OUTPUT_ON_FAILURE=1 ctest -V)
> 
>diff --git a/audio/libsndfile/files/extrapatch-cmake_SndFileChecks.cmake-disableexternallibs b/audio/libsndfile/files/extrapatch-cmake_SndFileChecks.cmake-disableexternallibs
>deleted file mode 100644
>index b54e202af521..000000000000
>--- a/audio/libsndfile/files/extrapatch-cmake_SndFileChecks.cmake-disableexternallibs
>+++ /dev/null
>@@ -1,32 +0,0 @@
>---- cmake/SndFileChecks.cmake.orig	2020-07-23 13:42:53 UTC
>-+++ cmake/SndFileChecks.cmake
>-@@ -31,28 +31,7 @@ if (VCPKG_TOOLCHAIN AND (NOT CMAKE_VERSION VERSION_LES
>- 	set (CMAKE_FIND_PACKAGE_PREFER_CONFIG ON)
>- endif ()
>- 
>--if (CMAKE_FIND_PACKAGE_PREFER_CONFIG)
>--	find_package (Ogg 1.3 CONFIG)
>--	find_package (Vorbis CONFIG COMPONENTS Enc)
>--	find_package (FLAC CONFIG)
>--	find_package (Opus CONFIG)
>--
>--	include (FindPackageHandleStandardArgs)
>--	find_package_handle_standard_args (Ogg CONFIG_MODE)
>--	find_package_handle_standard_args (Vorbis CONFIG_MODE)
>--	find_package_handle_standard_args (FLAC CONFIG_MODE)
>--	find_package_handle_standard_args (Opus CONFIG_MODE)
>--else ()
>--	find_package (Ogg 1.3)
>--	find_package (Vorbis COMPONENTS Enc)
>--	find_package (FLAC)
>--	find_package (Opus)
>--endif ()
>--if (Vorbis_FOUND AND FLAC_FOUND AND Opus_FOUND)
>--	set (HAVE_EXTERNAL_XIPH_LIBS 1)
>--else ()
>--	set (HAVE_EXTERNAL_XIPH_LIBS 0)
>--endif ()
>-+set (HAVE_EXTERNAL_XIPH_LIBS 0)
>- 
>- find_package (Speex)
>- find_package (SQLite3)
>diff --git a/audio/libsndfile/pkg-plist b/audio/libsndfile/pkg-plist
>index bbe7b16354cd..c29eeb7d94aa 100644
>--- a/audio/libsndfile/pkg-plist
>+++ b/audio/libsndfile/pkg-plist
>@@ -10,10 +10,6 @@ bin/sndfile-play
> bin/sndfile-salvage
> include/sndfile.h
> include/sndfile.hh
>-lib/cmake/SndFile/SndFileConfig.cmake
>-lib/cmake/SndFile/SndFileConfigVersion.cmake
>-lib/cmake/SndFile/SndFileTargets-%%CMAKE_BUILD_TYPE%%.cmake
>-lib/cmake/SndFile/SndFileTargets.cmake
> %%STATIC%%lib/libsndfile.a
> %%NO_STATIC%%lib/libsndfile.so
> %%NO_STATIC%%lib/libsndfile.so.1
Comment 4 WHR 2021-09-08 12:20:37 UTC
Created attachment 227758 [details]
audio.libsndfile-without-cmake.diff

Fix a small issue of my previous patch. I'm not aware that the 'Edit Attachment' in BugZilla didn't work.
Comment 5 Tobias C. Berner freebsd_committer freebsd_triage 2021-09-08 15:33:56 UTC
Moin moin 


As maintainer I highly prefer cmake/meson over configure-scripts. 
So I'd rather not switch "back". It is just way easier to maintain.


mfg Tobias
Comment 6 Daniel Engberg freebsd_committer freebsd_triage 2021-09-08 20:08:41 UTC
It's much easier to maintain, debug and in many cases also improves build time quite dramatically especially when you have many cores like ARM. What you also see is that many projects (especially audio/video/desktop related ones) are moving away from GNU Autotools to already mentioned CMake or Meson and in case of CMake an additional benefit are the cmake files in lib/cmake that aren't available using Autotools.

I recognize that it can be perceived as a "downgrade" if you're looking at single port in terms of build time but past that there are a lot of benefits and usually more consistent than Autotools (less hacks/patching in Makefile). Additionally since many users (ports) will pull in CMake anyway it's not an issue if you look at the bigger picture.
Comment 7 commit-hook freebsd_committer freebsd_triage 2021-09-20 10:08:26 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/ports/commit/?id=232ed872c5e412a2a492035c1b97befce1cb49f5

commit 232ed872c5e412a2a492035c1b97befce1cb49f5
Author:     Adriaan de Groot <adridg@FreeBSD.org>
AuthorDate: 2021-09-20 09:57:05 +0000
Commit:     Adriaan de Groot <adridg@FreeBSD.org>
CommitDate: 2021-09-20 10:07:48 +0000

    audio/libsndfile: apply upstream patch for ALSA

    When ALSA is installed, some preprocessor-defines may have
    overlapping values. This patch was proposed (I think) in the
    PR mentioned, and then upstreamed.

    PR:             258362
    Reported by:    user WHR
    Obtained from:  upstream repository
    Approved by:    tcberner (multimedia@)

 audio/libsndfile/Makefile | 4 ++++
 audio/libsndfile/distinfo | 4 +++-
 2 files changed, 7 insertions(+), 1 deletion(-)
Comment 8 Adriaan de Groot freebsd_committer freebsd_triage 2021-09-20 17:23:23 UTC
The ALSA parts have landed, thank you for the pointer to upstream, and the CMake-less parts have been rejected. So I'll close this as "done as much as feasible".