Don't statically link libxmp

[ankith26]

The libxmp dependency seems to be inconsistently handled when building with SDL2MIXER_DEPS_SHARED=0. Every other library uses "standard dynamic linking" instead of the "SDL style dynamic linking" (apologies if this is wrong terminology, but I don't know what else to call it). But libxmp does an actual static linking, which increases the size of SDL_mixer shared library. I would expect it to do the "standard dynamic linking" when I pass SDL2MIXER_DEPS_SHARED=0.

This is a cmake only issue, the autotools build handles this as expected. This fix if accepted, needs to be frontported to SDL3 branch as well I believe.

[madebr]

I cannot reproduce your issue

cmake -S path/to/SDL_mixer -B /tmp/SDL_mixer-build --fresh -GNinja -DSDL2MIXER_VENDORED=ON -DCMAKE_INSTALL_PREFIX=/tmp/SDL_mixer-build/prefix -DSDL2MIXER_BUILD_SHARED_LIBS=OFF
cmake --build /tmp/SDL_mixer-build
cmake --install /tmp/SDL_mixer-build

$ tree /tmp/SDL_mixer-build/prefix
/tmp/SDL_mixer-build/prefix
├── include
│   └── SDL2
│       └── SDL_mixer.h
├── lib64
│   ├── cmake
│   │   └── SDL2_mixer
│   │       ├── SDL2_mixerConfig.cmake
│   │       ├── SDL2_mixerConfigVersion.cmake
│   │       ├── SDL2_mixer-shared-targets.cmake
│   │       └── SDL2_mixer-shared-targets-release.cmake
│   ├── libogg.so -> libogg.so.0
│   ├── libogg.so.0 -> libogg.so.0.8.5
│   ├── libogg.so.0.8.5
│   ├── libopusfile.so -> libopusfile.so.0
│   ├── libopusfile.so.0 -> libopusfile.so.0.12
│   ├── libopusfile.so.0.12
│   ├── libopus.so -> libopus.so.0
│   ├── libopus.so.0 -> libopus.so.0.9.0
│   ├── libopus.so.0.9.0
│   ├── libSDL2_mixer-2.0.so -> libSDL2_mixer-2.0.so.0
│   ├── libSDL2_mixer-2.0.so.0 -> libSDL2_mixer-2.0.so.0.800.0
│   ├── libSDL2_mixer-2.0.so.0.800.0
│   ├── libSDL2_mixer.so -> libSDL2_mixer-2.0.so.0
│   ├── libwavpack.so -> libwavpack.so.1
│   ├── libwavpack.so.1 -> libwavpack.so.1.2.5
│   ├── libwavpack.so.1.2.5
│   ├── libxmp.so -> libxmp.so.4
│   ├── libxmp.so.4 -> libxmp.so.4.6.0
│   ├── libxmp.so.4.6.0
│   └── pkgconfig
│       └── SDL2_mixer.pc
└── share
    └── licenses
        └── SDL2_mixer
            └── LICENSE.txt

10 directories, 26 files

libxmp is built as an external library: libxmp.so.4

Perhaps you have configured with -DSDL2MIXER_MOD_XMP_SHARED=OFF? Then libxmp will be built as a static librarary, and become built-in SDL_mixer.

[ankith26]

The command I am using is

cmake -S .. -B . $PG_BASE_CMAKE_FLAGS \
      -DSDL2MIXER_DEPS_SHARED=0 -DSDL2MIXER_VENDORED=0 \
      -DSDL2MIXER_FLAC_LIBFLAC=1 -DSDL2MIXER_FLAC_DRFLAC=0 \
      -DSDL2MIXER_MP3_MPG123=1 -DSDL2MIXER_MP3_MINIMP3=0 \
      -DSDL2MIXER_VORBIS=VORBISFILE

Where

PG_BASE_CMAKE_FLAGS="-DCMAKE_INSTALL_PREFIX=/usr/local \
  -DCMAKE_INSTALL_LIBDIR:PATH=lib \
  -DCMAKE_BUILD_TYPE=Release \
  -DBUILD_SHARED_LIBS=true"

[madebr]

Applying this patch on top of the current SDL2 branch, does this patch what you want?

--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -693,6 +693,8 @@ if(SDL2MIXER_MOD_XMP)
             find_package(libxmp REQUIRED)
             if(TARGET libxmp::xmp_shared AND SDL2MIXER_MOD_XMP_SHARED)
                 set(tgt_xmp libxmp::xmp_shared)
+            elseif(TARGET libxmp::xmp_shared)
+                set(tgt_xmp libxmp::xmp_shared)
             elseif(TARGET libxmp::xmp_static)
                 set(tgt_xmp libxmp::xmp_static)
             else()

[ankith26]

Yes, that patch also does what I would expect. Further simplifying it to

--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -691,7 +691,7 @@ if(SDL2MIXER_MOD_XMP)
         else()
             message(STATUS "Using system libxmp")
             find_package(libxmp REQUIRED)
-            if(TARGET libxmp::xmp_shared AND SDL2MIXER_MOD_XMP_SHARED)
+            if(TARGET libxmp::xmp_shared)
                 set(tgt_xmp libxmp::xmp_shared)
             elseif(TARGET libxmp::xmp_static)
                 set(tgt_xmp libxmp::xmp_static)

also works. Let me know if I need to update this PR to do this patch instead.

Personally I don't particularly care about the vendored dependency case so I can leave that part as it is if you think that is better.

Also in my opening comment earlier on I confused the SDL2MIXER_DEPS_SHARED option with SDL2MIXER_BUILD_SHARED_LIBS so sorry about that, I have fixed the comment now.

[madebr]

Your patch looks fine too :) So please update this pr to it.

[ankith26]

Thanks for accepting this fix!

I also appreciate that SDL 2.x.x is still getting regular releases. I request the same for SDL2 branches on the SDL satellite libraries, because I have noticed a couple of good fixes across all satellite libraries.

[sezero]

This needs porting to SDL3 branch too (as noted in the ticket's original message.)

[ankith26]

Should I open the port PR?

[sezero]

Should I open the port PR?

Yes please