EDGEML-14581 - Fix copy_headers to use build-time GLOB via cmake -P - #5
Conversation
[Why] The previous aie_codegen_setup_build_include_layout() implementation collected headers at cmake configure time using file(GLOB CONFIGURE_DEPENDS) then copied or symlinked each one via configure_file COPYONLY or file(CREATE_LINK). The resolved file paths were baked into the generated build system (MSBuild .rule files under the Visual Studio generator) at configure time. In CI environments where the build directory is reused across runs, stale .rule files from a prior configure can contain paths that no longer exist or that point through defunct filesystem junctions, triggering MSB8066 errors and breaking incremental builds without requiring a full reconfigure. [How] Replace the configure-time header materialization with a build-time add_custom_target(copy_headers ALL) that invokes a new cmake -P script (cmake/CopyHeaders.cmake). The script GLOBs headers and copies them using file(COPY_FILE ... ONLY_IF_DIFFERENT) at build time, so the generated .rule file contains only stable variable-expanded paths that remain valid across build-directory reuse. The aie_codegen target gains an add_dependencies(... copy_headers) link so headers are always present before compilation begins. The AieCodegenHeaders.cmake module is retained for install-time use by AieCodegenInstall.cmake. Signed-off-by: John Hoy <John.Hoy@amd.com> Co-authored-by: Claude <noreply@anthropic.com>
|
@ramanjan-amd , please help merge this PR |
| "${SRC_DIR}/*/*/*.h" | ||
| "${REGDB_DIR}/*.h" | ||
| ) | ||
|
|
There was a problem hiding this comment.
This is now a second definition of "which headers are public" — AieCodegenHeaders.cmake still owns _AIE_CODEGEN_HEADER_DIRS and still drives the install path. They match today (I
checked: 68 headers each), but a future src/newdir/foo.h would land in the build include dir and not in the installed package — builds in-tree, breaks against the installed SDK.
Can we reuse the existing list?
include("${CMAKE_CURRENT_LIST_DIR}/AieCodegenHeaders.cmake")
set(_hdrs "")
foreach(_dir IN LISTS _AIE_CODEGEN_HEADER_DIRS)
file(GLOB _d "${SRC_DIR}/${_dir}/.h")
list(APPEND _hdrs ${_d})
endforeach()
file(GLOB _r "${REGDB_DIR}/.h")
list(APPEND _hdrs ${_r})
Use the variable, not aie_codegen_collect_headers() — that reads CMAKE_CURRENT_SOURCE_DIR, which is wrong under cmake -P. Keeping the recursive glob is fine too, as long as the install
path switches to it as well.
ramanjan-amd
left a comment
ramanjan-amd
left a comment
There was a problem hiding this comment.
src/cmake/AieCodegenIncludes.cmake, on add_custom_target(copy_headers ALL
Fixed target name inside a function — fine today (called once), but a duplicate-target error if it's ever called twice or pulled in via add_subdirectory. ${PROJECT_NAME}_copy_headers avoids it.
please check this also in latest code.
|
|
||
| file(GLOB_RECURSE _hdrs | ||
| "${SRC_DIR}/*/*.h" | ||
| "${SRC_DIR}/*/*/*.h" |
There was a problem hiding this comment.
Redundant — GLOB_RECURSE already descends. First pattern alone returns all 68, including the depth-3 files under io_backend/ext and io_backend/privilege.
suggestion
file(GLOB_RECURSE _hdrs
"${SRC_DIR}//.h"
"${REGDB_DIR}/*.h"
)
| # REGDB_DIR - aie-regdb/globalparams source directory | ||
| # DST_DIR - destination include/aie_codegen_inc directory | ||
|
|
||
| foreach(_var SRC_DIR REGDB_DIR DST_DIR) |
There was a problem hiding this comment.
Could this also check the dirs exist, not just that the vars are defined? aie-regdb is often an uninitialized submodule; when it's missing the GLOB silently returns nothing and you get a
confusing missing-header error later. A message(WARNING) on NOT IS_DIRECTORY would do.
- Remove redundant GLOB_RECURSE depth pattern (/*/*/*.h is a no-op alongside /*/*.h under GLOB_RECURSE) - Replace independent GLOB_RECURSE with include(AieCodegenHeaders.cmake) + loop over _AIE_CODEGEN_HEADER_DIRS so build-time copies and install-time copies always cover the same header set - Add IS_DIRECTORY guards with message(WARNING) for uninitialized submodules (SRC_DIR, REGDB_DIR) to surface missing headers clearly
2 participants
[Why]
The previous aie_codegen_setup_build_include_layout() implementation collected headers at cmake configure time using file(GLOB CONFIGURE_DEPENDS) then copied or symlinked each one via configure_file COPYONLY or file(CREATE_LINK). The resolved file paths were baked into the generated build system (MSBuild .rule files under the Visual Studio generator) at configure time. In CI environments where the build directory is reused across runs, stale .rule files from a prior configure can contain paths that no longer exist or that point through defunct filesystem junctions, triggering MSB8066 errors and breaking incremental builds without requiring a full reconfigure.
[How]
Replace the configure-time header materialization with a build-time add_custom_target(copy_headers ALL) that invokes a new cmake -P script (cmake/CopyHeaders.cmake). The script GLOBs headers and copies them using file(COPY_FILE ... ONLY_IF_DIFFERENT) at build time, so the generated .rule file contains only stable variable-expanded paths that remain valid across build-directory reuse. The aie_codegen target gains an add_dependencies(... copy_headers) link so headers are always present before compilation begins. The AieCodegenHeaders.cmake module is retained for install-time use by AieCodegenInstall.cmake.