New pip build system #2499

dbort · 2024-03-19T00:30:32Z

Summary:
Compatible with python -m build -wheel instead of using the deprecated python setup.py bdist_wheel.

A new custom build command builds all CMake targets, and a new custom build_ext command installs built files into the wheel.

A new custom build_py command copies platform-independent files into the wheel instead of copying them into the source tree.

Puts all generated files under pip-out instead of mixing with files in the tree.

Uses Command methods like mkpath and copy_file to benefit from setuptools concepts like dry_run, and to get logging for free.

Adds flatc to the wheel and puts it on the user's PATH. Removes the call to install_flatc.sh in install_requirements.sh; but, leaves the script in place since someone might want to run it on their own.

Also:

Enables runtime logging for pybindings even in release mode
Fixes another -fPIC requirement when building the flatcc host tools
Picks a number of build jobs based on the cores in the system, instead of defaulting to 9
Removes now-unnecessary steps from the Getting Started docs
Removes a use of ${arg^^}, which doesn't work with older versions of bash

Test Plan:
Iteration command to test wheel building without completely rebuilding the system:

rm -rf \
    /tmp/wheel-out \
    pip-out/lib* \
    pip-out/bdist.* \
    pip-out/temp.*/cmake-out/CMake* \
    executorch.egg-info \
    third-party/flatcc/{bin,lib} \
    ; \
  python setup.py bdist_wheel --dist-dir /tmp/wheel-out

To fully rebuild the system, also rm -rf pip-out.

The wheel file is written to (e.g.)

/tmp/wheel-out/executorch-0.1.0-cp310-cp310-linux_x86_64.whl

Examine it with unzip -l, or install it with pip install /tmp/wheel-out/executorch*.whl.

Also tested creating the wheel via install_requirements.sh and creating a simple model file.

./install_requirements.sh
python3 -m examples.portable.scripts.export --model_name="add"
<creates add.pte file>

Also tested that it can build pybindings with xnnpack when invoked via install_requirements.sh:

./install_requirements.sh --pybind xnnpack
python -m unittest backends/xnnpack/test/models/mobilenet_v2.py

Builds successfully, but trying to run the test currently fails during xnnpack init() due to T184024365. But it fails in the same way that the main branch does, so this new diff does not regress.

Reviewers:

Subscribers:

Tasks:

Tags:

pytorch-bot · 2024-03-19T00:30:36Z

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/2499

📄 Preview Python docs built from this PR

Note: Links to docs will display an error until the docs builds have been completed.

❌ 1 New Failure, 1 Unrelated Failure

As of commit fddd6c8 with merge base db7c057 ():

NEW FAILURE - The following job has failed:

trunk / test-arm-backend-delegation / linux-job (gh)
/pytorch/executorch/backends/xnnpack/third-party/pthreadpool/src/threadpool-object.h:737:9: error: unknown type name 'pthread_cond_t'

FLAKY - The following job failed but was likely due to flakiness present on trunk:

Apple / upload-frameworks-ios (gh)

This comment was automatically generated by Dr. CI and updates every 15 minutes.

dbort · 2024-03-19T00:31:23Z

I still need to clean this up, but now that it's sort of working I'd like to get some eyes on it. There's a lot going on here that's hard to separate, since it's a rewrite of setup.py.

Haven't tried it in CI yet so I bet things are broken.

larryliu0820

Thanks for putting this up! It would be great if you can paste the temp directory tree structure and also things in the wheel so that we know what's inside.

larryliu0820 · 2024-03-19T18:24:41Z

.gitignore

-exir/_serialize/scalar_type.fbs
-exir/_serialize/program.fbs
-sdk/bundled_program/serialize/bundled_program_schema.fbs
-sdk/bundled_program/serialize/scalar_type.fbs


Oh nice finding we probably don't want to ignore all these

We needed to before this, because setup.py copied them directly into the source tree.

larryliu0820 · 2024-03-19T18:25:45Z

CMakeLists.txt

@@ -301,10 +301,10 @@ endif()
 # ${CMAKE_INSTALL_PREFIX}/
 install(
  TARGETS executorch
-  DESTINATION lib
+  LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}


Does CMake provide a default value for CMAKE_INSTALL_LIBDIR or we need to always pass in a value

Yeah it defaults to lib but can be lib64 in some situations, or a big ProgramFiles path on windows. So this is more portable then just saying lib.

larryliu0820 · 2024-03-19T18:27:07Z

install_requirements.sh

+echo STOP # @nocommit
+exit 1


remove this?

larryliu0820 · 2024-03-19T18:30:05Z

pyproject.toml

+requires = [
+  "setuptools",
+  "wheel",
+# @nocommit: insted we're using --no-isolation


Yeah is there a way to force --no-build-isolation in pip install?

larryliu0820 · 2024-03-19T18:39:24Z

setup.py

+        #
+        # See build.initialize_options() in
+        # setuptools/_distutils/command/build.py for the default.
+        self.build_base = "pip-out"


so one thing I noticed in the previous pip install experience, is that the build base directory name contains useful information such as python version and os arch. Do we still have that in the new directory name?

Yeah, this just moves the root from build/... to pip-out/... so it doesn't sit on top of our existing build/ directory. All of the platform-based directories live under this.

dbort · 2024-03-23T03:12:47Z

It would be great if you can paste the temp directory tree structure and also things in the wheel so that we know what's inside.

Here's the listing from the latest version: https://gist.github.com/dbort/4a79eac7e1d1ebe7306f437bb2583482

The data directory is the interesting part that I've been getting into shape. Now you can see that it has flatc, and an __init__.py wrapper that knows how to execute it. See 9b5e454

     1811  03-23-2024 02:33   executorch/data/bin/__init__.py
  2250288  03-23-2024 01:35   executorch/data/bin/flatc
   179456  03-23-2024 01:38   executorch/data/lib/liboptimized_native_cpu_ops_lib.a
    16414  03-23-2024 01:35   executorch/data/lib64/libbundled_program.a
    41846  03-23-2024 01:35   executorch/data/lib64/libetdump.a
   174748  03-23-2024 02:42   executorch/data/lib64/libexecutorch.a
    12056  03-23-2024 01:35   executorch/data/lib64/libextension_data_loader.a
   203986  03-23-2024 02:42   executorch/data/lib64/libflatccrt.a
 88238252  03-23-2024 01:38   executorch/data/lib64/libportable_kernels.a
   179392  03-23-2024 01:38   executorch/data/lib64/libportable_ops_lib.a
     2715  03-23-2024 01:34   executorch/data/lib64/cmake/executorch/executorch-config.cmake

Oh whoops looks like I need to update liboptimized_native_cpu_ops_lib.a to use the proper LIBDIR. (lib64 is the LIBDIR value picked by the GNU Coding Conventions when building on my x86_64 machine)

larryliu0820

Not sure if ci is reliable at this stage..

facebook-github-bot · 2024-03-27T19:13:06Z

@dbort has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot · 2024-03-27T19:30:01Z

@dbort has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot · 2024-03-27T19:42:03Z

@dbort has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot · 2024-03-27T20:41:33Z

@dbort has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

GregoryComer · 2024-03-28T15:53:20Z

setup.py

+
+        # Default build parallelism based on number of cores, but allow
+        # overriding through the environment.
+        default_parallel = str(os.cpu_count() - 1)


This is nice QOL change. This should speed up builds significantly on higher core count systems.

GregoryComer · 2024-03-28T15:57:09Z

This looks great. Thanks for taking all of this on.

facebook-github-bot · 2024-03-30T22:00:03Z

@dbort has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

Summary: Compatible with `pythom -m build` instead of using the deprecated `python setup.py bdist_wheel`. A new custom build command builds all necessary CMake targets, and a new custom build_ext command installs built files into the wheel. A new custom build_py command copies platform-independent files into the wheel instead of copying them into the source tree. Puts all generated files under pip-out instead of mixing with files in the tree. Uses Command methods like `mkpath` and `copy_file` to benefit from setuptools concepts like dry_run, and to get logging for free. Adds `flatc` to the wheel and installs a wrapper to make it available on the path. Test Plan: Iteration command to test wheel building without completely rebuilding the system: ``` rm -rf \ /tmp/wheel-out \ pip-out/lib* \ pip-out/bdist.* \ pip-out/temp.*/cmake-out/CMake* \ executorch.egg-info \ third-party/flatcc/{bin,lib} \ ; \ python setup.py bdist_wheel --dist-dir /tmp/wheel-out ``` To fully rebuild the system, also `rm -rf pip-out`. The wheel file is written to (e.g.) ``` /tmp/wheel-out/executorch-0.1.0-cp310-cp310-linux_x86_64.whl ``` Examine it with `unzip -l`, or install it with `pip install /tmp/wheel-out/executorch*.whl`.

facebook-github-bot · 2024-03-30T23:09:29Z

@dbort has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot · 2024-03-31T00:55:39Z

@dbort merged this pull request in 250f681.

dbort requested review from larryliu0820 and GregoryComer March 19, 2024 00:30

facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Mar 19, 2024

dbort force-pushed the dbort-pip branch 2 times, most recently from 5b56340 to b41035e Compare March 19, 2024 18:01

larryliu0820 reviewed Mar 19, 2024

View reviewed changes

dbort force-pushed the dbort-pip branch 4 times, most recently from e4a18ec to 9b5e454 Compare March 23, 2024 03:05

larryliu0820 approved these changes Mar 23, 2024

View reviewed changes

salykova mentioned this pull request Mar 23, 2024

Can't run examples ExecuTorch #2618

Closed

dbort force-pushed the dbort-pip branch 7 times, most recently from 42282fb to 64748f5 Compare March 26, 2024 19:10

dbort added the ciflow/trunk label Mar 26, 2024

dbort force-pushed the dbort-pip branch 2 times, most recently from d4caf74 to 2dc73ea Compare March 27, 2024 19:12

dbort force-pushed the dbort-pip branch from 2dc73ea to 73e272e Compare March 27, 2024 19:29

dbort force-pushed the dbort-pip branch from 73e272e to 7338340 Compare March 27, 2024 19:41

dbort force-pushed the dbort-pip branch from 7338340 to a0bacb1 Compare March 27, 2024 20:40

GregoryComer reviewed Mar 28, 2024

View reviewed changes

GregoryComer approved these changes Mar 28, 2024

View reviewed changes

dbort force-pushed the dbort-pip branch 2 times, most recently from ba122bd to 98a17b3 Compare March 30, 2024 21:49

dbort force-pushed the dbort-pip branch from 98a17b3 to fddd6c8 Compare March 30, 2024 23:00

facebook-github-bot closed this in 250f681 Mar 31, 2024

facebook-github-bot added the Merged label Mar 31, 2024

dbort mentioned this pull request Mar 31, 2024

make install requirements work with older versions of bash #2762

Open

kirklandsign mentioned this pull request Apr 9, 2024

No such file or directory: executorch/exir/_serialize/program.fbs and scalar_type.fbs #2910

Closed

		echo STOP # @nocommit
		exit 1

New pip build system #2499

New pip build system #2499

Uh oh!

Conversation

dbort commented Mar 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pytorch-bot bot commented Mar 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/2499

❌ 1 New Failure, 1 Unrelated Failure

Uh oh!

dbort commented Mar 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

larryliu0820 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dbort commented Mar 23, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

larryliu0820 left a comment

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot commented Mar 27, 2024

Uh oh!

facebook-github-bot commented Mar 27, 2024

Uh oh!

facebook-github-bot commented Mar 27, 2024

Uh oh!

facebook-github-bot commented Mar 27, 2024

Uh oh!

GregoryComer Mar 28, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

GregoryComer commented Mar 28, 2024

Uh oh!

facebook-github-bot commented Mar 30, 2024

Uh oh!

facebook-github-bot commented Mar 30, 2024

Uh oh!

facebook-github-bot commented Mar 31, 2024

Uh oh!

Uh oh!

dbort commented Mar 19, 2024 •

edited

Loading

pytorch-bot bot commented Mar 19, 2024 •

edited

Loading

dbort commented Mar 19, 2024 •

edited

Loading

dbort commented Mar 23, 2024 •

edited

Loading

GregoryComer Mar 28, 2024 •

edited

Loading