WIP: Remove install; fix nocommits

Author: dbort

setup.py

@@ -49,7 +49,6 @@
 import re
 import sys
 
-from distutils import log
 from distutils.sysconfig import get_python_lib
 from pathlib import Path
 
@@ -62,6 +61,34 @@
 # https://setuptools.pypa.io/en/latest/userguide/extension.html
 
 
+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", ""):
+            return False
+        return True
+
+    @classmethod
+    @property
+    def pybindings(cls) -> bool:
+        return cls._is_env_enabled("EXECUTORCH_BUILD_PYBIND", default=False)
+
+    @classmethod
+    @property
+    def xnnpack(cls) -> bool:
+        return cls._is_env_enabled("EXECUTORCH_BUILD_XNNPACK", default=False)
+
+    @classmethod
+    @property
+    def aot_util(cls) -> bool:
+        return cls._is_env_enabled("EXECUTORCH_BUILD_AOT_UTIL", default=True)
+
+
 class _BaseExtension(Extension):
     """A base class that maps an abstract source to an abstract destination."""
 
@@ -89,8 +116,8 @@ def src_path(self, installer: "InstallerBuildExt") -> Path:
             installer: The InstallerBuildExt instance that is installing the
                 file.
         """
-        # @nocommit: share the cmake-out location with CustomBuild.
-        # Can get a handle with installer.get_finalized_command('build')
+        # TODO(dbort): share the cmake-out location with CustomBuild. Can get a
+        # handle with installer.get_finalized_command('build')
         cmake_cache_dir: Path = Path().cwd() / installer.build_temp / "cmake-out"
 
         # Construct the full source path, resolving globs. If there are no glob
@@ -104,7 +131,13 @@ def src_path(self, installer: "InstallerBuildExt") -> Path:
 
 
 class BuiltFile(_BaseExtension):
-    """An extension that installs a single file that was built by cmake."""
+    """An extension that installs a single file that was built by cmake.
+
+    This isn't technically a `build_ext` style python extension, but there's no
+    dedicated command for installing arbitrary data. It's convenient to use
+    this, though, because it lets us manage the files to install as entries in
+    `ext_modules`.
+    """
 
     def __init__(self, src: str, dst: str):
         """Initializes a BuiltFile.
@@ -149,7 +182,8 @@ def __init__(self, src: str, modpath: str):
         Args:
             src: The path to the file to install (typically a shared library),
                 relative to the cmake-out directory. May be an fnmatch-style
-                glob that matches exactly one file.
+                glob that matches exactly one file. If the path ends in `.so`,
+                this class will also look for similarly-named `.dylib` files.
             modpath: The dotted path of the python module that maps to the
                 extension.
         """
@@ -193,6 +227,8 @@ def dst_path(self, installer: "InstallerBuildExt") -> Path:
 
 
 class InstallerBuildExt(build_ext):
+    """Installs files that were built by cmake."""
+
     # TODO(dbort): Depend on the "build" command to ensure it runs first
 
     def build_extension(self, ext: _BaseExtension) -> None:
@@ -227,12 +263,6 @@ class CustomBuildPy(build_py):
     """
 
     def run(self):
-        if False:  # @nocommit
-            self.no_compile = "NO_COMPILE"
-            self.dump_options()  # @nocommit
-            print("STOP")
-            sys.exit(1)
-
         # Copy python files to the output directory. This set of files is
         # defined by the py_module list and package_data patterns.
         build_py.run(self)
@@ -297,7 +327,7 @@ def initialize_options(self):
         self.parallel = os.environ.get("CMAKE_BUILD_PARALLEL_LEVEL", "9")
 
     def run(self):
-        self.dump_options()  # @nocommit
+        self.dump_options()
 
         debug = int(os.environ.get("DEBUG", 0)) if self.debug is None else self.debug
         cfg = "Debug" if debug else "Release"
@@ -322,20 +352,18 @@ def run(self):
             f"-DCMAKE_BUILD_TYPE={cfg}",
         ]
 
-        # TODO(dbort): Separate build target options from the base cmake setup.
-        # @nocommit: Don't repeat the "build aot_util" env logic
-        if os.environ.get("EXECUTORCH_BUILD_AOT_UTIL", "ON") == "ON":
+        # TODO(dbort): This block should also assemble the list of targets to
+        # build so we only build what we need.
+        if ShouldBuild.aot_util:
             cmake_args += [
                 "-DEXECUTORCH_BUILD_EXTENSION_AOT_UTIL=ON",
             ]
-        # @nocommit: Don't repeat the "build pybind" env logic
-        if os.environ.get("EXECUTORCH_BUILD_PYBIND", "OFF") == "ON":
+        if ShouldBuild.pybindings:
             cmake_args += [
                 "-DEXECUTORCH_BUILD_PYBIND=ON",
             ]
-            if os.environ.get("EXECUTORCH_BUILD_XNNPACK", None):
+            if ShouldBuild.xnnpack:
                 cmake_args += [
-                    # @nocommit: fails in the xnnpack build files
                     "-DEXECUTORCH_BUILD_XNNPACK=ON",
                 ]
             # TODO(dbort): Add MPS/CoreML backends when building on macos.
@@ -348,10 +376,6 @@ def run(self):
 
         build_args = [f"-j{self.parallel}"]
 
-        self.dry_run = False  # @nocommit
-        if self.dry_run:
-            log.info("CustomBuild: DRY_RUN")
-
         # Put the cmake cache under the temp directory, like
         # "pip-out/temp.<plat>/cmake-out".
         cmake_cache_dir = os.path.join(repo_root, self.build_temp, "cmake-out")
@@ -363,33 +387,19 @@ def run(self):
         # Build the system.
         self.spawn(["cmake", "--build", cmake_cache_dir, *build_args])
 
-        # Install some files to a common spot under cmake-out. This will make
-        # them available install via BuiltExtension with a source under
-        # `_installed/`.
-        # TODO(dbort): Add --strip for release builds.
-        cmake_install_dir = os.path.join(cmake_cache_dir, "_installed")
-        self.spawn(
-            [
-                "cmake",
-                "--install",
-                cmake_cache_dir,
-                "--prefix",
-                cmake_install_dir,
-            ]
-        )
-
-        # Copy all installed files into the package, under data/. If names or
-        # locations need to change, handle that in the CMake config instead of
-        # adding a mapping here.
+        # Non-python files should live under this data directory.
         data_root = os.path.join(self.build_lib, "executorch", "data")
-        self.copy_tree(cmake_install_dir, data_root)
+
+        # Directories like bin/ and lib/ live under data/.
+        bin_dir = os.path.join(data_root, "bin")
 
         # Copy the bin wrapper so that users can run any executables under
         # data/bin, as long as they are listed in the [project.scripts] section
         # of pyproject.toml.
+        self.mkpath(bin_dir)
         self.copy_file(
             "build/pip_data_bin_init.py.in",
-            os.path.join(data_root, "bin", "__init__.py"),
+            os.path.join(bin_dir, "__init__.py"),
         )
 
         # Finally, run the underlying subcommands like build_py, build_ext.
@@ -400,19 +410,16 @@ def get_ext_modules() -> list[Extension]:
     """Returns the set of extension modules to build."""
 
     ext_modules = [
-        # @nocommit: remove this since now it's installed by the build command.
-        # but removing this requires potentially adding a no-op extension to
-        # mark the wheel as non-pure, when we're not building the pybindings.
         BuiltFile("third-party/flatbuffers/flatc", "executorch/data/bin/"),
     ]
-    if os.environ.get("EXECUTORCH_BUILD_AOT_UTIL", "ON") == "ON":
+    if ShouldBuild.aot_util:
         ext_modules.append(
             BuiltExtension(
                 "extension/aot_util/libaot_util.so",
                 "executorch.extension.aot_util.aot_util",
             )
         )
-    if os.environ.get("EXECUTORCH_BUILD_PYBIND", "OFF") == "ON":
+    if ShouldBuild.pybindings:
         ext_modules.append(
             # Install the prebuilt pybindings extension wrapper for the runtime,
             # portable kernels, and a selection of backends. This lets users
@@ -445,7 +452,7 @@ def get_ext_modules() -> list[Extension]:
         "executorch/sdk": "sdk",
         "executorch/sdk/bundled_program": "sdk/bundled_program",
         "executorch/util": "util",
-        # @nocommit: This will install a top-level module called "serializer",
+        # Note: This will install a top-level module called "serializer",
         # which seems too generic and might conflict with other pip packages.
         "serializer": "backends/arm/third-party/serialization_lib/python/serializer",
         "tosa": "backends/arm/third-party/serialization_lib/python/tosa",