From b6ff8c6d7bc2e8fe595604e0f36a6cdfa54000fc Mon Sep 17 00:00:00 2001 From: Oliver Reiche Date: Tue, 14 Nov 2023 14:28:19 +0100 Subject: Fix serialization of the target cache key ... which was accidentially a list of (a single) object, instead of only a single JSON object. (cherry-picked from 63a874517618a57dd5ca223d19a795c28a39c123) --- CHANGELOG.md | 22 +++++++++++++++------- src/buildtool/storage/target_cache_key.cpp | 8 ++++---- 2 files changed, 19 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8e762967..22ea953c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,13 +4,21 @@ Bug fixes on top of release `1.2.2`. ### Fixes -- The cache key used for an export target is now based on the export - target itself rather than that of the exported target. The latter - could lead to spourious cache hits, but only in the case where - an explicit file reference was exported and an regular target - with the same name exists. Where the new cache keys overlap with - the old ones, they refer to the same configured targets; so no - special action is to be taken when updating. +- The cache key used for an export target is now based on the + export target itself rather than that of the exported target. The + latter could lead to spurious cache hits, but only in the case + where the exported target was an explicit file reference, and a + regular target with the same name existed as well. Where the new + cache keys would overlap with the old ones, they would refer to + the same configured targets. However, we used the fact that we + changed the target cache key to also clean up the serialization + format to only contain the JSON object describing repository, + target, and effective configuration, instead of a singleton list + containing this object. Therefore, old and new cache keys do not + overlap at all. In particular, no special care has to be taken + on upgrading or downgrading. However, old target-level cache + entries will not be used leading potentially to rebuilding of + some targets. - Fixed a race condition in an internal cache of `just execute` used for keeping track of running operations. diff --git a/src/buildtool/storage/target_cache_key.cpp b/src/buildtool/storage/target_cache_key.cpp index 11ea5c84..7d300338 100644 --- a/src/buildtool/storage/target_cache_key.cpp +++ b/src/buildtool/storage/target_cache_key.cpp @@ -31,10 +31,10 @@ auto TargetCacheKey::Create(std::string const& repo_key, try { // target's repository is content-fixed, we can compute a cache key auto target_desc = nlohmann::json{ - {{"repo_key", repo_key}, - {"target_name", - nlohmann::json{target_name.module, target_name.name}.dump()}, - {"effective_config", effective_config.ToString()}}}; + {"repo_key", repo_key}, + {"target_name", + nlohmann::json{target_name.module, target_name.name}.dump()}, + {"effective_config", effective_config.ToString()}}; if (auto target_key = Storage::Instance().CAS().StoreBlob( target_desc.dump(2), /*is_executable=*/false)) { return TargetCacheKey{ -- cgit v1.2.3