Skip to content

Commit

Permalink
Build the wheel, install it on a separate machine, run the tests. Pro…
Browse files Browse the repository at this point in the history
…fit. (#30)

Summary:
This PR adds the following jobs to our OSS CI:

- Build a wheel (these are the same wheel(s) that would be updated on PyPI eventually)
- Save that wheel as an artifact (see `actions/upload-artifact@v4`)
- Spin up a new machine, download the wheel that was previously built
- Install `torchcodec` from that wheel: that's exactly what users will eventually do when they `pip install torchcodec` from PyPI
- Install FFmpeg - this can be done at any point, but we do it here to ensure there's no build/install dependency on FFmpeg
- Run the torchcodec tests to ensure the wheel installation went fine.

If this works, it means we're 90% done with the packaging on x86 Linux.

There is some overlap with the already-existing "Unit Test / python" job. I think we can get rid of those, but I'll leave that for a follow-up PR.

Pull Request resolved: #30

Reviewed By: ahmadsharif1

Differential Revision: D58592017

Pulled By: NicolasHug

fbshipit-source-id: d6cb6f8ca24144bb55e17353914057306340a10c
  • Loading branch information
NicolasHug authored and facebook-github-bot committed Jun 14, 2024
1 parent 94f3151 commit 1bc888d
Show file tree
Hide file tree
Showing 3 changed files with 135 additions and 5 deletions.
7 changes: 7 additions & 0 deletions .github/scripts/assert_ffmpeg_not_installed.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
#!/usr/bin/env bash

if command -v "ffmpeg" &> /dev/null
then
echo "ffmpeg is installed, but it shouldn't! Exiting!!"
exit 1
fi
6 changes: 1 addition & 5 deletions .github/workflows/unit_test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,7 @@ jobs:
python -m pip install --pre torch torchvision --index-url https://download.pytorch.org/whl/nightly/cpu
- name: Build and install torchcodec
run: |
if command -v "ffmpeg" &> /dev/null
then
echo "ffmpeg is installed, but it shouldn't! Exiting!!"
exit 1
fi
.github/scripts/assert_ffmpeg_not_installed.sh
# TODO: should we pass -DCMAKE_BUILD_TYPE=Debug here? That's what we
# do for the C++ tests.
BUILD_AGAINST_ALL_FFMPEG_FROM_S3=1 python -m pip install -e ".[dev]" --no-build-isolation -vvv
Expand Down
127 changes: 127 additions & 0 deletions .github/workflows/wheel.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
name: Build wheel

on:
push:
branches: [ main ]
pull_request:

concurrency:
group: unit-test${{ github.workflow }}-${{ github.ref == 'refs/heads/main' && github.run_number || github.ref }}
cancel-in-progress: true

defaults:
run:
shell: bash -l -eo pipefail {0}

jobs:
build:
runs-on: ubuntu-latest
strategy:
fail-fast: false
matrix:
python-version: ['3.8', '3.12']
steps:
- name: Check out repo
uses: actions/checkout@v3
- name: Setup conda env
uses: conda-incubator/setup-miniconda@v2
with:
auto-update-conda: true
miniconda-version: "latest"
activate-environment: test
python-version: ${{ matrix.python-version }}
- name: Update pip
run: python -m pip install --upgrade pip
- name: Install Pytorch
run: |
python -m pip install --pre torch --index-url https://download.pytorch.org/whl/nightly/cpu
- name: Build the torchcodec wheel
run: |
# Just for sanity, make sure FFmpeg isn't installed or needed for buidling.
.github/scripts/assert_ffmpeg_not_installed.sh
# TODO: Need to make sure the wheel only contains torchcodec and not
# the other folders (tests, docs, etc.)
python -m pip install build
BUILD_AGAINST_ALL_FFMPEG_FROM_S3=1 python -m build . --wheel -vvv --no-isolation
- name: Show the .so files in the wheel
run: |
# This should show libtorchcodec4.so, libtorchcodec5.so, etc.
wheel_path=`find dist -type f`
unzip -l $wheel_path | grep .so
- uses: actions/upload-artifact@v4
with:
name: sdist-linux_x86_${{ matrix.python-version }}
path: dist/*

install-and-test:
runs-on: ubuntu-latest
strategy:
fail-fast: false
matrix:
python-version: ['3.8', '3.12']
ffmpeg-version-for-tests: ['4.4.2', '5.1.2', '6.1.1', '7.0.1']
if: ${{ always() }}
needs: build
steps:
- uses: actions/download-artifact@v4
with:
name: sdist-linux_x86_${{ matrix.python-version }}
path: dist/
- name: Setup conda env
uses: conda-incubator/setup-miniconda@v2
with:
auto-update-conda: true
miniconda-version: "latest"
activate-environment: test
python-version: ${{ matrix.python-version }}
- name: Update pip
run: python -m pip install --upgrade pip
- name: Install PyTorch
run: |
python -m pip install --pre torch --index-url https://download.pytorch.org/whl/nightly/cpu
- name: Install torchcodec from the wheel
run: |
wheel_path=`find dist -type f`
echo Installing $wheel_path
python -m pip install $wheel_path
- name: Check out repo
uses: actions/checkout@v3
- name: Install ffmpeg, post build
run: |
# Ideally we would have checked for that before installing the wheel,
# but we need to checkout the repo to access this file, and we don't
# want to checkout the repo before installing the wheel to avoid any
# side-effect. It's OK.
.github/scripts/assert_ffmpeg_not_installed.sh
conda install "ffmpeg=${{ matrix.ffmpeg-version-for-tests }}" -c conda-forge
ffmpeg -version
- name: Install test dependencies
run: |
python -m pip install --pre torchvision --index-url https://download.pytorch.org/whl/nightly/cpu
# TODO: Find a way to get those dependencies from pyproject.toml
python -m pip install numpy pytest pillow opencv-python
- name: Delete the src/ folder just for fun
run: |
# The only reason we checked-out the repo is to get access to the
# tests. We don't care about the rest. Out of precaution, we delete
# the src/ folder to be extra sure that we're running the code from
# the installed wheel rather than from the source.
# This is just to be extra cautious and very overkill because a)
# there's no way the `torchcodec` package from src/ can be found from
# the PythonPath: the main point of `src/` is precisely to protect
# against that and b) if we ever were to execute code from
# `src/torchcodec`, it would fail loudly because the built .so files
# aren't present there.
rm -r src/
ls
- name: Smoke test
run: |
python test/decoders/manual_smoke_test.py
- name: Run Python tests
run: |
pytest test

0 comments on commit 1bc888d

Please sign in to comment.