fix: boost's include dir was listed first #2384
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bstaletic
approved these changes
Aug 12, 2020
YannickJadoul
approved these changes
Aug 12, 2020
There was a problem hiding this comment.
Agreed with @bstaletic. Minor fix; merge whenever you're happy with it! :-)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #2381
Boost's include directory (which is usually in a system include directory, such as in Homebrew) was being listed before the dependent target include directory, since it was directly set on the target rather than via a dependent target, like pybind11's headers, which meant that pybind11 in
/usr/local/includewas beating out local pybind11 in the compile line. This fix will work in CMake 3.12+ (where system includes are always placed after local includes, even if set directly instead on on a target. To fix it for CMake < 3.12, we probably would have to make a quick IMPORTED INTERFACE target for Boost (not a bad idea, but this only affects pybind11's own tests and when pybind11 is also in the boost directory - so not worried about old CMake's there)).The even more correct fix would be to use Boost's targets if they exist, and make the target if it does not.