-
Notifications
You must be signed in to change notification settings - Fork 208
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
Update CMake dependencies to rely on proper targets #261
Conversation
jdumas
commented
Mar 7, 2024
•
edited
Loading
edited
- This PR builds upon Update ImGui to 1.90.4 #260
- Updates glm to 0.9.9.8 using a submodule
- Clean up CMake code to use proper target-based dependencies
- Do not add subdirectories if a dependency already exist, making it easier to reuse deps coming from other parts of a larger project.
Thanks! I'm a little wary about unnecessarily breaking things for people who might be e.g. using Polyscope's copy of Regardless, I tried rebuilding so of my own projects with this and it all seems to work, so I say let's do it. The change to using targets and potentially pulling them from the outer project is quite useful. |
Merged! (Also we're about to get a bunch of issues about |
By the way, can you explain more about what |
Thanks for merging this quickly!
Oh man, I could go on for a while on this topic... Very briefly, I'd break it down to two items:
Anyhow if you have more specific concerns I'm always happy to answer any CMake question :D
Yep, this is why I've switched to FetchContent/CPM for dependency management in CMake...
Hmm I encountered some compilation errors at the time, so I bumped it down to 0.9.9.8. But now that I'm trying again it seems to work right away... the only thing I had to do was define |