Skip to content

Don't use CARGO_BUILD_JOBS in hash keys #2328

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

fsouza
Copy link

@fsouza fsouza commented Feb 10, 2025

CARGO_BUILD_JOBS only affects Cargo's parallelism, not rustc's outputs, so it can be safely excluded from the cache key.

This enables better cache reutilization when building from environments with different number of CPUs.

I ran into this while experimenting with sccache on a containerized system where a library can be shared/compiled between different job sizes, and was surprised that I was getting cache misses for those libraries.

Given how small the change is, I figured opening the PR wouldn't hurt, but please let me know if you prefer me to move this to an issue for further discussion before actually having the PR reviewed.

@fsouza
Copy link
Author

fsouza commented Feb 10, 2025

(Perhaps we can discuss what other variables should be excluded or whether sccache could have a knob that allows the user to select which environment variables to exclude from the key, though I understand that having a knob may complicate things unnecessarily)

`CARGO_BUILD_JOBS` only affects Cargo's parallelism, not rustc's
outputs, so it can be safely excluded from the cache key.

This enables better cache reutilization when building from environments
with different number of CPUs.
@sylvestre sylvestre force-pushed the exclude-CARGO_BUILD_JOBS-from-cache-key branch from c2851b2 to 85eabc0 Compare June 20, 2025 16:21
@drahnr
Copy link
Collaborator

drahnr commented Jun 23, 2025

I think a long-term solution would be to have a meta-key, that allows to exclude arbitrary environment variables from being included. Out of scope for this PR though.

@codecov-commenter
Copy link

codecov-commenter commented Jun 23, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 71.58%. Comparing base (a43cade) to head (85eabc0).
Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2328   +/-   ##
=======================================
  Coverage   71.58%   71.58%           
=======================================
  Files          65       65           
  Lines       36214    36221    +7     
=======================================
+ Hits        25923    25930    +7     
  Misses      10291    10291           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants