Skip to content

Clean up install scripts #4826

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 1 commit into from
Aug 23, 2024
Merged

Conversation

lucylq
Copy link
Contributor

@lucylq lucylq commented Aug 21, 2024

Now that setup-macos.sh and setup-linux.sh are using install_requirements, we can remove installing flatc from source, pytorch and pytorch domains install from them.

Copy link

pytorch-bot bot commented Aug 21, 2024

🔗 Helpful Links

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

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

✅ No Failures

As of commit bed6b96 with merge base ce4917c (image):
💚 Looks good so far! There are no failures yet. 💚

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 Aug 21, 2024
@lucylq lucylq force-pushed the lfq.clean-install-scripts branch 2 times, most recently from 481201d to bad7fc4 Compare August 22, 2024 17:28
@lucylq lucylq force-pushed the lfq.clean-install-scripts branch from bad7fc4 to bed6b96 Compare August 22, 2024 17:38
@lucylq lucylq marked this pull request as ready for review August 22, 2024 17:39
@lucylq lucylq requested review from larryliu0820 and dbort August 22, 2024 17:39
@facebook-github-bot
Copy link
Contributor

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

Copy link
Contributor

@larryliu0820 larryliu0820 left a comment

Choose a reason for hiding this comment

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

Thanks for the cleanup!

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.

This is great! :D I'm so glad to have the CI scripts using install_requirements now -- it means that this crucial script is actually tested, and it means that the CI jobs use the same environment that users will use.

@facebook-github-bot facebook-github-bot merged commit bf71fd4 into main Aug 23, 2024
113 checks passed
@facebook-github-bot facebook-github-bot deleted the lfq.clean-install-scripts branch August 23, 2024 22:52
huydhn added a commit that referenced this pull request Jan 9, 2025
huydhn added a commit that referenced this pull request Jan 9, 2025
* Fix the use of PyTorch pin commit in MacOS

* Fix typo

* Bring back install_pytorch_and_domains removed by #4826

* Bring back more changes from #4826

* Could it be this flag

* Try again

* Try this flag then

* Another attempt
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ciflow/trunk CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants