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.

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

Remove FindSDL2.cmake as SDL2 provides a sdl2-config.cmake since 2.0.4 #1530

Merged
merged 4 commits into from
Apr 16, 2020

Conversation

simonschmeisser
Copy link
Contributor

Apparently Debian messed up packaging SDL2 and has the headers in architecture dependent subfolders. This would not be an issue if ogre would use the sdl2-config.cmake provided by sdl2 since version 2.0.4 instead of guessing based on a list of possible locations

this is a test PR to see if your CI tells me if that also works on Windows and Mac

there is another option to do

FindPackage(SDL2 CONFIG)
if(NOT SDL2_FOUND)
  FindPackage(SDL2)
endif()

or to add /usr/include/${CMAKE_LIBRARY_ARCHITECTURE}/SDL2 to the list

@paroj
Copy link
Member

paroj commented Apr 10, 2020

On debian we only have SDL2_INCLUDE_DIRS, while current upstream only provides the SDL2::SDL2 target. And of course you cannot use both without issues.

welcome to the wonders of cross-platform development.

@simonschmeisser
Copy link
Contributor Author

Let's see if I'm allowed to simply set both ...

@paroj
Copy link
Member

paroj commented Apr 10, 2020

also build this locally as our CI builds without SDL on Ubuntu.

@simonschmeisser
Copy link
Contributor Author

May I ask why?

@paroj
Copy link
Member

paroj commented Apr 10, 2020

To cover a wide range of build configurations. Building with SDL2 is done on my machine 😉

@simonschmeisser
Copy link
Contributor Author

Since I don't have xenial either and my machine currently builds the debian image I added this test commit, can remove it later or while squashing on merge

@simonschmeisser
Copy link
Contributor Author

It fails with something about git diff on linux, are you running clang-format or similar?

@paroj
Copy link
Member

paroj commented Apr 14, 2020

it runs git diff --check master and finds issues in the reverse diff as master moved forward. If you rebase on master (while dropping the TESTING commit), the issue will disappear.

@paroj paroj merged commit 6de6f9b into OGRECave:master Apr 16, 2020
@paroj
Copy link
Member

paroj commented Apr 16, 2020

👍 thanks

@simonschmeisser simonschmeisser deleted the sdl2-config-cmake branch April 16, 2020 10:34
raymond-w-ko pushed a commit to syandus/ogre that referenced this pull request Jan 3, 2022
* replace SDL2_INCLUDE_DIRS by imported target
* create interface target for SDL2 if it doesn't exist
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants