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

cmake: use project-level variables, not top-level variables #214

Merged
merged 1 commit into from Aug 31, 2020
Merged

cmake: use project-level variables, not top-level variables #214

merged 1 commit into from Aug 31, 2020

Conversation

herrecito
Copy link
Contributor

This PR replaces CMAKE_SOURCE_DIR and CMAKE_BINARY_DIR with PROJECT_SOURCE_DIR and PROJECT_BINARY_DIR. This is more correct. It makes possible to add WildMidi as a submodule in a parent project and link it with:

add_subdirectory(lib/wildmidi EXCLUDE_FROM_ALL)
target_link_libraries(main PRIVATE libwildmidi_static)

It should have no effect on WildMidi itself.

@psi29a
Copy link
Member

psi29a commented Jun 8, 2020

Cool! Just needs a little love apparently...

CMakeLists.txt Outdated
CMAKE_MINIMUM_REQUIRED(VERSION 2.8.11)
CMAKE_POLICY(SET CMP0072 NEW)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of setting the policy, could we just bump our cmake requirement?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do we have to do with FindOpenGL() to need this CMP0072 policy setting? (see: https://cmake.org/cmake/help/v3.11/policy/CMP0072.html)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're not an OpenGL consumer, so we shouldn't need to bother with this nor set a policy for it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do not use: FindOpenGL

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, that's exactly the reason I asked, and it seems like the actual author lost interest in his own pull request..

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the radio silence! It's been very busy at work, but I'll try to find time this week to get this PR ready!

@herrecito
Copy link
Contributor Author

It should be OK now, though travis failed, but apparently while installing build dependencies on macOS?. I'm pretty sure is not related to this PR.

@psi29a
Copy link
Member

psi29a commented Aug 31, 2020

Yeah, I'll have a look at the MacOS stuff when I have free cycles.

Thanks

@psi29a psi29a merged commit 1a1fde7 into Mindwerks:master Aug 31, 2020
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.

None yet

3 participants