[BE] Remind users to update submodule #8203

larryliu0820 · 2025-02-05T00:58:57Z

Stack from ghstack (oldest at bottom):

-> [BE] Remind users to update submodule #8203

Summary:

Fixing #7243

As titled. This PR adds 2 things:

check_and_update_submodule() check if required submodule folders exist and at least contains a CMakeLists.txt file.
Give useful error when the submodule is corrupted (most likely missing a file, caused by submodule out of sync).

Test Plan:

Test if we can still install, if submodule is not updated

rm -rf third-party/prelude
./install_executorch.sh

See the following log

...
Processing /data/users/larryliu/executorch
  Preparing metadata (pyproject.toml): started
  Running command Preparing metadata (pyproject.toml)
  2025-02-05 11:28:46,430 [ExecuTorch] WARNING: Some required submodules are missing. Updating submodules...
  Submodule path 'third-party/prelude': checked out '851d3f09c452937fc5adef27e2c50f7f304f1646'
  2025-02-05 11:28:46,748 [ExecuTorch] INFO: All required submodules are present.
  2025-02-05 11:28:47,018 [ExecuTorch] INFO: running dist_info
...

This proves that we can update submodule for the user.

Test if we can give useful error message, if we are missing a file.

rm third-party/gflags/src/gflags.cc
./install_executorch.sh --clean
./install_executorch.sh

See the following error:

  2025-02-05 12:08:56,889 [ExecuTorch] ERROR: Failed to query buck for sources. Failed command:

     buck2 cquery inputs(deps('//runtime/executor:program'))

  This is likely due to missing git submodules or outdated CMake cache. Please run the following before retry:

      ./install_executorch.sh --clean
      git submodule update --init --recursive

Reviewers:

Subscribers:

Tasks:

Tags:

Differential Revision: D69156975

Summary: As titled. This adds messages when we suspect the user forgot to update git submodule or cleanup the CMake cache. Test Plan: See the following error messages Reviewers: Subscribers: Tasks: Tags: [ghstack-poisoned]

Summary: As titled. This adds messages when we suspect the user forgot to update git submodule or cleanup the CMake cache. Test Plan: See the following error messages Reviewers: Subscribers: Tasks: Tags: ghstack-source-id: 4021ed8242b88287aae6ab24b594a00e6332d2a7 Pull Request resolved: #8203

pytorch-bot · 2025-02-05T00:59:03Z

🔗 Helpful Links

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

📄 Preview Python docs built from this PR

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

❗ 1 Active SEVs

There are 1 currently active SEVs. If your PR is affected, please view them below:

ROCM Infra failures during checkout of PyTorch

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

larryliu0820 · 2025-02-05T01:12:42Z

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

Summary: Fixing #7243 As titled. This adds messages when we suspect the user forgot to update git submodule or cleanup the CMake cache. Test Plan: ``` rm -rf third-party/prelude ./install_executorch.sh ``` See the following error messages ``` Command failed: From load at implicit location Caused by: File not found: `prelude//prelude.bzl` CMake Error at build/Utils.cmake:230 (message): executorch: source list generation failed Call Stack (most recent call first): CMakeLists.txt:386 (extract_sources) -- Configuring incomplete, errors occurred! Traceback (most recent call last): File "<string>", line 643, in run File "/home/larryliu/miniconda3/envs/executorch/lib/python3.11/site-packages/setuptools/_distutils/cmd.py", line 388, in spawn spawn(cmd, search_path, dry_run=self.dry_run) File "/home/larryliu/miniconda3/envs/executorch/lib/python3.11/site-packages/setuptools/_distutils/spawn.py", line 68, in spawn raise DistutilsExecError(f"command {cmd!r} failed with exit code {exitcode}") distutils.errors.DistutilsExecError: command '/home/larryliu/miniconda3/envs/executorch/bin/cmake' failed with exit code 1 ... Exception: command '/home/larryliu/miniconda3/envs/executorch/bin/cmake' failed with exit code 1 ExecuTorch: Either CMake cache is outdated or git submodules are not synced. ExecuTorch: Please run the following before retry: ExecuTorch: ./install_executorch.sh --clean ExecuTorch: git submodule update --init --recursive error: subprocess-exited-with-error ``` Reviewers: Subscribers: Tasks: Tags: Differential Revision: [D69156975](https://our.internmc.facebook.com/intern/diff/D69156975) [ghstack-poisoned]

Summary: As titled. This adds messages when we suspect the user forgot to update git submodule or cleanup the CMake cache. Test Plan: See the following error messages Reviewers: Subscribers: Tasks: Tags: ghstack-source-id: 14d8e65e8ca2e2c9b4c0b44369817ee96061c7d6 Pull Request resolved: #8203

setup.py

mergennachin · 2025-02-05T19:11:51Z

What happens the submodule exists but it is out of sync, e.g., you just rebased and the underlying hash changed and the user hasn't updated?

In those cases, we should just warn people as opposed to actually syncing on their behalf.

Summary: Fixing #7243 As titled. This PR adds 2 things: 1. `check_and_update_submodule()` check if required submodule folders exist and at least contains a CMakeLists.txt file. 2. Give useful error when the submodule is corrupted (most likely missing a file, caused by submodule out of sync). Test Plan: 1. Test if we can still install, if submodule is not updated ``` rm -rf third-party/prelude ./install_executorch.sh ``` See the following log ``` ... Processing /data/users/larryliu/executorch Preparing metadata (pyproject.toml): started Running command Preparing metadata (pyproject.toml) 2025-02-05 11:28:46,430 [ExecuTorch] WARNING: Some required submodules are missing. Updating submodules... Submodule path 'third-party/prelude': checked out '851d3f09c452937fc5adef27e2c50f7f304f1646' 2025-02-05 11:28:46,748 [ExecuTorch] INFO: All required submodules are present. 2025-02-05 11:28:47,018 [ExecuTorch] INFO: running dist_info ... ``` This proves that we can update submodule for the user. 2. Test if we can give useful error message, if we are missing a file. ``` rm third-party/gflags/src/gflags.cc ./install_executorch.sh ``` See the following error: Reviewers: Subscribers: Tasks: Tags: Differential Revision: [D69156975](https://our.internmc.facebook.com/intern/diff/D69156975) [ghstack-poisoned]

Summary: As titled. This adds messages when we suspect the user forgot to update git submodule or cleanup the CMake cache. Test Plan: See the following error messages Reviewers: Subscribers: Tasks: Tags: ghstack-source-id: d38b105df4783d58c416a8946db84304c2ddfd01 Pull Request resolved: #8203

Summary: Fixing #7243 As titled. This PR adds 2 things: 1. `check_and_update_submodule()` check if required submodule folders exist and at least contains a CMakeLists.txt file. 2. Give useful error when the submodule is corrupted (most likely missing a file, caused by submodule out of sync). Test Plan: 1. Test if we can still install, if submodule is not updated ``` rm -rf third-party/prelude ./install_executorch.sh ``` See the following log ``` ... Processing /data/users/larryliu/executorch Preparing metadata (pyproject.toml): started Running command Preparing metadata (pyproject.toml) 2025-02-05 11:28:46,430 [ExecuTorch] WARNING: Some required submodules are missing. Updating submodules... Submodule path 'third-party/prelude': checked out '851d3f09c452937fc5adef27e2c50f7f304f1646' 2025-02-05 11:28:46,748 [ExecuTorch] INFO: All required submodules are present. 2025-02-05 11:28:47,018 [ExecuTorch] INFO: running dist_info ... ``` This proves that we can update submodule for the user. 2. Test if we can give useful error message, if we are missing a file. ``` rm third-party/gflags/src/gflags.cc ./install_executorch.sh --clean ./install_executorch.sh ``` See the following error: ``` 2025-02-05 12:08:56,889 [ExecuTorch] ERROR: Failed to query buck for sources. Failed command: buck2 cquery inputs(deps('//runtime/executor:program')) This is likely due to missing git submodules or outdated CMake cache. Please run the following before retry: ./install_executorch.sh --clean git submodule update --init --recursive ``` Reviewers: Subscribers: Tasks: Tags: Differential Revision: [D69156975](https://our.internmc.facebook.com/intern/diff/D69156975) [ghstack-poisoned]

Summary: As titled. This adds messages when we suspect the user forgot to update git submodule or cleanup the CMake cache. Test Plan: See the following error messages Reviewers: Subscribers: Tasks: Tags: ghstack-source-id: 4cbd0699092f8581d791b41593f63a4122b926c0 Pull Request resolved: #8203

larryliu0820 · 2025-02-05T20:29:13Z

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

build/extract_sources.py

setup.py

Summary: Fixing #7243 As titled. This PR adds 2 things: 1. `check_and_update_submodule()` check if required submodule folders exist and at least contains a CMakeLists.txt file. 2. Give useful error when the submodule is corrupted (most likely missing a file, caused by submodule out of sync). Test Plan: 1. Test if we can still install, if submodule is not updated ``` rm -rf third-party/prelude ./install_executorch.sh ``` See the following log ``` ... Processing /data/users/larryliu/executorch Preparing metadata (pyproject.toml): started Running command Preparing metadata (pyproject.toml) 2025-02-05 11:28:46,430 [ExecuTorch] WARNING: Some required submodules are missing. Updating submodules... Submodule path 'third-party/prelude': checked out '851d3f09c452937fc5adef27e2c50f7f304f1646' 2025-02-05 11:28:46,748 [ExecuTorch] INFO: All required submodules are present. 2025-02-05 11:28:47,018 [ExecuTorch] INFO: running dist_info ... ``` This proves that we can update submodule for the user. 2. Test if we can give useful error message, if we are missing a file. ``` rm third-party/gflags/src/gflags.cc ./install_executorch.sh --clean ./install_executorch.sh ``` See the following error: ``` 2025-02-05 12:08:56,889 [ExecuTorch] ERROR: Failed to query buck for sources. Failed command: buck2 cquery inputs(deps('//runtime/executor:program')) This is likely due to missing git submodules or outdated CMake cache. Please run the following before retry: ./install_executorch.sh --clean git submodule update --init --recursive ``` Reviewers: Subscribers: Tasks: Tags: Differential Revision: [D69156975](https://our.internmc.facebook.com/intern/diff/D69156975) [ghstack-poisoned]

Summary: As titled. This adds messages when we suspect the user forgot to update git submodule or cleanup the CMake cache. Test Plan: See the following error messages Reviewers: Subscribers: Tasks: Tags: ghstack-source-id: 6236566b68a6df7d485ef74566d53c67a771eace Pull Request resolved: #8203

larryliu0820 · 2025-02-05T20:38:19Z

@pytorchbot merge

pytorch-bot · 2025-02-05T20:38:23Z

Mergebot is not configured for this repository. Please use the merge button provided by GitHub.

larryliu0820 · 2025-02-05T20:38:25Z

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

Summary: As titled. This adds messages when we suspect the user forgot to update git submodule or cleanup the CMake cache. Test Plan: See the following error messages Reviewers: Subscribers: Tasks: Tags: ghstack-source-id: 6236566b68a6df7d485ef74566d53c67a771eace Pull Request resolved: #8203 Co-authored-by: Mengwei Liu <[email protected]>

dbort · 2025-02-05T22:28:45Z

setup.py

+    if missing_submodules:
+        logger.warning("Some required submodules are missing. Updating submodules...")
+        try:
+            subprocess.check_call(["git", "submodule", "sync"])


I don't like doing this in setup.py because it adds a new assumption that the code lives in git. Until now, we could build from a tarball of the entire tree outside of git. If we were going to do it anywhere I'd prefer doing it in install_executorch or install_requirements: setup.py should not be modifying its environment, only building it.

Summary: Per comment in #8203: #8203 (comment) Moving the logic of checking submodule and update out from setup.py and into install_executorch.py. Test Plan: Reviewers: Subscribers: Tasks: Tags:

* [BE] Move submodule check out from setup.py Summary: Per comment in #8203: #8203 (comment) Moving the logic of checking submodule and update out from setup.py and into install_executorch.py. Test Plan: Reviewers: Subscribers: Tasks: Tags: * lint Summary: Test Plan: Reviewers: Subscribers: Tasks: Tags: * Move submodule handling to before install requirements Summary: Test Plan: Reviewers: Subscribers: Tasks: Tags:

[BE] Remind users to update submodule

ec456c9

Summary: As titled. This adds messages when we suspect the user forgot to update git submodule or cleanup the CMake cache. Test Plan: See the following error messages Reviewers: Subscribers: Tasks: Tags: [ghstack-poisoned]

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 Feb 5, 2025

larryliu0820 added the topic: not user facing label Feb 5, 2025

tarun292 approved these changes Feb 5, 2025

View reviewed changes

mergennachin reviewed Feb 5, 2025

View reviewed changes

setup.py Outdated Show resolved Hide resolved

mergennachin reviewed Feb 5, 2025

View reviewed changes

setup.py Outdated Show resolved Hide resolved

mergennachin reviewed Feb 5, 2025

View reviewed changes

build/extract_sources.py Outdated Show resolved Hide resolved

mergennachin reviewed Feb 5, 2025

View reviewed changes

setup.py Outdated Show resolved Hide resolved

larryliu0820 merged commit 35369e0 into gh/larryliu0820/60/base Feb 5, 2025
41 checks passed

larryliu0820 deleted the gh/larryliu0820/60/head branch February 5, 2025 20:38

larryliu0820 temporarily deployed to cherry-pick-bot February 5, 2025 20:38 — with GitHub Actions Inactive

pytorchbot mentioned this pull request Feb 5, 2025

[BE] Remind users to update submodule #8229

Merged

dbort reviewed Feb 5, 2025

View reviewed changes

larryliu0820 mentioned this pull request Feb 7, 2025

[BE] Move submodule check out from setup.py #8315

Merged

This was referenced Feb 11, 2025

Weekly pr metrics report - 2025-02-01..2025-02-07 wdvr/pytorch#6

Open

Weekly pr metrics report - 2025-02-01..2025-02-07 wdvr/pytorch#8

Open

This was referenced Feb 24, 2025

Weekly pr metrics report - 2025-02-01..2025-02-07 wdvr/pytorch#10

Open

Weekly pr metrics report - 2025-02-01..2025-02-07 wdvr/pytorch#14

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[BE] Remind users to update submodule #8203

[BE] Remind users to update submodule #8203

larryliu0820 commented Feb 5, 2025 •

edited

Loading

pytorch-bot bot commented Feb 5, 2025 •

edited

Loading

larryliu0820 commented Feb 5, 2025

mergennachin commented Feb 5, 2025

larryliu0820 commented Feb 5, 2025

larryliu0820 commented Feb 5, 2025

pytorch-bot bot commented Feb 5, 2025

larryliu0820 commented Feb 5, 2025

dbort Feb 5, 2025

[BE] Remind users to update submodule #8203

[BE] Remind users to update submodule #8203

Conversation

larryliu0820 commented Feb 5, 2025 • edited Loading

pytorch-bot bot commented Feb 5, 2025 • edited Loading

🔗 Helpful Links

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

❗ 1 Active SEVs

larryliu0820 commented Feb 5, 2025

mergennachin commented Feb 5, 2025

larryliu0820 commented Feb 5, 2025

larryliu0820 commented Feb 5, 2025

pytorch-bot bot commented Feb 5, 2025

larryliu0820 commented Feb 5, 2025

dbort Feb 5, 2025

Choose a reason for hiding this comment

larryliu0820 commented Feb 5, 2025 •

edited

Loading

pytorch-bot bot commented Feb 5, 2025 •

edited

Loading