diff options
| -rw-r--r-- | CHANGELOG.md | 7 | ||||
| -rw-r--r-- | src/buildtool/build_engine/target_map/export.cpp | 77 |
2 files changed, 45 insertions, 39 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index f48f24fd..ba7b9a1d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,13 @@ A feature release on top of `1.2.0`, backwards compatible. ### 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. - Improved protability and update of the bundled dependencies. - Various minor improvements and typo fixes in the documentation. - Fixed a race condition in an internal cache of `just execute` used for keeping diff --git a/src/buildtool/build_engine/target_map/export.cpp b/src/buildtool/build_engine/target_map/export.cpp index 0ba82d23..d832b2ee 100644 --- a/src/buildtool/build_engine/target_map/export.cpp +++ b/src/buildtool/build_engine/target_map/export.cpp @@ -80,10 +80,10 @@ void FinalizeExport( } [[nodiscard]] auto ComputeTargetCacheKey( - BuildMaps::Base::EntityName const& exported_target, + BuildMaps::Base::EntityName const& export_target, Configuration const& target_config) -> std::optional<TargetCacheKey> { static auto const& repos = RepositoryConfig::Instance(); - auto const& target_name = exported_target.GetNamedTarget(); + auto const& target_name = export_target.GetNamedTarget(); if (auto repo_key = repos.RepositoryKey(target_name.repository)) { return TargetCacheKey::Create(*repo_key, target_name, target_config); } @@ -101,49 +101,13 @@ void ExportRule( const gsl::not_null<BuildMaps::Target::ResultTargetMap*> result_map) { auto desc = BuildMaps::Base::FieldReader::CreatePtr( desc_json, key.target, "export target", logger); - desc->ExpectFields(kExpectedFields); - auto exported_target_name = desc->ReadExpression("target"); - if (not exported_target_name) { - return; - } - auto exported_target = BuildMaps::Base::ParseEntityNameFromExpression( - exported_target_name, - key.target, - [&logger, &exported_target_name](std::string const& parse_err) { - (*logger)(fmt::format("Parsing target name {} failed with:\n{}", - exported_target_name->ToString(), - parse_err), - true); - }); - if (not exported_target) { - return; - } auto flexible_vars = desc->ReadStringList("flexible_config"); if (not flexible_vars) { return; } auto effective_config = key.config.Prune(*flexible_vars); + auto target_cache_key = ComputeTargetCacheKey(key.target, effective_config); - auto fixed_config = - desc->ReadOptionalExpression("fixed_config", Expression::kEmptyMap); - if (not fixed_config->IsMap()) { - (*logger)(fmt::format("fixed_config has to be a map, but found {}", - fixed_config->ToString()), - true); - return; - } - for (auto const& var : fixed_config->Map().Keys()) { - if (effective_config.VariableFixed(var)) { - (*logger)( - fmt::format("Variable {} is both fixed and flexible.", var), - true); - return; - } - } - auto target_config = effective_config.Update(fixed_config); - - auto target_cache_key = - ComputeTargetCacheKey(*exported_target, target_config); if (target_cache_key) { if (auto target_cache_value = Storage::Instance().TargetCache().Read(*target_cache_key)) { @@ -202,6 +166,41 @@ void ExportRule( key.target.ToString()); } + desc->ExpectFields(kExpectedFields); + auto exported_target_name = desc->ReadExpression("target"); + if (not exported_target_name) { + return; + } + auto exported_target = BuildMaps::Base::ParseEntityNameFromExpression( + exported_target_name, + key.target, + [&logger, &exported_target_name](std::string const& parse_err) { + (*logger)(fmt::format("Parsing target name {} failed with:\n{}", + exported_target_name->ToString(), + parse_err), + true); + }); + if (not exported_target) { + return; + } + auto fixed_config = + desc->ReadOptionalExpression("fixed_config", Expression::kEmptyMap); + if (not fixed_config->IsMap()) { + (*logger)(fmt::format("fixed_config has to be a map, but found {}", + fixed_config->ToString()), + true); + return; + } + for (auto const& var : fixed_config->Map().Keys()) { + if (effective_config.VariableFixed(var)) { + (*logger)( + fmt::format("Variable {} is both fixed and flexible.", var), + true); + return; + } + } + auto target_config = effective_config.Update(fixed_config); + (*subcaller)( {BuildMaps::Target::ConfiguredTarget{ .target = std::move(*exported_target), |