Change TargetContext.findDirectPrerequisite from config1 == config2 to config1.equals(c2).

This fixes an obscure bug between dynamic configurations, host configuration caching, and Skyframe skyKey interning that makes Bazel crash under certain sequences of builds. See changes for details.

--
PiperOrigin-RevId: 144766296
MOS_MIGRATED_REVID=144766296

1 files changed, 10 insertions, 0 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java
index a905a9f68a..c84cf2726a 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java
@@ -528,6 +528,16 @@ public final class SkyframeBuildView {
     if (hostConfig != null) {
       return hostConfig;
     }
+    // TODO(bazel-team): investigate getting the trimmed config from Skyframe instead of cloning.
+    // This is the only place we instantiate BuildConfigurations outside of Skyframe, This can
+    // produce surprising effects, such as requesting a configuration that's in the Skyframe cache
+    // but still produces a unique instance because we don't check that cache. It'd be nice to
+    // guarantee that *all* instantiations happen through Skyframe. That could, for example,
+    // guarantee that config1.equals(config2) implies config1 == config2, which is nice for
+    // verifying we don't accidentally create extra configurations. But unfortunately,
+    // hostConfigurationCache was specifically created because Skyframe is too slow for this use
+    // case. So further optimization is necessary to make that viable (proto_library in particular
+    // contributes to much of the difference).
     BuildConfiguration trimmedConfig =
         topLevelHostConfiguration.clone(fragmentClasses, ruleClassProvider);
     hostConfigurationCache.put(fragmentClasses, trimmedConfig);