Skip to content

Commit c5f2ea4

Browse files
committed
Update base for Update on "[build] Add editable mode unittest"
As titled. To do this we need to refactor the arguments being passed into `setup-linux.sh` `setup-macos.sh` and `unittest-linux.sh` `unittest-macos.sh`. [ghstack-poisoned]
1 parent f107e41 commit c5f2ea4

File tree

5 files changed

+51
-104
lines changed

5 files changed

+51
-104
lines changed

.github/workflows/pull.yml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,6 @@ jobs:
7979
PYTHON_EXECUTABLE=python bash ./install_executorch.sh --editable --pybind xnnpack --use-pt-pinned-commit
8080
# Try to import extension library
8181
python -c "from executorch.extension.llm.custom_ops import custom_ops"
82-
python -c "from executorch.data.bin import flatc"
8382
8483
test-models-linux:
8584
name: test-models-linux

.github/workflows/trunk.yml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,6 @@ jobs:
6060
PYTHON_EXECUTABLE=python ${CONDA_RUN} bash ./install_executorch.sh --editable --pybind xnnpack
6161
# Try to import extension library
6262
python -c "from executorch.extension.llm.custom_ops import custom_ops"
63-
python -c "from executorch.data.bin import flatc"
6463
6564
test-models-macos:
6665
name: test-models-macos

data/bin/README.md

Lines changed: 0 additions & 31 deletions
This file was deleted.

pyproject.toml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,6 @@ flatc = "executorch.data.bin:flatc"
8888
[tool.setuptools.package-dir]
8989
"executorch.backends" = "backends"
9090
"executorch.codegen" = "codegen"
91-
"executorch.data.bin" = "data/bin"
9291
# TODO(mnachin T180504136): Do not put examples/models
9392
# into core pip packages. Refactor out the necessary utils
9493
# or core models files into a separate package.

setup.py

Lines changed: 51 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -220,49 +220,36 @@ def src_path(self, installer: "InstallerBuildExt") -> Path:
220220
"""
221221
# Share the cmake-out location with CustomBuild.
222222
build_cmd = installer.get_finalized_command("build")
223-
if "%CMAKE_CACHE_DIR%" in self.src:
224-
if not hasattr(build_cmd, "cmake_cache_dir"):
225-
raise RuntimeError(
226-
f"Extension {self.name} has a src {self.src} that contains"
227-
" %CMAKE_CACHE_DIR% but CMake does not run in the `build` "
228-
"command. Please double check if the command is correct."
229-
)
230-
else:
231-
build_dir = Path(build_cmd.cmake_cache_dir)
223+
if hasattr(build_cmd, "cmake_cache_dir"):
224+
cmake_cache_dir = Path(build_cmd.cmake_cache_dir)
232225
else:
233-
# If the src path doesn't contain %CMAKE_CACHE_DIR% placeholder,
234-
# try to find it under the current directory.
235-
build_dir = Path(".")
236-
237-
src_path = self.src.replace("%CMAKE_CACHE_DIR%/", "")
238-
226+
# If we're in editable mode, use a default or fallback value for cmake_cache_dir
227+
# This could be a hardcoded path, or a path derived from the current working directory
228+
cmake_cache_dir = Path(".")
239229
cfg = get_build_type(installer.debug)
240230

241231
if os.name == "nt":
242232
# Replace %BUILD_TYPE% with the current build type.
243-
src_path = src_path.replace("%BUILD_TYPE%", cfg)
233+
self.src = self.src.replace("%BUILD_TYPE%", cfg)
244234
else:
245235
# Remove %BUILD_TYPE% from the path.
246-
src_path = src_path.replace("/%BUILD_TYPE%", "")
236+
self.src = self.src.replace("/%BUILD_TYPE%", "")
247237

248238
# Construct the full source path, resolving globs. If there are no glob
249239
# pattern characters, this will just ensure that the source file exists.
250-
srcs = tuple(build_dir.glob(src_path))
240+
srcs = tuple(cmake_cache_dir.glob(self.src))
251241
if len(srcs) != 1:
252242
raise ValueError(
253-
f"Expecting exactly 1 file matching {self.src} in {build_dir}, "
254-
f"found {repr(srcs)}. Resolved src pattern: {src_path}."
255-
)
256-
return srcs[0]
243+
f"""Expected exactly one file matching '{self.src}' in {cmake_cache_dir}; found {repr(srcs)}.
257244
258-
def inplace_dir(self, installer: "InstallerBuildExt") -> Path:
259-
"""Returns the path of this file to be installed to, under inplace mode.
245+
If that file is a CMake-built extension module file, and we are installing in editable mode, please disable the corresponding build option since it's not supported yet.
260246
261-
It will be a relative path to the project root directory. For more info
262-
related to inplace/editable mode, please checkout this doc:
263-
https://setuptools.pypa.io/en/latest/userguide/development_mode.html
264-
"""
265-
raise NotImplementedError()
247+
Try:
248+
249+
EXECUTORCH_BUILD_FLATC=OFF EXECUTORCH_BUILD_KERNELS_CUSTOM_AOT=OFF pip install -e .
250+
"""
251+
)
252+
return srcs[0]
266253

267254

268255
class BuiltFile(_BaseExtension):
@@ -329,18 +316,6 @@ def dst_path(self, installer: "InstallerBuildExt") -> Path:
329316
# Destination looks like a file.
330317
return dst_root / Path(self.dst)
331318

332-
def inplace_dir(self, installer: "InstallerBuildExt") -> Path:
333-
"""For a `BuiltFile`, we use self.dst as its inplace directory path.
334-
Need to handle directory vs file.
335-
"""
336-
# HACK: get rid of the leading "executorch" in ext.dst.
337-
# This is because we don't have a root level "executorch" module.
338-
package_dir = self.dst.removeprefix("executorch/")
339-
# If dst is a file, use it's directory
340-
if not package_dir.endswith("/"):
341-
package_dir = os.path.dirname(package_dir)
342-
return Path(package_dir)
343-
344319

345320
class BuiltExtension(_BaseExtension):
346321
"""An extension that installs a python extension that was built by cmake."""
@@ -360,7 +335,7 @@ def __init__(self, src: str, modpath: str):
360335
"/" not in modpath
361336
), f"modpath must be a dotted python module path: saw '{modpath}'"
362337
# This is a real extension, so use the modpath as the name.
363-
super().__init__(src=f"%CMAKE_CACHE_DIR%/{src}", dst=modpath, name=modpath)
338+
super().__init__(src=src, dst=modpath, name=modpath)
364339

365340
def src_path(self, installer: "InstallerBuildExt") -> Path:
366341
"""Returns the path to the source file, resolving globs.
@@ -394,15 +369,6 @@ def dst_path(self, installer: "InstallerBuildExt") -> Path:
394369
# path: that's the file we're creating.
395370
return Path(installer.get_ext_fullpath(self.dst))
396371

397-
def inplace_dir(self, installer: "InstallerBuildExt") -> Path:
398-
"""For BuiltExtension, deduce inplace dir path from extension name."""
399-
build_py = installer.get_finalized_command("build_py")
400-
modpath = self.name.split(".")
401-
package = ".".join(modpath[:-1])
402-
package_dir = os.path.abspath(build_py.get_package_dir(package))
403-
404-
return Path(package_dir)
405-
406372

