Skip to content
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

Reorganize XRT header files under core/include #8648

Merged
merged 8 commits into from
Dec 10, 2024

Conversation

stsoe
Copy link
Collaborator

@stsoe stsoe commented Dec 9, 2024

Problem solved by the commit

Under runtime_src/core/include we have a bunch of more or less random header files. Some are completely internal to XRT and are not exported, but several are exported in an install target to XRT_INSTALL_INCLUDE_DIR.

This PR, while work-in-progress, moves all exported header files under core/include/xrt. For legacy XRT installs, header wrappers are added in the old location(s) such that applications can continue to compile after the move.. It is strongly advised that application change to include headers from the new location(s), pragma messages will advise.

Bug / issue (if any) fixed, which PR introduced the bug, how it was discovered

We cannot have the content of the top-level include directory exported into e.g. /usr/include, so all the files have to be (re)moved or not exported.

How problem was solved, alternative solutions (if any) and why they were rejected

Move exported headers from -> to:

  • include/*.* -> include/xrt/detail
  • include/deprecated -> include/xrt/deprecated
  • include/experimental -> include/xrt/experimental

Create wrappers to preserve compilation of existing applications until they are changed.

Update driver sources for DKMS to copy the actual headers rather than the new wrappers.

Update all XRT source code to include from new location with the exception of code that is shared between
userspace and driver. For these latter situations, a few header wrappers remain in core/include and are included from this location. When DKMS builds sources the wrappers have been replaced with the actual files.

Risks (if any) associated the changes in the commit

Risks are primarily edge builds, which I couldn't validate sufficiently. This PR is bound to fail CI as is. and will be updated with help from others.

What has been tested and how, request additional testing if necessary

Manual build of legacy XRT application.

Documentation impact (if any)

stsoe added 8 commits December 6, 2024 12:37
Refactor header install per 4 buckets:

1. Internal to XRT, keep but stop exporting
2. Deprecated, delete from source tree
3. Indirectly included by xrt/xrt_<first class>.h, move to include/xrt/detail/
4. Included by application host code, move to include/xrt/legacy (and or other name(s))

Signed-off-by: Soren Soe <2106410+stsoe@users.noreply.github.com>
Signed-off-by: Soren Soe <2106410+stsoe@users.noreply.github.com>
Signed-off-by: Soren Soe <2106410+stsoe@users.noreply.github.com>
Signed-off-by: Soren Soe <2106410+stsoe@users.noreply.github.com>
Signed-off-by: Soren Soe <2106410+stsoe@users.noreply.github.com>
Signed-off-by: Soren Soe <2106410+stsoe@users.noreply.github.com>
Signed-off-by: Soren Soe <2106410+stsoe@users.noreply.github.com>
@stsoe stsoe added the do not merge hold off on merging label Dec 9, 2024
@stsoe stsoe removed the do not merge hold off on merging label Dec 10, 2024
Copy link
Collaborator

@AShivangi AShivangi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. We'll work on updating the includes in xrt-smi

@stsoe stsoe merged commit 07f11f2 into Xilinx:master Dec 10, 2024
20 checks passed
@stsoe stsoe deleted the xrt_packaging branch December 10, 2024 20:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants