-
Notifications
You must be signed in to change notification settings - Fork 1.4k
[cmake] Add core/base headers to target HEADERS file_set #18419
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
base: master
Are you sure you want to change the base?
Conversation
Test Results 20 files 20 suites 3d 7h 5m 6s ⏱️ Results for commit 347cde1. ♻️ This comment has been updated with latest results. |
Also potentially related: #17607 |
e9a87ee
to
347cde1
Compare
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.
@ferdymercury, would you agree that the same can be achieved with less code, since CMake already does the expansion to absolute paths for us?
as suggested by hageboeck
This version of CMake was released in March 2022 https://github.com/Kitware/CMake/releases/tag/v3.23.0 This version is required to use FILE_SETs, which could help towards root-project#16327 and root-project#18419.
This Pull request:
Changes or fixes:
Gives a taste on how it would look like to properly add HEADERS to targets in CMake.
This does not just help with the IDE workflow, it gives an idea on future paths:
One could make them PUBLIC instead of PRIVATE, then you would not need the target_include_directories line, and installing and cpacking would be easier.
Related:
Checklist:
This PR fixes #