[libshortfin] Python build ergonomics. (#177)

* Move bindings/python -> python
* Rework setup.py so that it can be the main driver for Python based
dev:
* Add env vars SHORTFIN_IREE_SOURCE_DIR, SHORTFIN_ENABLE_ASAN,
SHORTFIN_DEV_MODE, SHORTFIN_RUN_CTESTS
  * Update README with python based dev flow commands
  * Change build dir to build/cmake (from build/b)
* Removes a redundant build of IREE in the main CI (was not doing
anything in bundled mode).

With this, I can change my dev process to be entirely based on `pip
install` with a fully editable build and the ability to manipulate cmake
post build. This better aligns with how users will consume it.

Author: stellaraccident

.github/workflows/ci_linux_x64-libshortfin.yml

@@ -57,25 +57,6 @@ jobs:
         git submodule update --init --depth 1 -- third_party/googletest
         git submodule update --init --depth 1 -- third_party/hip-build-deps/
 
-    - name: Build IREE runtime
-      run: |
-        mkdir ${{ env.IREE_REPO_DIR }}/build
-        cd ${{ env.IREE_REPO_DIR }}/build
-        cmake -GNinja \
-          -DCMAKE_C_COMPILER=clang-18 \
-          -DCMAKE_CXX_COMPILER=clang++-18 \
-          -DIREE_ENABLE_LLD=ON \
-          -DIREE_ERROR_ON_MISSING_SUBMODULES=OFF \
-          -DIREE_HAL_DRIVER_DEFAULTS=OFF \
-          -DIREE_HAL_DRIVER_LOCAL_SYNC=ON \
-          -DIREE_HAL_DRIVER_LOCAL_TASK=ON \
-          -DIREE_HAL_DRIVER_HIP=ON \
-          -DIREE_BUILD_COMPILER=OFF \
-          -DIREE_BUILD_SAMPLES=OFF \
-          -DIREE_BUILD_TESTS=OFF \
-          ..
-        cmake --build . --target all
-
     - name: Setup Python
       uses: actions/setup-python@39cd14951b08e74b54015e9e001cdefcf80e669f # v5.1.1
       with:
@@ -97,7 +78,7 @@ jobs:
           -DCMAKE_CXX_COMPILER=clang++-18 \
           -DCMAKE_LINKER_TYPE=LLD \
           -DSHORTFIN_BUNDLE_DEPS=ON \
-          -DCMAKE_PREFIX_PATH=${{ env.IREE_REPO_DIR }}/build/lib/cmake/IREE \
+          -DSHORTFIN_IREE_SOURCE_DIR=${{ env.IREE_REPO_DIR }} \
           -DSHORTFIN_BUILD_PYTHON_BINDINGS=ON \
           ..
         cmake --build . --target all
@@ -120,7 +101,7 @@ jobs:
           -DCMAKE_C_COMPILER=clang-18 \
           -DCMAKE_CXX_COMPILER=clang++-18 \
           -DCMAKE_LINKER_TYPE=LLD \
-          -DCMAKE_PREFIX_PATH=${{ env.IREE_REPO_DIR }}/build/lib/cmake/IREE \
+          -DSHORTFIN_IREE_SOURCE_DIR=${{ env.IREE_REPO_DIR }} \
           -DSHORTFIN_BUILD_PYTHON_BINDINGS=ON \
           -DSHORTFIN_HAVE_AMDGPU=OFF \
           -DSHORTFIN_BUILD_STATIC=ON \

.github/workflows/ci_linux_x64_asan-libshortfin.yml

@@ -147,27 +147,13 @@ jobs:
     - name: Build libshortfin
       run: |
         eval "$(pyenv init -)"
-        mkdir ${{ env.LIBSHORTFIN_DIR }}/build
-        cd ${{ env.LIBSHORTFIN_DIR }}/build
-        cmake -GNinja \
-          -DCMAKE_BUILD_TYPE=Debug \
-          -DCMAKE_C_COMPILER=clang-18 \
-          -DCMAKE_CXX_COMPILER=clang++-18 \
-          -DCMAKE_LINKER_TYPE=LLD \
-          -DSHORTFIN_BUNDLE_DEPS=ON \
-          -DSHORTFIN_IREE_SOURCE_DIR=${{ env.IREE_SOURCE_DIR }} \
-          -DSHORTFIN_BUILD_PYTHON_BINDINGS=ON \
-          -DSHORTFIN_ENABLE_ASAN=ON \
-          ..
-        cmake --build . --target all
+        cd ${{ env.LIBSHORTFIN_DIR }}
+        SHORTFIN_IREE_SOURCE_DIR="${{ env.IREE_SOURCE_DIR }}" \
+        SHORTFIN_ENABLE_ASAN=ON \
+        SHORTFIN_DEV_MODE=ON \
+        SHORTFIN_RUN_CTESTS=ON \
         pip install -v -e .
 
-    - name: Run ctest
-      if: ${{ !cancelled() }}
-      run: |
-        cd ${{ env.LIBSHORTFIN_DIR }}/build
-        ctest --timeout 30 --output-on-failure
-
     - name: Run pytest
       if: ${{ !cancelled() }}
       env:

libshortfin/CMakeLists.txt

@@ -4,7 +4,7 @@
 # https://llvm.org/LICENSE.txt for license information. SPDX-License-Identifier:
 # Apache-2.0 WITH LLVM-exception
 
-cmake_minimum_required(VERSION 3.28)
+cmake_minimum_required(VERSION 3.29)
 
 if(CMAKE_SOURCE_DIR STREQUAL CMAKE_BINARY_DIR)
   message(
@@ -138,7 +138,7 @@ if (NOT SHORTFIN_IREE_SOURCE_DIR AND SHORTFIN_BUNDLE_DEPS)
   )
   FetchContent_GetProperties(iree)
   if(NOT iree_POPULATED)
-    FetchContent_Populate(iree)
+    FetchContent_MakeAvailable(iree)
   endif()
   set(SHORTFIN_IREE_SOURCE_DIR ${iree_SOURCE_DIR})
 endif()
@@ -184,7 +184,7 @@ add_subdirectory(src)
 
 if(SHORTFIN_BUILD_PYTHON_BINDINGS)
   find_package(Python 3.8 COMPONENTS Interpreter Development.Module REQUIRED)
-  add_subdirectory(bindings/python)
+  add_subdirectory(python)
   set(SHORTFIN_PYTHON_CPP_PREBUILT "TRUE") # See setup.py.
   configure_file(setup.py setup.py @ONLY)
   configure_file(pyproject.toml pyproject.toml COPYONLY)

libshortfin/README.md

@@ -1,43 +1,19 @@
 # libshortfin - SHARK C++ inference library
 
-## Dev Builds
-
-Library dependencies:
-
-* [spdlog](https://github.com/gabime/spdlog)
-* [xtensor](https://github.com/xtensor-stack/xtensor)
-* [iree runtime](https://github.com/iree-org/iree)
-
-On recent Ubuntu, the primary dependencies can be satisfied via:
-
-```
-apt install libspdlog-dev libxtensor-dev
-```
-
-CMake must be told how to find the IREE runtime, either from a distribution
-tarball, or local build/install dir. For a local build directory, pass:
-
-```
-# Assumes that the iree-build directory is adjacent to this repo.
--DCMAKE_PREFIX_PATH=$(pwd)/../../iree-build/lib/cmake/IREE
-```
-
-One liner recommended CMake command (note that CMAKE_LINKER_TYPE requires
-cmake>=3.29):
+## Native Dev Builds
 
 ```
 cmake -GNinja -S. -Bbuild \
     -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++ \
-    -DCMAKE_LINKER_TYPE=LLD \
-    -DCMAKE_PREFIX_PATH=$(pwd)/../../iree-build/lib/cmake/IREE
+    -DCMAKE_LINKER_TYPE=LLD
 ```
 
-## Building Python Bindings
+## Python Dev Builds
 
 If using a Python based development flow, there are two options:
 
-1. `pip install -v .` to build and install the library (TODO: Not yet implemented).
-2. Build with cmake and `-DSHORTFIN_BUILD_PYTHON_BINDINGS=ON` and then
+1. `pip install -e` based.
+2. Build with cmake as above and `-DSHORTFIN_BUILD_PYTHON_BINDINGS=ON` and then
    from the `build/` directory, run `pip install -v -e .` to create an
    editable install that will update as you build the C++ project.
 
@@ -46,6 +22,34 @@ recommended. Your python install should track any source file changes or
 builds without further interaction. Re-installing will be necessary if package
 structure changes significantly.
 
+For pure Python based dev, everything can be done from pip:
+
+```
+# Install build system pre-reqs (since we are building in dev mode, this
+# is not done for us). See source of truth in pyproject.toml:
+pip install setuptools wheel
+
+# Optionally install cmake and ninja if you don't have them or need a newer
+# version. If doing heavy development in Python, it is strongly recommended
+# to install these natively on your system as it will make it easier to
+# switch Python interpreters and build options (and the launcher in debug/asan
+# builds of Python is much slower). Note CMakeLists.txt for minimum CMake
+# version, which is usually quite recent.
+pip install cmake ninja
+
+# Optional env vars:
+#   SHORTFIN_IREE_SOURCE_DIR=$(pwd)/../../iree
+# Note that the `--no-build-isolation` flag is useful in development setups
+# because it does not create an intermediate venv that will keep later
+# invocations of cmake/ninja from working at the command line. If just doing
+# a one-shot build, it can be ommitted.
+SHORTFIN_DEV_MODE=ON pip install --no-build-isolation -v -e .
+
+Once built the first time, `cmake`, `ninja`, and `ctest` commands can be run
+directly from `build/cmake` and changes will apply directly to the next
+process launch.
+```
+
 ## Running Tests
 
 The project uses a combination of ctest for native C++ tests and pytest. Much

libshortfin/bindings/python/CMakeLists.txt libshortfin/python/CMakeLists.txt

@@ -3,3 +3,7 @@ requests
 fastapi
 onnx
 uvicorn
+
+# Libraries needed to build in dev setups.
+setuptools
+wheel

libshortfin/bindings/python/_shortfin/__init__.py libshortfin/python/_shortfin/__init__.py

@@ -15,6 +15,35 @@
 from setuptools.command.build_py import build_py as _build_py
 
 
+def get_env_boolean(name: str, default_value: bool = False) -> bool:
+    svalue = os.getenv(name)
+    if svalue is None:
+        return default_value
+    svalue = svalue.upper()
+    if svalue in ["1", "ON", "TRUE"]:
+        return True
+    elif svalue in ["0", "OFF", "FALSE"]:
+        return False
+    else:
+        print(f"WARNING: {name} env var cannot be interpreted as a boolean value")
+        return default_value
+
+
+def get_env_cmake_option(name: str, default_value: bool = False) -> str:
+    svalue = os.getenv(name)
+    if not svalue:
+        svalue = "ON" if default_value else "OFF"
+    return f"-D{name}={svalue}"
+
+
+def add_env_cmake_setting(args, env_name: str, cmake_name=None) -> str:
+    svalue = os.getenv(env_name)
+    if svalue is not None:
+        if not cmake_name:
+            cmake_name = env_name
+        args.append(f"-D{cmake_name}={svalue}")
+
+
 # This file can be generated into the build directory to allow an arbitrary
 # CMake built version of the project to be installed into a venv for development.
 # This can be detected if the CPP_PREBUILT global contains the string
@@ -30,23 +59,33 @@ def is_cpp_prebuilt():
     return CPP_PREBUILT == "TRUE"
 
 
+DEV_MODE = False
+
 if is_cpp_prebuilt():
-    print("setup.py running in pre-built mode:", file=sys.stderr)
+    print("setup.py running in pre-built mode:")
     SOURCE_DIR = Path(CPP_PREBUILT_SOURCE_DIR)
     BINARY_DIR = Path(CPP_PREBUILT_BINARY_DIR)
+    CMAKE_BUILD_DIR = BINARY_DIR
 else:
-    print("setup.py running in cmake build mode:", file=sys.stderr)
+    print("setup.py running in cmake build mode:")
     # setup.py is in the source directory.
     SOURCE_DIR = Path(SETUPPY_DIR)
-    BINARY_DIR = Path(os.path.join(SETUPPY_DIR, "build", "b"))
+    BINARY_DIR = Path(os.path.join(SETUPPY_DIR, "build"))
+    # TODO: Should build default and tracing version to different dirs.
+    CMAKE_BUILD_DIR = BINARY_DIR / "cmake"
+    DEV_MODE = get_env_boolean("SHORTFIN_DEV_MODE")
 
-print(f"  SOURCE_DIR = {SOURCE_DIR}", file=sys.stderr)
-print(f"  BINARY_DIR = {BINARY_DIR}", file=sys.stderr)
+print(f"  SOURCE_DIR = {SOURCE_DIR}")
+print(f"  BINARY_DIR = {BINARY_DIR}")
+
+if DEV_MODE:
+    print(f"  DEV MODE ENABLED: Building debug with clang/lld and other dev settings")
 
 # Due to a quirk of setuptools, that package_dir map must only contain
 # paths relative to the directory containing setup.py. Why? No one knows.
 REL_SOURCE_DIR = SOURCE_DIR.relative_to(SETUPPY_DIR, walk_up=True)
 REL_BINARY_DIR = BINARY_DIR.relative_to(SETUPPY_DIR, walk_up=True)
+REL_CMAKE_BUILD_DIR = CMAKE_BUILD_DIR.relative_to(SETUPPY_DIR, walk_up=True)
 
 
 class CMakeExtension(Extension):
@@ -98,7 +137,7 @@ def maybe_nuke_cmake_cache(cmake_build_dir):
     # Mismatch or not found. Clean it.
     cmake_cache_file = os.path.join(cmake_build_dir, "CMakeCache.txt")
     if os.path.exists(cmake_cache_file):
-        print("Removing CMakeCache.txt because Python version changed", file=sys.stderr)
+        print("Removing CMakeCache.txt because Python version changed")
         os.remove(cmake_cache_file)
 
     # And write.
@@ -115,67 +154,63 @@ def run(self):
         if not is_cpp_prebuilt():
 
             # Build extension using cmake.
-            print("*****************************", file=sys.stderr)
-            print("* Building libshortfin      *", file=sys.stderr)
-            print("*****************************", file=sys.stderr)
-
-            cfg = os.getenv("SHORTFIN_CMAKE_BUILD_TYPE", "Release")
-
-            CMAKE_BUILD_DIR = BINARY_DIR
+            print("Building libshortfin")
+            cfg = os.getenv(
+                "SHORTFIN_CMAKE_BUILD_TYPE", "Debug" if DEV_MODE else "Release"
+            )
 
             # Configure CMake.
-            os.makedirs(BINARY_DIR, exist_ok=True)
-            maybe_nuke_cmake_cache(CMAKE_BUILD_DIR)
-            print(f"CMake build dir: {CMAKE_BUILD_DIR}", file=sys.stderr)
+            os.makedirs(CMAKE_BUILD_DIR, exist_ok=True)
+            if not DEV_MODE:
+                maybe_nuke_cmake_cache(CMAKE_BUILD_DIR)
+            print(f"CMake build dir: {CMAKE_BUILD_DIR}")
             cmake_args = [
                 "-GNinja",
                 "--log-level=VERBOSE",
                 "-DSHORTFIN_BUNDLE_DEPS=ON",
                 f"-DCMAKE_BUILD_TYPE={cfg}",
                 "-DSHORTFIN_BUILD_PYTHON_BINDINGS=ON",
-                # TODO: This shouldn't be hardcoded... but shortfin doesn't
-                # compile without it.
-                "-DCMAKE_C_COMPILER=clang",
-                "-DCMAKE_CXX_COMPILER=clang++",
+                f"-DPython3_EXECUTABLE={sys.executable}",
             ]
 
+            if DEV_MODE:
+                cmake_args.extend(
+                    [
+                        "-DCMAKE_C_COMPILER=clang",
+                        "-DCMAKE_CXX_COMPILER=clang++",
+                        "-DCMAKE_LINKER_TYPE=LLD",
+                    ]
+                )
+
+            add_env_cmake_setting(cmake_args, "SHORTFIN_IREE_SOURCE_DIR")
+            add_env_cmake_setting(cmake_args, "SHORTFIN_ENABLE_ASAN")
+
             # Only do a from-scratch configure if not already configured.
             cmake_cache_file = os.path.join(CMAKE_BUILD_DIR, "CMakeCache.txt")
             if not os.path.exists(cmake_cache_file):
-                print(f"Configuring with: {cmake_args}", file=sys.stderr)
+                print(f"Configuring with: {cmake_args}")
                 subprocess.check_call(
                     ["cmake", SOURCE_DIR] + cmake_args, cwd=CMAKE_BUILD_DIR
                 )
             else:
-                print(f"Not re-configing (already configured)", file=sys.stderr)
+                print(f"Not re-configing (already configured)")
 
             # Build.
             subprocess.check_call(["cmake", "--build", "."], cwd=CMAKE_BUILD_DIR)
-            print("Build complete.", file=sys.stderr)
+            print("Build complete.")
 
-            # We only take _shortfin_default from the build.
-            target_dir = os.path.join(
-                os.path.abspath(self.build_lib), "_shortfin_default"
-            )
-            print(f"Building in target: {target_dir}", file=sys.stderr)
-            os.makedirs(target_dir, exist_ok=True)
-            print("Copying build to target.", file=sys.stderr)
-            if os.path.exists(target_dir):
-                shutil.rmtree(target_dir)
-            shutil.copytree(
-                os.path.join(
-                    CMAKE_BUILD_DIR,
-                    "bindings",
-                    "python",
-                    "_shortfin_default",
-                ),
-                target_dir,
-                symlinks=False,
-            )
+            # Optionally run CTests.
+            if get_env_boolean("SHORTFIN_RUN_CTESTS", False):
+                print("Running ctests...")
+                subprocess.check_call(
+                    ["ctest", "--timeout", "30", "--output-on-failure"],
+                    cwd=CMAKE_BUILD_DIR,
+                )
 
 
-PYTHON_SOURCE_DIR = REL_SOURCE_DIR / "bindings" / "python"
-PYTHON_BINARY_DIR = REL_BINARY_DIR / "bindings" / "python"
+PYTHON_SOURCE_DIR = REL_SOURCE_DIR / "python"
+# TODO: Need multiple binary dirs for different build variants.
+PYTHON_DEFAULT_BINARY_DIR = REL_CMAKE_BUILD_DIR / "python"
 
 # We need some directories to exist before setup.
 def populate_built_package(abs_dir):
@@ -190,7 +225,7 @@ def populate_built_package(abs_dir):
         pass
 
 
-populate_built_package(os.path.join(PYTHON_BINARY_DIR / "_shortfin_default"))
+populate_built_package(os.path.join(PYTHON_DEFAULT_BINARY_DIR / "_shortfin_default"))
 
 setup(
     name="shortfin",
@@ -206,7 +241,7 @@ def populate_built_package(abs_dir):
     zip_safe=False,
     package_dir={
         "_shortfin": str(PYTHON_SOURCE_DIR / "_shortfin"),
-        "_shortfin_default": str(PYTHON_BINARY_DIR / "_shortfin_default"),
+        "_shortfin_default": str(PYTHON_DEFAULT_BINARY_DIR / "_shortfin_default"),
         # TODO: Conditionally map additional native library variants.
         "shortfin": str(PYTHON_SOURCE_DIR / "shortfin"),
     },