-
Notifications
You must be signed in to change notification settings - Fork 169
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
Update pre_build_script.sh #390
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/ao/390
Note: Links to docs will display an error until the docs builds have been completed. ❗ 1 Active SEVsThere are 1 currently active SEVs. If your PR is affected, please view them below: ✅ You can merge normally! (2 Unrelated Failures)As of commit 5b3a639 with merge base 1b78dda (): FLAKY - The following job failed but was likely due to flakiness present on trunk:
BROKEN TRUNK - The following job failed but was present on the merge base:👉 Rebase onto the `viable/strict` branch to avoid these failures
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be good to add a bit more explanation, but sure, if it fixes the builds..
* Update pre_build_script.sh * Update smoke_test.py * Update post_build_script.sh * Update post_build_script.sh
When we merged #369 we completely got rid of any
requirements.txt
we had because we have no dependencies outside of torch. The torch dependency is also pulled defined directly inside asetup.py
because it's an environment variable that will decide which version of pytorch we'll install.This passed CI just fine but then when the script was merged to main and the binary builds were triggered it broke all our binary builds
This is because in our binary build scripts in
packaging/
we still runpip install -r requirements.txt
and we also need to installpip install -r dev-requirements.txt
because later in our post build validation we check we runpytest
. Since therequirements.txt
no longer exists the script fails.The reason why we didn't catch this in CI is because we don't run binary validation scripts per commit but we run it when code gets merged to main
In the future before making CI changes it's worth triggering a manual
workflow-dispatch
to make sure this change is safe to landDoing a manual dispatch to make sure this passes before merging as I've done here https://github.com/pytorch/ao/actions/workflows/build_wheels_linux.yml