From 4147f3afe8d9a9801d87713a544faa81d526e239 Mon Sep 17 00:00:00 2001 From: Ralph Giles Date: Thu, 7 Aug 2025 21:18:04 -0400 Subject: [PATCH 1/3] cmake: Fix disabling http support The cmake build has an option to conditionally disable network support. Similar to `./configure --disable-http` one can say cmake -B _build -D OP_DISABLE_HTTP=YES && cmake --build _build Unfortunately this worked by disabling `libopusurl` completely, which made building the examples fail. The url library can still be used to load local files or `file://` urls even when `http(s)://` support is disabled. Instead, always build the `opusurl` library like the autotools build does, and instead make the `OP_ENABLE_HTTP` pre-processor define and the `openssl` link library conditional on the cmake option. --- CMakeLists.txt | 126 ++++++++++++++++++++++++------------------------- 1 file changed, 63 insertions(+), 63 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index ee37291..af3d586 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -120,72 +120,72 @@ if(NOT OP_DISABLE_HTTP) endif() endif() cmake_pop_check_state() +endif() - add_library(opusurl - "${CMAKE_CURRENT_SOURCE_DIR}/include/opusfile.h" - "${CMAKE_CURRENT_SOURCE_DIR}/src/http.c" - "${CMAKE_CURRENT_SOURCE_DIR}/src/internal.c" - "${CMAKE_CURRENT_SOURCE_DIR}/src/internal.h" - ) - add_library(OpusFile::opusurl ALIAS opusurl) - if(WIN32) - target_sources(opusurl PRIVATE - "${CMAKE_CURRENT_SOURCE_DIR}/src/wincerts.c" - "${CMAKE_CURRENT_SOURCE_DIR}/src/winerrno.h" - ) - endif() - set_target_properties(opusurl PROPERTIES - PUBLIC_HEADER "${CMAKE_CURRENT_SOURCE_DIR}/include/opusfile.h" - VERSION ${PROJECT_VERSION} - SOVERSION ${PROJECT_VERSION_MAJOR} - ) - target_include_directories(opusurl - PRIVATE - "${CMAKE_CURRENT_SOURCE_DIR}/include" - INTERFACE - $ - $ - ) - target_compile_definitions(opusurl - PRIVATE - $<$:OP_DISABLE_FLOAT_API> - $<$:OP_FIXED_POINT> - $<$:OP_ENABLE_ASSERTIONS> - $<$:OP_HAVE_LRINTF> - $<$:OP_HAVE_CLOCK_GETTIME> - $<$:OP_HAVE_FTIME> - OP_ENABLE_HTTP - ) - target_link_libraries(opusurl - PRIVATE - opusfile - OpenSSL::SSL - $<$:ws2_32> - $<$:crypt32> - $<$:m> - ) - target_compile_options(opusurl - PRIVATE - $<$:/wd4267> - $<$:/wd4244> - $<$:/wd4090> - $<$:-std=c89> - $<$:-pedantic> - $<$:-Wall> - $<$:-Wextra> - $<$:-Wno-parentheses> - $<$:-Wno-long-long> - $<$:-fvisibility=hidden> - ) - install(TARGETS opusurl - EXPORT OpusFileTargets - RUNTIME DESTINATION "${CMAKE_INSTALL_BINDIR}" - LIBRARY DESTINATION "${CMAKE_INSTALL_LIBDIR}" - ARCHIVE DESTINATION "${CMAKE_INSTALL_LIBDIR}" - INCLUDES DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}/opus" - PUBLIC_HEADER DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}/opus" +add_library(opusurl + "${CMAKE_CURRENT_SOURCE_DIR}/include/opusfile.h" + "${CMAKE_CURRENT_SOURCE_DIR}/src/http.c" + "${CMAKE_CURRENT_SOURCE_DIR}/src/internal.c" + "${CMAKE_CURRENT_SOURCE_DIR}/src/internal.h" +) +add_library(OpusFile::opusurl ALIAS opusurl) +if(WIN32) + target_sources(opusurl PRIVATE + "${CMAKE_CURRENT_SOURCE_DIR}/src/wincerts.c" + "${CMAKE_CURRENT_SOURCE_DIR}/src/winerrno.h" ) endif() +set_target_properties(opusurl PROPERTIES + PUBLIC_HEADER "${CMAKE_CURRENT_SOURCE_DIR}/include/opusfile.h" + VERSION ${PROJECT_VERSION} + SOVERSION ${PROJECT_VERSION_MAJOR} +) +target_include_directories(opusurl + PRIVATE + "${CMAKE_CURRENT_SOURCE_DIR}/include" + INTERFACE + $ + $ +) +target_compile_definitions(opusurl + PRIVATE + $<$:OP_DISABLE_FLOAT_API> + $<$:OP_FIXED_POINT> + $<$:OP_ENABLE_ASSERTIONS> + $<$:OP_HAVE_LRINTF> + $<$:OP_HAVE_CLOCK_GETTIME> + $<$:OP_HAVE_FTIME> + $<$>:OP_ENABLE_HTTP> +) +target_link_libraries(opusurl + PRIVATE + opusfile + $<$>:OpenSSL::SSL> + $<$:ws2_32> + $<$:crypt32> + $<$:m> +) +target_compile_options(opusurl + PRIVATE + $<$:/wd4267> + $<$:/wd4244> + $<$:/wd4090> + $<$:-std=c89> + $<$:-pedantic> + $<$:-Wall> + $<$:-Wextra> + $<$:-Wno-parentheses> + $<$:-Wno-long-long> + $<$:-fvisibility=hidden> +) +install(TARGETS opusurl + EXPORT OpusFileTargets + RUNTIME DESTINATION "${CMAKE_INSTALL_BINDIR}" + LIBRARY DESTINATION "${CMAKE_INSTALL_LIBDIR}" + ARCHIVE DESTINATION "${CMAKE_INSTALL_LIBDIR}" + INCLUDES DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}/opus" + PUBLIC_HEADER DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}/opus" +) if(NOT OP_DISABLE_EXAMPLES) add_executable(opusfile_example From 38d4fd5d16056cc02274c8718bc23169b9d5ba55 Mon Sep 17 00:00:00 2001 From: Ralph Giles Date: Fri, 8 Aug 2025 08:53:24 -0400 Subject: [PATCH 2/3] cmake: add -lsocket if necessary On OpenIndiana (Illumos) the cmake build fails with indirect symbol references to functions declared by `sys/socket.h`. To address this, check for `getsockopt` linkage, and if it fails try again with `-lsocket` which provides the symbols on that operating system. This doesn't seem to be necessary with the autotools build; perhaps libtool is passing something extra to the linker to have it accept the transitive dependencies. Fixes https://github.com/xiph/opusfile/issues/60 Fixes https://gitlab.xiph.org/xiph/opusfile/-/issues/2337 --- CMakeLists.txt | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index af3d586..317bd33 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -103,6 +103,13 @@ if(NOT OP_DISABLE_HTTP) if(NOT OP_HAVE_SYS_SOCKET_H) message(FATAL_ERROR "HTTP support requires a POSIX socket library") endif() + check_symbol_exists(getsockopt "sys/socket.h" OP_HAVE_GETSOCKOPT) + if(NOT OP_HAVE_GETSOCKOPT) + cmake_push_check_state() + list(APPEND CMAKE_REQUIRED_LIBRARIES "socket") + check_symbol_exists(getsockopt "sys/socket.h" OP_NEED_SOCKET) + cmake_pop_check_state() + endif() endif() check_c_source_compiles( "#include @@ -164,6 +171,7 @@ target_link_libraries(opusurl $<$:ws2_32> $<$:crypt32> $<$:m> + $<$:socket> ) target_compile_options(opusurl PRIVATE From 8890ea3e96cbeb813c8660bfb4222bad10009c60 Mon Sep 17 00:00:00 2001 From: Ralph Giles Date: Fri, 8 Aug 2025 09:20:56 -0400 Subject: [PATCH 3/3] gitlab-ci: Add default cmake and cmake-nohttp jobs In addition to testing the build with a legacy cmake-3.16 upstream package, install the runner's default cmake version and test against that for broader coverage. On the current `gcc:9` container image this is cmake 3.18.4. Also test build with `-D OP_DISABLE_HTTP=YES` to confirm the build completes with this option, if not that it does what it's supposed to. This should protect against regression of the earlier fix. --- .gitlab-ci.yml | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 274bc97..5511729 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -16,7 +16,7 @@ autotools: - make - make distcheck -cmake: +cmake-3.16: stage: build before_script: - apt-get update && @@ -30,6 +30,30 @@ cmake: tags: - docker +cmake: + stage: build + before_script: + - apt-get update && + apt-get install -y libopus-dev libogg-dev libssl-dev + cmake doxygen + script: + - cmake -Bbuild -H. + - cmake --build build + tags: + - docker + +cmake-nohttp: + stage: build + before_script: + - apt-get update && + apt-get install -y libopus-dev libogg-dev libssl-dev + cmake doxygen + script: + - cmake -Bbuild -H. -D OP_DISABLE_HTTP=YES + - cmake --build build + tags: + - docker + makefile: stage: build before_script: