libsdl-org / SDL_ttf

Support for TrueType (.ttf) font files with Simple Directmedia Layer.
zlib License
343 stars 116 forks source link

cmake: use MACHO* properties to set macho version #349

Closed madebr closed 2 months ago

madebr commented 3 months ago

Currently, VERSION and SOVERSION determine the file name and the macho compatibility and current version.

With this changes, VERSION and SOVERSION only determine the filename. MACHO_COMPATIBILITY_VERSION and MACHO_CURRENT_VERSION set the internal macho properties.

This also bumps the minimum required version on macos to 3.17,

Should fix https://github.com/libsdl-org/SDL_ttf/issues/347

madebr commented 3 months ago

@traversaro

Since you obviously have a mac, can you compare the behavior of an install prefix generated by autotools and cmake? The installed CMake config files and SDL2_ttf.pc should have similar behavior.

traversaro commented 3 months ago

Thanks for the fast feedback!

Since you obviously have a mac, can you compare the behavior of an install prefix generated by autotools and cmake?

Actually I do not have a mac (the problem emerged debugging some CI failures in conda-forge), but I can try to setup some CI to check that.

madebr commented 3 months ago

Our GitHub workflow does a minimum functionality test on the installed config files. If you can improve those, we can avoid having this issue in the future.

traversaro commented 3 months ago

Our GitHub workflow does a minimum functionality test on the installed config files. If you can improve those, we can avoid having this issue in the future.

Sure, I can look into that, even if I am not sure what we want to check. At the moment I just did something a bit more stupid: I added some prints https://github.com/traversaro/SDL_ttf/pull/3/commits/cd02338e4b92fba17d8f3472bc8e6d5869ecf6e4 .

Let me report what I found.

Autotools build

The autotools build only installs libSDL2_ttf-2.0.0.dylib and libSDL2_ttf.dylib, so the issue was not present there.

The CMake imported target for it is:

         set(_sdl2ttf_shl "${_sdl2ttf_libdir}/${CMAKE_SHARED_LIBRARY_PREFIX}SDL2_ttf${CMAKE_SHARED_LIBRARY_SUFFIX}")
        if(EXISTS "${_sdl2ttf_shl}")
            add_library(SDL2_ttf::SDL2_ttf SHARED IMPORTED)
            set_target_properties(SDL2_ttf::SDL2_ttf
                PROPERTIES
                    IMPORTED_LOCATION "${_sdl2ttf_shl}"
            )
        endif()

Downstream projects will just link IMPORTED_LOCATION as no IMPORTED_SONAME is present, so the just link libSDL2_ttf.dylib, and at runtime I guess they expect to find libSDL2_ttf.dylib, so we should be good to go.

The .pc file is:

 Name: SDL2_ttf
Description: ttf library for Simple DirectMedia Layer with FreeType 2 support
Version: 2.22.0
Requires: sdl2 >= 2.0.10
Libs: -L${libdir} -lSDL2_ttf
Cflags: -I${includedir}/SDL2
Requires.private: 
Libs.private: 

Again, also in this case libSDL2_ttf.dylib is linked.

CMake build

The .dylib installed are libSDL2_ttf-2.0.0.dylib, libSDL2_ttf-2.0.dylib and libSDL2_ttf.dylib.

The imported target is defined as:

set_property(TARGET SDL2_ttf::SDL2_ttf APPEND PROPERTY IMPORTED_CONFIGURATIONS RELEASE)
set_target_properties(SDL2_ttf::SDL2_ttf PROPERTIES
  IMPORTED_LINK_DEPENDENT_LIBRARIES_RELEASE "SDL2::SDL2"
  IMPORTED_LOCATION_RELEASE "${_IMPORT_PREFIX}/lib/libSDL2_ttf-2.0.0.dylib"
  IMPORTED_SONAME_RELEASE "@rpath/libSDL2_ttf-2.0.0.dylib"
  )

The .pc file is instead:

Name: SDL2_ttf
Description: ttf library for Simple DirectMedia Layer with FreeType 2 support
Version: 2.22.0
Requires: sdl2 >= 2.0.10
Libs: -L${libdir} -lSDL2_ttf
Cflags: -I${includedir}/SDL2
Requires.private: 
Libs.private: 

The downstream projects would link libSDL2_ttf.dylib if they are using pkg-config, while libSDL2_ttf-2.0.0.dylib if they are using the CMake target. As no minor/patch information is included there that indeed would solve the specific problem reported in https://github.com/libsdl-org/SDL_ttf/issues/347 . However, I do not know if that is enough for you, given that you wanted that "The installed CMake config files and SDL2_ttf.pc should have similar behavior."

madebr commented 3 months ago

The difference between the cmake scripts is expected, since the cmake template used by autotools don't know about libtool versioning. What is important is that linking to SDL2_ttf::SDL2_ttf with both targets have the same result.

Ignoring lib/libSDL2_ttf-2.0.dylib, the_ installed library files look similar: autotools vs cmake

traversaro commented 3 months ago

@madebr just a follow up on this: on the conda-forge side we applied your fix on two sdl libraries (sdl_ttf and sdl_image) and it has been working fine, feel free to let us know if there is anything else we can do to help on this, thanks!

madebr commented 3 months ago

I created prs to all SDL2 branches of the satellite libraries. The main (SDL3) branches also need this change.

traversaro commented 3 months ago

I created prs to all SDL2 branches of the satellite libraries. The main (SDL3) branches also need this change.

Thanks!