Remove the "labels in the configuration must be in the transitive closure of command line labels" sanity check.

It's only used for --libc_top and makes configuration creation much more heavyweight that it would otherwise be.

--
MOS_MIGRATED_REVID=134513058

3 files changed, 0 insertions, 137 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 52dfd99bea..7b89d6f142 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
@@ -139,42 +139,6 @@ public final class BuildConfiguration {
     }
 
     /**
-     * Collects 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 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")
-    protected void addImplicitLabels(Multimap<String, Label> implicitLabels) {
-    }
-
-    /**
-     * Returns the roots used for the "all labels in the configuration must be reachable from the
-     * labels provided on the command line" sanity check.
-     */
-    public Iterable<Label> getSanityCheckRoots() {
-      return ImmutableList.of();
-    }
-
-    /**
-     * 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;
-    }
-
-    /**
      * Returns a fragment of the output directory name for this configuration. The output
      * directory for the whole configuration contains all the short names by all fragments.
      */
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java
index 6c5445e5e0..95f37e7d0c 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java
@@ -24,7 +24,6 @@ import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.ListMultimap;
 import com.google.common.collect.Maps;
-import com.google.common.collect.Multimap;
 import com.google.devtools.build.lib.analysis.RuleContext;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
 import com.google.devtools.build.lib.analysis.config.BuildOptions;
@@ -1999,26 +1998,6 @@ public class CppConfiguration extends BuildConfiguration.Fragment {
   }
 
   @Override
