Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Feb 7, 2025
Merged

Conversation

larryliu0820
Copy link
Contributor

@larryliu0820 larryliu0820 commented Feb 7, 2025

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:

Do a clean checkout

git clone https://github.com/pytorch/executorch.git

Run install_executorch.sh immediately after clone is done

./install_executorch.sh

Reviewers:

Subscribers:

Tasks:

Tags:

Summary

[PLEASE REMOVE] See CONTRIBUTING.md's Pull Requests for ExecuTorch PR guidelines.

[PLEASE REMOVE] If this PR closes an issue, please add a Fixes #<issue-id> line.

[PLEASE REMOVE] If this PR introduces a fix or feature that should be the upcoming release notes, please add a "Release notes: " label. For a list of available release notes labels, check out CONTRIBUTING.md's Pull Requests.

Test plan

[PLEASE REMOVE] How did you test this PR? Please write down any manual commands you used and note down tests that you have written if applicable.

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:
@larryliu0820 larryliu0820 requested a review from dbort February 7, 2025 19:34
Copy link

pytorch-bot bot commented Feb 7, 2025

🔗 Helpful Links

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

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

❌ 1 New Failure, 2 Pending

As of commit 5192049 with merge base 883d33a (image):

NEW FAILURE - The following job has failed:

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

@facebook-github-bot 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 7, 2025
Summary:

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:
Summary:

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:
Copy link
Contributor

@dbort dbort left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for moving this out of setup.py! This looks like exactly the right spot for it.

@larryliu0820 larryliu0820 added the release notes: build Changes related to build, including dependency upgrades, build flags, optimizations, etc. label Feb 7, 2025
@larryliu0820 larryliu0820 merged commit 2c8e706 into main Feb 7, 2025
45 of 46 checks passed
@larryliu0820 larryliu0820 deleted the submodule_install branch February 7, 2025 21:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. release notes: build Changes related to build, including dependency upgrades, build flags, optimizations, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants