Skip to content

Commit 15e9123

Browse files
jathukirklandsign
authored andcommitted
Remove usage of environment variables in wheel building (#9583)
### Summary We seem to be using a combination of CMAKE_ARGS and environment variables when creating wheels. Ultimately, CMake only uses the cmake args, however we redefine some of these flags as env vars to help `setup.py` determine if a certain feature is turned on. Specifically, it looks for pybinding vars to bundle pybindings. Let's remove this redundancy and just use the CMAKE_ARGS as the single source of truth. For more details and other considerations, see #9494 (abandoned). Note that even in the wheel building jobs, we use cmake args instead of environment variables to control features: https://github.com/pytorch/executorch/blob/644b7ddf14180d97e348faa627f576e13d367d69/.ci/scripts/wheel/envvar_base.sh#L20 https://github.com/pytorch/executorch/blob/644b7ddf14180d97e348faa627f576e13d367d69/.ci/scripts/wheel/envvar_macos.sh#L14-L15 ### Test plan build + check CMakeCache.txt to ensure flags are set ```bash # Expected: EXECUTORCH_BUILD_PYBIND=OFF EXECUTORCH_BUILD_XNNPACK=OFF EXECUTORCH_BUILD_COREML=OFF $ rm -rf pip-out dist && ./install_executorch.sh --pybind off # Expected: EXECUTORCH_BUILD_PYBIND=ON EXECUTORCH_BUILD_XNNPACK=ON EXECUTORCH_BUILD_COREML=OFF $ rm -rf pip-out dist && ./install_executorch.sh # Expected: EXECUTORCH_BUILD_PYBIND=ON EXECUTORCH_BUILD_XNNPACK=OFF EXECUTORCH_BUILD_COREML=ON $ rm -rf pip-out dist && ./install_executorch.sh --pybind coreml # Expected: EXECUTORCH_BUILD_PYBIND=ON EXECUTORCH_BUILD_XNNPACK=ON EXECUTORCH_BUILD_COREML=ON $ rm -rf pip-out dist && ./install_executorch.sh --pybind xnnpack coreml # Throws an error $ rm -rf pip-out dist && ./install_executorch.sh --pybind coreml off ``` cc @larryliu0820 @lucylq
1 parent 9be8768 commit 15e9123

File tree

7 files changed

+81
-58
lines changed

7 files changed

+81
-58
lines changed

.ci/scripts/unittest-linux.sh

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,7 @@ if [[ "$BUILD_TOOL" == "cmake" ]]; then
2121
source .ci/scripts/setup-vulkan-linux-deps.sh
2222

2323
PYTHON_EXECUTABLE=python \
24-
EXECUTORCH_BUILD_PYBIND=ON \
25-
CMAKE_ARGS="-DEXECUTORCH_BUILD_XNNPACK=ON -DEXECUTORCH_BUILD_KERNELS_QUANTIZED=ON" \
24+
CMAKE_ARGS="-DEXECUTORCH_BUILD_PYBIND=ON -DEXECUTORCH_BUILD_XNNPACK=ON -DEXECUTORCH_BUILD_KERNELS_QUANTIZED=ON" \
2625
.ci/scripts/setup-linux.sh "$@"
2726

2827
# Install llama3_2_vision dependencies.

.ci/scripts/unittest-macos.sh

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,7 @@ trap 'rm -rfv ${TMP_DIR}' EXIT
2121

2222
# Setup MacOS dependencies as there is no Docker support on MacOS atm
2323
PYTHON_EXECUTABLE=python \
24-
EXECUTORCH_BUILD_PYBIND=ON \
25-
CMAKE_ARGS="-DEXECUTORCH_BUILD_COREML=ON -DEXECUTORCH_BUILD_MPS=ON -DEXECUTORCH_BUILD_XNNPACK=ON -DEXECUTORCH_BUILD_KERNELS_QUANTIZED=ON" \
24+
CMAKE_ARGS="-DEXECUTORCH_BUILD_PYBIND=ON -DEXECUTORCH_BUILD_COREML=ON -DEXECUTORCH_BUILD_MPS=ON -DEXECUTORCH_BUILD_XNNPACK=ON -DEXECUTORCH_BUILD_KERNELS_QUANTIZED=ON" \
2625
${CONDA_RUN} --no-capture-output \
2726
.ci/scripts/setup-macos.sh "$@"
2827

.ci/scripts/wheel/envvar_base.sh

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,10 @@
88
# should typically only contain shell variable assignments. Be sure to export
99
# any variables so that subprocesses will see them.
1010

11-
# Enable pybindings so that users can execute ExecuTorch programs from python.
12-
export EXECUTORCH_BUILD_PYBIND=1
13-
1411
# Ensure that CMAKE_ARGS is defined before referencing it. Defaults to empty
1512
# if not defined.
1613
export CMAKE_ARGS="${CMAKE_ARGS:-}"
1714

1815
# Link the XNNPACK backend into the pybindings runtime so that users can execute
1916
# ExecuTorch programs that delegate to it.
20-
CMAKE_ARGS="${CMAKE_ARGS} -DEXECUTORCH_BUILD_XNNPACK=ON"
17+
CMAKE_ARGS="${CMAKE_ARGS} -DEXECUTORCH_BUILD_PYBIND=ON -DEXECUTORCH_BUILD_XNNPACK=ON"

.github/workflows/pull.yml

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -365,8 +365,7 @@ jobs:
365365
# build module for executorch.extension.pybindings.portable_lib
366366
BUILD_TOOL="cmake"
367367
PYTHON_EXECUTABLE=python \
368-
EXECUTORCH_BUILD_XNNPACK=ON \
369-
EXECUTORCH_BUILD_PYBIND=ON \
368+
CMAKE_ARGS="-DEXECUTORCH_BUILD_PYBIND=ON -DEXECUTORCH_BUILD_XNNPACK=ON" \
370369
bash .ci/scripts/setup-linux.sh --build-tool "${BUILD_TOOL}"
371370
372371
# see if we can import the module successfully
@@ -504,7 +503,7 @@ jobs:
504503
505504
# Setup MacOS dependencies as there is no Docker support on MacOS atm
506505
PYTHON_EXECUTABLE=python \
507-
EXECUTORCH_BUILD_PYBIND=ON \
506+
CMAKE_ARGS="-DEXECUTORCH_BUILD_PYBIND=ON" \
508507
EXECUTORCH_BUILD_ARM_BAREMETAL=ON \
509508
.ci/scripts/setup-linux.sh --build-tool "${BUILD_TOOL}"
510509

.github/workflows/trunk.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -261,7 +261,7 @@ jobs:
261261
262262
# build module for executorch.extension.pybindings.portable_lib
263263
BUILD_TOOL=${{ matrix.build-tool }}
264-
EXECUTORCH_BUILD_PYBIND=ON PYTHON_EXECUTABLE=python ${CONDA_RUN} bash .ci/scripts/setup-macos.sh --build-tool "${BUILD_TOOL}"
264+
CMAKE_ARGS="-DEXECUTORCH_BUILD_PYBIND=ON" PYTHON_EXECUTABLE=python ${CONDA_RUN} bash .ci/scripts/setup-macos.sh --build-tool "${BUILD_TOOL}"
265265
266266
# see if we can import the module successfully
267267
${CONDA_RUN} python -c "from executorch.extension.pybindings import portable_lib; print('success!')"

install_executorch.py

Lines changed: 37 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
import subprocess
1616
import sys
1717
from contextlib import contextmanager
18+
from typing import List, Tuple
1819

1920
from install_requirements import (
2021
install_requirements,
@@ -168,28 +169,30 @@ def build_args_parser() -> argparse.ArgumentParser:
168169
return parser
169170

170171

171-
def handle_pybind(args, cmake_args, executorch_build_pybind):
172+
# Returns (wants_off, wanted_pybindings)
173+
def _list_pybind_defines(args) -> Tuple[bool, List[str]]:
174+
if args.pybind is None:
175+
return False, []
176+
172177
# Flatten list of lists.
173178
args.pybind = list(itertools.chain(*args.pybind))
174179
if "off" in args.pybind:
175180
if len(args.pybind) != 1:
176181
raise Exception(f"Cannot combine `off` with other pybinds: {args.pybind}")
177-
executorch_build_pybind = "OFF"
178-
else:
179-
for pybind_arg in args.pybind:
180-
if pybind_arg not in VALID_PYBINDS:
181-
raise Exception(
182-
f"Unrecognized pybind argument {pybind_arg}; valid options are: {', '.join(VALID_PYBINDS)}"
183-
)
184-
if pybind_arg == "training":
185-
cmake_args += " -DEXECUTORCH_BUILD_EXTENSION_TRAINING=ON"
186-
os.environ["EXECUTORCH_BUILD_TRAINING"] = "ON"
187-
elif pybind_arg == "mps":
188-
cmake_args += " -DEXECUTORCH_BUILD_MPS=ON"
189-
else:
190-
cmake_args += f" -DEXECUTORCH_BUILD_{pybind_arg.upper()}=ON"
191-
executorch_build_pybind = "ON"
192-
return executorch_build_pybind, cmake_args
182+
return True, []
183+
184+
cmake_args = []
185+
for pybind_arg in args.pybind:
186+
if pybind_arg not in VALID_PYBINDS:
187+
raise Exception(
188+
f"Unrecognized pybind argument {pybind_arg}; valid options are: {', '.join(VALID_PYBINDS)}"
189+
)
190+
if pybind_arg == "training":
191+
cmake_args.append("-DEXECUTORCH_BUILD_EXTENSION_TRAINING=ON")
192+
else:
193+
cmake_args.append(f"-DEXECUTORCH_BUILD_{pybind_arg.upper()}=ON")
194+
195+
return False, cmake_args
193196

194197

195198
def main(args):
@@ -199,14 +202,23 @@ def main(args):
199202
parser = build_args_parser()
200203
args = parser.parse_args()
201204

202-
EXECUTORCH_BUILD_PYBIND = ""
203-
CMAKE_ARGS = os.getenv("CMAKE_ARGS", "")
205+
cmake_args = [os.getenv("CMAKE_ARGS", "")]
204206
use_pytorch_nightly = True
205207

206-
if args.pybind:
207-
EXECUTORCH_BUILD_PYBIND, CMAKE_ARGS = handle_pybind(
208-
args, CMAKE_ARGS, EXECUTORCH_BUILD_PYBIND
209-
)
208+
has_pybindings = False
209+
wants_pybindings_off, pybind_defines = _list_pybind_defines(args)
210+
if not wants_pybindings_off:
211+
has_pybindings = True
212+
if len(pybind_defines) > 0:
213+
# If the user explicitly provides a list of bindings, just use them
214+
cmake_args += pybind_defines
215+
else:
216+
# If the user has not set pybindings off but also has not provided
217+
# a list, then turn on xnnpack by default
218+
cmake_args.append("-DEXECUTORCH_BUILD_XNNPACK=ON")
219+
220+
if has_pybindings:
221+
cmake_args.append("-DEXECUTORCH_BUILD_PYBIND=ON")
210222

211223
if args.clean:
212224
clean()
@@ -218,18 +230,11 @@ def main(args):
218230
# latest PT commit otherwise
219231
use_pytorch_nightly = False
220232

221-
# If --pybind is not set explicitly for backends (e.g., --pybind xnnpack)
222-
# or is not turned off explicitly (--pybind off)
223-
# then install XNNPACK by default.
224-
if EXECUTORCH_BUILD_PYBIND == "":
225-
EXECUTORCH_BUILD_PYBIND = "ON"
226-
CMAKE_ARGS += " -DEXECUTORCH_BUILD_XNNPACK=ON"
227-
228233
# Use ClangCL on Windows.
229234
# ClangCL is an alias to Clang that configures it to work in an MSVC-compatible
230235
# mode. Using it on Windows to avoid compiler compatibility issues for MSVC.
231236
if os.name == "nt":
232-
CMAKE_ARGS += " -T ClangCL"
237+
cmake_args.append("-T ClangCL")
233238

234239
#
235240
# Install executorch pip package. This also makes `flatc` available on the path.
@@ -238,8 +243,7 @@ def main(args):
238243
#
239244

240245
# Set environment variables
241-
os.environ["EXECUTORCH_BUILD_PYBIND"] = EXECUTORCH_BUILD_PYBIND
242-
os.environ["CMAKE_ARGS"] = CMAKE_ARGS
246+
os.environ["CMAKE_ARGS"] = " ".join(cmake_args)
243247

244248
# Check if the required submodules are present and update them if not
245249
check_and_update_submodules()

setup.py

Lines changed: 38 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -60,42 +60,70 @@
6060

6161
from distutils import log
6262
from distutils.sysconfig import get_python_lib
63+
from functools import cache
6364
from pathlib import Path
64-
from typing import List, Optional
65+
from typing import Dict, List, Optional
6566

6667
from setuptools import Extension, setup
6768
from setuptools.command.build import build
6869
from setuptools.command.build_ext import build_ext
6970
from setuptools.command.build_py import build_py
7071

7172

73+
@cache
74+
def _cmake_args_defines() -> Dict[str, str]:
75+
result = {}
76+
77+
args = re.split(r"\s+", os.environ.get("CMAKE_ARGS", ""))
78+
for arg in args:
79+
if arg.startswith("-D") and "=" in arg:
80+
arg_key, value = arg.split("=")
81+
key = arg_key[2:] # Remove the leading "-D"
82+
result[key] = value
83+
84+
return result
85+
86+
7287
class ShouldBuild:
7388
"""Indicates whether to build various components."""
7489

7590
@staticmethod
76-
def _is_env_enabled(env_var: str, default: bool = False) -> bool:
77-
val = os.environ.get(env_var, None)
78-
if val is None:
79-
return default
80-
if val in ("OFF", "0", ""):
91+
def _is_truthy(value: Optional[str]) -> bool:
92+
if (value is None) or (value.lower() in ("off", "0", "")):
8193
return False
8294
return True
8395

96+
@staticmethod
97+
def _is_cmake_arg_enabled(var: str, default: bool) -> bool:
98+
if os.environ.get(var) is not None:
99+
raise RuntimeError(
100+
f"Python wheel building does not support setting '{var}' using environment variables. Use CMAKE_ARGS='-D{var}=ON' instead."
101+
)
102+
103+
value = _cmake_args_defines().get(var, None)
104+
if value is None:
105+
return default
106+
return ShouldBuild._is_truthy(value)
107+
84108
@classmethod
85109
def pybindings(cls) -> bool:
86-
return cls._is_env_enabled("EXECUTORCH_BUILD_PYBIND", default=False)
110+
return cls._is_cmake_arg_enabled("EXECUTORCH_BUILD_PYBIND", default=False)
87111

88112
@classmethod
89113
def training(cls) -> bool:
90-
return cls._is_env_enabled("EXECUTORCH_BUILD_TRAINING", default=False)
114+
return cls._is_cmake_arg_enabled(
115+
"EXECUTORCH_BUILD_EXTENSION_TRAINING", default=False
116+
)
91117

92118
@classmethod
93119
def llama_custom_ops(cls) -> bool:
94-
return cls._is_env_enabled("EXECUTORCH_BUILD_KERNELS_CUSTOM_AOT", default=True)
120+
return cls._is_cmake_arg_enabled(
121+
"EXECUTORCH_BUILD_KERNELS_CUSTOM_AOT", default=True
122+
)
95123

96124
@classmethod
97125
def flatc(cls) -> bool:
98-
return cls._is_env_enabled("EXECUTORCH_BUILD_FLATC", default=True)
126+
return cls._is_cmake_arg_enabled("EXECUTORCH_BUILD_FLATC", default=True)
99127

100128

101129
class Version:
@@ -667,9 +695,6 @@ def run(self):
667695
"-DEXECUTORCH_BUILD_KERNELS_QUANTIZED_AOT=ON",
668696
]
669697
if ShouldBuild.training():
670-
cmake_args += [
671-
"-DEXECUTORCH_BUILD_EXTENSION_TRAINING=ON",
672-
]
673698
build_args += ["--target", "_training_lib"]
674699
build_args += ["--target", "portable_lib"]
675700
# To link backends into the portable_lib target, callers should

0 commit comments

Comments
 (0)