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

Add FindSDL2 to simplify main CML #6686

Merged
merged 1 commit into from May 11, 2022
Merged

Add FindSDL2 to simplify main CML #6686

merged 1 commit into from May 11, 2022

Conversation

bencsikandrei
Copy link
Contributor

@bencsikandrei bencsikandrei commented May 5, 2022 •
edited

In summary:

  • the FindSDL2 module forwards to CONFIG mode first
  • if that fails (eg. no -version.cmake or config not even present) it
    uses pkg-config
  • exports SDL2::SDL2 as a target

I've tried it in the following scenarios:

  1. ubuntu 18 (no config present for SDL2) - uses pkg config, works
  2. ubuntu 20 (config present, but no version) - uses pkg-config, works
  3. ubuntu 20 (with vcpkg, static libSDL2) - uses custom -config/-version, works
  4. ubuntu 22 (has a good version of libSDL2) - uses -config/-version), works
  5. macos 11.6 (brew) - uses -config/-version, works

Haven't tried Windows, I was hoping the CI would help :D

Please let me know what you think about this one, I'd like to also make some Modules for SDL2_Image and Mixer

Have a good one!

EDIT: It also exports SDL2::SDL2main
EDIT2: I've also tested on windows (msvc/vcpkg/cmake/pkg-config-lite)

@github-actions github-actions bot added Building Build-time issues. CMake Issues involving the CMake build system. labels May 5, 2022
Pentarctagon
Pentarctagon reviewed May 6, 2022
View reviewed changes
CMakeLists.txt Outdated Show resolved Hide resolved
@Pentarctagon
Copy link
Member

Also it looks like the instances of using SDL2::SDL2 would no longer need to be in if(MSVC) checks?

@stevecotton stevecotton mentioned this pull request May 7, 2022
Closed
@bencsikandrei
Copy link
Contributor Author

Also it looks like the instances of using SDL2::SDL2 would no longer need to be in if(MSVC) checks?

working on it! thanks

bencsikandrei
bencsikandrei commented May 8, 2022
View reviewed changes
src/CMakeLists.txt Outdated Show resolved Hide resolved
@bencsikandrei
Add FindSDL2 to simplify main CML
f6aab3d 
* the FindSDL2 module forwards to CONFIG mode first
* if that fails (eg. no -version.cmake or config not even present) it
uses pkg-config
* exports SDL2::SDL2 and SDL2::SDL2main as targets
* fix missing IMPORTED_LOCATION for other build types in Vorbisfile
@bencsikandrei
Copy link
Contributor Author

Also it looks like the instances of using SDL2::SDL2 would no longer need to be in if(MSVC) checks?

I, hopefully, made all the changes that remove that necessity!

I'm actually trying this on a Windows machine I got my hands on, will report back.

Please let me know if you want me to make some other changes. Thanks

@Pentarctagon
Copy link
Member

Barring any potential issue you find on your Windows machine, this looks good to me.

@bencsikandrei
Copy link
Contributor Author

Barring any potential issue you find on your Windows machine, this looks good to me.

Happy to report it builds nicely on the following combo:

windows 11
msvc 19 (compiler version 16)
cmake 3.22
vcpkg  2022-03-30-692785ac944e81417840c6de244fb3e18a4b35eb
pkg-config-lite

@Pentarctagon Pentarctagon merged commit 1838041 into wesnoth:master May 11, 2022
@bencsikandrei bencsikandrei deleted the add_cross_platform_find_sdl branch May 13, 2022 19:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Pentarctagon Pentarctagon Pentarctagon left review comments

Assignees
No one assigned
Labels
Building Build-time issues. CMake Issues involving the CMake build system.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@bencsikandrei @Pentarctagon