Include clang-tidy binary in the cache fingerprint #4
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
It's simpler for users to get started if they don't need to create a config file at all, so this commit provides a default: simply use the `clang-tidy` that's on the path. It's also often useful to be able to override the config via env variables without touching the config file: `CLANG_TIDY_CACHE_BINARY` makes that possible.
Different clang-tidy versions affect the check results just like the `.clang-tidy` config file. Add the contents of the binary ensures that we don't get bad cache hits when upgrading clang-tidy versions.
|
Sorry for the delay @dean0x7d - this looks like a good update. Will have a look over it today and will look to include it in a new build next week |
|
Thanks, @ejfitzgerald! We've been running this on CI for the last month and it's been working great! One other thing that we realized a bit later is that we needed a feature equivalent to |
|
@dean0x7d I think the CCACHE_BASEDIR would be a good additional feature. Happy to accept another PR when you get time |
mathstuf
pushed a commit
to mathstuf/clang-tidy-cache
that referenced
this pull request
Jan 18, 2023
Windows paths
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hi @ejfitzgerald, thanks for this project! I've found it incredibly useful!
The speedup is amazing and I'm looking to integrate the cache in a CI pipeline but I've run into a couple of things that don't quite fit into our CI workflow. This PR resolves that and I'm hoping that these changes will be useful for all users.
clang-tidybinary in the cache fingerprintLet's say that you run
clang-tidy-cachewithclang-tidy-11on a project and warm up the cache. Then you upgrade toclang-tidy-12and run it again. Currently, that results in all cache hits but that isn't correct:clang-tidy-12has new checks that must run. The old cache entries aren't valid for this version. This is an important issue for correctness in CI with a shared cache whereclang-tidyupgrades can happen on any branch/repo (so it's not practical to wipe the cache).The fix proposed by this PR is to add the
clang-tidybinary itself to the cache fingerprint. This ensures that any changes in the executable (like version upgrades) will miss old cache entries. Addingclang-tidy --versioncould have been another option, but SHA256 felt both simpler and more robust.Config defaults and environment variable overrides
This one's just about ease of use. It's simpler for users to get started if they don't need to create a config file at all, so this PR provides a default: if there's no config file, simply use the
clang-tidythat's on the path.It's also often useful to be able to override the config via env variables without touching the config file:
CLANG_TIDY_CACHE_BINARYmakes that possible. I find the variable easier to automate in CI than writing to a config file.The priority of the configuration is set up the same way as
ccache. From highest to lowest priority: 1. Environment variables. 2. Config file. 3. Built-in defaults.