Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Debian package provides bad sdl2-config.cmake #2852

Closed
SDLBugzilla opened this issue Feb 11, 2021 · 3 comments
Closed

Debian package provides bad sdl2-config.cmake #2852

SDLBugzilla opened this issue Feb 11, 2021 · 3 comments
Assignees
@icculus
Milestone
2.0.22

Comments

@SDLBugzilla
Copy link
Collaborator

This bug report was migrated from our old Bugzilla tracker.

These attachments are available in the static archive:

Reported in version: HG 2.1
Reported for operating system, platform: Linux, x86_64

Comments on the original bug report:

On 2018-03-25 15:17:02 +0000, Raphnalor wrote:

Created attachment 3215
good and bad SDL2Config files

When I build SDL2 from source with cmake using the following command

mkdir build
cd build
cmake ..
cmake --build . --target install

it makes file "SDL2Config.cmake" and others (see folder "good..." in the attachment) which are perfect config files for CMake containing imported targets "SDL2::SDL2" and "SDL2::SDL2main" which makes SDL2 comfortable to use in any CMake-based project.

However when I install SDL with command "apt-get install libsdl2-dev" it provides file "sdl2-config.cmake" (see folder "bad..." in the attachment) which is completely different. It provides no imported targets, it just sets variables SDL2_INCLUDE_DIRS, SDL2_LIBRARIES. This config file is harder to use, it requires more lines to be written in a project's CMakeLists.txt, and these lines are completely different from those ones which are required for the good "SDL2Config.cmake" made from source with CMake.

VCPKG provides a good "SDL2Config.cmake". Is it possible to put the good "SDL2Config.cmake" file to a debian package too?

On 2018-04-17 10:35:39 +0000, Harry Wagstaff wrote:

I'd just like to point out that I've just experienced the exact opposite problem.

I'm working on a project which has SDL as a dependency, and it no longer builds with SDL 2.0.8 since it includes the imported targets rather than the legacy variables. This is essentially a breaking change for any project that uses SDL2 and cmake.

Really, I think that this is an issue with cmake rather than SDL (since the entire design of cmake seems to be constantly in flux).

On 2018-05-20 05:14:18 +0000, Tim Aitken wrote:

I have a similar problem to the one originally reported. In my case I believe SDL is correctly located by my build, but the version cannot be resolved. When I build the packages myself or with vcpkg it contains the package-import version which behaves as expected.

I believe these files should be included with the distributed package. In response to Harry's comment, the legacy variables should also be added to the CMakeTargets.cmake to support backwards-compatibility with pre-2.8 cmake builds reliant on variables.

On 2018-05-28 15:47:40 +0000, Leonardo wrote:

The problem is that sdl2-config.cmake is generated when building via the autotools (automake/configure/etc) path and it is not up to date with modern CMake. It is a workaround since CMake folks advised me to not create and submit a FindSDL* file but rather make SDL itself provide a config file (see 1).

This config file must be modernized, I'm sure a patch would be welcome.

On 2019-04-13 19:00:24 +0000, Leonardo wrote:

Oi. I have created bug 4597 that should fix your issue (it creates targets SDL and SDLmain, I couldn't create ALIAS targets on IMPORTED targets). Would you mind giving the new sdl2-config.cmake a try and reporting back if it works?

Thanks.

On 2020-02-07 15:22:49 +0000, wrote:

@Harry Wagstaff The best solution would be to provide uniform configs for autotools and CMake. I'd even go as far and drop autotools completely as CMake can do the same and more (e.g. cross-platform) and using CMake only relieves from the burden of having to maintain multiple versions of the same code.

For your use case one could provide targets AND the variables with the paths

On 2020-04-19 16:17:35 +0000, Matt Scheirer wrote:

This will start to become a rising problem in distro packaging - some distros build SDL via autotools while at least Arch uses CMake. As SDL2 based projects switch to using config to find SDL (Debian has been actively proposing patches to several games in recent months to drop FindSDL modules) new releases are failing on Arch.

For now I'm just injecting manual patches to use the SDL2 target in my Arch packages. A possible stopgap would be to insure the cmake built config exports SDL2_INCLUDE_DIRs and SDL2_LIBRARIES to have backwards compatibility with the hand written config. That doesn't fix the problem - tons of software would then start using these variables rather than the SDL2 target - but it would stop software from breaking when moving between the two SDL build types.

On 2020-04-20 07:03:10 +0000, wrote:

So now that the autotools build provides the targets we need the CMake build to provide the variables?

In the spirit of education I would choose to not do so and force users to use the targets when they are available. Yes this may break stuff but it is easy enough to fix and the right thing to do. And it creates pressure to actually do that.

So the message would be: Want to support newer SDL2 versions? Use the targets!

If you really must provide variables, set them like this:

set(SDL2_INCLUDE_DIRs "")
set(SDL2_LIBRARIES SDL2::SDL2)

If this breaks projects, they really do nasty stuff they should not.
@icculus icculus removed the bug label Nov 30, 2021
@icculus icculus self-assigned this Nov 30, 2021
@icculus icculus added this to the 2.0.20 milestone Nov 30, 2021
@icculus
Copy link
Collaborator

icculus commented Apr 6, 2022

@smcv Any chance you can look at this and decide if this is our fault or Debian's (or not an issue at all anymore)?

@smcv
Copy link
Contributor

smcv commented Apr 6, 2022

Debian is building SDL using Autotools, and that gives us a sdl2-config.cmake and a sdl2-config-version.cmake which seem to be setting up SDL2::SDL2 and SDL2::SDL2main imported libraries.

If I understand the bug report correctly, this was mostly the upstream issue that the Autotools build used to generate a sdl2-config.cmake and sdl2-config-version.cmake that lacked SDL2::SDL2 etc., but that was fixed in dd1d8ab. As far as I can see, the only thing we were doing "wrong" downstream in Debian (apart from some multiarch issues between Debian 10 and 11, which I fixed) was to be using Autotools, because some people want everything to be a CMake monoculture. However, if I understand correctly, SDL upstream still considers Autotools to be the recommended build system for production-quality builds, with the parallel CMake build considered experimental? (If that's not the case, we can look into switching.)

The only Debian-specific alterations we make to the build system, outside tests, are to support multiarch co-installation so that you can have i386 and x86_64 versions of SDL development files installed system-wide at the same time:

  • editing sdl2-config to not have -L@libdir@: this is unnecessary on Debian, because Debian packages install libraries into a directory that is in the compiler's search path already
  • making the SDL_config.h in the non-multiarch header directory redirect to SDL's real SDL_config.h in a multiarch header directory, because we need i386 and x86_64 builds to see a different SDL_config.h with appropriate constants for its word-size (Fedora does something similar)
  • some header file symlinks to work around packages that #include <SDL2/SDL.h> instead of <SDL.h>, which if I understand correctly was always considered to be wrong, but unfortunately seems to be quite common

I haven't sent those changes upstream, because they would be wrong when SDL is built locally and installed to paths other than the ones Debian uses.

We have automated tests for a couple of different use-cases for how to use SDL from CMake (once with ${SDL2_INCLUDE_DIRS}, and once with FindSDL2), but until today we didn't have an automated test for linking it via imported targets. Does this look like the correct way to link SDL to a simple program built with CMake?

cmake_minimum_required(VERSION 3.13)
project(Hello)
find_package(SDL2 REQUIRED)
add_executable(hello hello.c)
target_link_libraries(hello SDL2::SDL2)

If that's correct, then it seems to work fine as-is with the current libsdl2 (2.0.20+dfsg-2) in Debian unstable, and we now have test coverage that will stop me from uploading a new SDL where that doesn't build successfully.

@slouken
Copy link
Collaborator

slouken commented Apr 6, 2022

autotools and CMake are now considered roughly equivalent, with differences between them worthy of bug reports. I just use autotools myself, so it's easier for me to verify fixes and implement new changes there, but lots of people use CMake and it's fully supported.

That said, I don't know how to link a CMake program with SDL, so I'll have to defer to other folks on your question there.

It sounds like this bug is fixed, and new issues can be opened for specific deficiencies at this point.

Thanks!

@slouken slouken closed this as completed Apr 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@icculus icculus

Labels
None yet
Projects
None yet
Milestone
2.0.22
Development

No branches or pull requests
4 participants
@icculus @smcv @slouken @SDLBugzilla