-
Notifications
You must be signed in to change notification settings - Fork 2.5k
CMake: INCLUDE_DIRECTORIES -> TARGET_INCLUDE_DIRECTORIES #4284
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
Conversation
Why are you dropping the explicit linkages? This looks weird: If you're worried about this not being sufficiently DRY, then why not set a variable to this set of libraries and then let |
Agree. 👍 Thanks @pks-t |
Ok, I will fix it. But it seems quite strange for me that tests for the library don't link to that library. |
Again - they can't, at least not without making everything public. We want to be able to test our buffer implementation and our posix emulation on Windows, and we do not want those to be a public, exported part of the library. |
Apply `target_include_directories` when CMAKE_VERSION >= 2.8.12
I have reworked my commits to leave git history clean. |
🎉 Awesome, thanks, @AndreyG ! |
@ethomson thank you for clarification! |
I'd still prefer to pull only the commit regarding |
Manually merged via c26ce78. I've made a follow-up commit to also use Thanks for your work! |
It's preferable to use
target_include_directories
instead ofinclude_directories
in modern CMake. It's also more consistent withtarget_link_libraries
.