-  public void addImplicitLabels(Multimap<String, Label> implicitLabels) {
-    if (getLibcLabel() != null) {
-      implicitLabels.put("crosstool", getLibcLabel());
-    }
-
-    implicitLabels.put("crosstool", crosstoolTop);
-  }
-
-  @Override
-  public Iterable<Label> getSanityCheckRoots() {
-    ImmutableList.Builder<Label> result = ImmutableList.builder();
-    result.add(cppOptions.crosstoolTop);
-    if (cppOptions.libcTop != null) {
-      result.add(cppOptions.libcTop.getLabel());
-    }
-
-    return result.build();
-  }
-
-  @Override
   public String getOutputDirectoryName() {
     String lipoSuffix;
     if (getLipoMode() != LipoMode.OFF && !isAutoFdoLipo()) {
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ConfigurationFragmentFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ConfigurationFragmentFunction.java
index 075811797b..d560b8af11 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ConfigurationFragmentFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ConfigurationFragmentFunction.java
@@ -15,7 +15,6 @@ package com.google.devtools.build.lib.skyframe;
 
 import com.google.common.base.Supplier;
 import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ListMultimap;
 import com.google.devtools.build.lib.analysis.BlazeDirectories;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration.Fragment;
 import com.google.devtools.build.lib.analysis.config.BuildOptions;
@@ -25,16 +24,10 @@ import com.google.devtools.build.lib.analysis.config.InvalidConfigurationExcepti
 import com.google.devtools.build.lib.analysis.config.PackageProviderForConfigurations;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
-import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.events.EventHandler;
-import com.google.devtools.build.lib.packages.AggregatingAttributeMapper;
-import com.google.devtools.build.lib.packages.Attribute;
-import com.google.devtools.build.lib.packages.AttributeMap.AcceptsLabelAttribute;
 import com.google.devtools.build.lib.packages.NoSuchPackageException;
 import com.google.devtools.build.lib.packages.NoSuchTargetException;
-import com.google.devtools.build.lib.packages.NoSuchThingException;
 import com.google.devtools.build.lib.packages.Package;
-import com.google.devtools.build.lib.packages.Rule;
 import com.google.devtools.build.lib.packages.RuleClassProvider;
 import com.google.devtools.build.lib.packages.Target;
 import com.google.devtools.build.lib.skyframe.ConfigurationFragmentValue.ConfigurationFragmentKey;
@@ -44,10 +37,6 @@ import com.google.devtools.build.skyframe.SkyFunctionException;
 import com.google.devtools.build.skyframe.SkyKey;
 import com.google.devtools.build.skyframe.SkyValue;
 import java.io.IOException;
-import java.util.HashSet;
-import java.util.LinkedHashSet;
-import java.util.Map;
-import java.util.Set;
 
 /**
  * A builder for {@link ConfigurationFragmentValue}s.
@@ -79,10 +68,6 @@ public final class ConfigurationFragmentFunction implements SkyFunction {
       if (env.valuesMissing()) {
         return null;
       }
-      sanityCheck(fragment, packageProvider);
-      if (env.valuesMissing()) {
-        return null;
-      }
       return new ConfigurationFragmentValue(fragment);
     } catch (InvalidConfigurationException e) {
       // TODO(bazel-team): Rework the control-flow here so that we're not actually throwing this
@@ -94,71 +79,6 @@ public final class ConfigurationFragmentFunction implements SkyFunction {
     }
   }
 
-  /**
-   * Checks that the implicit labels are reachable from the loaded labels. The loaded labels are
-   * those returned from {@link BuildOptions#getAllLabels()}, and the implicit ones are those that
-   * are returned from {@link Fragment#getImplicitLabels}.
-   */
-  private static void sanityCheck(
-      Fragment fragment,
-      PackageProviderForConfigurations packageProvider)
-      throws InvalidConfigurationException, InterruptedException {
-    if (fragment == null) {
-      return;
-    }
-    ListMultimap<String, Label> implicitLabels = fragment.getImplicitLabels();
-    if (implicitLabels.isEmpty()) {
-      return;
-    }
-    // Sanity check that the implicit labels are all in the transitive closure of explicit ones.
-    // This also registers all targets in the cache entry and validates them on subsequent requests.
-    Set<Label> reachableLabels = new HashSet<>();
-    for (Label root : fragment.getSanityCheckRoots()) {
-      try {
-        collectAllTransitiveLabels(packageProvider, reachableLabels, root);
-      } catch (NoSuchThingException e) {
-        packageProvider.getEventHandler().handle(Event.error(e.getMessage()));
-        throw new InvalidConfigurationException(
-            String.format("Failed to load transitive closure of '%s': %s", root, e.getMessage()));
-      }
-    }
-    if (packageProvider.valuesMissing()) {
-      return;
-    }
-    for (Map.Entry<String, Label> entry : implicitLabels.entries()) {
-      if (!reachableLabels.contains(entry.getValue())) {
-        throw new InvalidConfigurationException(
-            String.format("The required %s target is not transitively reachable from a "
-            + "command-line option: '%s'", entry.getKey(), entry.getValue()));
-      }
-    }
-  }
-
-  private static void collectAllTransitiveLabels(
-      PackageProviderForConfigurations packageProvider, Set<Label> reachableLabels, Label from)
-      throws NoSuchThingException, InterruptedException {
-    if (!reachableLabels.add(from)) {
-      return;
-    }
-    Target fromTarget = packageProvider.getTarget(from);
-    if (fromTarget == null) {
-      return;
-    }
-    if (fromTarget instanceof Rule) {
-      Rule rule = (Rule) fromTarget;
-      final Set<Label> allLabels = new LinkedHashSet<>();
-      AggregatingAttributeMapper.of(rule).visitLabels(new AcceptsLabelAttribute() {
-        @Override
-        public void acceptLabelAttribute(Label label, Attribute attribute) {
-          allLabels.add(label);
-        }
-      });
-      for (Label label : allLabels) {
-        collectAllTransitiveLabels(packageProvider, reachableLabels, label);
-      }
-    }
-  }
-
   private ConfigurationFragmentFactory getFactory(Class<? extends Fragment> fragmentType) {
     for (ConfigurationFragmentFactory factory : configurationFragments.get()) {
       if (factory.creates().equals(fragmentType)) {