search

libsdl-org / SDL_mixer

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

SDL2_mixer cmake configuration uses custom Findvorbisfile.cmake instead of system libvorbis files #451

Closed maxmitti closed 2 years ago

maxmitti commented 2 years ago

SDL2_mixer includes and uses a Findvorbisfile.cmake to link with vorbisfile. It also installs and uses said cmake module for use in the installed cmake configuration files for SDL2_mixer.

At least when building libvorbis with cmake myself, it installs its own configuration files to /lib/cmake/Vorbis, which exports Vorbis::vorbisfile and also handles necessary dependencies correctly, which is especially important for static builds.

The included Findvorbisfile.cmake on the other hand fails to correctly link with vorbisfile’s dependencies.

madebr commented 2 years ago

I suppose we will switch to VorbisConfig.cmake, once it is generally available. But right now, it is not installed on my Fedora machine (and by extension not on many other machines):

$ rpm -ql libvorbis-devel.x86_64
/usr/include/vorbis
/usr/include/vorbis/codec.h
/usr/include/vorbis/vorbisenc.h
/usr/include/vorbis/vorbisfile.h
/usr/lib64/libvorbis.so
/usr/lib64/libvorbisenc.so
/usr/lib64/libvorbisfile.so
/usr/lib64/pkgconfig/vorbis.pc
/usr/lib64/pkgconfig/vorbisenc.pc
/usr/lib64/pkgconfig/vorbisfile.pc
/usr/share/aclocal/vorbis.m4

It looks like its availability depends on how vorbis is built: their autotools build script does not install a cmake config file. So right now, we can't just remove our Findvorbisfile.cmake module.

At least when building libvorbis with cmake myself, it installs its own configuration files to /lib/cmake/Vorbis, which exports Vorbis::vorbisfile and also handles necessary dependencies correctly, which is especially important for static builds.

I'll update the cmake scripts such that they are compatible with the upstream config file (if available).

The included Findvorbisfile.cmake on the other hand fails to correctly link with vorbisfile’s dependencies.

I added https://github.com/libsdl-org/SDL_mixer/blob/a3d843b027a06f11d745ed9543ee5ed29085dd72/cmake/Findvorbisfile.cmake#L7-L11 for users to add extra dependencies that our Findvorbisfile.cmake is unable to pick up.

maxmitti commented 2 years ago

So right now, we can't just remove our Findvorbisfile.cmake module.

I agree. In Arch Linux the cmake config files are also not available.

I'll update the cmake scripts such that they are compatible with the upstream config file (if available).

It would be nice if it could use the installed config if available and fall back to the included module otherwise.

madebr commented 2 years ago

I've opened #454 that addresses this. I had to use CMAKE_FIND_PACKAGE_PREFER_CONFIG to prefer VorbisConfig.cmake over FindVorbis.cmake though, but that's normal CMake behavior.