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] make CI $PATH equal for push & PR to cache nonhermetic builds #25852

Merged
merged 2 commits into from
Jan 13, 2025

Conversation

nbdd0121
Copy link
Contributor

@nbdd0121 nbdd0121 commented Jan 13, 2025

Change of PATH will cause change to hash of nonhermetic builds, which prevents Verilator build cached in master to be reused in pull request CI.

It appears that gcloud CLI is the one making difference, so fake a PATH without installing it (as it's additional ~0.5G ~300M to download).

Close #20844

Signed-off-by: Gary Guo <gary.guo@lowrisc.org>
@nbdd0121 nbdd0121 requested a review from rswarbrick as a code owner January 13, 2025 17:12
Change of `PATH` will cause change to hash of nonhermetic builds, which
prevents Verilator build cached in master to be reused in pull request
CI.

It appears that gcloud CLI is the one making difference, so fake a PATH
without installing it (as it's additional ~0.5G to download).

Signed-off-by: Gary Guo <gary.guo@lowrisc.org>
Copy link
Contributor

@jwnrt jwnrt left a comment

Choose a reason for hiding this comment

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

I'd rather not have this hack, it seems likely to break without us knowing. A real solution would be to fake the PATH inside Bazel to a proxy directory containing symlinks to the tools we need.

That said, I think the hack is harmless so I'll approve.

I would rather somebody tried the real fix though or at least opened an issue for it.

Copy link
Contributor

@AlexJones0 AlexJones0 left a comment

Choose a reason for hiding this comment

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

Thanks a lot @nbdd0121, this should fix the Verilator test caching issues.

I agree with what @jwnrt said above, and think in long term we should move to better delineating exactly which parts of our build need to be non-hermetic to avoid more fragile hacks like this.

@nbdd0121
Copy link
Contributor Author

I think the benefit of this PR is huge so we should merge the hack now while working towards a better solution. I opened #25855 to track this.

@nbdd0121 nbdd0121 merged commit 2ce0a29 into lowRISC:master Jan 13, 2025
38 checks passed
@nbdd0121 nbdd0121 deleted the verilator_cache branch January 13, 2025 19:15
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.

[ci] Bazel cache not being used
3 participants