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.

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

Build wheels on test-infra machines #160

Merged
merged 70 commits into from
Aug 7, 2024

Conversation

NicolasHug
Copy link
Member

@NicolasHug NicolasHug commented Aug 7, 2024

This PR moves our linux wheel building process from the default GA machines to the pytorch infra machine from test-infra. Testing is unchanged: we download the built wheels artifacts, install the wheels and run the tests on separate machines.

This moves ensures that torchcodec is built on the exact same infra as pytorch.

Additionally, this PR adds a renaming step for the wheels: we rename the wheels from "linux_x86_64.whl" to "manylinux_2_17_x86_64.manylinux2014_x86_64.whl" so that they can be properly uploaded on PyPI. In general that renaming step should be done with tools like auditwheel. We don't do that and instead do the renaming ourselves. There are multiple reasons to that:

  • auditwheel would by default try to vendor every single dynamic library that we depend on. That includes all of the ffmpeg libraries. We don't want to ship any ffmpeg library because we want users to be able to install their own. Using auditwheel would force us to use the following command, whose complexity hopefully speaks for itself
auditwheel repair --plat manylinux_2_17_x86_64 dist/* --exclude libtorch.so --exclude libtorch_cpu.so --exclude libc10.so --exclude libavutil.so.56 --exclude libavcodec.so.58 --exclude libavformat.so.58 --exclude libavdevice.so.58 --exclude libavfilter.so.7 --exclude libavutil.so.57 --exclude libavcodec.so.59 --exclude libavformat.so.59 --exclude libavdevice.so.59 --exclude libavfilter.so.8 --exclude libavutil.so.58 --exclude libavcodec.so.60 --exclude libavformat.so.60 --exclude libavdevice.so.60 --exclude libavfilter.so.9 --exclude libavutil.so.59 --exclude libavcodec.so.61 --exclude libavformat.so.61 --exclude libavdevice.so.61 --exclude libavfilter.so.10
  • pytorch doesn't use auditwheel itself, it just does a hard rename like we do here
  • The point of auditwheel is to ensure that the wheel doesn't depend on libraries or version symbols that are too new. We already ensure that ourselves with our custom check_glibcxx.py check.


install-and-test:
runs-on: ubuntu-latest
strategy:
fail-fast: false
matrix:
python-version: ['3.8', '3.12']
python-version: ['3.9']
Copy link
Member Author

Choose a reason for hiding this comment

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

We move from 3.8-3.12 to just 3.9.

  • We can't use 3.8 anymore because test-infra doesn't support it.
  • on top of that, we can only use 3.9 here (for now) because by default, test-infra will only generate job on 3.9 on PRs. Generating job for all python versions can be done by labeling the PR with ciflow/binaries/all (I will eventually document this). What we should be doing here is letting this install-and-test job rely on the exact same python versions that were generated by the generate-matrix job. I haven't found a way to do that yet, but this should be doable, possibly requirement extensions to test-infra.

For now though, this is OK. We will still be able to checks all necessary python version when we push a new release.

Copy link
Member Author

Choose a reason for hiding this comment

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

These are the same sanity checks that we used to run before, with the addition of the wheel renaming at the end.

@scotts scotts merged commit f023db8 into pytorch:main Aug 7, 2024
17 checks passed
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 Meta Open Source bot.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants