Skip to content

Conversation

rillian
Copy link
Contributor

@rillian rillian commented Aug 8, 2025

Fix cmake -D OP_DISABLE_HTTP=YES and pass -lsocket on illumos.

@h3ndrk @sezero Would appreciate your review on this.

rillian added 3 commits August 7, 2025 21:18
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.
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 #60
Fixes https://gitlab.xiph.org/xiph/opusfile/-/issues/2337
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.
@rillian rillian self-assigned this Aug 8, 2025
@rillian rillian closed this Aug 9, 2025
@rillian rillian merged commit 8890ea3 into main Aug 9, 2025
12 checks passed
@rillian rillian deleted the cmake-illumos branch August 9, 2025 15:12
@rillian
Copy link
Contributor Author

rillian commented Aug 9, 2025

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants