Add a method to get BuildConfigurationKey from ActionLookupKey.

PiperOrigin-RevId: 406831509
uber-common · Nov 1, 2021 · 216b2b6 · 216b2b6
1 parent 2c9721f
commit 216b2b6
Show file tree

Hide file tree

Showing 20 changed files with 116 additions and 38 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionLookupKey.java b/src/main/java/com/google/devtools/build/lib/actions/ActionLookupKey.java
@@ -13,8 +13,10 @@
 // limitations under the License.
 package com.google.devtools.build.lib.actions;
 
+import com.google.devtools.build.lib.skyframe.BuildConfigurationKey;
 import com.google.devtools.build.skyframe.CPUHeavySkyKey;
 import com.google.devtools.build.skyframe.SkyKey;
+import javax.annotation.Nullable;
 
 /**
  * {@link SkyKey} for an "analysis object": either an {@link ActionLookupValue} or a {@link
@@ -28,4 +30,12 @@
  * are subclasses of {@link ActionLookupKey}. This allows callers to easily find the value key,
  * while remaining agnostic to what action lookup values actually exist.
  */
-public interface ActionLookupKey extends ArtifactOwner, CPUHeavySkyKey {}
+public interface ActionLookupKey extends ArtifactOwner, CPUHeavySkyKey {
+
+  /**
+   * Returns the {@link BuildConfigurationKey} for the configuration associated with this key, or
+   * {@code null} if this key has no associated configuration.
+   */
+  @Nullable
+  BuildConfigurationKey getConfigurationKey();
+}
diff --git a/src/main/java/com/google/devtools/build/lib/actions/BUILD b/src/main/java/com/google/devtools/build/lib/actions/BUILD
@@ -168,8 +168,10 @@ java_library(
     srcs = ["ActionLookupKey.java"],
     deps = [
         ":artifact_owner",
+        "//src/main/java/com/google/devtools/build/lib/skyframe:build_configuration",
         "//src/main/java/com/google/devtools/build/skyframe:cpu_heavy_skykey",
         "//src/main/java/com/google/devtools/build/skyframe:skyframe-objects",
+        "//third_party:jsr305",
     ],
 )
 

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionValue.java
@@ -65,6 +65,11 @@ public Label getLabel() {
       return actionLookupKey.getLabel();
     }
 
+    @Override
+    public BuildConfigurationKey getConfigurationKey() {
+      return actionLookupKey.getConfigurationKey();
+    }
+
     public ActionLookupKey getActionLookupKey() {
       return actionLookupKey;
     }

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java
@@ -221,11 +221,11 @@ public SkyValue compute(SkyKey skyKey, Environment env)
           new BuildFileContainsErrorsException(key.getLabel().getPackageIdentifier()));
     }
 
-    boolean aspectHasConfiguration = key.getAspectConfigurationKey() != null;
+    boolean aspectHasConfiguration = key.getConfigurationKey() != null;
 
     ImmutableSet<SkyKey> keys =
         aspectHasConfiguration
-            ? ImmutableSet.of(key.getBaseConfiguredTargetKey(), key.getAspectConfigurationKey())
+            ? ImmutableSet.of(key.getBaseConfiguredTargetKey(), key.getConfigurationKey())
             : ImmutableSet.of(key.getBaseConfiguredTargetKey());
 
     Map<SkyKey, ValueOrException<ConfiguredValueCreationException>> baseAndAspectValues =
@@ -248,11 +248,12 @@ public SkyValue compute(SkyKey skyKey, Environment env)
     if (aspectHasConfiguration) {
       try {
         aspectConfiguration =
-            (BuildConfigurationValue)
-                baseAndAspectValues.get(key.getAspectConfigurationKey()).get();
+            (BuildConfigurationValue) baseAndAspectValues.get(key.getConfigurationKey()).get();
       } catch (ConfiguredValueCreationException e) {
-        throw new IllegalStateException("Unexpected exception from BuildConfigurationFunction when "
-            + "computing " + key.getAspectConfigurationKey(), e);
+        throw new IllegalStateException(
+            "Unexpected exception from BuildConfigurationFunction when computing "
+                + key.getConfigurationKey(),
+            e);
       }
     }
 

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/AspectKeyCreator.java b/src/main/java/com/google/devtools/build/lib/skyframe/AspectKeyCreator.java
@@ -192,22 +192,10 @@ public String getDescription() {
     /**
      * Returns the key of the configured target of the aspect; that is, the configuration in which
      * the aspect will be evaluated.
-     *
-     * <p>In trimmed configuration mode, the aspect may require more fragments than the target on
-     * which it is being evaluated; in addition to configuration fragments required by the target
-     * and its dependencies, an aspect has configuration fragment requirements of its own, as well
-     * as dependencies of its own with their own configuration fragment requirements.
-     *
-     * <p>The aspect configuration contains all of these fragments, and is used to create the
-     * aspect's RuleContext and to retrieve the dependencies. Note that dependencies will have their
-     * configurations trimmed from this one as normal.
-     *
-     * <p>Because of these properties, this configuration is always a superset of the base target's
-     * configuration. In untrimmed configuration mode, this configuration will be equivalent to the
-     * base target's configuration.
      */
+    @Override
     @Nullable
-    public BuildConfigurationKey getAspectConfigurationKey() {
+    public BuildConfigurationKey getConfigurationKey() {
       return aspectConfigurationKey;
     }
 
@@ -303,6 +291,11 @@ public SkyFunctionName functionName() {
       return SkyFunctions.TOP_LEVEL_ASPECTS;
     }
 
+    @Override
+    public BuildConfigurationKey getConfigurationKey() {
+      return getBaseConfiguredTargetKey().getConfigurationKey();
+    }
+
     ImmutableList<AspectClass> getTopLevelAspectsClasses() {
       return topLevelAspectsClasses;
     }
@@ -313,7 +306,7 @@ public Label getLabel() {
     }
 
     String getDescription() {
-      return topLevelAspectsClasses + " on " + getLabel();
+      return topLevelAspectsClasses + " on " + targetLabel;
     }
 
     @Override

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD
@@ -553,6 +553,7 @@ java_library(
     name = "action_template_expansion_value",
     srcs = ["ActionTemplateExpansionValue.java"],
     deps = [
+        ":build_configuration",
         ":sky_functions",
         "//src/main/java/com/google/devtools/build/lib/actions",
         "//src/main/java/com/google/devtools/build/lib/actions:action_lookup_key",
@@ -1226,6 +1227,7 @@ java_library(
     name = "coverage_report_value",
     srcs = ["CoverageReportValue.java"],
     deps = [
+        ":build_configuration",
         ":sky_functions",
         "//src/main/java/com/google/devtools/build/lib/actions",
         "//src/main/java/com/google/devtools/build/lib/actions:action_lookup_key",
@@ -2768,6 +2770,7 @@ java_library(
     name = "workspace_status_value",
     srcs = ["WorkspaceStatusValue.java"],
     deps = [
+        ":build_configuration",
         ":sky_functions",
         "//src/main/java/com/google/devtools/build/lib/actions",
         "//src/main/java/com/google/devtools/build/lib/actions:action_lookup_key",

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BuildInfoCollectionFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/BuildInfoCollectionFunction.java
@@ -57,7 +57,7 @@ public SkyValue compute(SkyKey skyKey, Environment env) throws InterruptedExcept
         ImmutableSet.of(
             WorkspaceStatusValue.BUILD_INFO_KEY,
             WorkspaceNameValue.key(),
-            keyAndConfig.getConfigKey());
+            keyAndConfig.getConfigurationKey());
     Map<SkyKey, SkyValue> result = env.getValues(keysToRequest);
     if (env.valuesMissing()) {
       return null;
@@ -66,7 +66,7 @@ public SkyValue compute(SkyKey skyKey, Environment env) throws InterruptedExcept
         (WorkspaceStatusValue) result.get(WorkspaceStatusValue.BUILD_INFO_KEY);
 
     BuildConfigurationValue config =
-        (BuildConfigurationValue) result.get(keyAndConfig.getConfigKey());
+        (BuildConfigurationValue) result.get(keyAndConfig.getConfigurationKey());
     Map<BuildInfoKey, BuildInfoFactory> buildInfoFactories = BUILD_INFO_FACTORIES.get(env);
     BuildInfoFactory buildInfoFactory = buildInfoFactories.get(keyAndConfig.getInfoKey());
     Preconditions.checkState(buildInfoFactory.isEnabled(config));

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BuildInfoCollectionValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/BuildInfoCollectionValue.java
@@ -79,12 +79,13 @@ public SkyFunctionName functionName() {
       return SkyFunctions.BUILD_INFO_COLLECTION;
     }
 
-    BuildInfoKey getInfoKey() {
-      return infoKey;
+    @Override
+    public BuildConfigurationKey getConfigurationKey() {
+      return configKey;
     }
 
-    BuildConfigurationKey getConfigKey() {
-      return configKey;
+    BuildInfoKey getInfoKey() {
+      return infoKey;
     }
 
     @Override

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/CoverageReportValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/CoverageReportValue.java
@@ -40,6 +40,12 @@ public SkyFunctionName functionName() {
       return SkyFunctions.COVERAGE_REPORT;
     }
 
+    @Nullable
+    @Override
+    public BuildConfigurationKey getConfigurationKey() {
+      return null;
+    }
+
     @Nullable
     @Override
     public Label getLabel() {

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
@@ -561,15 +561,12 @@ public SkyframeAnalysisResult configureTargets(
       for (ActionLookupKey ctKey : Iterables.concat(ctKeys, aspectKeys)) {
         if (!topLevelActionConflictReport.isErrorFree(ctKey)) {
           Optional<ConflictException> e = topLevelActionConflictReport.getConflictException(ctKey);
-          if (!e.isPresent()) {
+          if (e.isEmpty()) {
             continue;
           }
           AnalysisFailedCause failedCause =
               makeArtifactConflictAnalysisFailedCause(configurationLookupSupplier, e.get());
-          BuildConfigurationKey configKey =
-              ctKey instanceof ConfiguredTargetKey
-                  ? ((ConfiguredTargetKey) ctKey).getConfigurationKey()
-                  : ((AspectKey) ctKey).getAspectConfigurationKey();
+          BuildConfigurationKey configKey = ctKey.getConfigurationKey();
           eventBus.post(
               new AnalysisFailureEvent(
                   ctKey,

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceStatusValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceStatusValue.java
@@ -67,5 +67,11 @@ public SkyFunctionName functionName() {
     public Label getLabel() {
       return null;
     }
+
+    @Nullable
+    @Override
+    public BuildConfigurationKey getConfigurationKey() {
+      return null;
+    }
   }
 }
diff --git a/src/test/java/com/google/devtools/build/lib/actions/ArtifactTest.java b/src/test/java/com/google/devtools/build/lib/actions/ArtifactTest.java
@@ -36,6 +36,7 @@
 import com.google.devtools.build.lib.cmdline.LabelConstants;
 import com.google.devtools.build.lib.rules.cpp.CppFileTypes;
 import com.google.devtools.build.lib.rules.java.JavaSemantics;
+import com.google.devtools.build.lib.skyframe.BuildConfigurationKey;
 import com.google.devtools.build.lib.skyframe.serialization.AutoRegistry;
 import com.google.devtools.build.lib.skyframe.serialization.ObjectCodecs;
 import com.google.devtools.build.lib.skyframe.serialization.testutils.SerializationDepsUtils;
@@ -414,6 +415,11 @@ public Label getLabel() {
             return null;
           }
 
+          @Override
+          public BuildConfigurationKey getConfigurationKey() {
+            return null;
+          }
+
           @Override
           public SkyFunctionName functionName() {
             return null;
@@ -426,6 +432,11 @@ public Label getLabel() {
             return null;
           }
 
+          @Override
+          public BuildConfigurationKey getConfigurationKey() {
+            return null;
+          }
+
           @Override
           public SkyFunctionName functionName() {
             return null;

diff --git a/src/test/java/com/google/devtools/build/lib/actions/BUILD b/src/test/java/com/google/devtools/build/lib/actions/BUILD
@@ -52,6 +52,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/exec:single_build_file_cache",
         "//src/main/java/com/google/devtools/build/lib/rules/cpp",
         "//src/main/java/com/google/devtools/build/lib/rules/java:java-compilation",
+        "//src/main/java/com/google/devtools/build/lib/skyframe:build_configuration",
         "//src/main/java/com/google/devtools/build/lib/skyframe:tree_artifact_value",
         "//src/main/java/com/google/devtools/build/lib/skyframe/serialization",
         "//src/main/java/com/google/devtools/build/lib/skyframe/serialization/testutils",

diff --git a/src/test/java/com/google/devtools/build/lib/actions/util/ActionsTestUtil.java b/src/test/java/com/google/devtools/build/lib/actions/util/ActionsTestUtil.java
@@ -79,6 +79,7 @@
 import com.google.devtools.build.lib.exec.SingleBuildFileCache;
 import com.google.devtools.build.lib.skyframe.ActionTemplateExpansionValue;
 import com.google.devtools.build.lib.skyframe.ActionTemplateExpansionValue.ActionTemplateExpansionKey;
+import com.google.devtools.build.lib.skyframe.BuildConfigurationKey;
 import com.google.devtools.build.lib.skyframe.TreeArtifactValue;
 import com.google.devtools.build.lib.skyframe.serialization.autocodec.SerializationConstant;
 import com.google.devtools.build.lib.util.FileType;
@@ -387,6 +388,7 @@ public boolean restartPermitted() {
   @SerializationConstant
   public static final ActionLookupKey NULL_ARTIFACT_OWNER =
       new ActionLookupKey() {
+
         @Override
         public SkyFunctionName functionName() {
           return null;
@@ -397,6 +399,12 @@ public Label getLabel() {
           return NULL_LABEL;
         }
 
+        @Nullable
+        @Override
+        public BuildConfigurationKey getConfigurationKey() {
+          return null;
+        }
+
         @Override
         public String toString() {
           return "NULL_ARTIFACT_OWNER";

diff --git a/src/test/java/com/google/devtools/build/lib/actions/util/BUILD b/src/test/java/com/google/devtools/build/lib/actions/util/BUILD
@@ -36,6 +36,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/events",
         "//src/main/java/com/google/devtools/build/lib/exec:single_build_file_cache",
         "//src/main/java/com/google/devtools/build/lib/skyframe:action_template_expansion_value",
+        "//src/main/java/com/google/devtools/build/lib/skyframe:build_configuration",
         "//src/main/java/com/google/devtools/build/lib/skyframe:tree_artifact_value",
         "//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec",
         "//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec:serialization-constant",

diff --git a/src/test/java/com/google/devtools/build/lib/actions/util/InjectedActionLookupKey.java b/src/test/java/com/google/devtools/build/lib/actions/util/InjectedActionLookupKey.java
@@ -16,6 +16,7 @@
 
 import com.google.devtools.build.lib.actions.ActionLookupKey;
 import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.skyframe.BuildConfigurationKey;
 import com.google.devtools.build.skyframe.SkyFunctionName;
 import javax.annotation.Nullable;
 
@@ -44,6 +45,12 @@ public Label getLabel() {
     return null;
   }
 
+  @Nullable
+  @Override
+  public BuildConfigurationKey getConfigurationKey() {
+    return null;
+  }
+
   @Override
   public int hashCode() {
     return name.hashCode();

diff --git a/src/test/java/com/google/devtools/build/lib/analysis/RunfilesTest.java b/src/test/java/com/google/devtools/build/lib/analysis/RunfilesTest.java
@@ -31,6 +31,7 @@
 import com.google.devtools.build.lib.cmdline.LabelConstants;
 import com.google.devtools.build.lib.events.EventKind;
 import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions;
+import com.google.devtools.build.lib.skyframe.BuildConfigurationKey;
 import com.google.devtools.build.lib.testutil.FoundationTestCase;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import com.google.devtools.build.lib.vfs.Root;
@@ -71,7 +72,7 @@ private static StarlarkThread newStarlarkThread(String... options)
   }
 
   @Test
-  public void testFilterListForObscuringSymlinksCatchesBadObscurer() throws Exception {
+  public void testFilterListForObscuringSymlinksCatchesBadObscurer() {
     Map<PathFragment, Artifact> obscuringMap = new HashMap<>();
     PathFragment pathA = PathFragment.create("a");
     ArtifactRoot root = ArtifactRoot.asSourceRoot(Root.fromPath(scratch.resolve("/workspace")));
@@ -84,7 +85,7 @@ public void testFilterListForObscuringSymlinksCatchesBadObscurer() throws Except
   }
 
   @Test
-  public void testFilterListForObscuringSymlinksCatchesBadGrandParentObscurer() throws Exception {
+  public void testFilterListForObscuringSymlinksCatchesBadGrandParentObscurer() {
     Map<PathFragment, Artifact> obscuringMap = new HashMap<>();
     PathFragment pathA = PathFragment.create("a");
     ArtifactRoot root = ArtifactRoot.asSourceRoot(Root.fromPath(scratch.resolve("/workspace")));
@@ -97,7 +98,7 @@ public void testFilterListForObscuringSymlinksCatchesBadGrandParentObscurer() th
   }
 
   @Test
-  public void testFilterListForObscuringSymlinksCatchesBadObscurerNoListener() throws Exception {
+  public void testFilterListForObscuringSymlinksCatchesBadObscurerNoListener() {
     Map<PathFragment, Artifact> obscuringMap = new HashMap<>();
     PathFragment pathA = PathFragment.create("a");
     ArtifactRoot root = ArtifactRoot.asSourceRoot(Root.fromPath(scratch.resolve("/workspace")));
@@ -109,7 +110,7 @@ public void testFilterListForObscuringSymlinksCatchesBadObscurerNoListener() thr
   }
 
   @Test
-  public void testFilterListForObscuringSymlinksIgnoresOkObscurer() throws Exception {
+  public void testFilterListForObscuringSymlinksIgnoresOkObscurer() {
     Map<PathFragment, Artifact> obscuringMap = new HashMap<>();
     PathFragment pathA = PathFragment.create("a");
     ArtifactRoot root = ArtifactRoot.asSourceRoot(Root.fromPath(scratch.resolve("/workspace")));
@@ -123,7 +124,7 @@ public void testFilterListForObscuringSymlinksIgnoresOkObscurer() throws Excepti
   }
 
   @Test
-  public void testFilterListForObscuringSymlinksNoObscurers() throws Exception {
+  public void testFilterListForObscuringSymlinksNoObscurers() {
     Map<PathFragment, Artifact> obscuringMap = new HashMap<>();
     PathFragment pathA = PathFragment.create("a");
     ArtifactRoot root = ArtifactRoot.asSourceRoot(Root.fromPath(scratch.resolve("/workspace")));
@@ -171,6 +172,12 @@ public SkyFunctionName functionName() {
     public Label getLabel() {
       return null;
     }
+
+    @Nullable
+    @Override
+    public BuildConfigurationKey getConfigurationKey() {
+      return null;
+    }
   }
 
   @Test