search

libsdl-org / SDL_mixer

An audio mixer that supports various file formats for Simple Directmedia Layer.
zlib License
399 stars 134 forks source link

Failure to static link. #467

Closed FtZPetruska closed 1 year ago

FtZPetruska commented 1 year ago

When fixing the sdl2-mixer static builds on vcpkg, I noticed a few issues when trying to link the library. The same issues were noticed when building release-2.6.2 from source.

Configuration was done as follows:

cmake -S<src> -Bbuild -G Ninja -DCMAKE_BUILD_TYPE=Release
-DCMAKE_OSX_ARCHITECTURES=arm64
-DCMAKE_SYSTEM_NAME=Darwin
-DBUILD_SHARED_LIBS=OFF
-DSDL2MIXER_DEPS_SHARED=0
-DSDL2MIXER_VENDORED=OFF
-DSDL2MIXER_SAMPLES=OFF
-DSDL2MIXER_MIDI_FLUIDSYNTH=ON
-DSDL2MIXER_MIDI_NATIVE=ON
-DSDL2MIXER_FLAC_LIBFLAC=ON
-DSDL2MIXER_FLAC_DRFLAC=OFF
-DSDL2MIXER_MOD=ON
-DSDL2MIXER_MOD_MODPLUG=ON
-DSDL2MIXER_VORBIS=VORBISFILE
-DSDL2MIXER_VORBIS_VORBISFILE=ON
-DSDL2MIXER_MP3_MPG123=ON
-DSDL2MIXER_MP3_DRMP3=OFF
-DSDL2MIXER_OPUS=ON

My test project CMakeLists is as follows:

cmake_minimum_required(VERSION 3.0.0)
project(mixer-example VERSION 0.1.0 LANGUAGES C CXX)

add_executable(mixer-example main.cpp)

find_package(SDL2 CONFIG REQUIRED)
find_package(SDL2_mixer CONFIG REQUIRED)
target_link_libraries(mixer-example PRIVATE SDL2_mixer::SDL2_mixer-static SDL2::SDL2main SDL2::SDL2-static)

And the main.cpp file is just a simple hello world that initialises SDL_mixer:

#include <SDL.h>
#include <SDL_mixer.h>

int main(int, char**) {
  if (SDL_Init(SDL_INIT_AUDIO) < 0) {
    SDL_Log("Failed SDL_Init: %s\n", SDL_GetError());
    return 1;
  }

  constexpr int MIXER_FLAGS = MIX_INIT_FLAC | MIX_INIT_MID | MIX_INIT_MOD | MIX_INIT_MP3 | MIX_INIT_OGG | MIX_INIT_OPUS;
  if (Mix_Init(MIXER_FLAGS) != MIXER_FLAGS) {
    SDL_Log("Failed Mix_Init: %s\n", Mix_GetError());
    SDL_Quit();
    return 1;
  }

  Mix_Quit();
  SDL_Quit();
  return 0;
}

The first issue with this configuration is linking libFLAC, the LINK_LIBRARIES is:

vcpkg_installed/arm64-osx/debug/lib/libSDL2maind.a
vcpkg_installed/arm64-osx/debug/lib/libSDL2d.a
vcpkg_installed/arm64-osx/debug/lib/libSDL2_mixerd.a
-lm  -liconv  -Wl,-framework,CoreVideo  -Wl,-framework,Cocoa  -Wl,-framework,IOKit  -Wl,-framework,ForceFeedback  -Wl,-framework,Carbon  -Wl,-framework,CoreAudio  -Wl,-framework,AudioToolbox  -Wl,-framework,AVFoundation  -Wl,-framework,Foundation  -Wl,-weak_framework,GameController  -Wl,-weak_framework,Metal  -Wl,-weak_framework,QuartzCore  -Wl,-weak_framework,CoreHaptics  
-lFLAC
vcpkg_installed/arm64-osx/debug/lib/libmodplug.a
vcpkg_installed/arm64-osx/debug/lib/libmpg123.a
vcpkg_installed/arm64-osx/debug/lib/libfluidsynth.a

