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
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 52dfd99..7b89d6f 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 @@
}
/**
- * 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 6c5445e..95f37e7 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.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 @@
}
@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 0758117..d560b8a 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 @@
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.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.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 @@
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 @@
}
}
- /**
- * 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)) {
