Add minimal version info header to help compiler cache#9231
Closed
shramov wants to merge 1 commit intoXilinx:masterfrom
Closed
Add minimal version info header to help compiler cache#9231shramov wants to merge 1 commit intoXilinx:masterfrom
shramov wants to merge 1 commit intoXilinx:masterfrom
Conversation
Collaborator
|
shramov - is not a collaborator |
xrt/detail/abi.h includes autogenerated version.h that contains timestamp but use only major/minor version macros. Using smaller header enables use of compiler caches (like ccache). Signed-off-by: Pavel Shramov <shramov@mexmat.net>
Collaborator
|
shramov - is not a collaborator |
Collaborator
|
Can one of the admins verify this patch? |
4 tasks
stsoe
added a commit
to stsoe/XRT
that referenced
this pull request
Sep 16, 2025
…ache Refactor the autogenerated version.h into two separately configured headers, one that doesn't use timestamp (version-slim.h) and one that does (version.h). The XRT exported header files depend on ABI check that uses the XRT version only. These headers can include version-slim.h and not be affected by a changing timestamp everytime CMake is reconfigured. This in turn simplifies the logic for building with ccache. The two separate headers are generated under the build tree in a directory with same relatively location as where the corresponding headers will be installed. Thank you @shramov for Xilinx#9231, which seeded this PR. Signed-off-by: Soren Soe <2106410+stsoe@users.noreply.github.com>
stsoe
added a commit
to stsoe/XRT
that referenced
this pull request
Sep 16, 2025
…ache Refactor the autogenerated version.h into two separately configured headers, one that doesn't use timestamp (version-slim.h) and one that does (version.h). The XRT exported header files depend on ABI check that uses the XRT version only. These headers can include version-slim.h and not be affected by a changing timestamp everytime CMake is reconfigured. This in turn simplifies the logic for building with ccache. The two separate headers are generated under the build tree in a directory with same relatively location as where the corresponding headers will be installed. Thank you @shramov for Xilinx#9231, which seeded this PR. Signed-off-by: Soren Soe <2106410+stsoe@users.noreply.github.com>
Collaborator
stsoe
added a commit
that referenced
this pull request
Sep 17, 2025
…9238) * Amend #9231: Add minimal version info header to help compiler cache Refactor the autogenerated version.h into two separately configured headers, one that doesn't use timestamp (version-slim.h) and one that does (version.h). The XRT exported header files depend on ABI check that uses the XRT version only. These headers can include version-slim.h and not be affected by a changing timestamp everytime CMake is reconfigured. This in turn simplifies the logic for building with ccache. The two separate headers are generated under the build tree in a directory with same relatively location as where the corresponding headers will be installed. Thank you @shramov for #9231, which seeded this PR. Signed-off-by: Soren Soe <2106410+stsoe@users.noreply.github.com> * Fix review comment Signed-off-by: Soren Soe <2106410+stsoe@users.noreply.github.com> * Fix driver build error Make sure version-slim.h is installed in proper location for DKMS. Signed-off-by: Soren Soe <2106410+stsoe@users.noreply.github.com> --------- Signed-off-by: Soren Soe <2106410+stsoe@users.noreply.github.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
xrt/detail/abi.h includes autogenerated version.h that contains timestamp but use only major/minor version macros. Using smaller header enables use of compiler caches (like ccache).
Problem solved by the commit
Make it possible to use ccache (or other compiler cache) at buildhosts
Bug / issue (if any) fixed, which PR introduced the bug, how it was discovered
How problem was solved, alternative solutions (if any) and why they were rejected
Slim version header is added that contains only necessary info. Old version.h is left unchanged but it is possible to include slim one there to reduce code duplication
Why this solution is better then -DXRT_CCACHE?
XRT_CCACHE uses global define -DDISABLE_ABI_CHECK so technicaly compilation result is different and some errors can not be detected (redefinition of variables/macros from version.h for example).
This patch fixes root cause why -DDISABLE_ABI_CHECK was introduced and same code base with same flags can be compiled with or without ccache.
Risks (if any) associated the changes in the commit
Code that uses version macros without including version.h will break
What has been tested and how, request additional testing if necessary
Documentation impact (if any)