From 47ec689e6b9d0c042a11ed99cf475a90ee022b82 Mon Sep 17 00:00:00 2001 From: Brendan Duke Date: Fri, 28 Jun 2024 20:24:04 +0000 Subject: [PATCH] [mlir] Make MLIR Python sources idempotent Make MLIR Python sources idempotent by moving the custom commands off the custom target for each source. Previously, a custom target was created for each MLIR Python source file and a custom command added to symlink it to a destination path. However this causes the build to not be idempotent because custom targets always run. Instead, this PR separates the symlink/copy to destination paths into custom commands and makes the custom target depend on the output. That prevents re-running them and restores idempotency. Testing: - Build with `-D MLIR_ENABLE_BINDINGS_PYTHON=ON`. Prior to this change, the build is not idempotent (building twice re-runs the symlink/copy command for Python source targets). After this change it is idempotent. --- mlir/cmake/modules/AddMLIRPython.cmake | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/mlir/cmake/modules/AddMLIRPython.cmake b/mlir/cmake/modules/AddMLIRPython.cmake index f030798a94ae9..7b91f43e2d57f 100644 --- a/mlir/cmake/modules/AddMLIRPython.cmake +++ b/mlir/cmake/modules/AddMLIRPython.cmake @@ -565,8 +565,6 @@ function(add_mlir_python_sources_target name) message(FATAL_ERROR "Unhandled arguments to add_mlir_python_sources_target(${name}, ... : ${ARG_UNPARSED_ARGUMENTS}") endif() - add_custom_target(${name}) - # On Windows create_symlink requires special permissions. Use copy_if_different instead. if(CMAKE_HOST_WIN32) set(_link_or_copy copy_if_different) @@ -575,8 +573,6 @@ function(add_mlir_python_sources_target name) endif() foreach(_sources_target ${ARG_SOURCES_TARGETS}) - add_dependencies(${name} ${_sources_target}) - get_target_property(_src_paths ${_sources_target} SOURCES) if(NOT _src_paths) get_target_property(_src_paths ${_sources_target} INTERFACE_SOURCES) @@ -590,6 +586,8 @@ function(add_mlir_python_sources_target name) get_target_property(_root_dir ${_sources_target} INTERFACE_INCLUDE_DIRECTORIES) endif() + # Initialize an empty list of all Python source destination paths. + set(all_dest_paths "") foreach(_src_path ${_src_paths}) file(RELATIVE_PATH _source_relative_path "${_root_dir}" "${_src_path}") set(_dest_path "${ARG_OUTPUT_DIRECTORY}/${_source_relative_path}") @@ -598,13 +596,17 @@ function(add_mlir_python_sources_target name) file(MAKE_DIRECTORY "${_dest_dir}") add_custom_command( - TARGET ${name} PRE_BUILD + OUTPUT "${_dest_path}" + PRE_BUILD COMMENT "Copying python source ${_src_path} -> ${_dest_path}" DEPENDS "${_src_path}" - BYPRODUCTS "${_dest_path}" COMMAND "${CMAKE_COMMAND}" -E ${_link_or_copy} "${_src_path}" "${_dest_path}" ) + + # Track the symlink or copy command output. + list(APPEND all_dest_paths "${_dest_path}") + if(ARG_INSTALL_DIR) # We have to install each file individually because we need to preserve # the relative directory structure in the install destination. @@ -621,6 +623,9 @@ function(add_mlir_python_sources_target name) endif() endforeach() endforeach() + + # Create a new custom target that depends on all symlinked or copied sources. + add_custom_target("${name}" DEPENDS ${all_dest_paths}) endfunction() ################################################################################