From 35a174a5fb95aae4f3b11091bb0e2b199a81c964 Mon Sep 17 00:00:00 2001 From: Steven Johnson Date: Thu, 4 Aug 2022 15:53:27 -0700 Subject: [PATCH 01/15] Rework PYTHONPATH --- README_cmake.md | 2 +- python_bindings/CMakeLists.txt | 2 +- python_bindings/src/CMakeLists.txt | 10 ++++++++++ python_bindings/src/halide/CMakeLists.txt | 7 +++++-- python_bindings/test/apps/CMakeLists.txt | 1 - python_bindings/test/correctness/CMakeLists.txt | 3 +-- python_bindings/tutorial/CMakeLists.txt | 3 +-- src/autoschedulers/li2018/CMakeLists.txt | 1 - 8 files changed, 19 insertions(+), 10 deletions(-) create mode 100644 python_bindings/src/CMakeLists.txt diff --git a/README_cmake.md b/README_cmake.md index c152fd101912..0761030214cc 100644 --- a/README_cmake.md +++ b/README_cmake.md @@ -781,7 +781,7 @@ The following targets are not guaranteed to be available: | Imported target | Description | |-------------------------|------------------------------------------------------------------------------------------------------------------------------------------| -| `Halide::Python` | this is a Python 3 module that can be referenced as `$` when setting up Python tests or the like from CMake. | +| `Halide::Python` | this is a Python 3 package that can be referenced as `$` when setting up PYTHONPATH for Python tests or the like from CMake. | | `Halide::Adams19` | the Adams et.al. 2019 autoscheduler (no GPU support) | | `Halide::Li18` | the Li et.al. 2018 gradient autoscheduler (limited GPU support) | | `Halide::Mullapudi2016` | the Mullapudi et.al. 2016 autoscheduler (no GPU support) | diff --git a/python_bindings/CMakeLists.txt b/python_bindings/CMakeLists.txt index 979a3594b534..66214e97c862 100644 --- a/python_bindings/CMakeLists.txt +++ b/python_bindings/CMakeLists.txt @@ -53,7 +53,7 @@ endif () # Add our sources to this sub-tree. ## -add_subdirectory(src/halide) +add_subdirectory(src) add_subdirectory(stub) if (WITH_TEST_PYTHON) diff --git a/python_bindings/src/CMakeLists.txt b/python_bindings/src/CMakeLists.txt new file mode 100644 index 000000000000..56a541d44b3b --- /dev/null +++ b/python_bindings/src/CMakeLists.txt @@ -0,0 +1,10 @@ +add_subdirectory(halide) + +# Now add a 'target' that exists just to conveniently give us a way to find this directory; +# we can't use $ because we need `import halide` to see +# this directory (not the one below us). +add_library(Halide_Python_Target_File_Dir UNKNOWN IMPORTED GLOBAL) +set_target_properties(Halide_Python_Target_File_Dir PROPERTIES + IMPORTED_LOCATION ${CMAKE_CURRENT_BINARY_DIR}/nonexistent) + +add_library(Halide::Python ALIAS Halide_Python_Target_File_Dir) diff --git a/python_bindings/src/halide/CMakeLists.txt b/python_bindings/src/halide/CMakeLists.txt index a93aae90da9c..f3248d9620db 100644 --- a/python_bindings/src/halide/CMakeLists.txt +++ b/python_bindings/src/halide/CMakeLists.txt @@ -32,7 +32,6 @@ set(SOURCES list(TRANSFORM SOURCES PREPEND "halide_/") pybind11_add_module(Halide_Python MODULE ${SOURCES}) -add_library(Halide::Python ALIAS Halide_Python) set_target_properties(Halide_Python PROPERTIES LIBRARY_OUTPUT_NAME halide_ @@ -46,10 +45,14 @@ if (WIN32 AND BUILD_SHARED_LIBS) # Ref: https://bugs.python.org/issue36085 # Ref: https://docs.python.org/3/whatsnew/3.8.html#bpo-36085-whatsnew add_custom_command(TARGET Halide_Python POST_BUILD - COMMAND ${CMAKE_COMMAND} -E copy_if_different $ $ + COMMAND ${CMAKE_COMMAND} -E copy_if_different $ $ VERBATIM) endif () +# Copy our __init__.py file over so that we have a package laid out in the right fashion +# (halide_.so has already been built in this dir) +configure_file("__init__.py" "${CMAKE_CURRENT_BINARY_DIR}/__init__.py") + ## # Packaging ## diff --git a/python_bindings/test/apps/CMakeLists.txt b/python_bindings/test/apps/CMakeLists.txt index 90e8de28eadd..8f04b511cb38 100644 --- a/python_bindings/test/apps/CMakeLists.txt +++ b/python_bindings/test/apps/CMakeLists.txt @@ -8,7 +8,6 @@ set(SCRIPTS make_shell_path( PYTHONPATH "$" - "${Halide_SOURCE_DIR}/python_bindings/src" ) set(TEST_ENV diff --git a/python_bindings/test/correctness/CMakeLists.txt b/python_bindings/test/correctness/CMakeLists.txt index 205256e4d2f7..f868231c9f6c 100644 --- a/python_bindings/test/correctness/CMakeLists.txt +++ b/python_bindings/test/correctness/CMakeLists.txt @@ -29,10 +29,9 @@ set(TESTS # Use generator expressions to get the true output paths of these files. make_shell_path( PYTHONPATH + "$" "$" "$" - "$" - "${Halide_SOURCE_DIR}/python_bindings/src" ) foreach (TEST IN LISTS TESTS) diff --git a/python_bindings/tutorial/CMakeLists.txt b/python_bindings/tutorial/CMakeLists.txt index 09f2759a9e82..4569cfa7daec 100644 --- a/python_bindings/tutorial/CMakeLists.txt +++ b/python_bindings/tutorial/CMakeLists.txt @@ -18,9 +18,8 @@ set(TESTS make_shell_path( PYTHONPATH - "$" "$" - "${Halide_SOURCE_DIR}/python_bindings/src" + "$" ) foreach (TEST IN LISTS TESTS) diff --git a/src/autoschedulers/li2018/CMakeLists.txt b/src/autoschedulers/li2018/CMakeLists.txt index 31a1fb2f8eab..9a35af44416c 100644 --- a/src/autoschedulers/li2018/CMakeLists.txt +++ b/src/autoschedulers/li2018/CMakeLists.txt @@ -48,7 +48,6 @@ if (WITH_PYTHON_BINDINGS) make_shell_path( PYTHONPATH "$" - "${Halide_SOURCE_DIR}/python_bindings/src" ) if (WIN32) From 53a3362889fab4d39def9c8315edc8c443d2fa81 Mon Sep 17 00:00:00 2001 From: Alex Reinking Date: Mon, 8 Aug 2022 18:01:14 -0400 Subject: [PATCH 02/15] Move pure-Python file copying logic to build time. --- python_bindings/src/halide/CMakeLists.txt | 39 +++++++++++++++++------ 1 file changed, 30 insertions(+), 9 deletions(-) diff --git a/python_bindings/src/halide/CMakeLists.txt b/python_bindings/src/halide/CMakeLists.txt index e7bb2fbfd200..a2ffa17f89b4 100644 --- a/python_bindings/src/halide/CMakeLists.txt +++ b/python_bindings/src/halide/CMakeLists.txt @@ -1,4 +1,4 @@ -set(SOURCES +set(native_sources PyArgument.cpp PyBoundaryConditions.cpp PyBuffer.cpp @@ -29,9 +29,13 @@ set(SOURCES PyVar.cpp PyVarOrRVar.cpp ) -list(TRANSFORM SOURCES PREPEND "halide_/") +list(TRANSFORM native_sources PREPEND "halide_/") -pybind11_add_module(Halide_Python MODULE ${SOURCES}) +set(python_sources + __init__.py + ) + +pybind11_add_module(Halide_Python MODULE ${native_sources}) set_target_properties(Halide_Python PROPERTIES LIBRARY_OUTPUT_NAME halide_ @@ -49,9 +53,23 @@ if (WIN32 AND BUILD_SHARED_LIBS) VERBATIM) endif () -# Copy our __init__.py file over so that we have a package laid out in the right fashion -# (halide_.so has already been built in this dir) -configure_file("__init__.py" "${CMAKE_CURRENT_BINARY_DIR}/__init__.py") +# Copy our Python source files over so that we have a valid package in the binary directory. +set(build_tree_pys "") +foreach (pysrc IN LISTS python_sources) + # TODO: CMake 3.22 still doesn't allow target-dependent genex in OUTPUT, but we can hack around this using a stamp + # file. Fix this hack up if and when they ever improve this feature. + set(stamp_file "${CMAKE_CURRENT_BINARY_DIR}/.${pysrc}.stamp") + add_custom_command( + OUTPUT "${stamp_file}" + COMMAND ${CMAKE_COMMAND} -E copy "${CMAKE_CURRENT_SOURCE_DIR}/${pysrc}" "$/${pysrc}" + COMMAND ${CMAKE_COMMAND} -E touch "${stamp_file}" + DEPENDS "${CMAKE_CURRENT_SOURCE_DIR}/${pysrc}" + VERBATIM + ) + list(APPEND build_tree_pys "${stamp_file}") +endforeach () +add_custom_target(Halide_Python_sources ALL DEPENDS ${build_tree_pys}) +add_dependencies(Halide_Python Halide_Python_sources) ## # Packaging @@ -63,12 +81,15 @@ include(GNUInstallDirs) set(Halide_INSTALL_PYTHONDIR "${CMAKE_INSTALL_LIBDIR}/python3/site-packages" CACHE STRING "Path to the Python site-packages folder") -install(DIRECTORY "${CMAKE_CURRENT_SOURCE_DIR}" +# TODO: this is wrong for multi-config generators. It's not CMAKE_CURRENT_BINARY_DIR. +# we might need to lock down the directory structure by setting properties here. +install(DIRECTORY "${CMAKE_CURRENT_BINARY_DIR}" DESTINATION "${Halide_INSTALL_PYTHONDIR}" COMPONENT Halide_Python FILES_MATCHING PATTERN "*.py" PATTERN "*/halide_" EXCLUDE + PATTERN "*/CMakeFiles" EXCLUDE PATTERN "*/__pycache__" EXCLUDE) install(TARGETS Halide_Python @@ -123,8 +144,8 @@ if ( endif () file(RELATIVE_PATH lib_dir - "${CMAKE_CURRENT_BINARY_DIR}/${Halide_INSTALL_PYTHONDIR}/halide" - "${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_INSTALL_LIBDIR}") + "${CMAKE_CURRENT_BINARY_DIR}/${Halide_INSTALL_PYTHONDIR}/halide" + "${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_INSTALL_LIBDIR}") set_target_properties(Halide_Python PROPERTIES INSTALL_RPATH "${rbase}/${lib_dir}") endif () From 3fc32735697071c2b8075ecb5833eb78859fad5c Mon Sep 17 00:00:00 2001 From: Alex Reinking Date: Mon, 8 Aug 2022 18:01:51 -0400 Subject: [PATCH 03/15] Use TARGET_RUNTIME_DLLS to copy all DLLs instead of just Halide. --- python_bindings/src/halide/CMakeLists.txt | 26 ++++++++++++++--------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/python_bindings/src/halide/CMakeLists.txt b/python_bindings/src/halide/CMakeLists.txt index a2ffa17f89b4..25e5f75bfcca 100644 --- a/python_bindings/src/halide/CMakeLists.txt +++ b/python_bindings/src/halide/CMakeLists.txt @@ -42,16 +42,22 @@ set_target_properties(Halide_Python EXPORT_NAME Python) target_link_libraries(Halide_Python PRIVATE Halide::Halide) -if (WIN32 AND BUILD_SHARED_LIBS) - # There's precious little information about why Python only sometimes prevents DLLs from loading from the PATH on Windows. - # This workaround places a copy of Halide.dll next to our Python module. - # Ref: https://stackoverflow.com/questions/59860465/pybind11-importerror-dll-not-found-when-trying-to-import-pyd-in-python-int - # Ref: https://bugs.python.org/issue36085 - # Ref: https://docs.python.org/3/whatsnew/3.8.html#bpo-36085-whatsnew - add_custom_command(TARGET Halide_Python POST_BUILD - COMMAND ${CMAKE_COMMAND} -E copy_if_different $ $ - VERBATIM) -endif () +# TODO: There's precious little information about why Python only sometimes prevents DLLs from loading from the PATH +# on Windows. This workaround places a copy of Halide.dll (and any other dependencies) next to our Python module. +# Ref: https://stackoverflow.com/questions/59860465/pybind11-importerror-dll-not-found-when-trying-to-import-pyd-in-python-int +# Ref: https://bugs.python.org/issue36085 +# Ref: https://docs.python.org/3/whatsnew/3.8.html#bpo-36085-whatsnew +# TODO: copying a dummy file here works around a CMake limitation. The issue is that if $ is +# empty, then copy_if_different errors out, thinking it doesn't have enough arguments. +# Ref: https://gitlab.kitware.com/cmake/cmake/-/issues/23543 +set(dummy_file "${CMAKE_CURRENT_BINARY_DIR}/.dummy_file") +file(TOUCH "${dummy_file}") +add_custom_command( + TARGET Halide_Python POST_BUILD + COMMAND ${CMAKE_COMMAND} -E copy_if_different "${dummy_file}" $ $ + COMMAND_EXPAND_LISTS + VERBATIM +) # Copy our Python source files over so that we have a valid package in the binary directory. set(build_tree_pys "") From 4ea676fb02c90ab9ba48c9d4ab76791405c6ff93 Mon Sep 17 00:00:00 2001 From: Alex Reinking Date: Mon, 8 Aug 2022 18:31:22 -0400 Subject: [PATCH 04/15] Ensure that the last path component for Halide_Python is always `halide` --- python_bindings/src/halide/CMakeLists.txt | 26 ++++++++++++++--------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/python_bindings/src/halide/CMakeLists.txt b/python_bindings/src/halide/CMakeLists.txt index 25e5f75bfcca..2e6681fbe583 100644 --- a/python_bindings/src/halide/CMakeLists.txt +++ b/python_bindings/src/halide/CMakeLists.txt @@ -35,11 +35,19 @@ set(python_sources __init__.py ) +# It is technically still possible for a user to override the LIBRARY_OUTPUT_DIRECTORY by setting +# CMAKE_LIBRARY_OUTPUT_DIRECTORY_, but they do so at their own peril. If a user needs to +# do this, they should use the CMAKE_PROJECT_Halide_Python_INCLUDE_BEFORE variable to override it +# just for this project, rather than globally, and they should ensure that the last path component +# is `halide`. Otherwise, the tests will break. pybind11_add_module(Halide_Python MODULE ${native_sources}) -set_target_properties(Halide_Python - PROPERTIES - LIBRARY_OUTPUT_NAME halide_ - EXPORT_NAME Python) +set_target_properties( + Halide_Python + PROPERTIES + LIBRARY_OUTPUT_NAME halide_ + LIBRARY_OUTPUT_DIRECTORY "$/halide" + EXPORT_NAME Python +) target_link_libraries(Halide_Python PRIVATE Halide::Halide) # TODO: There's precious little information about why Python only sometimes prevents DLLs from loading from the PATH @@ -60,6 +68,7 @@ add_custom_command( ) # Copy our Python source files over so that we have a valid package in the binary directory. +# TODO: When upgrading to CMake 3.23 or beyond, investigate the FILE_SET feature. set(build_tree_pys "") foreach (pysrc IN LISTS python_sources) # TODO: CMake 3.22 still doesn't allow target-dependent genex in OUTPUT, but we can hack around this using a stamp @@ -87,10 +96,8 @@ include(GNUInstallDirs) set(Halide_INSTALL_PYTHONDIR "${CMAKE_INSTALL_LIBDIR}/python3/site-packages" CACHE STRING "Path to the Python site-packages folder") -# TODO: this is wrong for multi-config generators. It's not CMAKE_CURRENT_BINARY_DIR. -# we might need to lock down the directory structure by setting properties here. -install(DIRECTORY "${CMAKE_CURRENT_BINARY_DIR}" - DESTINATION "${Halide_INSTALL_PYTHONDIR}" +install(DIRECTORY "$/" + DESTINATION "${Halide_INSTALL_PYTHONDIR}/halide" COMPONENT Halide_Python FILES_MATCHING PATTERN "*.py" @@ -100,8 +107,7 @@ install(DIRECTORY "${CMAKE_CURRENT_BINARY_DIR}" install(TARGETS Halide_Python LIBRARY DESTINATION "${Halide_INSTALL_PYTHONDIR}/halide" - COMPONENT Halide_Python - NAMELINK_COMPONENT Halide_Python) + COMPONENT Halide_Python) get_property(halide_is_imported TARGET Halide::Halide PROPERTY IMPORTED) get_property(halide_type TARGET Halide::Halide PROPERTY TYPE) From a1947cfe595d379116f91277de65025c7bc9303d Mon Sep 17 00:00:00 2001 From: Steven Johnson Date: Tue, 9 Aug 2022 14:48:29 -0700 Subject: [PATCH 05/15] Attempt to fix PYTHONPATH stuff --- python_bindings/src/CMakeLists.txt | 9 --------- python_bindings/src/halide/CMakeLists.txt | 13 +++++++++++++ python_bindings/test/apps/CMakeLists.txt | 1 + 3 files changed, 14 insertions(+), 9 deletions(-) diff --git a/python_bindings/src/CMakeLists.txt b/python_bindings/src/CMakeLists.txt index 56a541d44b3b..aa44c7e36f7f 100644 --- a/python_bindings/src/CMakeLists.txt +++ b/python_bindings/src/CMakeLists.txt @@ -1,10 +1 @@ add_subdirectory(halide) - -# Now add a 'target' that exists just to conveniently give us a way to find this directory; -# we can't use $ because we need `import halide` to see -# this directory (not the one below us). -add_library(Halide_Python_Target_File_Dir UNKNOWN IMPORTED GLOBAL) -set_target_properties(Halide_Python_Target_File_Dir PROPERTIES - IMPORTED_LOCATION ${CMAKE_CURRENT_BINARY_DIR}/nonexistent) - -add_library(Halide::Python ALIAS Halide_Python_Target_File_Dir) diff --git a/python_bindings/src/halide/CMakeLists.txt b/python_bindings/src/halide/CMakeLists.txt index 2e6681fbe583..1d2ec28b94d6 100644 --- a/python_bindings/src/halide/CMakeLists.txt +++ b/python_bindings/src/halide/CMakeLists.txt @@ -86,6 +86,19 @@ endforeach () add_custom_target(Halide_Python_sources ALL DEPENDS ${build_tree_pys}) add_dependencies(Halide_Python Halide_Python_sources) +# Now add a 'target' that exists just to conveniently give us a way to find this directory; +# we can't use $ because we need `import halide` to see +# $, not $/halide. This is surprisingly complex, in part because +# $ is actually evaluated at build time (not config time), so we must +# only use it in expressions that are deferred until build time... +set(fake_cpp_file "${CMAKE_CURRENT_BINARY_DIR}/.fake_cpp_file.cpp") +file(WRITE "${fake_cpp_file}" "int halide_fake_unused_;\n") + +add_library(Halide_Python_Fake_Lib SHARED "${fake_cpp_file}") +# Note that LIBRARY_OUTPUT_DIRECTORY is only used for SHARED, not STATIC +set_target_properties(Halide_Python_Fake_Lib PROPERTIES LIBRARY_OUTPUT_DIRECTORY "$") +add_library(Halide::Python ALIAS Halide_Python_Fake_Lib) + ## # Packaging ## diff --git a/python_bindings/test/apps/CMakeLists.txt b/python_bindings/test/apps/CMakeLists.txt index 2115ccd85e3c..81907754f09a 100644 --- a/python_bindings/test/apps/CMakeLists.txt +++ b/python_bindings/test/apps/CMakeLists.txt @@ -8,6 +8,7 @@ set(scripts set(PYTHONPATH "$" ) +list(TRANSFORM PYTHONPATH PREPEND "PYTHONPATH=path_list_prepend:") set(TEST_ENV "PYTHONPATH=${PYTHONPATH}" From e3fcb66dd344d3e7c73b294687d335fa75d4acdc Mon Sep 17 00:00:00 2001 From: Steven Johnson Date: Tue, 9 Aug 2022 16:33:40 -0700 Subject: [PATCH 06/15] Update CMakeLists.txt --- python_bindings/src/halide/CMakeLists.txt | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/python_bindings/src/halide/CMakeLists.txt b/python_bindings/src/halide/CMakeLists.txt index 1d2ec28b94d6..7c94baf581c3 100644 --- a/python_bindings/src/halide/CMakeLists.txt +++ b/python_bindings/src/halide/CMakeLists.txt @@ -95,8 +95,12 @@ set(fake_cpp_file "${CMAKE_CURRENT_BINARY_DIR}/.fake_cpp_file.cpp") file(WRITE "${fake_cpp_file}" "int halide_fake_unused_;\n") add_library(Halide_Python_Fake_Lib SHARED "${fake_cpp_file}") -# Note that LIBRARY_OUTPUT_DIRECTORY is only used for SHARED, not STATIC -set_target_properties(Halide_Python_Fake_Lib PROPERTIES LIBRARY_OUTPUT_DIRECTORY "$") +# Note that LIBRARY_OUTPUT_DIRECTORY is only used for SHARED, not STATIC, +# and that the shared library won't get created on Windows if no symbols are exported. +set_target_properties(Halide_Python_Fake_Lib + PROPERTIES + LIBRARY_OUTPUT_DIRECTORY "$" + WINDOWS_EXPORT_ALL_SYMBOLS TRUE) add_library(Halide::Python ALIAS Halide_Python_Fake_Lib) ## From a7a78c8a4fc1b9d0a151ebdcefe392c18429fce1 Mon Sep 17 00:00:00 2001 From: Steven Johnson Date: Tue, 9 Aug 2022 17:26:55 -0700 Subject: [PATCH 07/15] Update CMakeLists.txt --- python_bindings/test/apps/CMakeLists.txt | 1 - 1 file changed, 1 deletion(-) diff --git a/python_bindings/test/apps/CMakeLists.txt b/python_bindings/test/apps/CMakeLists.txt index 81907754f09a..591cad02af44 100644 --- a/python_bindings/test/apps/CMakeLists.txt +++ b/python_bindings/test/apps/CMakeLists.txt @@ -11,7 +11,6 @@ set(PYTHONPATH list(TRANSFORM PYTHONPATH PREPEND "PYTHONPATH=path_list_prepend:") set(TEST_ENV - "PYTHONPATH=${PYTHONPATH}" "HL_TARGET=${Halide_TARGET}" "TEST_TMPDIR=$" "TEST_IMAGES_DIR=$" From 9f802ed729f698a10d0fbecf7edb3c7101bf2251 Mon Sep 17 00:00:00 2001 From: Steven Johnson Date: Tue, 9 Aug 2022 17:29:00 -0700 Subject: [PATCH 08/15] Remove Duplicates --- python_bindings/test/apps/CMakeLists.txt | 1 + python_bindings/test/correctness/CMakeLists.txt | 1 + python_bindings/tutorial/CMakeLists.txt | 1 + src/autoschedulers/li2018/CMakeLists.txt | 2 ++ 4 files changed, 5 insertions(+) diff --git a/python_bindings/test/apps/CMakeLists.txt b/python_bindings/test/apps/CMakeLists.txt index 591cad02af44..33a65cd427bd 100644 --- a/python_bindings/test/apps/CMakeLists.txt +++ b/python_bindings/test/apps/CMakeLists.txt @@ -8,6 +8,7 @@ set(scripts set(PYTHONPATH "$" ) +list(REMOVE_DUPLICATES PYTHONPATH) list(TRANSFORM PYTHONPATH PREPEND "PYTHONPATH=path_list_prepend:") set(TEST_ENV diff --git a/python_bindings/test/correctness/CMakeLists.txt b/python_bindings/test/correctness/CMakeLists.txt index 57a5d8e5e201..91e7347a6798 100644 --- a/python_bindings/test/correctness/CMakeLists.txt +++ b/python_bindings/test/correctness/CMakeLists.txt @@ -33,6 +33,7 @@ set( "$" "$" ) +list(REMOVE_DUPLICATES PYTHONPATH) list(TRANSFORM PYTHONPATH PREPEND "PYTHONPATH=path_list_prepend:") foreach (test IN LISTS tests) diff --git a/python_bindings/tutorial/CMakeLists.txt b/python_bindings/tutorial/CMakeLists.txt index 83b7688c8859..c5a28409b4ad 100644 --- a/python_bindings/tutorial/CMakeLists.txt +++ b/python_bindings/tutorial/CMakeLists.txt @@ -21,6 +21,7 @@ set( "$" "$" ) +list(REMOVE_DUPLICATES PYTHONPATH) list(TRANSFORM PYTHONPATH PREPEND "PYTHONPATH=path_list_prepend:") foreach (test IN LISTS tests) diff --git a/src/autoschedulers/li2018/CMakeLists.txt b/src/autoschedulers/li2018/CMakeLists.txt index 56433561c977..d30a70ee2eaa 100644 --- a/src/autoschedulers/li2018/CMakeLists.txt +++ b/src/autoschedulers/li2018/CMakeLists.txt @@ -49,6 +49,7 @@ if (WITH_PYTHON_BINDINGS) PYTHONPATH "$" ) + list(REMOVE_DUPLICATES PYTHONPATH) list(TRANSFORM PYTHONPATH PREPEND "PYTHONPATH=path_list_prepend:") set( @@ -56,6 +57,7 @@ if (WITH_PYTHON_BINDINGS) "$" "$" ) + list(REMOVE_DUPLICATES PATH) list(TRANSFORM PATH PREPEND "PATH=path_list_prepend:") set_tests_properties(gradient_autoscheduler_test_py PROPERTIES From 31e61cda1f95de26f7568ebb1756705ed6d64852 Mon Sep 17 00:00:00 2001 From: Steven Johnson Date: Tue, 9 Aug 2022 17:54:19 -0700 Subject: [PATCH 09/15] Update CMakeLists.txt --- python_bindings/src/halide/CMakeLists.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/python_bindings/src/halide/CMakeLists.txt b/python_bindings/src/halide/CMakeLists.txt index 7c94baf581c3..fcba7b685500 100644 --- a/python_bindings/src/halide/CMakeLists.txt +++ b/python_bindings/src/halide/CMakeLists.txt @@ -99,7 +99,9 @@ add_library(Halide_Python_Fake_Lib SHARED "${fake_cpp_file}") # and that the shared library won't get created on Windows if no symbols are exported. set_target_properties(Halide_Python_Fake_Lib PROPERTIES + # Must set both of these for DLL systems to get it right LIBRARY_OUTPUT_DIRECTORY "$" + RUNTIME_OUTPUT_DIRECTORY "$" WINDOWS_EXPORT_ALL_SYMBOLS TRUE) add_library(Halide::Python ALIAS Halide_Python_Fake_Lib) From 437b71c71ec2d31ed8bdcbcec2a4b21395210950 Mon Sep 17 00:00:00 2001 From: Steven Johnson Date: Tue, 9 Aug 2022 17:54:54 -0700 Subject: [PATCH 10/15] wip --- python_bindings/test/apps/CMakeLists.txt | 1 - python_bindings/test/correctness/CMakeLists.txt | 1 - python_bindings/tutorial/CMakeLists.txt | 1 - src/autoschedulers/li2018/CMakeLists.txt | 2 -- 4 files changed, 5 deletions(-) diff --git a/python_bindings/test/apps/CMakeLists.txt b/python_bindings/test/apps/CMakeLists.txt index 33a65cd427bd..591cad02af44 100644 --- a/python_bindings/test/apps/CMakeLists.txt +++ b/python_bindings/test/apps/CMakeLists.txt @@ -8,7 +8,6 @@ set(scripts set(PYTHONPATH "$" ) -list(REMOVE_DUPLICATES PYTHONPATH) list(TRANSFORM PYTHONPATH PREPEND "PYTHONPATH=path_list_prepend:") set(TEST_ENV diff --git a/python_bindings/test/correctness/CMakeLists.txt b/python_bindings/test/correctness/CMakeLists.txt index 91e7347a6798..57a5d8e5e201 100644 --- a/python_bindings/test/correctness/CMakeLists.txt +++ b/python_bindings/test/correctness/CMakeLists.txt @@ -33,7 +33,6 @@ set( "$" "$" ) -list(REMOVE_DUPLICATES PYTHONPATH) list(TRANSFORM PYTHONPATH PREPEND "PYTHONPATH=path_list_prepend:") foreach (test IN LISTS tests) diff --git a/python_bindings/tutorial/CMakeLists.txt b/python_bindings/tutorial/CMakeLists.txt index c5a28409b4ad..83b7688c8859 100644 --- a/python_bindings/tutorial/CMakeLists.txt +++ b/python_bindings/tutorial/CMakeLists.txt @@ -21,7 +21,6 @@ set( "$" "$" ) -list(REMOVE_DUPLICATES PYTHONPATH) list(TRANSFORM PYTHONPATH PREPEND "PYTHONPATH=path_list_prepend:") foreach (test IN LISTS tests) diff --git a/src/autoschedulers/li2018/CMakeLists.txt b/src/autoschedulers/li2018/CMakeLists.txt index d30a70ee2eaa..56433561c977 100644 --- a/src/autoschedulers/li2018/CMakeLists.txt +++ b/src/autoschedulers/li2018/CMakeLists.txt @@ -49,7 +49,6 @@ if (WITH_PYTHON_BINDINGS) PYTHONPATH "$" ) - list(REMOVE_DUPLICATES PYTHONPATH) list(TRANSFORM PYTHONPATH PREPEND "PYTHONPATH=path_list_prepend:") set( @@ -57,7 +56,6 @@ if (WITH_PYTHON_BINDINGS) "$" "$" ) - list(REMOVE_DUPLICATES PATH) list(TRANSFORM PATH PREPEND "PATH=path_list_prepend:") set_tests_properties(gradient_autoscheduler_test_py PROPERTIES From f9e7932165d6eeb630f894bfcf3c296172615f42 Mon Sep 17 00:00:00 2001 From: Alex Reinking Date: Wed, 10 Aug 2022 18:14:01 +0000 Subject: [PATCH 11/15] Drop the "one level up" dummy target --- README_cmake.md | 16 +++++++-------- python_bindings/src/halide/CMakeLists.txt | 20 +------------------ python_bindings/test/apps/CMakeLists.txt | 2 +- .../test/correctness/CMakeLists.txt | 2 +- python_bindings/tutorial/CMakeLists.txt | 2 +- src/autoschedulers/li2018/CMakeLists.txt | 2 +- 6 files changed, 13 insertions(+), 31 deletions(-) diff --git a/README_cmake.md b/README_cmake.md index 0761030214cc..1fd65e710622 100644 --- a/README_cmake.md +++ b/README_cmake.md @@ -370,7 +370,7 @@ Halide's own CI infrastructure, or as escape hatches for third-party packagers. |-----------------------------|--------------------------------------------------------------------|------------------------------------------------------------------------------------------| | `Halide_CLANG_TIDY_BUILD` | `OFF` | Used internally to generate fake compile jobs for runtime files when running clang-tidy. | | `Halide_CCACHE_BUILD` | `OFF` | Use ccache with Halide-recommended settings to accelerate rebuilds. | -| `Halide_CCACHE_PARAMS` | `CCACHE_CPP2=yes CCACHE_HASHDIR=yes CCACHE_SLOPPINESS=pch_defines` | Options to pass to `ccache` when using `Halide_CCACHE_BUILD`. | +| `Halide_CCACHE_PARAMS` | `CCACHE_CPP2=yes CCACHE_HASHDIR=yes CCACHE_SLOPPINESS=pch_defines` | Options to pass to `ccache` when using `Halide_CCACHE_BUILD`. | | `Halide_SOVERSION_OVERRIDE` | `${Halide_VERSION_MAJOR}` | Override the SOVERSION for libHalide. Expects a positive integer (i.e. not a version). | The following options are only available when building Halide directly, ie. not @@ -779,12 +779,12 @@ Halide defines the following targets that are available to users: The following targets are not guaranteed to be available: -| Imported target | Description | -|-------------------------|------------------------------------------------------------------------------------------------------------------------------------------| -| `Halide::Python` | this is a Python 3 package that can be referenced as `$` when setting up PYTHONPATH for Python tests or the like from CMake. | -| `Halide::Adams19` | the Adams et.al. 2019 autoscheduler (no GPU support) | -| `Halide::Li18` | the Li et.al. 2018 gradient autoscheduler (limited GPU support) | -| `Halide::Mullapudi2016` | the Mullapudi et.al. 2016 autoscheduler (no GPU support) | +| Imported target | Description | +|-------------------------|-------------------------------------------------------------------------------------------------------------------------------------------------------------------| +| `Halide::Python` | this is a Python 3 package that can be referenced as `$/..` when setting up `PYTHONPATH` for Python tests or the like from CMake. | +| `Halide::Adams19` | the Adams et.al. 2019 autoscheduler (no GPU support) | +| `Halide::Li18` | the Li et.al. 2018 gradient autoscheduler (limited GPU support) | +| `Halide::Mullapudi2016` | the Mullapudi et.al. 2016 autoscheduler (no GPU support) | ### Functions @@ -972,7 +972,7 @@ would call `add_halide_library` with no `TARGETS` option and set `FROM` equal to the name of the imported generator executable. Obviously, this is a significant increase in complexity over a typical CMake project. -This is very compatible with the `add_halide_generator` strategy above. +This is very compatible with the `add_halide_generator` strategy above. ### Use `ExternalProject` directly diff --git a/python_bindings/src/halide/CMakeLists.txt b/python_bindings/src/halide/CMakeLists.txt index fcba7b685500..f837a33c534e 100644 --- a/python_bindings/src/halide/CMakeLists.txt +++ b/python_bindings/src/halide/CMakeLists.txt @@ -41,6 +41,7 @@ set(python_sources # just for this project, rather than globally, and they should ensure that the last path component # is `halide`. Otherwise, the tests will break. pybind11_add_module(Halide_Python MODULE ${native_sources}) +add_library(Halide::Python ALIAS Halide_Python) set_target_properties( Halide_Python PROPERTIES @@ -86,25 +87,6 @@ endforeach () add_custom_target(Halide_Python_sources ALL DEPENDS ${build_tree_pys}) add_dependencies(Halide_Python Halide_Python_sources) -# Now add a 'target' that exists just to conveniently give us a way to find this directory; -# we can't use $ because we need `import halide` to see -# $, not $/halide. This is surprisingly complex, in part because -# $ is actually evaluated at build time (not config time), so we must -# only use it in expressions that are deferred until build time... -set(fake_cpp_file "${CMAKE_CURRENT_BINARY_DIR}/.fake_cpp_file.cpp") -file(WRITE "${fake_cpp_file}" "int halide_fake_unused_;\n") - -add_library(Halide_Python_Fake_Lib SHARED "${fake_cpp_file}") -# Note that LIBRARY_OUTPUT_DIRECTORY is only used for SHARED, not STATIC, -# and that the shared library won't get created on Windows if no symbols are exported. -set_target_properties(Halide_Python_Fake_Lib - PROPERTIES - # Must set both of these for DLL systems to get it right - LIBRARY_OUTPUT_DIRECTORY "$" - RUNTIME_OUTPUT_DIRECTORY "$" - WINDOWS_EXPORT_ALL_SYMBOLS TRUE) -add_library(Halide::Python ALIAS Halide_Python_Fake_Lib) - ## # Packaging ## diff --git a/python_bindings/test/apps/CMakeLists.txt b/python_bindings/test/apps/CMakeLists.txt index 591cad02af44..a845d748617e 100644 --- a/python_bindings/test/apps/CMakeLists.txt +++ b/python_bindings/test/apps/CMakeLists.txt @@ -6,7 +6,7 @@ set(scripts local_laplacian.py) set(PYTHONPATH - "$" + "$/.." ) list(TRANSFORM PYTHONPATH PREPEND "PYTHONPATH=path_list_prepend:") diff --git a/python_bindings/test/correctness/CMakeLists.txt b/python_bindings/test/correctness/CMakeLists.txt index 57a5d8e5e201..90a350b4d7d2 100644 --- a/python_bindings/test/correctness/CMakeLists.txt +++ b/python_bindings/test/correctness/CMakeLists.txt @@ -29,7 +29,7 @@ set(tests # Use generator expressions to get the true output paths of these files. set( PYTHONPATH - "$" + "$/.." "$" "$" ) diff --git a/python_bindings/tutorial/CMakeLists.txt b/python_bindings/tutorial/CMakeLists.txt index 83b7688c8859..c8c72e5703b4 100644 --- a/python_bindings/tutorial/CMakeLists.txt +++ b/python_bindings/tutorial/CMakeLists.txt @@ -18,7 +18,7 @@ set(tests set( PYTHONPATH - "$" + "$/.." "$" ) list(TRANSFORM PYTHONPATH PREPEND "PYTHONPATH=path_list_prepend:") diff --git a/src/autoschedulers/li2018/CMakeLists.txt b/src/autoschedulers/li2018/CMakeLists.txt index 56433561c977..f1d5b3c6f90a 100644 --- a/src/autoschedulers/li2018/CMakeLists.txt +++ b/src/autoschedulers/li2018/CMakeLists.txt @@ -47,7 +47,7 @@ if (WITH_PYTHON_BINDINGS) set( PYTHONPATH - "$" + "$/.." ) list(TRANSFORM PYTHONPATH PREPEND "PYTHONPATH=path_list_prepend:") From 0954bcf1ed335d3b84154ed909ce0c7f57c2743d Mon Sep 17 00:00:00 2001 From: Alex Reinking Date: Wed, 10 Aug 2022 18:16:12 +0000 Subject: [PATCH 12/15] Simplify __init__.py now that it's copied to build tree Fixes #6870 --- python_bindings/src/halide/__init__.py | 26 ++------------------------ 1 file changed, 2 insertions(+), 24 deletions(-) diff --git a/python_bindings/src/halide/__init__.py b/python_bindings/src/halide/__init__.py index 88f00caab722..20c9f793e327 100644 --- a/python_bindings/src/halide/__init__.py +++ b/python_bindings/src/halide/__init__.py @@ -1,24 +1,2 @@ -# TODO(#6870): The following three lines are a stop-gap. This file should just -# contain the last two imports, at least until the pure-Python part of the -# library grows. -# -# There are three main reasons why this exists: -# -# 1. relative imports cannot be overloaded with sys.path -# 2. for a variety of reasons, copying the python sources next to the -# halide_.*.so module is difficult to make work in 100% of cases in CMake -# 3. even if we could copy them reliably, copying these files into the build -# folder seems inelegant -# -# Fortunately, there are apparently other hooks besides sys.path that we could -# use to redirect a failing relative import. -# -# https://docs.python.org/3/reference/import.html#finders-and-loaders -# https://github.com/halide/Halide/issues/6870 - -import sys -from pathlib import Path -sys.path.append(str(Path(__file__).parent.resolve(strict=True))) - -from halide_ import * -from halide_ import _, _1, _2, _3, _4, _5, _6, _7, _8, _9 +from .halide_ import * +from .halide_ import _, _1, _2, _3, _4, _5, _6, _7, _8, _9 From 06939eaab3324880f5140bb75c50bbbe197bd78b Mon Sep 17 00:00:00 2001 From: Alex Reinking Date: Wed, 10 Aug 2022 18:39:14 +0000 Subject: [PATCH 13/15] Add helper to de-duplicate PYTHONPATH test logic --- python_bindings/CMakeLists.txt | 29 +++++++++++++++++++ python_bindings/test/apps/CMakeLists.txt | 29 +++++-------------- .../test/correctness/CMakeLists.txt | 21 +++----------- python_bindings/tutorial/CMakeLists.txt | 20 +++---------- 4 files changed, 45 insertions(+), 54 deletions(-) diff --git a/python_bindings/CMakeLists.txt b/python_bindings/CMakeLists.txt index 66214e97c862..6824f037134a 100644 --- a/python_bindings/CMakeLists.txt +++ b/python_bindings/CMakeLists.txt @@ -49,6 +49,35 @@ if (NOT Halide_ENABLE_RTTI OR NOT Halide_ENABLE_EXCEPTIONS) message(FATAL_ERROR "Python bindings require RTTI and exceptions to be enabled.") endif () +## +# A helper for creating tests with correct PYTHONPATH +## + +function(add_python_test) + cmake_parse_arguments(ARG "" "FILE;LABEL" "PYTHONPATH;ENVIRONMENT" ${ARGN}) + + set(ARG_PYTHONPATH "$/.." ${ARG_PYTHONPATH}) + list(TRANSFORM ARG_PYTHONPATH PREPEND "PYTHONPATH=path_list_prepend:") + + list(PREPEND ARG_ENVIRONMENT "HL_TARGET=${Halide_TARGET}") + + cmake_path(GET ARG_FILE STEM test_name) + set(test_name "${ARG_LABEL}_${test_name}") + + add_test( + NAME "${test_name}" + COMMAND Python3::Interpreter "$" + ) + set_tests_properties( + "${test_name}" + PROPERTIES + LABELS "python;${ARG_LABEL}" + ENVIRONMENT "${ARG_ENVIRONMENT}" + ENVIRONMENT_MODIFICATION "${ARG_PYTHONPATH}" + ) +endfunction() + + ## # Add our sources to this sub-tree. ## diff --git a/python_bindings/test/apps/CMakeLists.txt b/python_bindings/test/apps/CMakeLists.txt index a845d748617e..70daeb483fd6 100644 --- a/python_bindings/test/apps/CMakeLists.txt +++ b/python_bindings/test/apps/CMakeLists.txt @@ -1,29 +1,16 @@ -set(scripts +set(tests bilateral_grid.py blur.py erode.py interpolate.py local_laplacian.py) -set(PYTHONPATH - "$/.." -) -list(TRANSFORM PYTHONPATH PREPEND "PYTHONPATH=path_list_prepend:") - -set(TEST_ENV - "HL_TARGET=${Halide_TARGET}" - "TEST_TMPDIR=$" - "TEST_IMAGES_DIR=$" -) - -foreach (script IN LISTS scripts) - cmake_path(GET script STEM base) - add_test(NAME python_apps_${base} - COMMAND Python3::Interpreter "$") - set_tests_properties( - python_apps_${base} PROPERTIES - LABELS python - ENVIRONMENT "${TEST_ENV}" - ENVIRONMENT_MODIFICATION "${PYTHONPATH}" +foreach (test IN LISTS tests) + add_python_test( + FILE "${test}" + LABEL python_apps + ENVIRONMENT + "TEST_TMPDIR=$" + "TEST_IMAGES_DIR=$" ) endforeach () diff --git a/python_bindings/test/correctness/CMakeLists.txt b/python_bindings/test/correctness/CMakeLists.txt index 90a350b4d7d2..7180fa6d0732 100644 --- a/python_bindings/test/correctness/CMakeLists.txt +++ b/python_bindings/test/correctness/CMakeLists.txt @@ -26,23 +26,10 @@ set(tests var.py ) -# Use generator expressions to get the true output paths of these files. -set( - PYTHONPATH - "$/.." - "$" - "$" -) -list(TRANSFORM PYTHONPATH PREPEND "PYTHONPATH=path_list_prepend:") - foreach (test IN LISTS tests) - cmake_path(GET test STEM test_name) - add_test(NAME python_correctness_${test_name} - COMMAND Python3::Interpreter "$") - set_tests_properties( - python_correctness_${test_name} PROPERTIES - LABELS "python" - ENVIRONMENT "HL_TARGET=${Halide_TARGET}" - ENVIRONMENT_MODIFICATION "${PYTHONPATH}" + add_python_test( + FILE "${test}" + LABEL python_correctness + PYTHONPATH "$" "$" ) endforeach () diff --git a/python_bindings/tutorial/CMakeLists.txt b/python_bindings/tutorial/CMakeLists.txt index c8c72e5703b4..da1a55fd2a86 100644 --- a/python_bindings/tutorial/CMakeLists.txt +++ b/python_bindings/tutorial/CMakeLists.txt @@ -16,23 +16,11 @@ set(tests lesson_14_types.py ) -set( - PYTHONPATH - "$/.." - "$" -) -list(TRANSFORM PYTHONPATH PREPEND "PYTHONPATH=path_list_prepend:") - foreach (test IN LISTS tests) - cmake_path(GET test STEM test_name) - add_test(NAME python_tutorial_${test_name} - COMMAND Python3::Interpreter "$") - - set_tests_properties( - python_tutorial_${test_name} PROPERTIES - LABELS python - ENVIRONMENT "HL_TARGET=${Halide_TARGET}" - ENVIRONMENT_MODIFICATION "${PYTHONPATH}" + add_python_test( + FILE "${test}" + LABEL python_tutorial + PYTHONPATH "$" ) endforeach () From 1d2b23be4837c8827b3e835daf36008c1cc69699 Mon Sep 17 00:00:00 2001 From: Alex Reinking Date: Wed, 10 Aug 2022 18:43:01 +0000 Subject: [PATCH 14/15] Remove CTest label to avoid duplicate test runs --- python_bindings/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python_bindings/CMakeLists.txt b/python_bindings/CMakeLists.txt index 6824f037134a..c3430b25ffb2 100644 --- a/python_bindings/CMakeLists.txt +++ b/python_bindings/CMakeLists.txt @@ -71,7 +71,7 @@ function(add_python_test) set_tests_properties( "${test_name}" PROPERTIES - LABELS "python;${ARG_LABEL}" + LABELS "python" ENVIRONMENT "${ARG_ENVIRONMENT}" ENVIRONMENT_MODIFICATION "${ARG_PYTHONPATH}" ) From 593d264094786c63f4f0bf74e9148731617824e8 Mon Sep 17 00:00:00 2001 From: Alex Reinking Date: Wed, 10 Aug 2022 18:43:58 +0000 Subject: [PATCH 15/15] Use list(PREPEND) consistently --- python_bindings/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python_bindings/CMakeLists.txt b/python_bindings/CMakeLists.txt index c3430b25ffb2..119ae9bbbea3 100644 --- a/python_bindings/CMakeLists.txt +++ b/python_bindings/CMakeLists.txt @@ -56,7 +56,7 @@ endif () function(add_python_test) cmake_parse_arguments(ARG "" "FILE;LABEL" "PYTHONPATH;ENVIRONMENT" ${ARGN}) - set(ARG_PYTHONPATH "$/.." ${ARG_PYTHONPATH}) + list(PREPEND ARG_PYTHONPATH "$/..") list(TRANSFORM ARG_PYTHONPATH PREPEND "PYTHONPATH=path_list_prepend:") list(PREPEND ARG_ENVIRONMENT "HL_TARGET=${Halide_TARGET}")