Skip to content
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.

Sign up for GitHub

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

Jump to bottom

stop double-installing ExecuTorch in one-off linux jobs #7578

Closed
wants to merge 10 commits into from
Closed

stop double-installing ExecuTorch in one-off linux jobs #7578

wants to merge 10 commits into from

Conversation

swolchok
Copy link
Contributor

@swolchok swolchok commented Jan 9, 2025 •
edited
Loading

@swolchok
stop double-installing ExecuTorch in one-off linux jobs
1a77767 
setup-linux.sh already installs ExecuTorch with XNNPACK (and it passes use-pt-pinned-commit as it should).

Differential Revision: [D67996460](https://our.internmc.facebook.com/intern/diff/D67996460/)

[ghstack-poisoned]
@pytorch-bot PyTorch Bot
Copy link

pytorch-bot bot commented Jan 9, 2025 •
edited
Loading

🔗 Helpful Links

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

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

❌ 1 New Failure, 2 Cancelled Jobs

As of commit e1862fc with merge base 8494b90 (image):

NEW FAILURE - The following job has failed:

CANCELLED JOBS - The following jobs were cancelled. Please retry:

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 Jan 9, 2025
@swolchok swolchok mentioned this pull request Jan 9, 2025
Open
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D67996460

This was referenced Jan 9, 2025
Merged
Draft
@swolchok swolchok added the release notes: build Changes related to build, including dependency upgrades, build flags, optimizations, etc. label Jan 9, 2025
@swolchok swolchok requested review from helunwencser, digantdesai and larryliu0820 and removed request for helunwencser and digantdesai January 9, 2025 23:53
Update on "stop double-installing ExecuTorch in one-off linux jobs"
9f523e5 
setup-linux.sh already installs ExecuTorch with XNNPACK (and it passes use-pt-pinned-commit as it should).

Differential Revision: [D67996460](https://our.internmc.facebook.com/intern/diff/D67996460/)

[ghstack-poisoned]
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D67996460

Update on "stop double-installing ExecuTorch in one-off linux jobs"
db84935 
setup-linux.sh already installs ExecuTorch with XNNPACK (and it passes use-pt-pinned-commit as it should).

Differential Revision: [D67996460](https://our.internmc.facebook.com/intern/diff/D67996460/)

[ghstack-poisoned]
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D67996460

Update on "stop double-installing ExecuTorch in one-off linux jobs"
981173c 
setup-linux.sh already installs ExecuTorch with XNNPACK (and it passes use-pt-pinned-commit as it should).

Differential Revision: [D67996460](https://our.internmc.facebook.com/intern/diff/D67996460/)

[ghstack-poisoned]
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D67996460

Update on "stop double-installing ExecuTorch in one-off linux jobs"
7ffed3b 
setup-linux.sh already installs ExecuTorch with XNNPACK (and it passes use-pt-pinned-commit as it should).

Differential Revision: [D67996460](https://our.internmc.facebook.com/intern/diff/D67996460/)

[ghstack-poisoned]
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D67996460

Update on "stop double-installing ExecuTorch in one-off linux jobs"
a854f7f 
setup-linux.sh already installs ExecuTorch with XNNPACK (and it passes use-pt-pinned-commit as it should).

Differential Revision: [D67996460](https://our.internmc.facebook.com/intern/diff/D67996460/)

[ghstack-poisoned]
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D67996460

Update on "stop double-installing ExecuTorch in one-off linux jobs"
eb186b6 
setup-linux.sh already installs ExecuTorch with XNNPACK (and it passes use-pt-pinned-commit as it should).

Differential Revision: [D67996460](https://our.internmc.facebook.com/intern/diff/D67996460/)

[ghstack-poisoned]
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D67996460

@swolchok swolchok mentioned this pull request Jan 13, 2025
Open
@swolchok swolchok mentioned this pull request Jan 13, 2025
Open
@swolchok
Update on "stop double-installing ExecuTorch in one-off linux jobs"
483f67a 
setup-linux.sh already installs ExecuTorch with XNNPACK (and it passes use-pt-pinned-commit as it should).

Differential Revision: [D67996460](https://our.internmc.facebook.com/intern/diff/D67996460/)

[ghstack-poisoned]
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D67996460

@swolchok
Update on "stop double-installing ExecuTorch in one-off linux jobs"
d09a372 
setup-linux.sh already installs ExecuTorch with XNNPACK (and it passes use-pt-pinned-commit as it should).

Differential Revision: [D67996460](https://our.internmc.facebook.com/intern/diff/D67996460/)

[ghstack-poisoned]
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D67996460

@swolchok
Update on "stop double-installing ExecuTorch in one-off linux jobs"
e1862fc 
setup-linux.sh already installs ExecuTorch with XNNPACK (and it passes use-pt-pinned-commit as it should).

Differential Revision: [D67996460](https://our.internmc.facebook.com/intern/diff/D67996460/)

[ghstack-poisoned]
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D67996460

@swolchok
Copy link
Contributor Author

I don't know why, but this is apparently causing timeouts for affected jobs. Any ideas? @digantdesai @dbort @larryliu0820 @mergennachin

dbort
dbort reviewed Jan 22, 2025
View reviewed changes
.github/workflows/pull.yml
@@ -199,9 +199,6 @@ jobs:

PYTHON_EXECUTABLE=python bash .ci/scripts/setup-linux.sh "cmake"

# install pybind
Copy link
Contributor

Choose a reason for hiding this comment

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

I agree that we should stop double-installing, but I'd much rather remove the bespoke CI-only installation stuff in favor of install_requirements. I think that if we remove these calls to install_requirements, then none (or very few) of the CI jobs will actually exercise install_requirements. And removing the CI-only logic would reduce overall maintenance costs.

@swolchok
Copy link
Contributor Author

abandoning in favor of #7922

@swolchok swolchok closed this Jan 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dbort dbort dbort left review comments

@digantdesai digantdesai digantdesai approved these changes

@helunwencser helunwencser Awaiting requested review from helunwencser

@larryliu0820 larryliu0820 Awaiting requested review from larryliu0820

Assignees
No one assigned
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported release notes: build Changes related to build, including dependency upgrades, build flags, optimizations, etc.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@swolchok @facebook-github-bot @digantdesai @dbort