Remove the unnecessary BuildConfigurationValue wrapper around `Buil…

…dConfiguration`.

Instead, have `BuildConfiguration` implement `SkyValue` directly, and rename it to `BuildConfigurationValue`. Its `equals` and `hashCode` methods are removed - they were incomplete (notably, they did not consider the repository name). This was covered up by the fact that the wrapping `SkyValue` did not implement equality, preventing false change pruning.

`BuildConfigurationValue.Key` is promoted to a top-level class `BuildConfigurationKey`.

PiperOrigin-RevId: 406166220

site/docs/configurable-attributes.md

@@ -974,7 +974,7 @@ values with `bazel config`:
 
 ```sh
 $ bazel config 12e23b9a2b534a
-BuildConfiguration 12e23b9a2b534a
+BuildConfigurationValue 12e23b9a2b534a
 Fragment com.google.devtools.build.lib.analysis.config.CoreOptions {
   cpu: darwin
   compilation_mode: fastbuild

src/main/java/com/google/devtools/build/lib/BUILD

@@ -308,7 +308,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/skyframe:action_execution_inactivity_watchdog",
         "//src/main/java/com/google/devtools/build/lib/skyframe:actiongraph/v2/aquery_output_handler",
         "//src/main/java/com/google/devtools/build/lib/skyframe:aspect_key_creator",
-        "//src/main/java/com/google/devtools/build/lib/skyframe:build_configuration_value",
+        "//src/main/java/com/google/devtools/build/lib/skyframe:build_configuration",
         "//src/main/java/com/google/devtools/build/lib/skyframe:builder",
         "//src/main/java/com/google/devtools/build/lib/skyframe:configuration_phase_started_event",
         "//src/main/java/com/google/devtools/build/lib/skyframe:configured_target_key",

src/main/java/com/google/devtools/build/lib/actions/Action.java

@@ -65,10 +65,10 @@
  *   <li>As much as possible, make the cache key computation obvious - fully hash every field
  *       (except input contents, but including input and output names if they appear in the command
  *       line) in the class, and avoid referencing anything that isn't needed for action execution,
- *       such as {@link com.google.devtools.build.lib.analysis.config.BuildConfiguration} objects or
- *       even fragments thereof; if the action has a command line, err on the side of hashing the
- *       entire command line, even if that seems expensive. It's always safe to hash too much - the
- *       negative effect on incremental build times is usually negligible.
+ *       such as {@link com.google.devtools.build.lib.analysis.config.BuildConfigurationValue}
+ *       objects or even fragments thereof; if the action has a command line, err on the side of
+ *       hashing the entire command line, even if that seems expensive. It's always safe to hash too
+ *       much - the negative effect on incremental build times is usually negligible.
  *   <li>Add test coverage for the cache key computation; use {@link
  *       com.google.devtools.build.lib.analysis.util.ActionTester} to generate as many combinations
  *       of field values as possible; add test coverage every time you add another field.

src/main/java/com/google/devtools/build/lib/actions/ActionInput.java

@@ -46,7 +46,7 @@ public interface ActionInput {
 
   /**
    * Returns if this input's file system path includes a digest of its content. See {@link
-   * com.google.devtools.build.lib.analysis.config.BuildConfiguration#useContentBasedOutputPaths}.
+   * com.google.devtools.build.lib.analysis.config.BuildConfigurationValue#useContentBasedOutputPaths}.
    */
   default boolean contentBasedPath() {
     return false;

src/main/java/com/google/devtools/build/lib/actions/Artifact.java

@@ -383,7 +383,7 @@ public static class DerivedArtifact extends Artifact {
     /**
      * Content-based output paths are experimental. Only derived artifacts that are explicitly opted
      * in by their creating rules should use them and only when {@link
-     * com.google.devtools.build.lib.analysis.config.BuildConfiguration#useContentBasedOutputPaths}
+     * com.google.devtools.build.lib.analysis.config.BuildConfigurationValue#useContentBasedOutputPaths}
      * is on.
      */
     private final boolean contentBasedPath;
@@ -397,7 +397,7 @@ public static DerivedArtifact create(
     /**
      * Same as {@link #create(ArtifactRoot, PathFragment, ActionLookupKey)} but includes the option
      * to use a content-based path for this artifact (see {@link
-     * com.google.devtools.build.lib.analysis.config.BuildConfiguration#useContentBasedOutputPaths}).
+     * com.google.devtools.build.lib.analysis.config.BuildConfigurationValue#useContentBasedOutputPaths}).
      */
     public static DerivedArtifact create(
         ArtifactRoot root, PathFragment execPath, ActionLookupKey owner, boolean contentBasedPath) {

src/main/java/com/google/devtools/build/lib/actions/ArtifactFactory.java

@@ -216,7 +216,7 @@ public Artifact.DerivedArtifact getDerivedArtifact(
   /**
    * Same as {@link #getDerivedArtifact(PathFragment, ArtifactRoot, ArtifactOwner)} but includes the
    * option to use a content-based path for this artifact (see {@link
-   * com.google.devtools.build.lib.analysis.config.BuildConfiguration#useContentBasedOutputPaths}).
+   * com.google.devtools.build.lib.analysis.config.BuildConfigurationValue#useContentBasedOutputPaths}).
    */
   public Artifact.DerivedArtifact getDerivedArtifact(
       PathFragment rootRelativePath,

src/main/java/com/google/devtools/build/lib/actions/BuildConfigurationEvent.java

@@ -25,7 +25,7 @@
 
 /**
  * Encapsulation of {@link BuildEvent} info associated with a {@link
- * com.google.devtools.build.lib.analysis.config.BuildConfiguration}.
+ * com.google.devtools.build.lib.analysis.config.BuildConfigurationValue}.
  */
 @AutoCodec
 public class BuildConfigurationEvent implements BuildEvent {

src/main/java/com/google/devtools/build/lib/analysis/AnalysisEnvironment.java

@@ -25,7 +25,7 @@
 import com.google.devtools.build.lib.actions.ArtifactRoot;
 import com.google.devtools.build.lib.actions.MiddlemanFactory;
 import com.google.devtools.build.lib.analysis.buildinfo.BuildInfoKey;
-import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
+import com.google.devtools.build.lib.analysis.config.BuildConfigurationValue;
 import com.google.devtools.build.lib.events.ExtendedEventHandler;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import com.google.devtools.build.lib.vfs.Root;
@@ -67,7 +67,7 @@ public interface AnalysisEnvironment extends ActionRegistry {
   /**
    * Same as {@link #getDerivedArtifact(PathFragment, ArtifactRoot)} but includes the option to use
    * a content-based path for this artifact (see {@link
-   * BuildConfiguration#useContentBasedOutputPaths()}).
+   * BuildConfigurationValue#useContentBasedOutputPaths()}).
    */
   Artifact.DerivedArtifact getDerivedArtifact(
       PathFragment rootRelativePath, ArtifactRoot root, boolean contentBasedPath);
@@ -178,8 +178,8 @@ Artifact.DerivedArtifact getDerivedArtifact(
    * @param stamp whether stamping is enabled
    * @param config the current build configuration.
    */
-  ImmutableList<Artifact> getBuildInfo(boolean stamp, BuildInfoKey key, BuildConfiguration config)
-      throws InterruptedException;
+  ImmutableList<Artifact> getBuildInfo(
+      boolean stamp, BuildInfoKey key, BuildConfigurationValue config) throws InterruptedException;
 
   /**
    * Returns the set of orphan Artifacts (i.e. Artifacts without generating action). Should only be

src/main/java/com/google/devtools/build/lib/analysis/AnalysisOptions.java

@@ -25,7 +25,7 @@
 /**
  * Options that affect the <i>mechanism</i> of analysis. These are distinct from {@link
  * com.google.devtools.build.lib.analysis.config.BuildOptions}, which affect the <i>value</i> of a
- * BuildConfiguration.
+ * BuildConfigurationValue.
  */
 public class AnalysisOptions extends OptionsBase {
   @Option(

src/main/java/com/google/devtools/build/lib/analysis/AnalysisRootCauseEvent.java

@@ -16,7 +16,7 @@
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.collect.ImmutableList;
-import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
+import com.google.devtools.build.lib.analysis.config.BuildConfigurationValue;
 import com.google.devtools.build.lib.buildeventstream.BuildEvent;
 import com.google.devtools.build.lib.buildeventstream.BuildEventContext;
 import com.google.devtools.build.lib.buildeventstream.BuildEventIdUtil;
@@ -35,12 +35,12 @@
  * cause. It also allows UIs to collate errors by root cause.
  */
 public class AnalysisRootCauseEvent implements BuildEventWithConfiguration {
-  private final BuildConfiguration configuration;
+  private final BuildConfigurationValue configuration;
   private final Label label;
   private final String errorMessage;
 
   public AnalysisRootCauseEvent(
-      @Nullable BuildConfiguration configuration, Label label, String errorMessage) {
+      @Nullable BuildConfigurationValue configuration, Label label, String errorMessage) {
     this.configuration = configuration;
     this.label = label;
     this.errorMessage = errorMessage;

src/main/java/com/google/devtools/build/lib/analysis/AnalysisUtils.java

@@ -19,8 +19,8 @@
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Multimap;
-import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
 import com.google.devtools.build.lib.analysis.config.BuildConfigurationCollection;
+import com.google.devtools.build.lib.analysis.config.BuildConfigurationValue;
 import com.google.devtools.build.lib.analysis.config.ConfigurationResolver;
 import com.google.devtools.build.lib.analysis.config.ConfigurationResolver.TopLevelTargetsAndConfigsResult;
 import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException;
@@ -56,7 +56,7 @@ private AnalysisUtils() {
    * it returns the value of the stamp attribute, or of the stamp option if the attribute value is
    * -1.
    */
-  public static boolean isStampingEnabled(RuleContext ruleContext, BuildConfiguration config) {
+  public static boolean isStampingEnabled(RuleContext ruleContext, BuildConfigurationValue config) {
     if (config.isToolConfiguration()) {
       return false;
     }
@@ -180,24 +180,25 @@ public static TopLevelTargetsAndConfigsResult getTargetsWithConfigs(
     // We use a hash set here to remove duplicate nodes; this can happen for input files and package
     // groups.
     LinkedHashSet<TargetAndConfiguration> nodes = new LinkedHashSet<>(targets.size());
-    for (BuildConfiguration config : configurations.getTargetConfigurations()) {
+    for (BuildConfigurationValue config : configurations.getTargetConfigurations()) {
       for (Target target : targets) {
         nodes.add(new TargetAndConfiguration(target, config));
       }
     }
 
     // We'll get the configs from ConfigurationsCollector#getConfigurations, which gets
     // configurations for deps including transitions.
-    Multimap<BuildConfiguration, DependencyKey> asDeps = targetsToDeps(nodes, ruleClassProvider);
+    Multimap<BuildConfigurationValue, DependencyKey> asDeps =
+        targetsToDeps(nodes, ruleClassProvider);
 
     return ConfigurationResolver.getConfigurationsFromExecutor(
         nodes, asDeps, eventHandler, configurationsCollector);
   }
 
   @VisibleForTesting
-  public static Multimap<BuildConfiguration, DependencyKey> targetsToDeps(
+  public static Multimap<BuildConfigurationValue, DependencyKey> targetsToDeps(
       Collection<TargetAndConfiguration> nodes, ConfiguredRuleClassProvider ruleClassProvider) {
-    Multimap<BuildConfiguration, DependencyKey> asDeps = ArrayListMultimap.create();
+    Multimap<BuildConfigurationValue, DependencyKey> asDeps = ArrayListMultimap.create();
     for (TargetAndConfiguration targetAndConfig : nodes) {
       ConfigurationTransition transition =
           TransitionResolver.evaluateTransition(

src/main/java/com/google/devtools/build/lib/analysis/AspectConfiguredEvent.java

@@ -14,7 +14,7 @@
 package com.google.devtools.build.lib.analysis;
 
 import com.google.common.collect.ImmutableList;
-import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
+import com.google.devtools.build.lib.analysis.config.BuildConfigurationValue;
 import com.google.devtools.build.lib.buildeventstream.BuildEvent;
 import com.google.devtools.build.lib.buildeventstream.BuildEventContext;
 import com.google.devtools.build.lib.buildeventstream.BuildEventIdUtil;
@@ -30,10 +30,10 @@
 public class AspectConfiguredEvent implements BuildEventWithConfiguration {
   private final Label target;
   private final String aspect;
-  private final Collection<BuildConfiguration> configurations;
+  private final Collection<BuildConfigurationValue> configurations;
 
   AspectConfiguredEvent(
-      Label target, String aspect, Collection<BuildConfiguration> configurations) {
+      Label target, String aspect, Collection<BuildConfigurationValue> configurations) {
     this.configurations = configurations;
     this.target = target;
     this.aspect = aspect;
@@ -42,7 +42,7 @@ public class AspectConfiguredEvent implements BuildEventWithConfiguration {
   @Override
   public Collection<BuildEvent> getConfigurations() {
     ImmutableList.Builder<BuildEvent> builder = new ImmutableList.Builder<>();
-    for (BuildConfiguration config : configurations) {
+    for (BuildConfigurationValue config : configurations) {
       if (config != null) {
         builder.add(config.toBuildEvent());
       } else {
@@ -60,7 +60,7 @@ public BuildEventId getEventId() {
   @Override
   public Collection<BuildEventId> getChildrenEvents() {
     ImmutableList.Builder<BuildEventId> childrenBuilder = ImmutableList.builder();
-    for (BuildConfiguration config : configurations) {
+    for (BuildConfigurationValue config : configurations) {
       if (config != null) {
         childrenBuilder.add(BuildEventIdUtil.targetCompleted(target, config.getEventId()));
       } else {

src/main/java/com/google/devtools/build/lib/analysis/BUILD

@@ -408,7 +408,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/shell",
         "//src/main/java/com/google/devtools/build/lib/skyframe:aspect_creation_exception",
         "//src/main/java/com/google/devtools/build/lib/skyframe:aspect_key_creator",
-        "//src/main/java/com/google/devtools/build/lib/skyframe:build_configuration_value",
+        "//src/main/java/com/google/devtools/build/lib/skyframe:build_configuration",
         "//src/main/java/com/google/devtools/build/lib/skyframe:build_info_collection_value",
         "//src/main/java/com/google/devtools/build/lib/skyframe:configured_target_and_data",
         "//src/main/java/com/google/devtools/build/lib/skyframe:configured_target_key",
@@ -614,7 +614,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/pkgcache",
         "//src/main/java/com/google/devtools/build/lib/profiler",
         "//src/main/java/com/google/devtools/build/lib/skyframe:aspect_key_creator",
-        "//src/main/java/com/google/devtools/build/lib/skyframe:build_configuration_value",
+        "//src/main/java/com/google/devtools/build/lib/skyframe:build_configuration",
         "//src/main/java/com/google/devtools/build/lib/skyframe:configured_target_key",
         "//src/main/java/com/google/devtools/build/lib/skyframe:coverage_report_value",
         "//src/main/java/com/google/devtools/build/lib/skyframe:skyframe_cluster",
@@ -683,7 +683,7 @@ java_library(
         ":config/config_matching_provider",
         ":transitive_info_collection",
         "//src/main/java/com/google/devtools/build/lib/cmdline",
-        "//src/main/java/com/google/devtools/build/lib/skyframe:build_configuration_value",
+        "//src/main/java/com/google/devtools/build/lib/skyframe:build_configuration",
         "//src/main/java/net/starlark/java/eval",
         "//third_party:guava",
         "//third_party:jsr305",
@@ -1512,7 +1512,7 @@ java_library(
 java_library(
     name = "config/build_configuration",
     srcs = [
-        "config/BuildConfiguration.java",
+        "config/BuildConfigurationValue.java",
         "config/OutputDirectories.java",
     ],
     deps = [
@@ -1533,12 +1533,14 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/cmdline",
         "//src/main/java/com/google/devtools/build/lib/concurrent",
         "//src/main/java/com/google/devtools/build/lib/events",
+        "//src/main/java/com/google/devtools/build/lib/skyframe:build_configuration",
         "//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec",
         "//src/main/java/com/google/devtools/build/lib/starlarkbuildapi",
         "//src/main/java/com/google/devtools/build/lib/util",
         "//src/main/java/com/google/devtools/build/lib/util:os",
         "//src/main/java/com/google/devtools/build/lib/vfs",
         "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
+        "//src/main/java/com/google/devtools/build/skyframe:skyframe-objects",
         "//src/main/java/net/starlark/java/annot",
         "//src/main/protobuf:failure_details_java_proto",
         "//third_party:guava",
@@ -2072,7 +2074,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/events",
         "//src/main/java/com/google/devtools/build/lib/packages",
         "//src/main/java/com/google/devtools/build/lib/pkgcache",
-        "//src/main/java/com/google/devtools/build/lib/skyframe:build_configuration_value",
+        "//src/main/java/com/google/devtools/build/lib/skyframe:build_configuration",
         "//src/main/protobuf:failure_details_java_proto",
         "//third_party:auto_value",
         "//third_party:guava",

src/main/java/com/google/devtools/build/lib/analysis/BaseRuleClasses.java

@@ -29,7 +29,7 @@
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
-import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
+import com.google.devtools.build.lib.analysis.config.BuildConfigurationValue;
 import com.google.devtools.build.lib.analysis.config.HostTransition;
 import com.google.devtools.build.lib.analysis.config.RunUnder;
 import com.google.devtools.build.lib.analysis.constraints.ConstraintConstants;
@@ -121,7 +121,7 @@ public boolean resolvableWithRawAttributes() {
   @SerializationConstant @AutoCodec.VisibleForSerialization @VisibleForTesting
   static final LabelListLateBoundDefault<?> ACTION_LISTENER =
       LabelListLateBoundDefault.fromTargetConfiguration(
-          BuildConfiguration.class,
+          BuildConfigurationValue.class,
           (rule, attributes, configuration) -> configuration.getActionListeners());
 
   public static final String DEFAULT_COVERAGE_SUPPORT_VALUE = "//tools/test:coverage_support";
@@ -152,13 +152,13 @@ public static LabelLateBoundDefault<TestConfiguration> coverageReportGeneratorAt
         TestConfiguration.class, defaultValue, COVERAGE_REPORT_GENERATOR_CONFIGURATION_RESOLVER);
   }
 
-  public static LabelLateBoundDefault<BuildConfiguration> getCoverageOutputGeneratorLabel() {
+  public static LabelLateBoundDefault<BuildConfigurationValue> getCoverageOutputGeneratorLabel() {
     return LabelLateBoundDefault.fromTargetConfiguration(
-        BuildConfiguration.class, null, COVERAGE_OUTPUT_GENERATOR_RESOLVER);
+        BuildConfigurationValue.class, null, COVERAGE_OUTPUT_GENERATOR_RESOLVER);
   }
 
   @SerializationConstant @AutoCodec.VisibleForSerialization
-  static final Resolver<BuildConfiguration, Label> COVERAGE_OUTPUT_GENERATOR_RESOLVER =
+  static final Resolver<BuildConfigurationValue, Label> COVERAGE_OUTPUT_GENERATOR_RESOLVER =
       (rule, attributes, configuration) -> {
         if (configuration.isCodeCoverageEnabled()) {
           return Label.parseAbsoluteUnchecked(DEFAULT_COVERAGE_OUTPUT_GENERATOR_VALUE);
@@ -172,7 +172,7 @@ public static LabelLateBoundDefault<BuildConfiguration> getCoverageOutputGenerat
   @SerializationConstant @AutoCodec.VisibleForSerialization
   public static final LabelLateBoundDefault<?> RUN_UNDER =
       LabelLateBoundDefault.fromTargetConfiguration(
-          BuildConfiguration.class,
+          BuildConfigurationValue.class,
           null,
           (rule, attributes, configuration) -> {
             RunUnder runUnder = configuration.getRunUnder();