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.

(cherry-picked from 180d8d89ffa92f8c3e8b8bcb912ec1a4990569c9)

2 files changed, 45 insertions, 39 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index c6c9f854..8e762967 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -4,6 +4,13 @@ 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.
 - Fixed a race condition in an internal cache of `just execute` used for keeping
   track of running operations.
 
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),