From 977c1f5cf013ca8c8fd861e9b06b64ceda9a385c Mon Sep 17 00:00:00 2001 From: Jeremy Leibs Date: Thu, 13 Apr 2023 17:43:43 +0200 Subject: [PATCH 01/10] Install the rerun-sdk by the expected version --- .github/workflows/python.yml | 7 ++++++- scripts/version_util.py | 6 ++++-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/.github/workflows/python.yml b/.github/workflows/python.yml index 4dd13a4833c4..faeae1202ed9 100644 --- a/.github/workflows/python.yml +++ b/.github/workflows/python.yml @@ -194,6 +194,11 @@ jobs: run: | python3 scripts/version_util.py --check_version + - name: Store the expected version + # This call to version_util.py will assert version from Cargo.toml matches git tagged version vX.Y.Z + run: | + python3 scripts/version_util.py --expected_version >> $GITHUB_ENV + - name: Build Wheel uses: PyO3/maturin-action@v1 with: @@ -212,7 +217,7 @@ jobs: - name: Install built wheel shell: bash run: | - pip install pre-dist/*${{ matrix.wheel_suffix }}.whl --force-reinstall + pip install rerun-sdk==${{ env.expected_version }} --find-links pre-dist --force-reinstall - name: Verify built wheel version shell: bash diff --git a/scripts/version_util.py b/scripts/version_util.py index 08792ad481af..61502cb23eb6 100755 --- a/scripts/version_util.py +++ b/scripts/version_util.py @@ -89,10 +89,10 @@ def main() -> None: cargo_toml = f.read() cargo_version = get_cargo_version(cargo_toml) + date = datetime.now().strftime("%Y-%m-%d") + new_version = f"{cargo_version}+{date}-{get_git_sha()}" if sys.argv[1] == "--patch_prerelease": - date = datetime.now().strftime("%Y-%m-%d") - new_version = f"{cargo_version}+{date}-{get_git_sha()}" new_cargo_toml = patch_cargo_version(cargo_toml, new_version) # Write the patched Cargo.toml back to disk @@ -106,6 +106,8 @@ def main() -> None: f"Version number in Cargo.toml ({cargo_version}) does not match tag version ({ref_version})" ) print(f"Version numbers match: {cargo_version} == {ref_version}") + elif sys.argv[1] == "--expected_version": + print(f"expected_version={new_version}") else: raise Exception("Invalid argument") From 7dac6f2047023f5426e6f70826f891da43636684 Mon Sep 17 00:00:00 2001 From: Jeremy Leibs Date: Thu, 13 Apr 2023 17:52:02 +0200 Subject: [PATCH 02/10] Fix comment --- .github/workflows/python.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/python.yml b/.github/workflows/python.yml index faeae1202ed9..1574ff3b2dcb 100644 --- a/.github/workflows/python.yml +++ b/.github/workflows/python.yml @@ -195,7 +195,7 @@ jobs: python3 scripts/version_util.py --check_version - name: Store the expected version - # This call to version_util.py will assert version from Cargo.toml matches git tagged version vX.Y.Z + # This call to version_util.py store the expected version number in the github env `expected_version` run: | python3 scripts/version_util.py --expected_version >> $GITHUB_ENV From d809bcc602efde5912787ca0b95f9a9795485496 Mon Sep 17 00:00:00 2001 From: Jeremy Leibs Date: Thu, 13 Apr 2023 17:53:46 +0200 Subject: [PATCH 03/10] typo --- .github/workflows/python.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/python.yml b/.github/workflows/python.yml index 1574ff3b2dcb..c06d6b87137a 100644 --- a/.github/workflows/python.yml +++ b/.github/workflows/python.yml @@ -195,7 +195,7 @@ jobs: python3 scripts/version_util.py --check_version - name: Store the expected version - # This call to version_util.py store the expected version number in the github env `expected_version` + # This call to version_util.py store the expected version number in the GitHub env `expected_version` run: | python3 scripts/version_util.py --expected_version >> $GITHUB_ENV From bc48bfd8205fda1329fe0e9e4e2a4feee2c4649f Mon Sep 17 00:00:00 2001 From: Jeremy Leibs Date: Thu, 13 Apr 2023 17:57:51 +0200 Subject: [PATCH 04/10] Use --no-index when installing the rerun wheel --- .github/workflows/python.yml | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/.github/workflows/python.yml b/.github/workflows/python.yml index c06d6b87137a..f49643322a0b 100644 --- a/.github/workflows/python.yml +++ b/.github/workflows/python.yml @@ -215,9 +215,14 @@ jobs: --out pre-dist - name: Install built wheel + # First we install the dependencies manually so we can use `--no-index` when installing the + # main rerun wheel to guarantee we only get it from the pre-dist folder. We additionally + # specify the version explicitly to make sure we have built what we expected. + # TODO(jleibs): pull these deps from pyproject.toml shell: bash run: | - pip install rerun-sdk==${{ env.expected_version }} --find-links pre-dist --force-reinstall + pip install deprecated numpy>=1.23 pyarrow==10.0.1 + pip install rerun-sdk==${{ env.expected_version }} --no-index --find-links pre-dist --force-reinstall - name: Verify built wheel version shell: bash From afaa78d7a0dd3cb66edf4ad9a2ad93963573e736 Mon Sep 17 00:00:00 2001 From: Jeremy Leibs Date: Thu, 13 Apr 2023 18:56:08 +0200 Subject: [PATCH 05/10] Use the cargo_version, not the new_version --- scripts/version_util.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/scripts/version_util.py b/scripts/version_util.py index 61502cb23eb6..d14339c57c5b 100755 --- a/scripts/version_util.py +++ b/scripts/version_util.py @@ -89,10 +89,10 @@ def main() -> None: cargo_toml = f.read() cargo_version = get_cargo_version(cargo_toml) - date = datetime.now().strftime("%Y-%m-%d") - new_version = f"{cargo_version}+{date}-{get_git_sha()}" if sys.argv[1] == "--patch_prerelease": + date = datetime.now().strftime("%Y-%m-%d") + new_version = f"{cargo_version}+{date}-{get_git_sha()}" new_cargo_toml = patch_cargo_version(cargo_toml, new_version) # Write the patched Cargo.toml back to disk @@ -107,7 +107,9 @@ def main() -> None: ) print(f"Version numbers match: {cargo_version} == {ref_version}") elif sys.argv[1] == "--expected_version": - print(f"expected_version={new_version}") + # We always expect the version built to match what is in cargo.toml + # NOTE: what is in Cargo.toml may have been modified by an earlier call to `--patch-prerelease` + print(f"expected_version={cargo_version}") else: raise Exception("Invalid argument") From a9f78be73ea00dae9a454442084491e5a142d185 Mon Sep 17 00:00:00 2001 From: Jeremy Leibs Date: Thu, 13 Apr 2023 19:41:11 +0200 Subject: [PATCH 06/10] Split dependency install into its own step --- .github/workflows/python.yml | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/.github/workflows/python.yml b/.github/workflows/python.yml index f49643322a0b..7ef76e52cb9f 100644 --- a/.github/workflows/python.yml +++ b/.github/workflows/python.yml @@ -214,14 +214,20 @@ jobs: --features pypi --out pre-dist - - name: Install built wheel - # First we install the dependencies manually so we can use `--no-index` when installing the - # main rerun wheel to guarantee we only get it from the pre-dist folder. We additionally - # specify the version explicitly to make sure we have built what we expected. + - name: Install wheel dependencies + # First we install the dependencies manually so we can use `--no-index` when installing the wheel. + # This needs to be a separate step for some reason or the following step fails # TODO(jleibs): pull these deps from pyproject.toml + # TODO(jleibs): understand why deps can't be installed in the same step as the wheel shell: bash run: | pip install deprecated numpy>=1.23 pyarrow==10.0.1 + + - name: Install built wheel + # Now install the wheel using a specific version and --no-index to guarantee we get the version from + # the pre-dist folder. + shell: bash + run: | pip install rerun-sdk==${{ env.expected_version }} --no-index --find-links pre-dist --force-reinstall - name: Verify built wheel version From 0e10deabfde04fc47da5aeb57760530048b5cb23 Mon Sep 17 00:00:00 2001 From: Jeremy Leibs Date: Thu, 13 Apr 2023 19:56:55 +0200 Subject: [PATCH 07/10] Don't use force-reinstall --- .github/workflows/python.yml | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/.github/workflows/python.yml b/.github/workflows/python.yml index 7ef76e52cb9f..9bf222a397c8 100644 --- a/.github/workflows/python.yml +++ b/.github/workflows/python.yml @@ -225,10 +225,12 @@ jobs: - name: Install built wheel # Now install the wheel using a specific version and --no-index to guarantee we get the version from - # the pre-dist folder. + # the pre-dist folder. Note we don't use --force-reinstall here because --no-index means it wouldn't + # find the dependencies to reinstall them. shell: bash run: | - pip install rerun-sdk==${{ env.expected_version }} --no-index --find-links pre-dist --force-reinstall + pip uninstall rerun-sdk + pip install rerun-sdk==${{ env.expected_version }} --no-index --find-links pre-dist - name: Verify built wheel version shell: bash From dd158dd7263966565f95706d4a3dcc8281d82b5f Mon Sep 17 00:00:00 2001 From: Jeremy Leibs Date: Thu, 13 Apr 2023 20:09:17 +0200 Subject: [PATCH 08/10] Refactor setting of expected_version variable. --- .github/workflows/python.yml | 4 ++-- scripts/version_util.py | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/.github/workflows/python.yml b/.github/workflows/python.yml index 9bf222a397c8..9f2abb4d32f5 100644 --- a/.github/workflows/python.yml +++ b/.github/workflows/python.yml @@ -195,9 +195,9 @@ jobs: python3 scripts/version_util.py --check_version - name: Store the expected version - # This call to version_util.py store the expected version number in the GitHub env `expected_version` + # Find the current cargo version and store it in the GITHUB_ENV var: `expected_version` run: | - python3 scripts/version_util.py --expected_version >> $GITHUB_ENV + echo "expected_version=$(python3 scripts/version_util.py --bare_cargo_version)" >> $GITHUB_ENV - name: Build Wheel uses: PyO3/maturin-action@v1 diff --git a/scripts/version_util.py b/scripts/version_util.py index d14339c57c5b..52f555966f23 100755 --- a/scripts/version_util.py +++ b/scripts/version_util.py @@ -106,10 +106,10 @@ def main() -> None: f"Version number in Cargo.toml ({cargo_version}) does not match tag version ({ref_version})" ) print(f"Version numbers match: {cargo_version} == {ref_version}") - elif sys.argv[1] == "--expected_version": - # We always expect the version built to match what is in cargo.toml - # NOTE: what is in Cargo.toml may have been modified by an earlier call to `--patch-prerelease` - print(f"expected_version={cargo_version}") + elif sys.argv[1] == "--bare_cargo_version": + # Print the bare cargo version. NOTE: do not add additional formatting here. This output + # is expected to be fed into an environment variable. + print(f"{cargo_version}") else: raise Exception("Invalid argument") From 2b926bb98b24655e95e831d6a39442ae092cfc1e Mon Sep 17 00:00:00 2001 From: Jeremy Leibs Date: Thu, 13 Apr 2023 20:14:11 +0200 Subject: [PATCH 09/10] Use bash when setting env --- .github/workflows/python.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/python.yml b/.github/workflows/python.yml index 9f2abb4d32f5..2067b0801fcf 100644 --- a/.github/workflows/python.yml +++ b/.github/workflows/python.yml @@ -196,6 +196,7 @@ jobs: - name: Store the expected version # Find the current cargo version and store it in the GITHUB_ENV var: `expected_version` + shell: bash run: | echo "expected_version=$(python3 scripts/version_util.py --bare_cargo_version)" >> $GITHUB_ENV From a86ff05469d6b7a7171bf51fca96df2ca0037b2d Mon Sep 17 00:00:00 2001 From: Jeremy Leibs Date: Fri, 14 Apr 2023 05:05:02 +0200 Subject: [PATCH 10/10] Always run the linux job first and use its rrds for the other wheels --- .github/workflows/python.yml | 227 +++++++++++++++++++++++++++-------- 1 file changed, 179 insertions(+), 48 deletions(-) diff --git a/.github/workflows/python.yml b/.github/workflows/python.yml index 2067b0801fcf..3d31474d0933 100644 --- a/.github/workflows/python.yml +++ b/.github/workflows/python.yml @@ -63,7 +63,167 @@ jobs: just py-requirements # --------------------------------------------------------------------------- + # We need one wheel-build to be special so the other builds (namely mac arm) can use its rrd + # This copy-paste is awful, but we'll refactor the build soon. + wheels-linux: + if: github.event_name == 'push' || github.event.inputs.force_build_wheel + name: Build Python Wheels (Linux) + runs-on: ubuntu-latest + container: + image: rerunio/ci_docker:0.6 + steps: + - uses: actions/checkout@v3 + + # These should already be in the docker container, but run for good measure. A no-op install + # should be fast, and this way things don't break if we add new packages without rebuilding + # docker + - name: Cache APT Packages + uses: awalsh128/cache-apt-pkgs-action@v1.2.2 + with: + packages: ${{ env.UBUNTU_REQUIRED_PKGS }} + version: 2.0 # Increment this to pull newer packages + execute_install_scripts: true + + - name: Set up cargo cache + uses: Swatinem/rust-cache@v2 + with: + env-vars: CARGO CC CFLAGS CXX CMAKE RUST CACHE_KEY + # Don't update the cache -- it will be updated by the lint job + # TODO(jleibs): this job will likely run before rust.yml updates + # the cache. Better cross-job sequencing would be nice here + save-if: False + + # These should already be in the docker container, but run for good measure. A no-op install + # should be fast, and this way things don't break if we add new packages without rebuilding + # docker + - run: pip install -r rerun_py/requirements-build.txt + + # ---------------------------------------------------------------------------------- + + - name: Patch Cargo.toml for pre-release + if: github.ref == 'refs/heads/main' + # After patching the pre-release version, run cargo update. + # This updates the cargo.lock file with the new version numbers and keeps the wheel build from failing + run: | + python3 scripts/version_util.py --patch_prerelease + cargo update -w + + - name: Version check for tagged-release + if: startsWith(github.ref, 'refs/tags/v') + # This call to version_util.py will assert version from Cargo.toml matches git tagged version vX.Y.Z + run: | + python3 scripts/version_util.py --check_version + + - name: Store the expected version + # Find the current cargo version and store it in the GITHUB_ENV var: `expected_version` + shell: bash + run: | + echo "expected_version=$(python3 scripts/version_util.py --bare_cargo_version)" >> $GITHUB_ENV + - name: Build Wheel + uses: PyO3/maturin-action@v1 + with: + maturin-version: "0.14.10" + manylinux: manylinux_2_31 + container: off + command: build + args: | + --manifest-path rerun_py/Cargo.toml + --release + --target x86_64-unknown-linux-gnu + --no-default-features + --features pypi + --out pre-dist + + - name: Install wheel dependencies + # First we install the dependencies manually so we can use `--no-index` when installing the wheel. + # This needs to be a separate step for some reason or the following step fails + # TODO(jleibs): pull these deps from pyproject.toml + # TODO(jleibs): understand why deps can't be installed in the same step as the wheel + shell: bash + run: | + pip install deprecated numpy>=1.23 pyarrow==10.0.1 + + - name: Install built wheel + # Now install the wheel using a specific version and --no-index to guarantee we get the version from + # the pre-dist folder. Note we don't use --force-reinstall here because --no-index means it wouldn't + # find the dependencies to reinstall them. + shell: bash + run: | + pip uninstall rerun-sdk + pip install rerun-sdk==${{ env.expected_version }} --no-index --find-links pre-dist + + - name: Verify built wheel version + shell: bash + run: | + python3 -m rerun --version + which rerun + rerun --version + + - name: Run unit tests + shell: bash + run: cd rerun_py/tests && pytest + + - name: Install requriements for e2e test + run: | + pip install -r examples/python/api_demo/requirements.txt + pip install -r examples/python/car/requirements.txt + pip install -r examples/python/multithreading/requirements.txt + pip install -r examples/python/plots/requirements.txt + pip install -r examples/python/text_logging/requirements.txt + + - name: Run e2e test + shell: bash + run: scripts/run_python_e2e_test.py --no-build # rerun-sdk is already built and installed + + - name: Unpack the wheel + shell: bash + run: | + mkdir unpack-dist + wheel unpack pre-dist/*.whl --dest unpack-dist + + - name: Get the folder name + shell: bash + run: | + echo "pkg_folder=$(ls unpack-dist)" >> $GITHUB_ENV + + - name: Cache RRD dataset + id: dataset + uses: actions/cache@v3 + with: + path: examples/python/colmap/dataset/ + # TODO(jleibs): Derive this key from the invocation below + key: colmap-dataset-colmap-fiat-v0 + + - name: Generate Embedded RRD file + shell: bash + # If you change the line below you should almost definitely change the `key:` line above by giving it a new, unique name + run: | + mkdir rrd + pip install -r examples/python/colmap/requirements.txt + python3 examples/python/colmap/main.py --dataset colmap_fiat --resize 800x600 --save rrd/colmap_fiat.rrd + cp rrd/colmap_fiat.rrd unpack-dist/${{ env.pkg_folder }}/rerun_sdk/rerun_demo/colmap_fiat.rrd + + - name: Repack the wheel + shell: bash + run: | + mkdir dist + wheel pack unpack-dist/${{ env.pkg_folder }} --dest dist/ + + - name: Upload wheels + uses: actions/upload-artifact@v3 + with: + name: wheels + path: dist + + # All platforms are currently creating the same rrd file, upload one of them + - name: Save RRD artifact + uses: actions/upload-artifact@v3 + with: + name: rrd + path: rrd + + # --------------------------------------------------------------------------- matrix-setup: # Building all the wheels is expensive, so we only run this job when we push (to main or release tags), # or if the job was manually triggered with `force_build_wheel` set to true. @@ -87,40 +247,26 @@ jobs: shell: bash run: | matrix=() - matrix+=('{"platform": "macos", "target": "x86_64-apple-darwin", "wheel_suffix": "x86_64", "runs_on": "macos-latest" },') - matrix+=('{"platform": "macos", "target": "aarch64-apple-darwin", "wheel_suffix": "x86_64", "runs_on": "macos-latest" },') # NOTE: we test the x86_64 wheel AGAIN, because the runner is x86_64 - matrix+=('{"platform": "windows", "target": "x86_64-pc-windows-msvc", "wheel_suffix": "", "runs_on": "windows-latest-8-cores"},') - matrix+=('{"platform": "linux", "target": "x86_64-unknown-linux-gnu", "wheel_suffix": "", "runs_on": "ubuntu-latest-16-cores", container: {"image": "rerunio/ci_docker:0.6"}}') + matrix+=('{"platform": "macos", "target": "x86_64-apple-darwin", "run_tests": true, "runs_on": "macos-latest" },') + matrix+=('{"platform": "macos", "target": "aarch64-apple-darwin", "run_tests": false, "runs_on": "macos-latest" },') # NOTE: we can't run tests on arm since our macos runner is x86_64 + matrix+=('{"platform": "windows", "target": "x86_64-pc-windows-msvc", "run_tests": true, "runs_on": "windows-latest-8-cores"},') echo "Matrix values: ${matrix[@]}" echo "matrix={\"include\":[${matrix[@]}]}" >> $GITHUB_OUTPUT wheels: - name: Build Python Wheels - needs: [lint, matrix-setup] + name: Build Remaining Python Wheels + needs: [lint, matrix-setup, wheels-linux] strategy: matrix: ${{fromJson(needs.matrix-setup.outputs.matrix)}} runs-on: ${{ matrix.runs_on }} - container: ${{ matrix.container }} - steps: - uses: actions/checkout@v3 - # These should already be in the docker container, but run for good measure. A no-op install - # should be fast, and this way things don't break if we add new packages without rebuilding - # docker - - name: Cache APT Packages - if: matrix.platform == 'linux' - uses: awalsh128/cache-apt-pkgs-action@v1.2.2 - with: - packages: ${{ env.UBUNTU_REQUIRED_PKGS }} - version: 2.0 # Increment this to pull newer packages - execute_install_scripts: true - - name: Set up cargo cache uses: Swatinem/rust-cache@v2 with: @@ -133,7 +279,6 @@ jobs: # The pip-cache setup logic doesn't work in the ubuntu docker container # That's probably fine since we bake these deps into the container already - name: Setup python - if: matrix.platform != 'linux' uses: actions/setup-python@v4 with: python-version: ${{ env.PYTHON_VERSION }} @@ -215,25 +360,21 @@ jobs: --features pypi --out pre-dist - - name: Install wheel dependencies - # First we install the dependencies manually so we can use `--no-index` when installing the wheel. - # This needs to be a separate step for some reason or the following step fails - # TODO(jleibs): pull these deps from pyproject.toml - # TODO(jleibs): understand why deps can't be installed in the same step as the wheel - shell: bash - run: | - pip install deprecated numpy>=1.23 pyarrow==10.0.1 - - name: Install built wheel - # Now install the wheel using a specific version and --no-index to guarantee we get the version from + if: ${{ matrix.run_tests }} + # First we install the dependencies manually so we can use `--no-index` when installing the wheel. + # Then install the wheel using a specific version and --no-index to guarantee we get the version from # the pre-dist folder. Note we don't use --force-reinstall here because --no-index means it wouldn't # find the dependencies to reinstall them. + # TODO(jleibs): pull these deps from pyproject.toml shell: bash run: | pip uninstall rerun-sdk + pip install deprecated numpy>=1.23 pyarrow==10.0.1 pip install rerun-sdk==${{ env.expected_version }} --no-index --find-links pre-dist - name: Verify built wheel version + if: ${{ matrix.run_tests }} shell: bash run: | python3 -m rerun --version @@ -241,10 +382,12 @@ jobs: rerun --version - name: Run unit tests + if: ${{ matrix.run_tests }} shell: bash run: cd rerun_py/tests && pytest - name: Install requriements for e2e test + if: ${{ matrix.run_tests }} run: | pip install -r examples/python/api_demo/requirements.txt pip install -r examples/python/car/requirements.txt @@ -253,6 +396,7 @@ jobs: pip install -r examples/python/text_logging/requirements.txt - name: Run e2e test + if: ${{ matrix.run_tests }} shell: bash run: scripts/run_python_e2e_test.py --no-build # rerun-sdk is already built and installed @@ -267,21 +411,16 @@ jobs: run: | echo "pkg_folder=$(ls unpack-dist)" >> $GITHUB_ENV - - name: Cache RRD dataset - id: dataset - uses: actions/cache@v3 + - name: Download RRD + uses: actions/download-artifact@v3 with: - path: examples/python/colmap/dataset/ - # TODO(jleibs): Derive this key from the invocation below - key: colmap-dataset-colmap-fiat-v0 + name: rrd + path: rrd - - name: Generate Embedded RRD file + - name: Insert the rrd shell: bash # If you change the line below you should almost definitely change the `key:` line above by giving it a new, unique name run: | - mkdir rrd - pip install -r examples/python/colmap/requirements.txt - python3 examples/python/colmap/main.py --dataset colmap_fiat --resize 800x600 --save rrd/colmap_fiat.rrd cp rrd/colmap_fiat.rrd unpack-dist/${{ env.pkg_folder }}/rerun_sdk/rerun_demo/colmap_fiat.rrd - name: Repack the wheel @@ -296,14 +435,6 @@ jobs: name: wheels path: dist - # All platforms are currently creating the same rrd file, upload one of them - - name: Save RRD artifact - if: matrix.platform == 'linux' - uses: actions/upload-artifact@v3 - with: - name: rrd - path: rrd - # --------------------------------------------------------------------------- upload_rrd: