-
Notifications
You must be signed in to change notification settings - Fork 97
[C/C++] CMake: Set include install interface to include
#1535
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
[C/C++] CMake: Set include install interface to include
#1535
Conversation
|
One thing to point out: The headers are installed to Lines 81 to 83 in 0a93888
|
| target_include_directories(sbp | ||
| PUBLIC | ||
| $<BUILD_INTERFACE:${PROJECT_SOURCE_DIR}/include> | ||
| $<BUILD_INTERFACE:${CMAKE_CURRENT_BINARY_DIR}> |
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.
I am wondering whether this line is still needed?
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.
I don't see any reason you would need it. But I didn't want to break any of your existing tooling/setups that might depend on it in an unexpected way.
| $<BUILD_INTERFACE:${PROJECT_SOURCE_DIR}/include> | ||
| $<BUILD_INTERFACE:${CMAKE_CURRENT_BINARY_DIR}> | ||
| $<INSTALL_INTERFACE:${CMAKE_INSTALL_LIBDIR}> | ||
| $<INSTALL_INTERFACE:${CMAKE_INSTALL_INCLUDEDIR}> |
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.
👍
martin4861
left a comment
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.
Thanks for the fix 👍
Description
@swift-nav/algint-team
Resolves #1532.
API compatibility
Does this change introduce a API compatibility risk?
No, the API is unchanged. It does introduce a build compatibility risk depending on how consumers use the configured install interface.
API compatibility plan
If the above is "Yes", please detail the compatibility (or migration) plan:
The change should be documented in the release notes with instructions that consumers will need to reconfigure their existing build caches.
JIRA Reference
N/A