-
Notifications
You must be signed in to change notification settings - Fork 140
Adopt Swatinem/rust-cache for faster CI #1676
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -32,6 +32,12 @@ jobs: | |
- uses: actions/checkout@v4 | ||
- name: Bootc Ubuntu Setup | ||
uses: ./.github/actions/bootc-ubuntu-setup | ||
- name: Setup Rust cache | ||
uses: Swatinem/rust-cache@v2 | ||
with: | ||
cache-all-crates: true | ||
save-if: ${{ github.ref == 'refs/heads/main' }} | ||
lookup-only: ${{ github.ref == 'refs/heads/main' }} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why are you setting There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will ensure that on There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Yes, that's the effect the change has. I'm asking about why we want that effect - it's not the default, so you went to some effort to turn it on. A github-wide search shows it's definitely used elsewhere but...why? |
||
- name: Validate (default) | ||
run: just validate | ||
# Build container with continuous repository enabled | ||
|
@@ -41,6 +47,12 @@ jobs: | |
- uses: actions/checkout@v4 | ||
- name: Bootc Ubuntu Setup | ||
uses: ./.github/actions/bootc-ubuntu-setup | ||
- name: Setup Rust cache | ||
uses: Swatinem/rust-cache@v2 | ||
with: | ||
cache-all-crates: true | ||
save-if: ${{ github.ref == 'refs/heads/main' }} | ||
lookup-only: ${{ github.ref == 'refs/heads/main' }} | ||
- name: Build with continuous repo enabled | ||
run: sudo just build --build-arg=continuous_repo=1 | ||
# Check for security vulnerabilities and license compliance | ||
|
@@ -62,6 +74,12 @@ jobs: | |
uses: actions/checkout@v4 | ||
- name: Bootc Ubuntu Setup | ||
uses: ./.github/actions/bootc-ubuntu-setup | ||
- name: Setup Rust cache | ||
uses: Swatinem/rust-cache@v2 | ||
with: | ||
cache-all-crates: true | ||
save-if: ${{ github.ref == 'refs/heads/main' }} | ||
lookup-only: ${{ github.ref == 'refs/heads/main' }} | ||
- name: Enable fsverity for / | ||
run: sudo tune2fs -O verity $(findmnt -vno SOURCE /) | ||
- name: Install utils | ||
|
@@ -76,7 +94,6 @@ jobs: | |
sudo podman build -t localhost/bootc-fsverity -f ci/Containerfile.install-fsverity | ||
|
||
# TODO move into a container, and then have this tool run other containers | ||
export CARGO_INCREMENTAL=0 # because we aren't caching the test runner bits | ||
cargo build --release -p tests-integration | ||
|
||
df -h / | ||
|
@@ -116,6 +133,12 @@ jobs: | |
- uses: actions/checkout@v4 | ||
- name: Bootc Ubuntu Setup | ||
uses: ./.github/actions/bootc-ubuntu-setup | ||
- name: Setup Rust cache | ||
uses: Swatinem/rust-cache@v2 | ||
with: | ||
cache-all-crates: true | ||
save-if: ${{ github.ref == 'refs/heads/main' }} | ||
lookup-only: ${{ github.ref == 'refs/heads/main' }} | ||
- name: Build mdbook | ||
run: just build-mdbook | ||
# Build containers and disk images for integration testing across OS matrix | ||
|
@@ -131,6 +154,12 @@ jobs: | |
- uses: actions/checkout@v4 | ||
- name: Bootc Ubuntu Setup | ||
uses: ./.github/actions/bootc-ubuntu-setup | ||
- name: Setup Rust cache | ||
uses: Swatinem/rust-cache@v2 | ||
with: | ||
cache-all-crates: true | ||
save-if: ${{ github.ref == 'refs/heads/main' }} | ||
lookup-only: ${{ github.ref == 'refs/heads/main' }} | ||
- name: Install qemu-utils | ||
run: sudo apt install -y qemu-utils | ||
|
||
|
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.
Why? My understanding is that due to how github handles caching we're safe against e.g. malicious PRs.
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.
This will ensure that caches are saved only on
main
, not on any other PRs. I think that's enough caching. It may also have some vague security benefits.But if you don't like it, it doesn't have to be there.
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.
I think anything that's not the default should have a comment with a rationale. When you say "enough caching" - is that to ensure that we're not evicting caches from git main with pull requests?