Failing with:

ld: library not found for -lFLAC

Manually replacing -lFLAC with vcpkg_installed/arm64-osx/debug/lib/libFLAC.a fixes this issue, but now returns undefined references for fluidsynth and FLAC. It seems to be lacking glib, openmp (for fluidsynth), and libogg (for FLAC).

Here is the details of the error ``` [build] Undefined symbols for architecture arm64: [build] "___kmpc_barrier", referenced from: [build] _.omp_outlined._debug__ in libfluidsynth.a(fluid_defsfont.c.o) [build] "___kmpc_critical", referenced from: [build] _.omp_task_entry. in libfluidsynth.a(fluid_defsfont.c.o) [build] _.omp_task_entry..10 in libfluidsynth.a(fluid_defsfont.c.o) [build] "___kmpc_end_critical", referenced from: [build] _.omp_task_entry. in libfluidsynth.a(fluid_defsfont.c.o) [build] _.omp_task_entry..10 in libfluidsynth.a(fluid_defsfont.c.o) [build] "___kmpc_end_single", referenced from: [build] _.omp_outlined._debug__ in libfluidsynth.a(fluid_defsfont.c.o) [build] "___kmpc_fork_call", referenced from: [build] _fluid_defsfont_load_all_sampledata in libfluidsynth.a(fluid_defsfont.c.o) [build] "___kmpc_omp_task", referenced from: [build] _.omp_outlined._debug__ in libfluidsynth.a(fluid_defsfont.c.o) [build] "___kmpc_omp_task_alloc", referenced from: [build] _.omp_outlined._debug__ in libfluidsynth.a(fluid_defsfont.c.o) [build] "___kmpc_single", referenced from: [build] _.omp_outlined._debug__ in libfluidsynth.a(fluid_defsfont.c.o) [build] "_g_assertion_message_expr", referenced from: [build] _delete_fluid_num_setting in libfluidsynth.a(fluid_settings.c.o) [build] _delete_fluid_int_setting in libfluidsynth.a(fluid_settings.c.o) [build] _delete_fluid_str_setting in libfluidsynth.a(fluid_settings.c.o) [build] _delete_fluid_set_setting in libfluidsynth.a(fluid_settings.c.o) [build] _fluid_align_ptr in libfluidsynth.a(fluid_rvoice_mixer.c.o) [build] _fluid_rvoice_buffers_mix in libfluidsynth.a(fluid_rvoice_mixer.c.o) [build] "_g_clear_error", referenced from: [build] _new_fluid_thread in libfluidsynth.a(fluid_sys.c.o) [build] "_g_cond_broadcast", referenced from: [build] _delete_rvoice_mixer_threads in libfluidsynth.a(fluid_rvoice_mixer.c.o) [build] _fluid_render_loop_multithread in libfluidsynth.a(fluid_rvoice_mixer.c.o) [build] "_g_cond_clear", referenced from: [build] _delete_fluid_cond in libfluidsynth.a(fluid_rvoice_mixer.c.o) [build] "_g_cond_init", referenced from: [build] _new_fluid_cond in libfluidsynth.a(fluid_rvoice_mixer.c.o) [build] "_g_cond_signal", referenced from: [build] _fluid_mixer_thread_func in libfluidsynth.a(fluid_rvoice_mixer.c.o) [build] "_g_cond_wait", referenced from: [build] _fluid_mixer_thread_func in libfluidsynth.a(fluid_rvoice_mixer.c.o) [build] _fluid_render_loop_multithread in libfluidsynth.a(fluid_rvoice_mixer.c.o) [build] "_g_file_test", referenced from: [build] _fluid_file_open in libfluidsynth.a(fluid_sys.c.o) [build] "_g_free", referenced from: [build] _delete_fluid_cond in libfluidsynth.a(fluid_rvoice_mixer.c.o) [build] _delete_fluid_cond_mutex in libfluidsynth.a(fluid_rvoice_mixer.c.o) [build] "_g_get_monotonic_time", referenced from: [build] _fluid_utime in libfluidsynth.a(fluid_sys.c.o) [build] "_g_malloc_n", referenced from: [build] _new_fluid_cond in libfluidsynth.a(fluid_rvoice_mixer.c.o) [build] _new_fluid_cond_mutex in libfluidsynth.a(fluid_rvoice_mixer.c.o) [build] "_g_mutex_clear", referenced from: [build] _delete_fluid_cond_mutex in libfluidsynth.a(fluid_rvoice_mixer.c.o) [build] _delete_fluid_midi_router in libfluidsynth.a(fluid_midi_router.c.o) [build] "_g_mutex_init", referenced from: [build] _new_fluid_cond_mutex in libfluidsynth.a(fluid_rvoice_mixer.c.o) [build] _new_fluid_server2 in libfluidsynth.a(fluid_cmd.c.o) [build] _new_fluid_midi_router in libfluidsynth.a(fluid_midi_router.c.o) [build] "_g_mutex_lock", referenced from: [build] _delete_rvoice_mixer_threads in libfluidsynth.a(fluid_rvoice_mixer.c.o) [build] _fluid_mixer_thread_func in libfluidsynth.a(fluid_rvoice_mixer.c.o) [build] _fluid_render_loop_multithread in libfluidsynth.a(fluid_rvoice_mixer.c.o) [build] _fluid_server_add_client in libfluidsynth.a(fluid_cmd.c.o) [build] _fluid_server_remove_client in libfluidsynth.a(fluid_cmd.c.o) [build] _fluid_server_close in libfluidsynth.a(fluid_cmd.c.o) [build] _fluid_midi_router_set_default_rules in libfluidsynth.a(fluid_midi_router.c.o) [build] ... [build] "_g_mutex_unlock", referenced from: [build] _delete_rvoice_mixer_threads in libfluidsynth.a(fluid_rvoice_mixer.c.o) [build] _fluid_mixer_thread_func in libfluidsynth.a(fluid_rvoice_mixer.c.o) [build] _fluid_render_loop_multithread in libfluidsynth.a(fluid_rvoice_mixer.c.o) [build] _fluid_server_add_client in libfluidsynth.a(fluid_cmd.c.o) [build] _fluid_server_remove_client in libfluidsynth.a(fluid_cmd.c.o) [build] _fluid_server_close in libfluidsynth.a(fluid_cmd.c.o) [build] _fluid_midi_router_set_default_rules in libfluidsynth.a(fluid_midi_router.c.o) [build] ... [build] "_g_private_get", referenced from: [build] _fluid_synth_tuning_iteration_next in libfluidsynth.a(fluid_synth.c.o) [build] "_g_private_set", referenced from: [build] _fluid_synth_tuning_iteration_start in libfluidsynth.a(fluid_synth.c.o) [build] _fluid_synth_tuning_iteration_next in libfluidsynth.a(fluid_synth.c.o) [build] "_g_rec_mutex_clear", referenced from: [build] _delete_fluid_settings in libfluidsynth.a(fluid_settings.c.o) [build] _delete_fluid_synth in libfluidsynth.a(fluid_synth.c.o) [build] _fluid_sffile_close in libfluidsynth.a(fluid_sffile.c.o) [build] "_g_rec_mutex_init", referenced from: [build] _new_fluid_settings in libfluidsynth.a(fluid_settings.c.o) [build] _new_fluid_synth in libfluidsynth.a(fluid_synth.c.o) [build] _fluid_sffile_open in libfluidsynth.a(fluid_sffile.c.o) [build] "_g_rec_mutex_lock", referenced from: [build] _fluid_settings_register_str in libfluidsynth.a(fluid_settings.c.o) [build] _fluid_settings_register_num in libfluidsynth.a(fluid_settings.c.o) [build] _fluid_settings_register_int in libfluidsynth.a(fluid_settings.c.o) [build] _fluid_settings_callback_str in libfluidsynth.a(fluid_settings.c.o) [build] _fluid_settings_callback_num in libfluidsynth.a(fluid_settings.c.o) [build] _fluid_settings_callback_int in libfluidsynth.a(fluid_settings.c.o) [build] _fluid_settings_get_user_data in libfluidsynth.a(fluid_settings.c.o) [build] ... [build] "_g_rec_mutex_unlock", referenced from: [build] _fluid_settings_register_str in libfluidsynth.a(fluid_settings.c.o) [build] _fluid_settings_register_num in libfluidsynth.a(fluid_settings.c.o) [build] _fluid_settings_register_int in libfluidsynth.a(fluid_settings.c.o) [build] _fluid_settings_callback_str in libfluidsynth.a(fluid_settings.c.o) [build] _fluid_settings_callback_num in libfluidsynth.a(fluid_settings.c.o) [build] _fluid_settings_callback_int in libfluidsynth.a(fluid_settings.c.o) [build] _fluid_settings_get_user_data in libfluidsynth.a(fluid_settings.c.o) [build] ... [build] "_g_return_if_fail_warning", referenced from: [build] _new_fluid_thread in libfluidsynth.a(fluid_sys.c.o) [build] "_g_shell_parse_argv", referenced from: [build] _fluid_command in libfluidsynth.a(fluid_cmd.c.o) [build] "_g_strfreev", referenced from: [build] _fluid_command in libfluidsynth.a(fluid_cmd.c.o) [build] "_g_thread_join", referenced from: [build] _fluid_thread_join in libfluidsynth.a(fluid_sys.c.o) [build] "_g_thread_try_new", referenced from: [build] _new_fluid_thread in libfluidsynth.a(fluid_sys.c.o) [build] "_g_thread_unref", referenced from: [build] _new_fluid_thread in libfluidsynth.a(fluid_sys.c.o) [build] "_g_usleep", referenced from: [build] _fluid_msleep in libfluidsynth.a(fluid_sys.c.o) [build] "_ogg_page_serialno", referenced from: [build] _FLAC__ogg_decoder_aspect_read_callback_wrapper in libFLAC.a(ogg_decoder_aspect.c.o) [build] "_ogg_stream_clear", referenced from: [build] _FLAC__ogg_decoder_aspect_finish in libFLAC.a(ogg_decoder_aspect.c.o) [build] "_ogg_stream_init", referenced from: [build] _FLAC__ogg_decoder_aspect_init in libFLAC.a(ogg_decoder_aspect.c.o) [build] "_ogg_stream_packetout", referenced from: [build] _FLAC__ogg_decoder_aspect_read_callback_wrapper in libFLAC.a(ogg_decoder_aspect.c.o) [build] "_ogg_stream_pagein", referenced from: [build] _FLAC__ogg_decoder_aspect_read_callback_wrapper in libFLAC.a(ogg_decoder_aspect.c.o) [build] "_ogg_stream_reset", referenced from: [build] _FLAC__ogg_decoder_aspect_flush in libFLAC.a(ogg_decoder_aspect.c.o) [build] "_ogg_sync_buffer", referenced from: [build] _FLAC__ogg_decoder_aspect_read_callback_wrapper in libFLAC.a(ogg_decoder_aspect.c.o) [build] "_ogg_sync_clear", referenced from: [build] _FLAC__ogg_decoder_aspect_finish in libFLAC.a(ogg_decoder_aspect.c.o) [build] "_ogg_sync_init", referenced from: [build] _FLAC__ogg_decoder_aspect_init in libFLAC.a(ogg_decoder_aspect.c.o) [build] "_ogg_sync_pageout", referenced from: [build] _FLAC__ogg_decoder_aspect_read_callback_wrapper in libFLAC.a(ogg_decoder_aspect.c.o) [build] "_ogg_sync_reset", referenced from: [build] _FLAC__ogg_decoder_aspect_flush in libFLAC.a(ogg_decoder_aspect.c.o) [build] "_ogg_sync_wrote", referenced from: [build] _FLAC__ogg_decoder_aspect_read_callback_wrapper in libFLAC.a(ogg_decoder_aspect.c.o) ```

This can be fixed by adding: vcpkg_installed/arm64-osx/debug/lib/libogg.a vcpkg_installed/arm64-osx/debug/lib/libglib-2.0.a vcpkg_installed/arm64-osx/debug/lib/libintl.a /opt/homebrew/lib/libomp.a to the LINK_LIBRARIES. OpenMP may be an upstream issue as Apple's toolchains do not ship with it, but the fluidsynth build found one from another toolchain.

Lastly, despite building with OGG support using VORBISFILE and Opus support, they do not seem to be built in as even manually adding them to the build command fails at runtime with OPUS/OGG support not available.

Additional info:

The issue could not be reproduced when building shared libraries, apart from SDL_mixer being built without OPUS support.

CMake configuration logs ``` -- The C compiler identification is AppleClang 14.0.0.14000029 -- Detecting C compiler ABI info -- Detecting C compiler ABI info - done -- Check for working C compiler: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/cc - skipped -- Detecting C compile features -- Detecting C compile features - done -- Configuring SDL2_mixer 2.6.1 -- Using system opusfile -- Found opusfile: /Users/.../git/vcpkg/installed/arm64-osx/lib/libopusfile.a -- Dynamic opus (opusfile): libopusfile.a -- Using system vorbisfile -- Found vorbisfile: /Users/.../git/vcpkg/installed/arm64-osx/lib/libvorbisfile.a -- Dynamic vorbisfile: libvorbisfile.a -- Using system libflac -- Found FLAC: /Users/.../git/vcpkg/installed/arm64-osx/lib/libFLAC.a -- Using system libmodplug -- Found modplug: /Users/.../git/vcpkg/installed/arm64-osx/lib/libmodplug.a -- Using system mpg123 -- Found MPG123: /Users/.../git/vcpkg/installed/arm64-osx/lib/libmpg123.a -- Using system FluidSynth -- Found FluidSynth: /Users/.../git/vcpkg/installed/arm64-osx/lib/libfluidsynth.a -- Configuring done -- Generating done ```
Variables set in SDL_mixerConfig.cmake ```cmake set(SDL2_mixer_FOUND ON) set(SDL2MIXER_VENDORED OFF) set(SDL2MIXER_CMD OFF) set(SDL2MIXER_FLAC_LIBFLAC ON) set(SDL2MIXER_FLAC_DRFLAC OFF) set(SDL2MIXER_MOD ON) set(SDL2MIXER_MOD_MODPLUG ON) set(SDL2MIXER_MOD_XMP OFF) set(SDL2MIXER_MOD_XMP_LITE OFF) set(SDL2MIXER_MP3 ON) set(SDL2MIXER_MP3_DRMP3 OFF) set(SDL2MIXER_MP3_MPG123 ON) set(SDL2MIXER_MIDI ON) set(SDL2MIXER_MIDI_FLUIDSYNTH ON) set(SDL2MIXER_MIDI_NATIVE ON) set(SDL2MIXER_MIDI_TIMIDITY ON) set(SDL2MIXER_OPUS ON) set(SDL2MIXER_VORBIS VORBISFILE) set(SDL2MIXER_VORBIS_STB OFF) set(SDL2MIXER_VORBIS_TREMOR OFF) set(SDL2MIXER_VORBIS_VORBISFILE ON) set(SDL2MIXER_WAVE ON) ```
FtZPetruska commented 1 year ago

Here's an update after testing on the latest commit:

[cmake] The link interface of target "SDL2_mixer::SDL2_mixer-static" contains: [cmake] [cmake] opusfile::opusfile [cmake] [cmake] but the target was not found.

This can be fixed by modifying `SDL2_mixerConfig.cmake`:
```cmake
    if(SDL2MIXER_VORBIS_VORBISFILE AND NOT SDL2MIXER_VENDORED AND NOT TARGET Vorbis::vorbisfile)
        find_dependency(Vorbis)
    endif()

    if(SDL2MIXER_OPUS AND NOT SDL2MIXER_VENDORED AND NOT TARGET opusfile::opusfile)
        find_dependency(opusfile)
    endif()
madebr commented 1 year ago

Those missing find_dependency's in SDL2_mixerConfig.cmake are bugs and should be added.

About the dependencies of static libraries, this is a hard problem to get right and is either the job of the package manager, a config file of a dependency or the user. A user can modify things by setting certain cache variables. So at least, there's no need for patching.

In https://github.com/libsdl-org/SDL_mixer/pull/468, I've renamed Findopusfile.cmake to FindOpusFile.cmake such that it is compatible with OpusFileConfig.cmake installed by upstream. Usually, config files have precedence over modules so if you have installed the dependencies through CMake these config files are used.

FindFluidsynth.cmake should also be compatible FluidSynthConfig.cmake installed here

FindVorbis.cmake should also be compatible with VorbisConfig.cmake installed here

FtZPetruska commented 1 year ago

Thank you for your reply!

FindVorbis.cmake should also be compatible with VorbisConfig.cmake installed here

Regarding the current implementation, it lacks the find_dependency(Ogg REQUIRED). Moreover, SDL_mixerConfig.cmake seems to be using the target vorbisfile::vorbisfile instead of upstream's Vorbis::vorbisfile.

Similarly, libflac's upstream config file calls find_dependency(Ogg), and opusfile calls find_package on both Ogg and Opus.

FindFluidsynth.cmake should also be compatible FluidSynthConfig.cmake installed here

As to fluidsynth, it's a little trickier. Upstream uses this:

# Create imported target FluidSynth::libfluidsynth-OBJ
add_library(FluidSynth::libfluidsynth-OBJ INTERFACE IMPORTED)

set_target_properties(FluidSynth::libfluidsynth-OBJ PROPERTIES
  INTERFACE_LINK_LIBRARIES "-Wl,-framework,CoreAudio,-framework,AudioUnit;-Wl,-framework,CoreMIDI,-framework,CoreServices;m;PkgConfig::GLIB;PkgConfig::LIBSNDFILE;PkgConfig::PORTAUDIO;PkgConfig::READLINE"
)

# Create imported target FluidSynth::fluidsynth
add_executable(FluidSynth::fluidsynth IMPORTED)

# Create imported target FluidSynth::libfluidsynth
add_library(FluidSynth::libfluidsynth SHARED IMPORTED)

set_target_properties(FluidSynth::libfluidsynth PROPERTIES
  INTERFACE_INCLUDE_DIRECTORIES "${_IMPORT_PREFIX}/include"
)

Where all the dependencies get searched and added through PkgConfig.

A user can modify things by setting certain cache variables. So at least, there's no need for patching.

While I agree that this is a good workaround where I even used it in CI. It exhibits a different behaviour from using the packages individually. If you were to statically link all the direct dependencies of SDL_mixer build would succeed, but if you statically link SDL_mixer it fails as the transitive dependencies are not pulled.

Anyway, I'd be happy to help so let me know if you need testing!

FtZPetruska commented 1 year ago

Okay, so I've been working on a patch to address this issue.

For now, the patch only fixes the undefined references to libogg functions in FLAC, vorbis and tremor. But it should be trivial to extend to Opus after you merge your aforementioned PR.

The patch is designed to follow what the other libraries do in their CMake config file:

I was able to confirm that it fixes the linkage issues (for those specific libraries at least) when doing static builds. While not conflicting with vendored builds (static and shared) or shared builds with system libs.

The patch in question ```diff diff --git a/CMakeLists.txt b/CMakeLists.txt index de7b2120..411dc481 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -809,6 +809,7 @@ if(SDL2MIXER_INSTALL) cmake/FindMPG123.cmake cmake/FindVorbis.cmake cmake/Findtremor.cmake + cmake/FindOgg.cmake DESTINATION "${SDLMIXER_INSTALL_CMAKEDIR}" COMPONENT devel ) diff --git a/cmake/FindFLAC.cmake b/cmake/FindFLAC.cmake index b5ad76eb..a1a7a5e4 100644 --- a/cmake/FindFLAC.cmake +++ b/cmake/FindFLAC.cmake @@ -18,6 +18,11 @@ find_package_handle_standard_args(FLAC REQUIRED_VARS FLAC_LIBRARY FLAC_INCLUDE_PATH ) +include(CMakeFindDependencyMacro) +if (NOT TARGET Ogg::ogg) + find_dependency(Ogg REQUIRED) +endif() + if(FLAC_FOUND) if(NOT TARGET FLAC::FLAC) add_library(FLAC::FLAC UNKNOWN IMPORTED) @@ -25,7 +30,7 @@ if(FLAC_FOUND) IMPORTED_LOCATION "${FLAC_LIBRARY}" INTERFACE_INCLUDE_DIRECTORIES "${FLAC_INCLUDE_PATH}" INTERFACE_COMPILE_OPTIONS "${FLAC_COMPILE_OPTIONS}" - INTERFACE_LINK_LIBRARIES "${FLAC_LINK_LIBRARIES}" + INTERFACE_LINK_LIBRARIES "${FLAC_LINK_LIBRARIES};Ogg::ogg" INTERFACE_LINK_FLAGS "${FLAC_LINK_FLAGS}" ) endif() diff --git a/cmake/FindOgg.cmake b/cmake/FindOgg.cmake new file mode 100644 index 00000000..b60c3526 --- /dev/null +++ b/cmake/FindOgg.cmake @@ -0,0 +1,26 @@ +find_package(PkgConfig) +pkg_check_modules(_OGG QUIET ogg) + +find_path(OGG_INCLUDE_DIR + NAMES "ogg/ogg.h" + PATHS ${_OGG_INCLUDE_DIRS}) + +find_library(OGG_LIBRARY + NAMES ogg libogg + HINTS ${_OGG_LIBRARY_DIRS}) + +mark_as_advanced( + OGG_INCLUDE_DIR + OGG_LIBRARY) + +include(FindPackageHandleStandardArgs) +find_package_handle_standard_args(Ogg + REQUIRED_VARS OGG_INCLUDE_DIR OGG_LIBRARY + VERSION_VAR _OGG_VERSION) + +if(OGG_FOUND AND NOT TARGET Ogg::ogg) + add_library(Ogg::ogg UNKNOWN IMPORTED) + set_target_properties(Ogg::ogg PROPERTIES + INTERFACE_INCLUDE_DIRECTORIES "${OGG_INCLUDE_DIR}" + IMPORTED_LOCATION "${OGG_LIBRARY}") +endif() diff --git a/cmake/FindVorbis.cmake b/cmake/FindVorbis.cmake index cd6c7909..dead65cc 100644 --- a/cmake/FindVorbis.cmake +++ b/cmake/FindVorbis.cmake @@ -18,6 +18,11 @@ find_package_handle_standard_args(Vorbis REQUIRED_VARS Vorbis_vorbisfile_LIBRARY Vorbis_vorbisfile_INCLUDE_PATH ) +include(CMakeFindDependencyMacro) +if (NOT TARGET Ogg::ogg) + find_dependency(Ogg REQUIRED) +endif() + if (Vorbis_FOUND) if (NOT TARGET Vorbis::vorbisfile) add_library(Vorbis::vorbisfile UNKNOWN IMPORTED) @@ -25,7 +30,7 @@ if (Vorbis_FOUND) IMPORTED_LOCATION "${Vorbis_vorbisfile_LIBRARY}" INTERFACE_INCLUDE_DIRECTORIES "${Vorbis_vorbisfile_INCLUDE_PATH}" INTERFACE_COMPILE_OPTIONS "${Vorbis_vorbisfile_COMPILE_OPTIONS}" - INTERFACE_LINK_LIBRARIES "${Vorbis_vorbisfile_LINK_LIBRARIES}" + INTERFACE_LINK_LIBRARIES "${Vorbis_vorbisfile_LINK_LIBRARIES};Ogg::ogg" INTERFACE_LINK_FLAGS "${Vorbis_vorbisfile_LINK_FLAGS}" ) endif() diff --git a/cmake/Findtremor.cmake b/cmake/Findtremor.cmake index 8bdacfd1..921788d1 100644 --- a/cmake/Findtremor.cmake +++ b/cmake/Findtremor.cmake @@ -18,6 +18,11 @@ find_package_handle_standard_args(tremor REQUIRED_VARS tremor_LIBRARY tremor_INCLUDE_PATH ) +include(CMakeFindDependencyMacro) +if (NOT TARGET Ogg::ogg) + find_dependency(Ogg REQUIRED) +endif() + if (tremor_FOUND) if (NOT TARGET tremor::tremor) add_library(tremor::tremor UNKNOWN IMPORTED) @@ -25,7 +30,7 @@ if (tremor_FOUND) IMPORTED_LOCATION "${tremor_LIBRARY}" INTERFACE_INCLUDE_DIRECTORIES "${tremor_INCLUDE_PATH}" INTERFACE_COMPILE_OPTIONS "${tremor_COMPILE_OPTIONS}" - INTERFACE_LINK_LIBRARIES "${tremor_LINK_LIBRARIES}" + INTERFACE_LINK_LIBRARIES "${tremor_LINK_LIBRARIES};Ogg::ogg" INTERFACE_LINK_FLAGS "${tremor_LINK_FLAGS}" ) endif() ```

Let me know what you think!

madebr commented 1 year ago

I forgot about this issue. Sorry about that.

The issue with your approach is that SDL2_mixer will unnecessarily link to dependencies of the static library. Also, find_dependency cannot be used in a FindXXX.cmake module (according to its documentation).

Your issue is that a static flac and tremor library needs to additionally link to the ogg static library. I think, when configuring the project as follows, a static FLAC and tremor library will link:

cmake .. -Dtremor_LINK_FLAGS=-logg -DFLAC_LINK_FLAGS=-logg

Can you check this?

An alternative fix is to make our cmake find modules smarter. See e.g. this CMake WIP pr to detect the library type. If

FtZPetruska commented 1 year ago

I was able to link successfully by setting the _LINK_LIBRARIES to -logg (instead of _LINK_FLAGS).

To maybe rephrase the issue I am facing:

While the approach of setting _LINK_LIBRARIES to the missing libraries does work, it leaves the issue of:

I like the idea of making the find modules smarter, CMake's documentation mentions that modern find modules commonly behave like config file package and propagate requirements:

The more modern approach is to behave as much like config file packages files as possible, by providing imported target. This has the advantage of propagating Transitive Usage Requirements to consumers.

I can also see the limit of that approach, while libFLAC, libvorbis, and libopusfile always have the same set of dependencies, a library such as fluidsynth may be built with optional features that would add transitive dependencies, but which cannot be predicted in a Find module.

madebr commented 1 year ago

In that aspect, I like the approach conan is taking by generating FindXXX.cmake, xxx-config.cmake, xxx.pc, ... Its build recipes are more complicated, but they capture all dependency information.

sezero commented 1 year ago

468 is in. Close this?

madebr commented 1 year ago

Okay by me. Static dependencies will always need special care.