halide · alexreinking · Aug 10, 2022 · Aug 4, 2022 · Aug 8, 2022 · Aug 8, 2022
diff --git 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 module that can be referenced as `$<TARGET_FILE:Halide::Python>` when setting up 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 `$<TARGET_FILE_DIR:Halide::Python>/..` 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/CMakeLists.txt b/python_bindings/CMakeLists.txt
@@ -49,11 +49,40 @@ 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})
+
+    list(PREPEND ARG_PYTHONPATH "$<TARGET_FILE_DIR:Halide::Python>/..")
+    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 "$<SHELL_PATH:${CMAKE_CURRENT_SOURCE_DIR}/${ARG_FILE}>"
+    )
+    set_tests_properties(
+        "${test_name}"
+        PROPERTIES
+        LABELS "python"
+        ENVIRONMENT "${ARG_ENVIRONMENT}"
+        ENVIRONMENT_MODIFICATION "${ARG_PYTHONPATH}"
+    )
+endfunction()
+
+
 ##
 # 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
@@ -0,0 +1 @@
+add_subdirectory(halide)
diff --git 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,26 +29,63 @@ 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
+    )
+
+# It is technically still possible for a user to override the LIBRARY_OUTPUT_DIRECTORY by setting
+# CMAKE_LIBRARY_OUTPUT_DIRECTORY_<CONFIG>, 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})
 add_library(Halide::Python ALIAS Halide_Python)
-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 "$<CONFIG>/halide"
+    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 $<TARGET_FILE:Halide::Halide> $<TARGET_FILE_DIR:Halide::Python>
-                       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 $<TARGET_RUNTIME_DLLS:...> 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}" $<TARGET_RUNTIME_DLLS:Halide_Python> $<TARGET_FILE_DIR:Halide_Python>
+    COMMAND_EXPAND_LISTS
+    VERBATIM
+)
+
+# 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
+    #   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}" "$<TARGET_FILE_DIR:Halide_Python>/${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
@@ -60,18 +97,18 @@ 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}"
-        DESTINATION "${Halide_INSTALL_PYTHONDIR}"
+install(DIRECTORY "$<TARGET_FILE_DIR:Halide_Python>/"
+        DESTINATION "${Halide_INSTALL_PYTHONDIR}/halide"
         COMPONENT Halide_Python
         FILES_MATCHING
         PATTERN "*.py"
         PATTERN "*/halide_" EXCLUDE
+        PATTERN "*/CMakeFiles" EXCLUDE
         PATTERN "*/__pycache__" EXCLUDE)
 
 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)
@@ -120,8 +157,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 ()
diff --git 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
diff --git 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
-    "$<TARGET_FILE_DIR:Halide::Python>"
-    "${Halide_SOURCE_DIR}/python_bindings/src")
-list(TRANSFORM PYTHONPATH PREPEND "PYTHONPATH=path_list_prepend:")
-
-set(TEST_ENV
-    "HL_TARGET=${Halide_TARGET}"
-    "TEST_TMPDIR=$<SHELL_PATH:${CMAKE_CURRENT_BINARY_DIR}>"
-    "TEST_IMAGES_DIR=$<SHELL_PATH:${CMAKE_CURRENT_SOURCE_DIR}/../../../apps/images>"
-)
-
-foreach (script IN LISTS scripts)
-    cmake_path(GET script STEM base)
-    add_test(NAME python_apps_${base}
-             COMMAND Python3::Interpreter "$<SHELL_PATH:${CMAKE_CURRENT_SOURCE_DIR}/${script}>")
-    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=$<SHELL_PATH:${CMAKE_CURRENT_BINARY_DIR}>"
+        "TEST_IMAGES_DIR=$<SHELL_PATH:${CMAKE_CURRENT_SOURCE_DIR}/../../../apps/images>"
     )
 endforeach ()
diff --git a/python_bindings/test/correctness/CMakeLists.txt b/python_bindings/test/correctness/CMakeLists.txt
@@ -26,24 +26,10 @@ set(tests
     var.py
     )
 
-# Use generator expressions to get the true output paths of these files.
-set(
-    PYTHONPATH
-    "$<TARGET_FILE_DIR:py_aot_bit>"
-    "$<TARGET_FILE_DIR:py_stub_bit>"
-    "$<TARGET_FILE_DIR:Halide::Python>"
-    "${Halide_SOURCE_DIR}/python_bindings/src"
-)
-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 "$<SHELL_PATH:${CMAKE_CURRENT_SOURCE_DIR}/${test}>")
-    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 "$<TARGET_FILE_DIR:py_aot_bit>" "$<TARGET_FILE_DIR:py_stub_bit>"
     )
 endforeach ()
diff --git a/python_bindings/tutorial/CMakeLists.txt b/python_bindings/tutorial/CMakeLists.txt
@@ -16,24 +16,11 @@ set(tests
     lesson_14_types.py
     )
 
-set(
-    PYTHONPATH
-    "$<TARGET_FILE_DIR:lesson_10_halide>"
-    "$<TARGET_FILE_DIR:Halide::Python>"
-    "${Halide_SOURCE_DIR}/python_bindings/src"
-)
-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 "$<SHELL_PATH:${CMAKE_CURRENT_SOURCE_DIR}/${test}>")
-
-    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 "$<TARGET_FILE_DIR:lesson_10_halide>"
     )
 endforeach ()
 

diff --git a/src/autoschedulers/li2018/CMakeLists.txt b/src/autoschedulers/li2018/CMakeLists.txt
@@ -47,8 +47,7 @@ if (WITH_PYTHON_BINDINGS)
 
         set(
             PYTHONPATH
-            "$<TARGET_FILE_DIR:Halide::Python>"
-            "${Halide_SOURCE_DIR}/python_bindings/src"
+            "$<TARGET_FILE_DIR:Halide::Python>/.."
         )
         list(TRANSFORM PYTHONPATH PREPEND "PYTHONPATH=path_list_prepend:")