Skip to content

Conversation

@frozenspider
Copy link
Contributor

@frozenspider frozenspider commented Dec 25, 2025

Installing cargo-llvm-cov each time takes a long time and is wasteful. Cache it the same way we cache cargo-deny.

GH Action Time before Time after
Coverage (PR) 166 seconds 38 seconds
Coverage (Push) 87 seconds 18 seconds

@frozenspider frozenspider requested a review from a team as a code owner December 25, 2025 15:50
@Fantoni0
Copy link
Contributor

I think it works, and times are much better.
But recently, I had some issues with the cache of cargo deny https://github.com/NethermindEth/stellar-private-transactions/pull/57
We were using an outdated version that was not compatible with newer advisory databases. Even if I updated the version, which cargo deny succeeded, and the new version was not installed.
Could something like this also happen for cache-llvm-cov?

@frozenspider
Copy link
Contributor Author

frozenspider commented Dec 26, 2025

@Fantoni0, I've ran into that too. However, the short-term fix is pretty simple - go to Actions > Caches and delete a relevant cache.
I think caching is still worth it even though this might become a problem occasionally.

Copy link
Contributor

@Fantoni0 Fantoni0 left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Collaborator

@maksimryndin maksimryndin left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@frozenspider frozenspider merged commit 9513a3a into main Dec 26, 2025
9 checks passed
@frozenspider frozenspider deleted the f/cache-llvm-cov branch December 26, 2025 12:55
frozenspider added a commit that referenced this pull request Dec 26, 2025
A follow-up to #34 
Once cache is formed, it's not updated anymore (see
actions/toolkit#505). As such, it doesn't make
sense having the cache shared between unrelated actions.
Made each action using a cache key tied to its tool.
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.

4 participants