xbmgmt2: use XBMGMT2_NAME in XRT_EDGE target_compile_definitions#9830
Open
talhaHavadar wants to merge 1 commit into
Open
xbmgmt2: use XBMGMT2_NAME in XRT_EDGE target_compile_definitions#9830talhaHavadar wants to merge 1 commit into
talhaHavadar wants to merge 1 commit into
Conversation
The XRT_EDGE branch of xbmgmt2/CMakeLists.txt references ${XBUTIL2_NAME}
when calling target_compile_definitions. That variable is defined in the
sibling xbutil2/CMakeLists.txt and is not visible in this directory
scope, so it expands to an empty string. CMake then parses the call as
target_compile_definitions(PRIVATE ENABLE_DEFAULT_ONE_DEVICE_OPTION) and
fails the configure step with:
Cannot specify compile definitions for target "PRIVATE" which is
not built by this project.
The non-edge branch above already uses the correct ${XBMGMT2_NAME}.
Replace the ${XBUTIL2_NAME} reference with ${XBMGMT2_NAME} so the
compile definition is applied to the xbmgmt executable built in this
directory.
Fixes: Xilinx#9829
Signed-off-by: Talha Can Havadar <havadartalha@gmail.com>
|
@talhaHavadar is not a repository collaborator. To proceed:
|
Contributor
|
clang-tidy review says "All clean, LGTM! 👍" |
chvamshi-xilinx
requested changes
May 25, 2026
Collaborator
chvamshi-xilinx
left a comment
There was a problem hiding this comment.
xbmgmt is not compiled for EDGE flows. I think, we should remove the else part completely.
ManojTakasi
reviewed
May 25, 2026
| @@ -65,7 +65,7 @@ add_executable(${XBMGMT2_NAME} ${XBMGMT_V2_SRCS}) | |||
| if (NOT XRT_EDGE) | |||
| target_compile_definitions(${XBMGMT2_NAME} PRIVATE ENABLE_NATIVE_SUBCMDS_AND_REPORTS) | |||
| else() | |||
Collaborator
There was a problem hiding this comment.
Agree with removing else() entirely. Edge handling already lives in xbutil2/CMakeLists.txt. xbmgmt2 is only built for XRT_XRT/XRT_ALVEO, and no xbmgmt2 source uses that macro. so, we can drop this else()
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
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.
Fixes #9829.
Summary
src/runtime_src/core/tools/xbmgmt2/CMakeLists.txtreferences${XBUTIL2_NAME}in itsXRT_EDGEbranch when callingtarget_compile_definitions.XBUTIL2_NAMEis defined in the siblingxbutil2/CMakeLists.txtand is not visible in this directory scope, so it expands to an empty string. CMake then parses the call astarget_compile_definitions(PRIVATE ENABLE_DEFAULT_ONE_DEVICE_OPTION)and aborts configure with:The non-edge branch above already uses the correct
${XBMGMT2_NAME}. This change makes the edge branch consistent.Reproduction (before this PR)
Configure fails as above. With this patch, configure proceeds past xbmgmt2.
Impact
-DXRT_EDGE=ON(arm64 / edge / Zynq).-DXRT_EDGEunset orOFF) are unaffected — the else-branch is never entered.Notes