Simplify the createConfiguration method in SkyframeExecutor.

Instead of passing BuildConfigurationKey instances around, just pass in the
little data we actually need. This allows removing the BuildConfigurationKey
class.

--
MOS_MIGRATED_REVID=91865340

8 files changed, 34 insertions, 143 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ConfigurationCollectionFactory.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfigurationCollectionFactory.java
index 340af2826b..4ce7426433 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/ConfigurationCollectionFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfigurationCollectionFactory.java
@@ -41,7 +41,7 @@ public interface ConfigurationCollectionFactory {
    * @throws InvalidConfigurationException
    */
   @Nullable
-  public BuildConfiguration createConfigurations(
+  BuildConfiguration createConfigurations(
       ConfigurationFactory configurationFactory,
       PackageProviderForConfigurations loadedPackageProvider,
       BuildOptions buildOptions,
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfigurationKey.java b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfigurationKey.java
deleted file mode 100644
index 065f3937b2..0000000000
--- a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfigurationKey.java
+++ /dev/null
@@ -1,82 +0,0 @@
-// Copyright 2014 Google Inc. All rights reserved.
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//    http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-package com.google.devtools.build.lib.analysis.config;
-
-import com.google.common.base.Preconditions;
-import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.ImmutableSortedSet;
-import com.google.common.collect.ListMultimap;
-import com.google.devtools.build.lib.analysis.BlazeDirectories;
-import com.google.devtools.build.lib.syntax.Label;
-
-import java.util.Objects;
-import java.util.Set;
-
-/**
- * A key for the creation of {@link BuildConfigurationCollection} instances.
- */
-public final class BuildConfigurationKey {
-
-  private final BuildOptions buildOptions;
-  private final BlazeDirectories directories;
-  private final ImmutableSortedSet<String> multiCpu;
-
-  /**
-   * Creates a key for the creation of {@link BuildConfigurationCollection} instances.
-   *
-   * Note that the BuildConfiguration.Options instance must not contain unresolved relative paths.
-   */
-  public BuildConfigurationKey(
-      BuildOptions buildOptions, BlazeDirectories directories, Set<String> multiCpu) {
-    this.buildOptions = Preconditions.checkNotNull(buildOptions);
-    this.directories = Preconditions.checkNotNull(directories);
-    this.multiCpu = ImmutableSortedSet.copyOf(multiCpu);
-  }
-
-  public BuildConfigurationKey(BuildOptions buildOptions, BlazeDirectories directories) {
-    this(buildOptions, directories, ImmutableSet.<String>of());
-  }
-
-  public BuildOptions getBuildOptions() {
-    return buildOptions;
-  }
-
-  public BlazeDirectories getDirectories() {
-    return directories;
-  }
-
-  public ImmutableSortedSet<String> getMultiCpu() {
-    return multiCpu;
-  }
-
-  public ListMultimap<String, Label> getLabelsToLoadUnconditionally() {
-    return buildOptions.getAllLabels();
-  }
-
-  @Override
-  public boolean equals(Object o) {
-    if (!(o instanceof BuildConfigurationKey)) {
-      return false;
-    }
-    BuildConfigurationKey k = (BuildConfigurationKey) o;
-    return buildOptions.equals(k.buildOptions)
-        && directories.equals(k.directories)
-        && multiCpu.equals(k.multiCpu);
-  }
-
-  @Override
-  public int hashCode() {
-    return Objects.hash(buildOptions, directories, multiCpu);
-  }
-}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigurationFactory.java b/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigurationFactory.java
index 56b259e004..e623bdf38d 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigurationFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/ConfigurationFactory.java
@@ -81,7 +81,7 @@ public final class ConfigurationFactory {
   @Nullable
   public BuildConfiguration createConfiguration(
       PackageProviderForConfigurations loadedPackageProvider, BuildOptions buildOptions,
-      BuildConfigurationKey key, EventHandler errorEventListener)
+      EventHandler errorEventListener)
           throws InvalidConfigurationException {
     return configurationCollectionFactory.createConfigurations(this,
         loadedPackageProvider, buildOptions, errorEventListener, performSanityCheck);
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelConfigurationCollection.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelConfigurationCollection.java
index 10eca9d5bc..47689efa0d 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelConfigurationCollection.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelConfigurationCollection.java
@@ -24,7 +24,6 @@ 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.BuildConfigurationCollection.ConfigurationHolder;
 import com.google.devtools.build.lib.analysis.config.BuildConfigurationCollection.Transitions;
-import com.google.devtools.build.lib.analysis.config.BuildConfigurationKey;
 import com.google.devtools.build.lib.analysis.config.BuildOptions;
 import com.google.devtools.build.lib.analysis.config.ConfigurationFactory;
 import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException;
@@ -195,8 +194,8 @@ public class BazelConfigurationCollection implements ConfigurationCollectionFact
 
   /**
    * Checks that the implicit labels are reachable from the loaded labels. The loaded labels are
-   * those returned from {@link BuildConfigurationKey#getLabelsToLoadUnconditionally()}, and the
-   * implicit ones are those that need to be available for late-bound attributes.
+   * those returned from {@link BuildOptions#getAllLabels()}, and the implicit ones are those that
+   * need to be available for late-bound attributes.
    */
   private void sanityCheckImplicitLabels(Collection<Label> reachableLabels,
       BuildConfiguration config) throws InvalidConfigurationException {
diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java b/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java
index a9c3ec8eba..f7fc0f8b2c 100644
--- a/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java
+++ b/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java
@@ -39,7 +39,6 @@ import com.google.devtools.build.lib.analysis.RuleConfiguredTarget;
 import com.google.devtools.build.lib.analysis.ViewCreationFailedException;
 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.BuildConfigurationKey;
 import com.google.devtools.build.lib.analysis.config.BuildOptions;
 import com.google.devtools.build.lib.analysis.config.DefaultsPackage;
 import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException;
@@ -175,8 +174,7 @@ public class BuildTool {
               + "'test' right now!");
         }
       }
-      configurations = getConfigurations(
-          runtime.getBuildConfigurationKey(buildOptions, request.getMultiCpus()),
+      configurations = getConfigurations(buildOptions, request.getMultiCpus(),
           request.getViewOptions().keepGoing);
 
       getEventBus().post(new ConfigurationsCreatedEvent(configurations));
@@ -378,13 +376,15 @@ public class BuildTool {
     return result;
   }
 
-  protected final BuildConfigurationCollection getConfigurations(BuildConfigurationKey key,
-      boolean keepGoing)
+  private final BuildConfigurationCollection getConfigurations(BuildOptions buildOptions,
+      Set<String> multiCpu, boolean keepGoing)
       throws InvalidConfigurationException, InterruptedException {
     SkyframeExecutor executor = runtime.getSkyframeExecutor();
     // TODO(bazel-team): consider a possibility of moving ConfigurationFactory construction into
     // skyframe.
-    return executor.createConfigurations(keepGoing, runtime.getConfigurationFactory(), key);
+    return executor.createConfigurations(
+        runtime.getConfigurationFactory(), buildOptions, runtime.getDirectories(), multiCpu,
+        keepGoing);
   }
 
   @VisibleForTesting
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java
index 2309b6e9a8..704cf32ad4 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java
@@ -24,7 +24,6 @@ import com.google.common.base.Predicates;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.ImmutableSortedSet;
 import com.google.common.collect.Lists;
 import com.google.common.collect.Range;
 import com.google.common.collect.Sets;
@@ -45,7 +44,6 @@ import com.google.devtools.build.lib.analysis.WorkspaceStatusAction;
 import com.google.devtools.build.lib.analysis.config.BinTools;
 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.BuildConfigurationKey;
 import com.google.devtools.build.lib.analysis.config.BuildOptions;
 import com.google.devtools.build.lib.analysis.config.ConfigurationFactory;
 import com.google.devtools.build.lib.analysis.config.DefaultsPackage;
@@ -982,32 +980,23 @@ public final class BlazeRuntime {
   }
 
   /**
-   * Constructs a build configuration key for the given options.
-   */
-  public BuildConfigurationKey getBuildConfigurationKey(BuildOptions buildOptions,
-      ImmutableSortedSet<String> multiCpu) {
-    return new BuildConfigurationKey(buildOptions, directories, multiCpu);
-  }
-
-  /**
    * This method only exists for the benefit of InfoCommand, which needs to construct a {@link
    * BuildConfigurationCollection} without running a full loading phase. Don't add any more clients;
    * instead, we should change info so that it doesn't need the configuration.
    */
   public BuildConfigurationCollection getConfigurations(OptionsProvider optionsProvider)
       throws InvalidConfigurationException, InterruptedException {
-    BuildConfigurationKey configurationKey = getBuildConfigurationKey(
-        createBuildOptions(optionsProvider), ImmutableSortedSet.<String>of());
+    BuildOptions buildOptions = createBuildOptions(optionsProvider);
     boolean keepGoing = optionsProvider.getOptions(BuildView.Options.class).keepGoing;
     LoadedPackageProvider loadedPackageProvider =
         loadingPhaseRunner.loadForConfigurations(reporter,
-            ImmutableSet.copyOf(configurationKey.getLabelsToLoadUnconditionally().values()),
+            ImmutableSet.copyOf(buildOptions.getAllLabels().values()),
             keepGoing);
     if (loadedPackageProvider == null) {
       throw new InvalidConfigurationException("Configuration creation failed");
     }
-    return skyframeExecutor.createConfigurations(keepGoing, configurationFactory,
-        configurationKey);
+    return skyframeExecutor.createConfigurations(configurationFactory,
+        buildOptions, directories, ImmutableSet.<String>of(), keepGoing);
   }
 
   /**
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ConfigurationCollectionFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ConfigurationCollectionFunction.java
index 5e16f0c00b..b00f2c8cb8 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ConfigurationCollectionFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ConfigurationCollectionFunction.java
@@ -14,10 +14,9 @@
 package com.google.devtools.build.lib.skyframe;
 
 import com.google.common.base.Supplier;
-import com.google.devtools.build.lib.analysis.BlazeDirectories;
+import com.google.common.collect.ImmutableSet;
 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.BuildConfigurationKey;
 import com.google.devtools.build.lib.analysis.config.BuildOptions;
 import com.google.devtools.build.lib.analysis.config.ConfigurationFactory;
 import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException;
@@ -57,16 +56,10 @@ public class ConfigurationCollectionFunction implements SkyFunction {
       ConfigurationCollectionFunctionException {
     ConfigurationCollectionKey collectionKey = (ConfigurationCollectionKey) skyKey.argument();
     try {
-      BlazeDirectories directories = PrecomputedValue.BLAZE_DIRECTORIES.get(env);
-      if (env.valuesMissing()) {
-        return null;
-      }
-
       BuildConfigurationCollection result =
           getConfigurations(env.getListener(),
-          new SkyframePackageLoaderWithValueEnvironment(env, configurationPackages.get()),
-          new BuildConfigurationKey(
-              collectionKey.getBuildOptions(), directories, collectionKey.getMultiCpu()));
+              new SkyframePackageLoaderWithValueEnvironment(env, configurationPackages.get()),
+              collectionKey.getBuildOptions(), collectionKey.getMultiCpu());
 
       // BuildConfigurationCollection can be created, but dependencies to some files might be
       // missing. In that case we need to build configurationCollection again.
@@ -88,13 +81,14 @@ public class ConfigurationCollectionFunction implements SkyFunction {
 
   /** Create the build configurations with the given options. */
   private BuildConfigurationCollection getConfigurations(EventHandler eventHandler,
-      PackageProviderForConfigurations loadedPackageProvider, BuildConfigurationKey key)
+      PackageProviderForConfigurations loadedPackageProvider, BuildOptions buildOptions,
+      ImmutableSet<String> multiCpu)
           throws InvalidConfigurationException {
     List<BuildConfiguration> targetConfigurations = new ArrayList<>();
-    if (!key.getMultiCpu().isEmpty()) {
-      for (String cpu : key.getMultiCpu()) {
+    if (!multiCpu.isEmpty()) {
+      for (String cpu : multiCpu) {
         BuildConfiguration targetConfiguration = createConfiguration(
-            eventHandler, loadedPackageProvider, key, cpu);
+            eventHandler, loadedPackageProvider, buildOptions, cpu);
         if (targetConfiguration == null || targetConfigurations.contains(targetConfiguration)) {
           continue;
         }
@@ -105,7 +99,7 @@ public class ConfigurationCollectionFunction implements SkyFunction {
       }
     } else {
       BuildConfiguration targetConfiguration = createConfiguration(
-          eventHandler, loadedPackageProvider, key, null);
+          eventHandler, loadedPackageProvider, buildOptions, null);
       if (targetConfiguration == null) {
         return null;
       }
@@ -118,9 +112,8 @@ public class ConfigurationCollectionFunction implements SkyFunction {
   public BuildConfiguration createConfiguration(
       EventHandler originalEventListener,
       PackageProviderForConfigurations loadedPackageProvider,
-      BuildConfigurationKey key, String cpuOverride) throws InvalidConfigurationException {
+      BuildOptions buildOptions, String cpuOverride) throws InvalidConfigurationException {
     StoredEventHandler errorEventListener = new StoredEventHandler();
-    BuildOptions buildOptions = key.getBuildOptions();
     if (cpuOverride != null) {
       // TODO(bazel-team): Options classes should be immutable. This is a bit of a hack.
       buildOptions = buildOptions.clone();
@@ -128,7 +121,7 @@ public class ConfigurationCollectionFunction implements SkyFunction {
     }
 
     BuildConfiguration targetConfig = configurationFactory.get().createConfiguration(
-        loadedPackageProvider, buildOptions, key, errorEventListener);
+        loadedPackageProvider, buildOptions, errorEventListener);
     if (targetConfig == null) {
       return null;
     }
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
index ed41872152..8a8035c7df 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
@@ -22,6 +22,7 @@ import com.google.common.base.Throwables;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.ImmutableSortedSet;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Maps;
 import com.google.common.collect.Range;
@@ -53,7 +54,6 @@ import com.google.devtools.build.lib.analysis.buildinfo.BuildInfoFactory.BuildIn
 import com.google.devtools.build.lib.analysis.config.BinTools;
 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.BuildConfigurationKey;
 import com.google.devtools.build.lib.analysis.config.BuildOptions;
 import com.google.devtools.build.lib.analysis.config.ConfigurationFactory;
 import com.google.devtools.build.lib.analysis.config.ConfigurationFragmentFactory;
@@ -846,31 +846,23 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
     this.configurationPackages.set(Sets.<Package>newConcurrentHashSet());
   }
 
-  @VisibleForTesting
-  public BuildConfigurationCollection createConfigurations(
-      ConfigurationFactory configurationFactory, BuildConfigurationKey configurationKey)
-      throws InvalidConfigurationException, InterruptedException {
-    return createConfigurations(false, configurationFactory, configurationKey);
-  }
-
   /**
-   * Asks the Skyframe evaluator to build the value for BuildConfigurationCollection and
-   * returns result. Also invalidates {@link PrecomputedValue#TEST_ENVIRONMENT_VARIABLES} and
-   * {@link PrecomputedValue#BLAZE_DIRECTORIES} if they have changed.
+   * Asks the Skyframe evaluator to build the value for BuildConfigurationCollection and returns the
+   * result. Also invalidates {@link PrecomputedValue#BLAZE_DIRECTORIES} if it has changed.
    */
-  public BuildConfigurationCollection createConfigurations(boolean keepGoing,
-      ConfigurationFactory configurationFactory, BuildConfigurationKey configurationKey)
+  public BuildConfigurationCollection createConfigurations(
+      ConfigurationFactory configurationFactory, BuildOptions buildOptions,
+      BlazeDirectories directories, Set<String> multiCpu, boolean keepGoing)
       throws InvalidConfigurationException, InterruptedException {
-
     this.configurationPackages.set(Sets.<Package>newConcurrentHashSet());
     this.configurationFactory.set(configurationFactory);
     this.configurationFragments.set(ImmutableList.copyOf(configurationFactory.getFactories()));
     // TODO(bazel-team): find a way to use only BuildConfigurationKey instead of
     // BlazeDirectories.
-    PrecomputedValue.BLAZE_DIRECTORIES.set(injectable(), configurationKey.getDirectories());
+    PrecomputedValue.BLAZE_DIRECTORIES.set(injectable(), directories);
 
-    SkyKey skyKey = ConfigurationCollectionValue.key(configurationKey.getBuildOptions(),
-        configurationKey.getMultiCpu());
+    SkyKey skyKey = ConfigurationCollectionValue.key(
+        buildOptions, ImmutableSortedSet.copyOf(multiCpu));
     setConfigurationSkyKey(skyKey);
     EvaluationResult<ConfigurationCollectionValue> result = buildDriver.evaluate(
             Arrays.asList(skyKey), keepGoing, DEFAULT_THREAD_COUNT, errorEventListener);