Provides a clearer message when target analysis fails because its dynamic
config is missing required fragments.
Before this change, Bazel crashes with the mysterious error: "Fragment
foo can't load missing options BarOptions" with no details on which target or dep
needed Foo. So figuring out the source of the error is painful.
With this change, we instead get:
//foo:foo: dependency //bar:bar from attribute "deps" is missing required config fragments: JavaConfiguration
--
MOS_MIGRATED_REVID=129143764
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java
index 18fbe8c..2295561 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java
@@ -14,6 +14,7 @@
package com.google.devtools.build.lib.skyframe;
import com.google.common.base.Function;
+import com.google.common.base.Joiner;
import com.google.common.base.Verify;
import com.google.common.collect.ArrayListMultimap;
import com.google.common.collect.ImmutableList;
@@ -23,6 +24,7 @@
import com.google.common.collect.ListMultimap;
import com.google.common.collect.Maps;
import com.google.common.collect.Multimap;
+import com.google.common.collect.Sets;
import com.google.devtools.build.lib.actions.ActionAnalysisMetadata;
import com.google.devtools.build.lib.actions.Actions;
import com.google.devtools.build.lib.actions.Artifact;
@@ -472,6 +474,10 @@
}
Set<Class<? extends BuildConfiguration.Fragment>> depFragments =
transitiveDepInfo.getTransitiveConfigFragments().toSet();
+ // TODO(gregce): remove the below call once we have confidence dynamic configurations always
+ // provide needed fragments. This unnecessarily drags performance on the critical path.
+ checkForMissingFragments(env, ctgValue, attributeAndLabel.attribute.getName(), dep,
+ depFragments);
boolean sameFragments = depFragments.equals(ctgFragments);
Attribute.Transition transition = dep.getTransition();
@@ -573,6 +579,35 @@
}
/**
+ * Diagnostic helper method for dynamic configurations: checks the config fragments required by
+ * a dep against the fragments in its actual configuration. If any are missing, triggers a
+ * descriptive "missing fragments" error.
+ */
+ private static void checkForMissingFragments(Environment env, TargetAndConfiguration ctgValue,
+ String attribute, Dependency dep,
+ Set<Class<? extends BuildConfiguration.Fragment>> expectedDepFragments)
+ throws DependencyEvaluationException {
+ Set<String> ctgFragmentNames = new HashSet<>();
+ for (BuildConfiguration.Fragment fragment :
+ ctgValue.getConfiguration().getAllFragments().values()) {
+ ctgFragmentNames.add(fragment.getClass().getSimpleName());
+ }
+ Set<String> depFragmentNames = new HashSet<>();
+ for (Class<? extends BuildConfiguration.Fragment> fragmentClass : expectedDepFragments) {
+ depFragmentNames.add(fragmentClass.getSimpleName());
+ }
+ Set<String> missing = Sets.difference(depFragmentNames, ctgFragmentNames);
+ if (!missing.isEmpty()) {
+ String msg = String.format(
+ "%s: dependency %s from attribute \"%s\" is missing required config fragments: %s",
+ ctgValue.getLabel(), dep.getLabel(), attribute, Joiner.on(", ").join(missing));
+ env.getListener().handle(Event.error(msg));
+ throw new DependencyEvaluationException(new InvalidConfigurationException(msg));
+ }
+ }
+
+
+ /**
* Merges the each direct dependency configured target with the aspects associated with it.
*
* <p>Note that the combination of a configured target and its associated aspects are not
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java b/src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java
index 3efe26b..8d45c93 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java
@@ -1230,6 +1230,28 @@
ruleClassProvider.getUniversalFragment());
}
+ @Test
+ public void errorOnMissingDepFragments() throws Exception {
+ scratch.file("foo/BUILD",
+ "cc_library(",
+ " name = 'ccbin', ",
+ " srcs = ['c.cc'],",
+ " data = [':javalib'])",
+ "java_library(",
+ " name = 'javalib',",
+ " srcs = ['javalib.java'])");
+ useConfiguration("--experimental_dynamic_configs", "--experimental_disable_jvm");
+ reporter.removeHandler(failFastHandler);
+ try {
+ update("//foo:ccbin");
+ fail();
+ } catch (ViewCreationFailedException e) {
+ // Expected.
+ }
+ assertContainsEvent("//foo:ccbin: dependency //foo:javalib from attribute \"data\" is missing "
+ + "required config fragments: Jvm");
+ }
+
/** Runs the same test with the reduced loading phase. */
@TestSpec(size = Suite.SMALL_TESTS)
@RunWith(JUnit4.class)
