Base export target chache key on the exported target

The cache key for an export target should contain as target name
that of the export target (and its effective configuration) rather
than the exported target. As we computed the repository part of
the cache key for the target included in the key, this was still a
correct cache key except in the case an explicit file reference was
exported (as here, the information that the file was to be taken
rather than the target of the same name got lost). We still fix
this issue by making the implementation match our design (rather
than by including the file-reference bit in the cache key), as the
original design gives the cleaner protocol for target-level caching
as a service.

1 files changed, 38 insertions, 39 deletions

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),