Reimplement the configuration sanity check as a per-fragment check.

This is conceptually better (because each fragment should be
self-contained), and gives us better performance, as we no longer need to
load all explicit labels.

--
MOS_MIGRATED_REVID=118674470

2 files changed, 21 insertions, 25 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java
index 96a79209d8..9935a305b9 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java
@@ -148,9 +148,26 @@ public final class BuildConfiguration {
      * The resulting set only contains labels that were derived from command-line options; the
      * intention is that it can be used to sanity-check that the command-line options actually
      * contain these in their transitive closure.
+     *
+     * <p>This functionality only exists for legacy configuration fragments that compute labels from
+     * command-line option values. Don't do that! Instead, use a rule that specifies the mapping
+     * explicitly.
      */
     @SuppressWarnings("unused")
-    public void addImplicitLabels(Multimap<String, Label> implicitLabels) {
+    protected void addImplicitLabels(Multimap<String, Label> implicitLabels) {
+    }
+
+    /**
+     * Returns a multimap of all labels that should be implicitly loaded from labels that were
+     * specified as options, keyed by the name to be displayed to the user if something goes wrong.
+     * The returned set only contains labels that were derived from command-line options; the
+     * intention is that it can be used to sanity-check that the command-line options actually
+     * contain these in their transitive closure.
+     */
+    public final ListMultimap<String, Label> getImplicitLabels() {
+      ListMultimap<String, Label> implicitLabels = ArrayListMultimap.create();
+      addImplicitLabels(implicitLabels);
+      return implicitLabels;
     }
 
     /**
@@ -1849,21 +1866,6 @@ public final class BuildConfiguration {
   }
 
   /**
-   * Returns a multimap of all labels that should be implicitly loaded from labels that were
-   * specified as options, keyed by the name to be displayed to the user if something goes wrong.
-   * The returned set only contains labels that were derived from command-line options; the
-   * intention is that it can be used to sanity-check that the command-line options actually contain
-   * these in their transitive closure.
-   */
-  public ListMultimap<String, Label> getImplicitLabels() {
-    ListMultimap<String, Label> implicitLabels = ArrayListMultimap.create();
-    for (Fragment fragment : fragments.values()) {
-      fragment.addImplicitLabels(implicitLabels);
-    }
-    return implicitLabels;
-  }
-
-  /**
    * For an given environment, returns a subset containing all
    * variables in the given list if they are defined in the given
    * environment.
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 cd8e10cc93..31eb150cab 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
@@ -14,7 +14,6 @@
 
 package com.google.devtools.build.lib.analysis.config;
 
-import com.google.common.annotations.VisibleForTesting;
 import com.google.common.cache.Cache;
 import com.google.common.collect.ImmutableList;
 import com.google.devtools.build.lib.analysis.BlazeDirectories;
@@ -47,7 +46,6 @@ import javax.annotation.Nullable;
 public final class ConfigurationFactory {
   private final List<ConfigurationFragmentFactory> configurationFragmentFactories;
   private final ConfigurationCollectionFactory configurationCollectionFactory;
-  private boolean performSanityCheck = true;
 
   public ConfigurationFactory(
       ConfigurationCollectionFactory configurationCollectionFactory,
@@ -63,12 +61,8 @@ public final class ConfigurationFactory {
     this.configurationFragmentFactories = ImmutableList.copyOf(fragmentFactories);
   }
 
-  @VisibleForTesting
-  public void setSanityCheck(boolean performSanityCheck) {
-    this.performSanityCheck = performSanityCheck;
-  }
-
-  /** Creates a set of build configurations with top-level configuration having the given options.
+  /**
+   * Creates a set of build configurations with top-level configuration having the given options.
    *
    * <p>The rest of the configurations are created based on the set of transitions available.
    */
@@ -79,7 +73,7 @@ public final class ConfigurationFactory {
       EventHandler errorEventListener)
           throws InvalidConfigurationException {
     return configurationCollectionFactory.createConfigurations(this, cache,
-        loadedPackageProvider, buildOptions, errorEventListener, performSanityCheck);
+        loadedPackageProvider, buildOptions, errorEventListener);
   }
 
   /**