-
Notifications
You must be signed in to change notification settings - Fork 35
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
Fix build errors on macOS #950
Conversation
test/CMakeLists.txt
Outdated
celeritas_target_link_libraries(testcel_harness | ||
PUBLIC nlohmann_json::nlohmann_json | ||
) | ||
endif() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- The
include_directories
isn't necessary because the modern cmake targets propagate that information - This should be PRIVATE not PUBLIC because it's only used in a .cc file
if (
->if(
- Our top-level cmakelists defines
nlohmann_json_LIBRARIES
so that it's empty if json isn't available, so that makes it easier to add for the optional cases - It turns out I'm not directly using JSON anyway for now, so we could've just deleted the include 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wait, so target_link_libraries(...)
sets the include dirs as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Exactly. Modern cmake targets propagate build flags, library flags, include directories, etc.
This PR fixes two build errors on macOS: