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

Mark git directories as safe for our docker_ci container and fix cargo caching #1187

Merged
merged 4 commits into from
Feb 10, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 21 additions & 18 deletions .github/workflows/python.yml
Original file line number Diff line number Diff line change
Expand Up @@ -62,26 +62,26 @@ jobs:
matrix: ${{ steps.set-matrix.outputs.matrix }}

steps:
# Sets TAGGED_OR_MAIN if this workflow is running on a tag or the main branch.
- name: Set TAGGED_OR_MAIN
if: startsWith(github.ref, 'refs/tags/v') || github.ref == 'refs/heads/main'
run: echo "TAGGED_OR_MAIN=1" >> $GITHUB_ENV
# Sets TAGGED_OR_MAIN if this workflow is running on a tag or the main branch.
- name: Set TAGGED_OR_MAIN
if: startsWith(github.ref, 'refs/tags/v') || github.ref == 'refs/heads/main'
run: echo "TAGGED_OR_MAIN=1" >> $GITHUB_ENV

- id: set-matrix
shell: bash
run: |
matrix=()
- id: set-matrix
shell: bash
run: |
matrix=()
if [[ $TAGGED_OR_MAIN ]]; then
# MacOS build is really slow (>30 mins); uses up a lot of CI minutes
matrix+=('{"platform": "macos", "runs_on": "macos-latest"},')
fi
matrix+=('{"platform": "windows", "runs_on": "windows-latest-8-cores"},')
matrix+=('{"platform": "linux", "runs_on": "ubuntu-latest-16-cores"}')
if [[ $TAGGED_OR_MAIN ]]; then
# MacOS build is really slow (>30 mins); uses up a lot of CI minutes
matrix+=('{"platform": "macos", "runs_on": "macos-latest"},')
fi
matrix+=('{"platform": "windows", "runs_on": "windows-latest-8-cores"},')
matrix+=('{"platform": "linux", "runs_on": "ubuntu-latest-16-cores"}')
echo "Matrix values: ${matrix[@]}"
echo "Matrix values: ${matrix[@]}"
echo "matrix={\"include\":[${matrix[@]}]}" >> $GITHUB_OUTPUT
echo "matrix={\"include\":[${matrix[@]}]}" >> $GITHUB_OUTPUT
wheels:
name: Build Python Wheels
Expand All @@ -108,8 +108,11 @@ jobs:
- name: Set up cargo cache
uses: Swatinem/rust-cache@v2
with:
# See: https://github.com/rerun-io/rerun/pull/497
save-if: ${{ github.event_name == 'push'}}
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

- name: Setup python
uses: actions/setup-python@v4
Expand Down
5 changes: 4 additions & 1 deletion .github/workflows/rust.yml
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,10 @@ jobs:
uses: Swatinem/rust-cache@v2
with:
env-vars: CARGO CC CFLAGS CXX CMAKE RUST CACHE_KEY
save-if: ${{ github.event_name == 'push'}}
# 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

- name: Run benchmark
# Use bash shell so we get pipefail behavior with tee
Expand Down
6 changes: 5 additions & 1 deletion .github/workflows/toml.yml
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,11 @@ jobs:
uses: Swatinem/rust-cache@v2
with:
# See: https://github.com/rerun-io/rerun/pull/497
save-if: ${{ github.event_name == 'push'}}
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

- name: Install taplo-cli
uses: baptiste0928/cargo-install@v1
Expand Down
9 changes: 7 additions & 2 deletions ci_docker/Dockerfile
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
FROM rust:1.67
LABEL maintainer="[email protected]"
LABEL version="0.1"
# Remember to update the version in publish.sh
# TODO(jleibs) use this version in the publish.sh script and below in the CACHE_KEY
LABEL version="0.4"
Copy link
Member

@emilk emilk Feb 10, 2023

Choose a reason for hiding this comment

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

How does this relate to the VERSION in ci_docker/publish.sh? Should we always keep them in sync? If so, can you please add a comment to that effect, reminding us to update it in all places

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, added a comment, though would be nice to have one place to update.

Copy link
Member Author

Choose a reason for hiding this comment

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

LABEL description="Docker image used for the CI of https://github.com/rerun-io/rerun"

# Install some cargo tools we know we'll always need
Expand Down Expand Up @@ -34,4 +36,7 @@ RUN pip install -r requirements-build.txt
RUN cargo install [email protected]

# Increment this to invalidate cache
ENV CACHE_KEY=rerun_docker_v0.1
ENV CACHE_KEY=rerun_docker_v0.4

# See: https://github.com/actions/runner-images/issues/6775#issuecomment-1410270956
RUN git config --system --add safe.directory '*'
8 changes: 5 additions & 3 deletions ci_docker/publish.sh
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
VERSION=0.4 # Bump on each new version. Maybe it should also match the version in ci_docker/Dockerfile ?
VERSION=0.4 # Bump on each new version. Remember to update the version in the Dockerfile too.

# The build needs to run from top of repo to access the requirments.txt
cd `git rev-parse --show-toplevel`
Expand All @@ -7,8 +7,10 @@ cd `git rev-parse --show-toplevel`
docker pull rerunio/ci_docker

# Build the image
# docker build -t ci_docker -f ci_docker/Dockerfile .
docker buildx build --platform=linux/amd64 -t ci_docker -f ci_docker/Dockerfile .
docker build -t ci_docker -f ci_docker/Dockerfile .
# This is necessary to build on mac, but is doing something weird with the Cache
# TODO(jleibs): Make this all work portably with caching
# docker buildx build --platform=linux/amd64 -t ci_docker -f ci_docker/Dockerfile .

# Tag latest and version
docker tag ci_docker rerunio/ci_docker
Expand Down