dbry / WavPack

WavPack encode/decode library, command-line programs, and several plugins
BSD 3-Clause "New" or "Revised" License
362 stars 66 forks source link

CMake default options #163

Closed redtide closed 1 year ago

redtide commented 1 year ago

We are using WavPack as git submodule/directory in our project to build it statically as part of an "audiofile static library" for our sfz library project.

Just noticed in CMake configuration:

-- The following features have been enabled:

 * WAVPACK_ENABLE_DSD, Enable support for WavPack DSD files.
 * WAVPACK_ENABLE_THREADS, Enable support for threading in libwavpack.
 * WAVPACK_INSTALL_CMAKE_MODULE, Generate and install CMake package configuration module.
 * WAVPACK_ENABLE_ASM, Enable assembly optimizations.
 * WAVPACK_ENABLE_LIBCRYPTO, Use OpenSSL::Crypto library.
 * WAVPACK_BUILD_PROGRAMS, Build programs.
 * WAVPACK_INSTALL_DOCS, Install documentation.
 * WAVPACK_INSTALL_PKGCONFIG_MODULE, Generate and install wavpack.pc.

maybe the build programs and install xyz options could be disabled automatically if the project is not the top most (sub project), what do you think? Though I'm not sure about others like libcrypto and dsd.

set(PROJECT_IS_MAIN OFF)
if(PROJECT_SOURCE_DIR STREQUAL CMAKE_SOURCE_DIR)
    set(PROJECT_IS_MAIN ON)
endif()
cmake_dependent_option(WAVPACK_BUILD_PROGRAMS "Build programs" ON
    "(WIN32 OR Iconv_FOUND) AND PROJECT_IS_MAIN" OFF)
# ...

though it seems there are 2 problems: from cmake_dependent_option documentation:

New in version 3.22: Full Condition Syntax is now supported. See policy CMP0127.

Isn't OR (already there, despite my added AND) part of that full condition syntax, so that it requires at least CMake 3.22?

2nd problem: BUILD_TESTING is used also in another library (Google Abseil). I don't think it's a good idea generic variable names like that, can you rename it to WAVPACK_BUILD_TESTING?

dbry commented 1 year ago

Unfortunately I'm not well versed in using CMake with a git submodule (or really, CMake in general), so I'll have to defer to @sezero , @madebr , and @SoapGentoo for those specifics.

What I can say is that is does make sense to not build the command-line programs if all you want is libwavpack, but some projects use the command-line executables with pipes, so they would need them. Is it possible to pass options into the CMake submodule based on the specific needs of the high-level project? Or maybe just delete the artifacts that aren't needed? Like I say, this is not my wheelhouse.

I think for the other options, the defaults are pretty reasonable, and the only reason for disabling a given option (like DSD or threading) would be a tiny savings in space, or in rare cases eliminating a dependency that wasn't available on a specific platform.

redtide commented 1 year ago

Thank you for the quick response. It's not that important, just something similar to a "silence the warnings" issue; the messages seems to be a "false alarm".

Is it possible to pass options into the CMake submodule based on the specific needs of the high-level project?

I solved by setting them OFF before our add_subdirectory("thirdparty/wavpack" EXCLUDE_FROM_ALL), where I forgot that EXCLUDE_FROM_ALL should avoid the unwanted builds.

I think for the other options, the defaults are pretty reasonable, and the only reason for disabling a given option (like DSD or threading) would be a tiny savings in space, or in rare cases eliminating a dependency that wasn't available on a specific platform.

Sure, in the end it's mainly referred to the build/install variables/options and only for the CMake message above that might be confusing, though they can be disabled (as I did) in the top project.