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

manual SDL2 library search #199

Merged
merged 1 commit into from
Jan 25, 2019
Merged

manual SDL2 library search #199

merged 1 commit into from
Jan 25, 2019

Conversation

jpcima
Copy link
Collaborator

@jpcima jpcima commented Jan 24, 2019

This detects SDL2 without using the FindModule shipped with the library,
which is unreliable. It's the manual search which works in all configurations.

("old way" copied back from libOPNMIDI)
see #196 (comment)

CMakeLists.txt Outdated Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
@jpcima jpcima force-pushed the find-sdl2 branch 3 times, most recently from 74cd6ab to 21a2dbf Compare January 24, 2019 08:54
@jpcima
Copy link
Collaborator Author

jpcima commented Jan 24, 2019

I rewrote the whole PR @Flamefire thanks to your advice, do you think it's fine?

@Flamefire
Copy link
Contributor

Problem now is that you duplicate the conditions used in the subfolders. They might easily become out-of-sync. I'd suggest either:

  • Duplicate the target check at the point of usage. This is kinda fine as one sees why things are done
  • Create a very simple find module containing basically your code for ADLMIDI_SDL2 and find_package that.
  • Create a wrapper function like function(find_sdl2_targets) find_package(SDL2 ${ARGV}) <your code> and include+call that: find_sdl2_targets(REQUIRED) target_link_libraries(... SDL2::SDL2)

This allows to handle two variants of SDL2 find modules, which are incompatible:
- the first kind of module sets the SDL2_* variables
- the other kind exports the library target SDL2::SDL2
@jpcima
Copy link
Collaborator Author

jpcima commented Jan 25, 2019

Ok to merge this @Wohlstand ?
It's an annoyance to work from the master branch, because of the cmake failure.
(the case when SDL2 was built with cmake, if I understand right)

@Wohlstand
Copy link
Owner

Looks much better! Yeah, I think, the same set of works are must go into libOPNMIDI's build too

@Wohlstand Wohlstand merged commit de2aa69 into Wohlstand:master Jan 25, 2019
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.

3 participants