407373
class InstallerBuildExt(build_ext):
408374
"""Installs files that were built by cmake."""
@@ -433,15 +399,23 @@ def copy_extensions_to_source(self) -> None:
433399
434400
Returns:
435401
"""
402+
build_py = self.get_finalized_command("build_py")
436403
for ext in self.extensions:
437-
package_dir = ext.inplace_dir(self)
404+
if isinstance(ext, BuiltExtension):
405+
modpath = ext.name.split(".")
406+
package = ".".join(modpath[:-1])
407+
package_dir = os.path.abspath(build_py.get_package_dir(package))
408+
else:
409+
# HACK: get rid of the leading "executorch" in ext.dst.
410+
# This is because we don't have a root level "executorch" module.
411+
package_dir = ext.dst.removeprefix("executorch/")
438412

439413
# Ensure that the destination directory exists.
440414
self.mkpath(os.fspath(package_dir))
441415

442416
regular_file = ext.src_path(self)
443417
inplace_file = os.path.join(
444-
package_dir, os.path.basename(ext.dst_path(self))
418+
package_dir, os.path.basename(ext.src_path(self))
445419
)
446420

447421
# Always copy, even if source is older than destination, to ensure
@@ -750,6 +724,20 @@ def run(self):
750724
# Build the system.
751725
self.spawn(["cmake", "--build", cmake_cache_dir, *build_args])
752726

727+
# Non-python files should live under this data directory.
728+
data_root = os.path.join(self.build_lib, "executorch", "data")
729+
730+
# Directories like bin/ and lib/ live under data/.
731+
bin_dir = os.path.join(data_root, "bin")
732+
733+
# Copy the bin wrapper so that users can run any executables under
734+
# data/bin, as long as they are listed in the [project.scripts] section
735+
# of pyproject.toml.
736+
self.mkpath(bin_dir)
737+
self.copy_file(
738+
"build/pip_data_bin_init.py.in",
739+
os.path.join(bin_dir, "__init__.py"),
740+
)
753741
# Share the cmake-out location with _BaseExtension.
754742
self.cmake_cache_dir = cmake_cache_dir
755743

@@ -761,20 +749,13 @@ def get_ext_modules() -> List[Extension]:
761749
"""Returns the set of extension modules to build."""
762750
ext_modules = []
763751
if ShouldBuild.flatc():
764-
ext_modules.extend(
765-
[
766-
BuiltFile(
767-
src_dir="%CMAKE_CACHE_DIR%/third-party/flatbuffers/%BUILD_TYPE%/",
768-
src_name="flatc",
769-
dst="executorch/data/bin/",
770-
is_executable=True,
771-
),
772-
BuiltFile(
773-
src_dir="build/",
774-
src_name="pip_data_bin_init.py.in",
775-
dst="executorch/data/bin/__init__.py",
776-
),
777-
]
752+
ext_modules.append(
753+
BuiltFile(
754+
src_dir="third-party/flatbuffers/%BUILD_TYPE%/",
755+
src_name="flatc",
756+
dst="executorch/data/bin/",
757+
is_executable=True,
758+
)
778759
)
779760

780761
if ShouldBuild.pybindings():
@@ -797,16 +778,16 @@ def get_ext_modules() -> List[Extension]:
797778
if ShouldBuild.llama_custom_ops():
798779
ext_modules.append(
799780
BuiltFile(
800-
src_dir="%CMAKE_CACHE_DIR%/extension/llm/custom_ops/%BUILD_TYPE%/",
781+
src_dir="extension/llm/custom_ops/%BUILD_TYPE%/",
801782
src_name="custom_ops_aot_lib",
802-
dst="executorch/extension/llm/custom_ops/",
783+
dst="executorch/extension/llm/custom_ops",
803784
is_dynamic_lib=True,
804785
)
805786
)
806787
ext_modules.append(
807788
# Install the prebuilt library for quantized ops required by custom ops.
808789
BuiltFile(
809-
src_dir="%CMAKE_CACHE_DIR%/kernels/quantized/%BUILD_TYPE%/",
790+
src_dir="kernels/quantized/%BUILD_TYPE%/",
810791
src_name="quantized_ops_aot_lib",
811792
dst="executorch/kernels/quantized/",
812793
is_dynamic_lib=True,

0 commit comments

Comments
 (0)