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

CI cache not including installed Cargo tools? #1312

Closed
joshlf opened this issue May 19, 2024 · 2 comments
Closed

CI cache not including installed Cargo tools? #1312

joshlf opened this issue May 19, 2024 · 2 comments
Labels
experience-easy This issue is easy, and shouldn't require much experience help wanted Extra attention is needed

Comments

@joshlf
Copy link
Member

joshlf commented May 19, 2024

Our generate_cache CI job, which is a dependency of the main build_test job, runs cargo install cargo-nextest. However, in this run, we can see that it's getting re-installed in build_test. This likely means that binaries installed via cargo install are not getting cached.

@joshlf joshlf added help wanted Extra attention is needed experience-easy This issue is easy, and shouldn't require much experience labels May 19, 2024
@joshlf joshlf mentioned this issue May 19, 2024
@sl4m
Copy link

sl4m commented May 30, 2024

Is it possible it's due to the cargo clean call? I see the job you linked matches the target specified here: https://github.com/google/zerocopy/blob/889ac1bc2f38b1ccf338a1bf12f222102b338bc0/.github/workflows/ci.yml#L249C23-L256.

Never mind, it should not affect cargo dependencies.

Note: Saw this issue from This Week in Rust

@joshlf
Copy link
Member Author

joshlf commented Jun 1, 2024

Credit to @sl4m for figuring out that this was actually a red herring (#1375 (comment)):

Amazing, thank you for fixing this! I'm not sure I understand why this fix works, though?

Ok, I think I know what's really going on. My change worked in my fork because I did not have existing caches.

I believe cargo-nextest was recently introduced in #1307? If so, it may be a result of an old cache. A new cache is created only if Cargo.toml changes (see here) and since GitHub Actions cache are immutable, it did not preserve the cargo-nextest installation.

Cargo.toml has since been updated via google-pr-creation bot, so the caches should now have cargo-nextest.

I checked a recent build and it looks like it's ignoring the cargo-nextest installation as expected:

Running Miri tests with 8 threads
    Updating crates.io index
     Ignored package `cargo-nextest v0.9.72` is already installed, use --force to override
Preparing a sysroot for Miri (target: aarch64-unknown-linux-gnu)... done

I think I will close this PR if you agree and you can probably close #1312.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
experience-easy This issue is easy, and shouldn't require much experience help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants