diff options
-rw-r--r-- | src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java | 35 | ||||
-rw-r--r-- | src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java | 22 |
2 files changed, 57 insertions, 0 deletions
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 18fbe8ce74..22955612a1 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.LinkedListMultimap; 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 @@ final class ConfiguredTargetFunction implements SkyFunction { } 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 @@ final class ConfiguredTargetFunction implements SkyFunction { } /** + * 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 3efe26b16d..8d45c93362 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 @@ public class BuildViewTest extends BuildViewTestBase { 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) |