From 21493a85abac5d0e07b24a543a6502f717bd391e Mon Sep 17 00:00:00 2001 From: jathu Date: Tue, 25 Mar 2025 09:42:32 -0700 Subject: [PATCH 1/5] remove env vars --- install_executorch.py | 62 +++++++++++++++++++++---------------------- setup.py | 41 +++++++++++++++++++++------- 2 files changed, 62 insertions(+), 41 deletions(-) diff --git a/install_executorch.py b/install_executorch.py index 1c5959cd8bb..759add09c66 100644 --- a/install_executorch.py +++ b/install_executorch.py @@ -15,6 +15,7 @@ import subprocess import sys from contextlib import contextmanager +from typing import List from install_requirements import ( install_requirements, @@ -168,28 +169,24 @@ def build_args_parser() -> argparse.ArgumentParser: return parser -def handle_pybind(args, cmake_args, executorch_build_pybind): +def _list_pybind_defines(args) -> List[str]: + cmake_args = [] # Flatten list of lists. args.pybind = list(itertools.chain(*args.pybind)) - if "off" in args.pybind: - if len(args.pybind) != 1: - raise Exception(f"Cannot combine `off` with other pybinds: {args.pybind}") - executorch_build_pybind = "OFF" - else: - for pybind_arg in args.pybind: - if pybind_arg not in VALID_PYBINDS: - raise Exception( - f"Unrecognized pybind argument {pybind_arg}; valid options are: {', '.join(VALID_PYBINDS)}" - ) - if pybind_arg == "training": - cmake_args += " -DEXECUTORCH_BUILD_EXTENSION_TRAINING=ON" - os.environ["EXECUTORCH_BUILD_TRAINING"] = "ON" - elif pybind_arg == "mps": - cmake_args += " -DEXECUTORCH_BUILD_MPS=ON" - else: - cmake_args += f" -DEXECUTORCH_BUILD_{pybind_arg.upper()}=ON" - executorch_build_pybind = "ON" - return executorch_build_pybind, cmake_args + if ("off" in args.pybind) and (len(args.pybind) != 1): + raise Exception(f"Cannot combine `off` with other pybinds: {args.pybind}") + + for pybind_arg in args.pybind: + if pybind_arg not in VALID_PYBINDS: + raise Exception( + f"Unrecognized pybind argument {pybind_arg}; valid options are: {', '.join(VALID_PYBINDS)}" + ) + if pybind_arg == "training": + cmake_args.append("-DEXECUTORCH_BUILD_EXTENSION_TRAINING=ON") + else: + cmake_args.append(f"-DEXECUTORCH_BUILD_{pybind_arg.upper()}=ON") + + return cmake_args def main(args): @@ -199,14 +196,15 @@ def main(args): parser = build_args_parser() args = parser.parse_args() - EXECUTORCH_BUILD_PYBIND = "" - CMAKE_ARGS = os.getenv("CMAKE_ARGS", "") + has_pybindings = False + cmake_args = [os.getenv("CMAKE_ARGS", "")] use_pytorch_nightly = True if args.pybind: - EXECUTORCH_BUILD_PYBIND, CMAKE_ARGS = handle_pybind( - args, CMAKE_ARGS, EXECUTORCH_BUILD_PYBIND - ) + pybind_defines = _list_pybind_defines(args) + if len(pybind_defines) > 0: + has_pybindings = True + cmake_args += pybind_defines if args.clean: clean() @@ -221,15 +219,15 @@ def main(args): # If --pybind is not set explicitly for backends (e.g., --pybind xnnpack) # or is not turned off explicitly (--pybind off) # then install XNNPACK by default. - if EXECUTORCH_BUILD_PYBIND == "": - EXECUTORCH_BUILD_PYBIND = "ON" - CMAKE_ARGS += " -DEXECUTORCH_BUILD_XNNPACK=ON" + if not has_pybindings: + has_pybindings = True + cmake_args.append("-DEXECUTORCH_BUILD_XNNPACK=ON") # Use ClangCL on Windows. # ClangCL is an alias to Clang that configures it to work in an MSVC-compatible # mode. Using it on Windows to avoid compiler compatibility issues for MSVC. if os.name == "nt": - CMAKE_ARGS += " -T ClangCL" + cmake_args.append("-T ClangCL") # # Install executorch pip package. This also makes `flatc` available on the path. @@ -238,8 +236,10 @@ def main(args): # # Set environment variables - os.environ["EXECUTORCH_BUILD_PYBIND"] = EXECUTORCH_BUILD_PYBIND - os.environ["CMAKE_ARGS"] = CMAKE_ARGS + if has_pybindings: + cmake_args.append("-DEXECUTORCH_BUILD_PYBIND=ON") + + os.environ["CMAKE_ARGS"] = " ".join(cmake_args) # Check if the required submodules are present and update them if not check_and_update_submodules() diff --git a/setup.py b/setup.py index b40d6f4738c..2fe44fe22b6 100644 --- a/setup.py +++ b/setup.py @@ -60,8 +60,9 @@ from distutils import log from distutils.sysconfig import get_python_lib +from functools import cache from pathlib import Path -from typing import List, Optional +from typing import Dict, List, Optional from setuptools import Extension, setup from setuptools.command.build import build @@ -69,33 +70,53 @@ from setuptools.command.build_py import build_py +@cache +def _cmake_args_defines() -> Dict[str, str]: + result = {} + + args = re.split(r"\s+", os.environ.get("CMAKE_ARGS", "")) + for arg in args: + if arg.startswith("-D") and "=" in arg: + arg_key, value = arg.split("=") + key = arg_key[2:] # Remove the leading "-D" + result[key] = value + + return result + + class ShouldBuild: """Indicates whether to build various components.""" @staticmethod - def _is_env_enabled(env_var: str, default: bool = False) -> bool: - val = os.environ.get(env_var, None) - if val is None: - return default - if val in ("OFF", "0", ""): + def _is_truthy(value: Optional[str]) -> bool: + if (value is None) or (value.lower() in ("off", "0", "")): return False return True + @staticmethod + def _is_cmake_arg_enabled(var: str, default: bool) -> bool: + value = _cmake_args_defines().get(var, None) + if value is None: + return default + return ShouldBuild._is_truthy(value) + @classmethod def pybindings(cls) -> bool: - return cls._is_env_enabled("EXECUTORCH_BUILD_PYBIND", default=False) + return cls._is_cmake_arg_enabled("EXECUTORCH_BUILD_PYBIND", default=False) @classmethod def training(cls) -> bool: - return cls._is_env_enabled("EXECUTORCH_BUILD_TRAINING", default=False) + return cls._is_cmake_arg_enabled("EXECUTORCH_BUILD_TRAINING", default=False) @classmethod def llama_custom_ops(cls) -> bool: - return cls._is_env_enabled("EXECUTORCH_BUILD_KERNELS_CUSTOM_AOT", default=True) + return cls._is_cmake_arg_enabled( + "EXECUTORCH_BUILD_KERNELS_CUSTOM_AOT", default=True + ) @classmethod def flatc(cls) -> bool: - return cls._is_env_enabled("EXECUTORCH_BUILD_FLATC", default=True) + return cls._is_cmake_arg_enabled("EXECUTORCH_BUILD_FLATC", default=True) class Version: From 503fd8e0fe33a961e7cf2c041f186c989b3522f7 Mon Sep 17 00:00:00 2001 From: jathu Date: Tue, 25 Mar 2025 11:17:46 -0700 Subject: [PATCH 2/5] warn users who are using env vars --- setup.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/setup.py b/setup.py index 2fe44fe22b6..4d430727b72 100644 --- a/setup.py +++ b/setup.py @@ -95,6 +95,11 @@ def _is_truthy(value: Optional[str]) -> bool: @staticmethod def _is_cmake_arg_enabled(var: str, default: bool) -> bool: + if os.environ.get(var) is not None: + raise RuntimeError( + f"Python wheel building does not support setting '{var}' using environment variables. Use CMAKE_ARGS='-D{var}=ON' instead." + ) + value = _cmake_args_defines().get(var, None) if value is None: return default From 96c85fb573e3b27298d26b509dcbe54d6cb983df Mon Sep 17 00:00:00 2001 From: jathu Date: Tue, 25 Mar 2025 11:31:11 -0700 Subject: [PATCH 3/5] fix issue of not allowing pybind off --- install_executorch.py | 44 +++++++++++++++++++++++-------------------- 1 file changed, 24 insertions(+), 20 deletions(-) diff --git a/install_executorch.py b/install_executorch.py index 759add09c66..dc06a5849f9 100644 --- a/install_executorch.py +++ b/install_executorch.py @@ -15,7 +15,7 @@ import subprocess import sys from contextlib import contextmanager -from typing import List +from typing import List, Tuple from install_requirements import ( install_requirements, @@ -169,13 +169,19 @@ def build_args_parser() -> argparse.ArgumentParser: return parser -def _list_pybind_defines(args) -> List[str]: - cmake_args = [] +# Returns (wants_off, wanted_pybindings) +def _list_pybind_defines(args) -> Tuple[bool, List[str]]: + if args.pybind is None: + return False, [] + # Flatten list of lists. args.pybind = list(itertools.chain(*args.pybind)) - if ("off" in args.pybind) and (len(args.pybind) != 1): - raise Exception(f"Cannot combine `off` with other pybinds: {args.pybind}") + if "off" in args.pybind: + if len(args.pybind) != 1: + raise Exception(f"Cannot combine `off` with other pybinds: {args.pybind}") + return True, [] + cmake_args = [] for pybind_arg in args.pybind: if pybind_arg not in VALID_PYBINDS: raise Exception( @@ -186,7 +192,7 @@ def _list_pybind_defines(args) -> List[str]: else: cmake_args.append(f"-DEXECUTORCH_BUILD_{pybind_arg.upper()}=ON") - return cmake_args + return False, cmake_args def main(args): @@ -196,15 +202,23 @@ def main(args): parser = build_args_parser() args = parser.parse_args() - has_pybindings = False cmake_args = [os.getenv("CMAKE_ARGS", "")] use_pytorch_nightly = True - if args.pybind: - pybind_defines = _list_pybind_defines(args) + has_pybindings = False + wants_pybindings_off, pybind_defines = _list_pybind_defines(args) + if not wants_pybindings_off: + has_pybindings = True if len(pybind_defines) > 0: - has_pybindings = True + # If the user explicitly provides a list of bindings, just use them cmake_args += pybind_defines + else: + # If the user has not set pybindings off but also has not provided + # a list, then turn on xnnpack by default + cmake_args.append("-DEXECUTORCH_BUILD_XNNPACK=ON") + + if has_pybindings: + cmake_args.append("-DEXECUTORCH_BUILD_PYBIND=ON") if args.clean: clean() @@ -216,13 +230,6 @@ def main(args): # latest PT commit otherwise use_pytorch_nightly = False - # If --pybind is not set explicitly for backends (e.g., --pybind xnnpack) - # or is not turned off explicitly (--pybind off) - # then install XNNPACK by default. - if not has_pybindings: - has_pybindings = True - cmake_args.append("-DEXECUTORCH_BUILD_XNNPACK=ON") - # Use ClangCL on Windows. # ClangCL is an alias to Clang that configures it to work in an MSVC-compatible # mode. Using it on Windows to avoid compiler compatibility issues for MSVC. @@ -236,9 +243,6 @@ def main(args): # # Set environment variables - if has_pybindings: - cmake_args.append("-DEXECUTORCH_BUILD_PYBIND=ON") - os.environ["CMAKE_ARGS"] = " ".join(cmake_args) # Check if the required submodules are present and update them if not From 9189d629681ba12a3174e152addcfe3c84100335 Mon Sep 17 00:00:00 2001 From: jathu Date: Tue, 25 Mar 2025 11:48:58 -0700 Subject: [PATCH 4/5] done explicitly define binding --- .ci/scripts/wheel/envvar_base.sh | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/.ci/scripts/wheel/envvar_base.sh b/.ci/scripts/wheel/envvar_base.sh index 6379dee6b5a..15f590f0f68 100755 --- a/.ci/scripts/wheel/envvar_base.sh +++ b/.ci/scripts/wheel/envvar_base.sh @@ -8,13 +8,10 @@ # should typically only contain shell variable assignments. Be sure to export # any variables so that subprocesses will see them. -# Enable pybindings so that users can execute ExecuTorch programs from python. -export EXECUTORCH_BUILD_PYBIND=1 - # Ensure that CMAKE_ARGS is defined before referencing it. Defaults to empty # if not defined. export CMAKE_ARGS="${CMAKE_ARGS:-}" # Link the XNNPACK backend into the pybindings runtime so that users can execute # ExecuTorch programs that delegate to it. -CMAKE_ARGS="${CMAKE_ARGS} -DEXECUTORCH_BUILD_XNNPACK=ON" +CMAKE_ARGS="${CMAKE_ARGS} -DEXECUTORCH_BUILD_PYBIND=ON -DEXECUTORCH_BUILD_XNNPACK=ON" From 69a5ba4ba4f5756e5401d840437b6d42125b4681 Mon Sep 17 00:00:00 2001 From: jathu Date: Tue, 25 Mar 2025 12:00:25 -0700 Subject: [PATCH 5/5] fix ci --- .ci/scripts/unittest-linux.sh | 3 +-- .ci/scripts/unittest-macos.sh | 3 +-- .github/workflows/pull.yml | 5 ++--- .github/workflows/trunk.yml | 2 +- setup.py | 7 +++---- 5 files changed, 8 insertions(+), 12 deletions(-) diff --git a/.ci/scripts/unittest-linux.sh b/.ci/scripts/unittest-linux.sh index f8ff9df773e..a05211d8e0e 100755 --- a/.ci/scripts/unittest-linux.sh +++ b/.ci/scripts/unittest-linux.sh @@ -21,8 +21,7 @@ if [[ "$BUILD_TOOL" == "cmake" ]]; then source .ci/scripts/setup-vulkan-linux-deps.sh PYTHON_EXECUTABLE=python \ - EXECUTORCH_BUILD_PYBIND=ON \ - CMAKE_ARGS="-DEXECUTORCH_BUILD_XNNPACK=ON -DEXECUTORCH_BUILD_KERNELS_QUANTIZED=ON" \ + CMAKE_ARGS="-DEXECUTORCH_BUILD_PYBIND=ON -DEXECUTORCH_BUILD_XNNPACK=ON -DEXECUTORCH_BUILD_KERNELS_QUANTIZED=ON" \ .ci/scripts/setup-linux.sh "$@" # Install llama3_2_vision dependencies. diff --git a/.ci/scripts/unittest-macos.sh b/.ci/scripts/unittest-macos.sh index d5ca97404aa..12c9d3f1508 100755 --- a/.ci/scripts/unittest-macos.sh +++ b/.ci/scripts/unittest-macos.sh @@ -21,8 +21,7 @@ trap 'rm -rfv ${TMP_DIR}' EXIT # Setup MacOS dependencies as there is no Docker support on MacOS atm PYTHON_EXECUTABLE=python \ -EXECUTORCH_BUILD_PYBIND=ON \ -CMAKE_ARGS="-DEXECUTORCH_BUILD_COREML=ON -DEXECUTORCH_BUILD_MPS=ON -DEXECUTORCH_BUILD_XNNPACK=ON -DEXECUTORCH_BUILD_KERNELS_QUANTIZED=ON" \ +CMAKE_ARGS="-DEXECUTORCH_BUILD_PYBIND=ON -DEXECUTORCH_BUILD_COREML=ON -DEXECUTORCH_BUILD_MPS=ON -DEXECUTORCH_BUILD_XNNPACK=ON -DEXECUTORCH_BUILD_KERNELS_QUANTIZED=ON" \ ${CONDA_RUN} --no-capture-output \ .ci/scripts/setup-macos.sh "$@" diff --git a/.github/workflows/pull.yml b/.github/workflows/pull.yml index 9a2221b3aac..c3eafc02c39 100644 --- a/.github/workflows/pull.yml +++ b/.github/workflows/pull.yml @@ -365,8 +365,7 @@ jobs: # build module for executorch.extension.pybindings.portable_lib BUILD_TOOL="cmake" PYTHON_EXECUTABLE=python \ - EXECUTORCH_BUILD_XNNPACK=ON \ - EXECUTORCH_BUILD_PYBIND=ON \ + CMAKE_ARGS="-DEXECUTORCH_BUILD_PYBIND=ON -DEXECUTORCH_BUILD_XNNPACK=ON" \ bash .ci/scripts/setup-linux.sh --build-tool "${BUILD_TOOL}" # see if we can import the module successfully @@ -504,7 +503,7 @@ jobs: # Setup MacOS dependencies as there is no Docker support on MacOS atm PYTHON_EXECUTABLE=python \ - EXECUTORCH_BUILD_PYBIND=ON \ + CMAKE_ARGS="-DEXECUTORCH_BUILD_PYBIND=ON" \ EXECUTORCH_BUILD_ARM_BAREMETAL=ON \ .ci/scripts/setup-linux.sh --build-tool "${BUILD_TOOL}" diff --git a/.github/workflows/trunk.yml b/.github/workflows/trunk.yml index 9da65f6083b..9d4e0201f0e 100644 --- a/.github/workflows/trunk.yml +++ b/.github/workflows/trunk.yml @@ -261,7 +261,7 @@ jobs: # build module for executorch.extension.pybindings.portable_lib BUILD_TOOL=${{ matrix.build-tool }} - EXECUTORCH_BUILD_PYBIND=ON PYTHON_EXECUTABLE=python ${CONDA_RUN} bash .ci/scripts/setup-macos.sh --build-tool "${BUILD_TOOL}" + CMAKE_ARGS="-DEXECUTORCH_BUILD_PYBIND=ON" PYTHON_EXECUTABLE=python ${CONDA_RUN} bash .ci/scripts/setup-macos.sh --build-tool "${BUILD_TOOL}" # see if we can import the module successfully ${CONDA_RUN} python -c "from executorch.extension.pybindings import portable_lib; print('success!')" diff --git a/setup.py b/setup.py index 4d430727b72..fd19b5c690f 100644 --- a/setup.py +++ b/setup.py @@ -111,7 +111,9 @@ def pybindings(cls) -> bool: @classmethod def training(cls) -> bool: - return cls._is_cmake_arg_enabled("EXECUTORCH_BUILD_TRAINING", default=False) + return cls._is_cmake_arg_enabled( + "EXECUTORCH_BUILD_EXTENSION_TRAINING", default=False + ) @classmethod def llama_custom_ops(cls) -> bool: @@ -693,9 +695,6 @@ def run(self): "-DEXECUTORCH_BUILD_KERNELS_QUANTIZED_AOT=ON", ] if ShouldBuild.training(): - cmake_args += [ - "-DEXECUTORCH_BUILD_EXTENSION_TRAINING=ON", - ] build_args += ["--target", "_training_lib"] build_args += ["--target", "portable_lib"] # To link backends into the portable_lib target, callers should