Create top-level configs dynamically: period.

Before this change, top-level configs were still built statically,
then cloned into dynamic configs to run the actual build.

After this change, no static configuration should ever be created in
a dynamically configured build.

We're still keeping support for --experimental_dynamic_configs=off a
bit longer to provide easy reversion on any bad surprises. But barring
that we'll quickly move toward making --experimental_dynamic_configs=off
a no-op, then ripping out Bazel's static configuration logic.

PiperOrigin-RevId: 161005150

6 files changed, 163 insertions, 26 deletions

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 1459eece70..cd11d56728 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
@@ -41,7 +41,9 @@ import java.util.Set;
 import javax.annotation.Nullable;
 
 /**
- * Configuration collection used by the rules Bazel knows.
+ * Configuration collection used by the rules Bazel knows for statically configured builds.
+  *
+  * <p>Dynamically configured builds should never touch this file.
  */
 public class BazelConfigurationCollection implements ConfigurationCollectionFactory {
   @Override
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 32490cda8e..c825ccfce2 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
@@ -195,6 +195,9 @@ public final class BuildTool {
       env.throwPendingException();
 
       // Configuration creation.
+      // TODO(gregce): BuildConfigurationCollection is important for static configs, less so for
+      // dynamic configs. Consider dropping it outright and passing on-the-fly target / host configs
+      // directly when needed (although this could be hard when Skyframe is unavailable).
       BuildConfigurationCollection configurations =
           env.getSkyframeExecutor()
               .createConfigurations(
@@ -251,8 +254,8 @@ public final class BuildTool {
     } catch (RuntimeException e) {
       // Print an error message for unchecked runtime exceptions. This does not concern Error
       // subclasses such as OutOfMemoryError.
-      request.getOutErr().printErrLn("Unhandled exception thrown during build; message: " +
-          e.getMessage());
+      request.getOutErr().printErrLn(
+          "Unhandled exception thrown during build; message: " + e.getMessage());
       catastrophe = true;
       throw e;
     } catch (Error e) {
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/CommandEnvironment.java b/src/main/java/com/google/devtools/build/lib/runtime/CommandEnvironment.java
index 48092a1a70..43c5faa4fd 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/CommandEnvironment.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/CommandEnvironment.java
@@ -17,7 +17,6 @@ package com.google.devtools.build.lib.runtime;
 import static com.google.devtools.build.lib.profiler.AutoProfiler.profiled;
 
 import com.google.common.annotations.VisibleForTesting;
-import com.google.common.collect.ImmutableSet;
 import com.google.common.eventbus.EventBus;
 import com.google.devtools.build.lib.actions.PackageRootResolver;
 import com.google.devtools.build.lib.actions.cache.ActionCache;
@@ -25,10 +24,7 @@ import com.google.devtools.build.lib.analysis.BlazeDirectories;
 import com.google.devtools.build.lib.analysis.BuildView;
 import com.google.devtools.build.lib.analysis.SkyframePackageRootResolver;
 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.BuildOptions;
 import com.google.devtools.build.lib.analysis.config.DefaultsPackage;
-import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.events.Reporter;
 import com.google.devtools.build.lib.exec.OutputService;
@@ -416,19 +412,6 @@ public final class CommandEnvironment {
   }
 
   /**
-   * 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 {
-    BuildOptions buildOptions = runtime.createBuildOptions(optionsProvider);
-    boolean keepGoing = optionsProvider.getOptions(BuildView.Options.class).keepGoing;
-    return getSkyframeExecutor().createConfigurations(reporter, runtime.getConfigurationFactory(),
-        buildOptions, ImmutableSet.<String>of(), keepGoing);
-  }
-
-  /**
    * Prevents any further interruption of this command by modules, and returns the final exit code
    * from modules, or null if no modules requested an abrupt exit.
    *
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/InfoCommand.java b/src/main/java/com/google/devtools/build/lib/runtime/commands/InfoCommand.java
index 802d18c1d9..134b531e00 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/commands/InfoCommand.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/commands/InfoCommand.java
@@ -121,10 +121,9 @@ public class InfoCommand implements BlazeCommand {
           env.setupPackageCache(
               optionsProvider, runtime.getDefaultsPackageContent(optionsProvider));
           // TODO(bazel-team): What if there are multiple configurations? [multi-config]
-          configuration = env
-              .getConfigurations(optionsProvider)
-              .getTargetConfigurations().get(0);
-          return configuration;
+          env.getSkyframeExecutor().setConfigurationFactory(runtime.getConfigurationFactory());
+          return env.getSkyframeExecutor().getConfiguration(
+              env.getReporter(), runtime.createBuildOptions(optionsProvider));
         } catch (InvalidConfigurationException e) {
           env.getReporter().handle(Event.error(e.getMessage()));
           throw new ExitCausingRuntimeException(ExitCode.COMMAND_LINE_ERROR);
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 f782e15b24..243d3d56c2 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
@@ -13,6 +13,7 @@
 // limitations under the License.
 package com.google.devtools.build.lib.skyframe;
 
+import com.google.common.base.Preconditions;
 import com.google.common.base.Supplier;
 import com.google.common.cache.Cache;
 import com.google.common.cache.CacheBuilder;
@@ -62,6 +63,11 @@ public class ConfigurationCollectionFunction implements SkyFunction {
       return null;
     }
     ConfigurationCollectionKey collectionKey = (ConfigurationCollectionKey) skyKey.argument();
+    Preconditions.checkState(collectionKey.getBuildOptions()
+        .get(BuildConfiguration.Options.class).useDynamicConfigurations
+            == BuildConfiguration.Options.DynamicConfigsMode.OFF,
+        "configuration collections don't need to be Skyframe-loaded for dynamic configurations");
+
     try {
       BuildConfigurationCollection result = getConfigurations(env,
           new SkyframePackageLoaderWithValueEnvironment(env, ruleClassProvider),
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 db4528105a..3bf6102e58 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
@@ -70,13 +70,16 @@ import com.google.devtools.build.lib.analysis.config.BuildConfigurationCollectio
 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;
+import com.google.devtools.build.lib.analysis.config.HostTransition;
 import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException;
+import com.google.devtools.build.lib.analysis.config.PatchTransition;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
 import com.google.devtools.build.lib.cmdline.PackageIdentifier;
 import com.google.devtools.build.lib.cmdline.TargetParsingException;
 import com.google.devtools.build.lib.concurrent.ThreadSafety;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadCompatible;
+import com.google.devtools.build.lib.events.ErrorSensingEventHandler;
 import com.google.devtools.build.lib.events.ExtendedEventHandler;
 import com.google.devtools.build.lib.events.Reporter;
 import com.google.devtools.build.lib.exec.OutputService;
@@ -1038,6 +1041,14 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
   }
 
   /**
+   * Sets the configuration factory and known fragment set.
+   */
+  public void setConfigurationFactory(ConfigurationFactory configurationFactory) {
+    this.configurationFactory.set(configurationFactory);
+    this.configurationFragments.set(ImmutableList.copyOf(configurationFactory.getFactories()));
+  }
+
+  /**
    * Asks the Skyframe evaluator to build the value for BuildConfigurationCollection and returns the
    * result. Also invalidates {@link PrecomputedValue#BLAZE_DIRECTORIES} if it has changed.
    */
@@ -1048,9 +1059,24 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
       Set<String> multiCpu,
       boolean keepGoing)
       throws InvalidConfigurationException, InterruptedException {
-    this.configurationFactory.set(configurationFactory);
-    this.configurationFragments.set(ImmutableList.copyOf(configurationFactory.getFactories()));
+    setConfigurationFactory(configurationFactory);
+    if (buildOptions.get(BuildConfiguration.Options.class).useDynamicConfigurations
+        == BuildConfiguration.Options.DynamicConfigsMode.OFF) {
+      return createStaticConfigurations(eventHandler, buildOptions, multiCpu, keepGoing);
+    } else {
+      return createDynamicConfigurations(eventHandler, buildOptions, multiCpu);
+    }
+  }
 
+  /**
+   * {@link #createConfigurations} implementation that creates the configurations statically.
+   */
+  private BuildConfigurationCollection createStaticConfigurations(
+      ExtendedEventHandler eventHandler,
+      BuildOptions buildOptions,
+      Set<String> multiCpu,
+      boolean keepGoing)
+      throws InvalidConfigurationException, InterruptedException {
     SkyKey skyKey = ConfigurationCollectionValue.key(
         buildOptions, ImmutableSortedSet.copyOf(multiCpu));
     EvaluationResult<ConfigurationCollectionValue> result =
@@ -1074,6 +1100,61 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
     return configurationValue.getConfigurationCollection();
   }
 
+  /**
+   * {@link #createConfigurations} implementation that creates the configurations dynamically.
+   */
+  private BuildConfigurationCollection createDynamicConfigurations(
+      ExtendedEventHandler eventHandler,
+      BuildOptions buildOptions,
+      Set<String> multiCpu)
+      throws InvalidConfigurationException, InterruptedException {
+    List<BuildConfiguration> topLevelTargetConfigs =
+        getConfigurations(eventHandler, getTopLevelBuildOptions(buildOptions, multiCpu));
+
+    // The host configuration inherits the data, not target options. This is so host tools don't
+    // apply LIPO.
+    BuildConfiguration firstTargetConfig = topLevelTargetConfigs.get(0);
+    Attribute.Transition dataTransition = firstTargetConfig.getTransitions()
+        .getDynamicTransition(Attribute.ConfigurationTransition.DATA);
+    BuildOptions dataOptions = dataTransition != Attribute.ConfigurationTransition.NONE
+        ? ((PatchTransition) dataTransition).apply(firstTargetConfig.getOptions())
+        : firstTargetConfig.getOptions();
+
+    BuildOptions hostOptions =
+        dataOptions.get(BuildConfiguration.Options.class).useDistinctHostConfiguration
+            ? HostTransition.INSTANCE.apply(dataOptions)
+            : dataOptions;
+    BuildConfiguration hostConfig = getConfiguration(eventHandler, hostOptions);
+
+    // TODO(gregce): cache invalid option errors in BuildConfigurationFunction, then use a dedicated
+    // accessor (i.e. not the event handler) to trigger the exception below.
+    ErrorSensingEventHandler nosyEventHandler = new ErrorSensingEventHandler(eventHandler);
+    topLevelTargetConfigs.forEach(config -> config.reportInvalidOptions(nosyEventHandler));
+    if (nosyEventHandler.hasErrors()) {
+      throw new InvalidConfigurationException("Build options are invalid");
+    }
+    return new BuildConfigurationCollection(topLevelTargetConfigs, hostConfig);
+  }
+
+  /**
+   * Returns the {@link BuildOptions} to apply to the top-level build configurations. This can be
+   * plural because of {@code multiCpu}.
+   */
+  private static List<BuildOptions> getTopLevelBuildOptions(BuildOptions buildOptions,
+      Set<String> multiCpu) {
+    if (multiCpu.isEmpty()) {
+      return ImmutableList.of(buildOptions);
+    }
+    ImmutableList.Builder<BuildOptions> multiCpuOptions = ImmutableList.builder();
+    for (String cpu : multiCpu) {
+      BuildOptions clonedOptions = buildOptions.clone();
+      clonedOptions.get(BuildConfiguration.Options.class).cpu = cpu;
+      clonedOptions.get(BuildConfiguration.Options.class).experimentalMultiCpuDistinguisher = cpu;
+      multiCpuOptions.add(clonedOptions);
+    }
+    return multiCpuOptions.build();
+  }
+
   private Iterable<ActionLookupValue> getActionLookupValues() {
     // This filter keeps subclasses of ActionLookupValue.
     return Iterables.filter(memoizingEvaluator.getDoneValues().values(), ActionLookupValue.class);
@@ -1299,6 +1380,69 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
   }
 
   /**
+   * Returns the configuration corresponding to the given set of build options.
+   *
+   * @throws InvalidConfigurationException if the build options produces an invalid configuration
+   */
+  public BuildConfiguration getConfiguration(ExtendedEventHandler eventHandler,
+      BuildOptions options) throws InvalidConfigurationException {
+    return Iterables.getOnlyElement(
+        getConfigurations(eventHandler, ImmutableList.<BuildOptions>of(options)));
+  }
+
+  /**
+   * Returns the configurations corresponding to the given sets of build options. Output order is
+   * the same as input order.
+   *
+   * @throws InvalidConfigurationException if any build options produces an invalid configuration
+   */
+  public List<BuildConfiguration> getConfigurations(ExtendedEventHandler eventHandler,
+      List<BuildOptions> optionsList) throws InvalidConfigurationException {
+    Preconditions.checkArgument(!Iterables.isEmpty(optionsList));
+
+    // Prepare the Skyframe inputs.
+    // TODO(gregce): support trimmed configs.
+    Set<Class<? extends BuildConfiguration.Fragment>> allFragments =
+        configurationFragments.get()
+            .stream()
+            .map(factory -> factory.creates())
+            .collect(ImmutableSet.toImmutableSet());
+    final ImmutableList<SkyKey> configSkyKeys =
+        optionsList
+            .stream()
+            .map(elem -> BuildConfigurationValue.key(allFragments, elem))
+            .collect(ImmutableList.toImmutableList());
+
+    // Skyframe-evaluate the configurations and throw errors if any.
+    EvaluationResult<SkyValue> evalResult =
+        evaluateSkyKeys(eventHandler, configSkyKeys, /*keepGoing=*/true);
+    if (evalResult.hasError()) {
+      Map.Entry<SkyKey, ErrorInfo> firstError = Iterables.get(evalResult.errorMap().entrySet(), 0);
+      ErrorInfo error = firstError.getValue();
+      Throwable e = error.getException();
+      // Wrap loading failed exceptions
+      if (e instanceof NoSuchThingException) {
+        e = new InvalidConfigurationException(e);
+      } else if (e == null && !Iterables.isEmpty(error.getCycleInfo())) {
+        getCyclesReporter().reportCycles(error.getCycleInfo(), firstError.getKey(), eventHandler);
+        e = new InvalidConfigurationException(
+            "cannot load build configuration because of this cycle");
+      }
+      if (e != null) {
+        Throwables.throwIfInstanceOf(e, InvalidConfigurationException.class);
+      }
+      throw new IllegalStateException(
+          "Unknown error during ConfigurationCollectionValue evaluation", e);
+    }
+
+    // Prepare and return the results.
+    return configSkyKeys
+        .stream()
+        .map(key -> ((BuildConfigurationValue) evalResult.get(key)).getConfiguration())
+        .collect(ImmutableList.toImmutableList());
+}
+
+  /**
    * Retrieves the configurations needed for the given deps. If {@link
    * BuildConfiguration.Options#trimConfigurations()} is true, trims their fragments to only those
    * needed by their transitive closures. Else unconditionally includes all fragments.