Remove unqualified global aliases and use qualified aliases consistently (#5807)
This consistently aliases targets to `package::path::target` style, allowing us to remove a
bunch of string munging in various places. It also makes the decision to have a global
alias for binaries consistent and documented.
diff --git a/build_tools/cmake/external_cc_library.cmake b/build_tools/cmake/external_cc_library.cmake
index 88c0f82..6411cb9 100644
--- a/build_tools/cmake/external_cc_library.cmake
+++ b/build_tools/cmake/external_cc_library.cmake
@@ -188,7 +188,12 @@
endif()
add_library(${_RULE_PACKAGE}::${_RULE_NAME} ALIAS ${_NAME})
- if(${_RULE_PACKAGE} STREQUAL ${_RULE_NAME})
+ # If the library name matches the final component of the package then treat it
+ # as a default. For example, 'foo::bar' library 'bar' would end up as
+ # 'foo::bar'.
+ string(REGEX REPLACE "^.*::" "" _PACKAGE_DIR ${_RULE_PACKAGE})
+ if(${_PACKAGE_DIR} STREQUAL ${_RULE_NAME})
+
add_library(${_RULE_PACKAGE} ALIAS ${_NAME})
endif()
endfunction()
diff --git a/build_tools/cmake/iree_cc_binary.cmake b/build_tools/cmake/iree_cc_binary.cmake
index 31e13f0..e89f55e 100644
--- a/build_tools/cmake/iree_cc_binary.cmake
+++ b/build_tools/cmake/iree_cc_binary.cmake
@@ -31,7 +31,9 @@
#
# Note:
# iree_cc_binary will create a binary called ${PACKAGE_NAME}_${NAME}, e.g.
-# iree_base_foo with an alias (readonly) target called ${NAME}.
+# iree_base_foo with two alias (readonly) targets, a qualified
+# ${PACKAGE_NS}::${NAME} and an unqualified ${NAME}. Thus NAME must be globally
+# unique in the project.
#
# Usage:
# iree_cc_library(
@@ -67,10 +69,30 @@
# Prefix the library with the package name, so we get: iree_package_name
iree_package_name(_PACKAGE_NAME)
+ iree_package_ns(_PACKAGE_NS)
set(_NAME "${_PACKAGE_NAME}_${_RULE_NAME}")
add_executable(${_NAME} "")
+ # Alias the iree_package_name binary to iree::package::name.
+ # This lets us more clearly map to Bazel and makes it possible to
+ # disambiguate the underscores in paths vs. the separators.
+ add_executable(${_PACKAGE_NS}::${_RULE_NAME} ALIAS ${_NAME})
+
+ # If the binary name matches the package then treat it as a default. For
+ # example, foo/bar/ library 'bar' would end up as 'foo::bar'. This isn't
+ # likely to be common for binaries, but is consistent with the behavior for
+ # libraries and in Bazel.
+ iree_package_dir(_PACKAGE_DIR)
+ if(${_RULE_NAME} STREQUAL ${_PACKAGE_DIR})
+ add_executable(${_PACKAGE_NS} ALIAS ${_NAME})
+ endif()
+
+ # Finally, since we have so few binaries and we also want to support
+ # installing from a separate host build, binaries get an unqualified global
+ # alias. This means binary names must be unique across the whole project.
+ # (We could consider making this configurable).
add_executable(${_RULE_NAME} ALIAS ${_NAME})
+
set_target_properties(${_NAME} PROPERTIES OUTPUT_NAME "${_RULE_NAME}")
if(_RULE_SRCS)
target_sources(${_NAME}
@@ -106,7 +128,6 @@
)
# Replace dependencies passed by ::name with iree::package::name
- iree_package_ns(_PACKAGE_NS)
list(TRANSFORM _RULE_DEPS REPLACE "^::" "${_PACKAGE_NS}::")
target_link_libraries(${_NAME}
diff --git a/build_tools/cmake/iree_cc_library.cmake b/build_tools/cmake/iree_cc_library.cmake
index a50b947..076762f 100644
--- a/build_tools/cmake/iree_cc_library.cmake
+++ b/build_tools/cmake/iree_cc_library.cmake
@@ -184,10 +184,12 @@
# This lets us more clearly map to Bazel and makes it possible to
# disambiguate the underscores in paths vs. the separators.
add_library(${_PACKAGE_NS}::${_RULE_NAME} ALIAS ${_NAME})
+
+ # If the library name matches the final component of the package then treat
+ # it as a default. For example, foo/bar/ library 'bar' would end up as
+ # 'foo::bar'.
iree_package_dir(_PACKAGE_DIR)
if(${_RULE_NAME} STREQUAL ${_PACKAGE_DIR})
- # If the library name matches the package then treat it as a default.
- # For example, foo/bar/ library 'bar' would end up as 'foo::bar'.
add_library(${_PACKAGE_NS} ALIAS ${_NAME})
endif()
endfunction()
diff --git a/build_tools/cmake/iree_cc_test.cmake b/build_tools/cmake/iree_cc_test.cmake
index 6d32fc7..13026b1 100644
--- a/build_tools/cmake/iree_cc_test.cmake
+++ b/build_tools/cmake/iree_cc_test.cmake
@@ -70,9 +70,24 @@
# Prefix the library with the package name, so we get: iree_package_name
iree_package_name(_PACKAGE_NAME)
+ iree_package_ns(_PACKAGE_NS)
set(_NAME "${_PACKAGE_NAME}_${_RULE_NAME}")
add_executable(${_NAME} "")
+ # Alias the iree_package_name test binary to iree::package::name.
+ # This lets us more clearly map to Bazel and makes it possible to
+ # disambiguate the underscores in paths vs. the separators.
+ add_executable(${_PACKAGE_NS}::${_RULE_NAME} ALIAS ${_NAME})
+
+ # If the test binary name matches the package then treat it as a default.
+ # For example, foo/bar/ library 'bar' would end up as 'foo::bar'. This isn't
+ # likely to be common for tests, but is consistent with the behavior for
+ # libraries.
+ iree_package_dir(_PACKAGE_DIR)
+ if(${_RULE_NAME} STREQUAL ${_PACKAGE_DIR})
+ add_executable(${_PACKAGE_NS} ALIAS ${_NAME})
+ endif()
+
set_target_properties(${_NAME} PROPERTIES OUTPUT_NAME "${_RULE_NAME}")
target_sources(${_NAME}
PRIVATE
@@ -99,7 +114,7 @@
)
# Replace dependencies passed by ::name with iree::package::name
- iree_package_ns(_PACKAGE_NS)
+
list(TRANSFORM _RULE_DEPS REPLACE "^::" "${_PACKAGE_NS}::")
target_link_libraries(${_NAME}
diff --git a/build_tools/cmake/iree_lit_test.cmake b/build_tools/cmake/iree_lit_test.cmake
index 9d18495..e7b3b84 100644
--- a/build_tools/cmake/iree_lit_test.cmake
+++ b/build_tools/cmake/iree_lit_test.cmake
@@ -63,8 +63,7 @@
list(TRANSFORM _RULE_DATA REPLACE "^::" "${_PACKAGE_NS}::")
set(_DATA_DEP_PATHS)
foreach(_DATA_DEP ${_RULE_DATA})
- string(REPLACE "::" "_" _DATA_DEP_NAME ${_DATA_DEP})
- list(APPEND _DATA_DEP_PATHS $<TARGET_FILE:${_DATA_DEP_NAME}>)
+ list(APPEND _DATA_DEP_PATHS $<TARGET_FILE:${_DATA_DEP}>)
endforeach(_DATA_DEP)
iree_package_ns(_PACKAGE_NS)
diff --git a/build_tools/cmake/iree_macros.cmake b/build_tools/cmake/iree_macros.cmake
index dc272ca..16a0750 100644
--- a/build_tools/cmake/iree_macros.cmake
+++ b/build_tools/cmake/iree_macros.cmake
@@ -110,7 +110,10 @@
#
# Paramters:
# - OUTPUT_PATH_VAR: variable name for receiving the path to the built target.
-# - EXECUTABLE: the executable to get its path.
+# - EXECUTABLE: the executable to get its path. Note that this needs to be the
+# name of the executable target when not cross compiling and the basename of
+# the binary when importing a binary from a host build. Thus this should be
+# the global unqualified name of the binary, not the fully-specified name.
function(iree_get_executable_path OUTPUT_PATH_VAR EXECUTABLE)
if (NOT CMAKE_CROSSCOMPILING OR TARGET "${EXECUTABLE}")
# We can either expect the target to be defined as part of this CMake
diff --git a/build_tools/cmake/iree_run_binary_test.cmake b/build_tools/cmake/iree_run_binary_test.cmake
index 7b5b373..3747e51 100644
--- a/build_tools/cmake/iree_run_binary_test.cmake
+++ b/build_tools/cmake/iree_run_binary_test.cmake
@@ -63,11 +63,11 @@
iree_package_name(_PACKAGE_NAME)
set(_NAME "${_PACKAGE_NAME}_${_RULE_NAME}")
iree_package_ns(_PACKAGE_NS)
- string(REPLACE "::" "/" _PACKAGE_PATH ${_PACKAGE_NS})
+ iree_package_path(_PACKAGE_PATH)
set(_TEST_NAME "${_PACKAGE_PATH}/${_RULE_NAME}")
+ # Replace binary passed by relative ::name with iree::package::name
string(REGEX REPLACE "^::" "${_PACKAGE_NS}::" _TEST_BINARY_TARGET ${_RULE_TEST_BINARY})
- string(REPLACE "::" "_" _TEST_BINARY_EXECUTABLE ${_TEST_BINARY_TARGET})
if(ANDROID)
set(_ANDROID_REL_DIR "${_PACKAGE_PATH}/${_RULE_NAME}")
@@ -80,7 +80,7 @@
${_TEST_NAME}
COMMAND
"${CMAKE_SOURCE_DIR}/build_tools/cmake/run_android_test.${IREE_HOST_SCRIPT_EXT}"
- "${_ANDROID_REL_DIR}/$<TARGET_FILE_NAME:${_TEST_BINARY_EXECUTABLE}>"
+ "${_ANDROID_REL_DIR}/$<TARGET_FILE_NAME:${_TEST_BINARY_TARGET}>"
${_RULE_ARGS}
)
# Use environment variables to instruct the script to push artifacts
@@ -89,7 +89,7 @@
set(
_ENVIRONMENT_VARS
TEST_ANDROID_ABS_DIR=${_ANDROID_ABS_DIR}
- TEST_EXECUTABLE=$<TARGET_FILE:${_TEST_BINARY_EXECUTABLE}>
+ TEST_EXECUTABLE=$<TARGET_FILE:${_TEST_BINARY_TARGET}>
TEST_TMPDIR=${_ANDROID_ABS_DIR}/test_tmpdir
)
set_property(TEST ${_TEST_NAME} PROPERTY ENVIRONMENT ${_ENVIRONMENT_VARS})
@@ -99,7 +99,7 @@
${_TEST_NAME}
COMMAND
"${CMAKE_SOURCE_DIR}/build_tools/cmake/run_test.${IREE_HOST_SCRIPT_EXT}"
- "$<TARGET_FILE:${_TEST_BINARY_EXECUTABLE}>"
+ "$<TARGET_FILE:${_TEST_BINARY_TARGET}>"
${_RULE_ARGS}
)
set_property(TEST ${_TEST_NAME} PROPERTY ENVIRONMENT "TEST_TMPDIR=${CMAKE_BINARY_DIR}/${_NAME}_test_tmpdir")
diff --git a/iree/tools/CMakeLists.txt b/iree/tools/CMakeLists.txt
index af3ab84..15c925d 100644
--- a/iree/tools/CMakeLists.txt
+++ b/iree/tools/CMakeLists.txt
@@ -379,24 +379,24 @@
# CMake always needs to custom targets to drive custom commands. We have to
# give it a different name.
- add_custom_target(_IreeFileCheck_Driver ALL DEPENDS IreeFileCheck)
+ add_custom_target(iree_tools_IreeFileCheck_Driver ALL DEPENDS IreeFileCheck)
# But custom target doesn't have the TARGET_FILE property, which we need, so
- # wrap it yet again in an executable. This is marke as an imported executable
- # because otherwise CMake would try to compile it as C++. The executable
- # target has to have a different name than the output file of the custom
- # command... (https://gitlab.kitware.com/cmake/cmake/-/issues/18627).
- add_executable(IreeFileCheck_exe IMPORTED GLOBAL)
+ # wrap it yet again in an executable. This is marked as an imported executable
+ # because otherwise CMake would try to compile it as C++.
+ add_executable(iree_tools_IreeFileCheck IMPORTED GLOBAL)
set_property(
TARGET
- IreeFileCheck_exe
+ iree_tools_IreeFileCheck
PROPERTY IMPORTED_LOCATION
"${CMAKE_CURRENT_BINARY_DIR}/IreeFileCheck"
)
- add_dependencies(IreeFileCheck_exe _IreeFileCheck_Driver)
- # Finally do our normal aliasing trick to make this accessible as a qualified
- # target name throughout the project.
- add_executable(iree_tools_IreeFileCheck ALIAS IreeFileCheck_exe)
+ add_dependencies(iree_tools_IreeFileCheck iree_tools_IreeFileCheck_Driver)
+ # Finally do our normal aliasing tricks to make this accessible as a qualified
+ # target name throughout the project. Accessing this as an unqualified target
+ # is not supported as the name would conflict with the file name, which CMake
+ # does not support (https://gitlab.kitware.com/cmake/cmake/-/issues/18627).
+ add_executable(iree::tools::IreeFileCheck ALIAS iree_tools_IreeFileCheck)
if(${IREE_MLIR_DEP_MODE} STREQUAL "BUNDLED")
add_custom_target(BundledLLVMFileCheck ALL