Fix a bug, when we have dependency from one aspect to another aspect1->aspect2 and aspect2 fails then aspect1 throws IllegalStateException.
--
MOS_MIGRATED_REVID=102976139
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java
index a4d54c0..87523c1 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java
@@ -129,6 +129,8 @@
transitivePackages);
} catch (DependencyEvaluationException e) {
throw new AspectFunctionException(e.getRootCauseSkyKey(), e.getCause());
+ } catch (AspectCreationException e) {
+ throw new AspectFunctionException(e);
}
}
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 8067e60..7fe19c5 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
@@ -193,6 +193,9 @@
return ans;
} catch (DependencyEvaluationException e) {
throw new ConfiguredTargetFunctionException(e.getRootCauseSkyKey(), e.getCause());
+ } catch (AspectCreationException e) {
+ throw new ConfiguredTargetFunctionException(
+ new ConfiguredValueCreationException(e.getMessage()));
}
}
@@ -224,7 +227,7 @@
AspectDefinition aspectDefinition, AspectParameters aspectParameters,
Set<ConfigMatchingProvider> configConditions, RuleClassProvider ruleClassProvider,
BuildConfiguration hostConfiguration, NestedSetBuilder<Package> transitivePackages)
- throws DependencyEvaluationException {
+ throws DependencyEvaluationException, AspectCreationException {
// Create the map from attributes to list of (target, configuration) pairs.
ListMultimap<Attribute, Dependency> depValueNames;
@@ -483,7 +486,7 @@
private static ListMultimap<SkyKey, Aspect> resolveAspectDependencies(Environment env,
Map<SkyKey, ConfiguredTarget> configuredTargetMap, Iterable<Dependency> deps,
NestedSetBuilder<Package> transitivePackages)
- throws DependencyEvaluationException {
+ throws AspectCreationException {
ListMultimap<SkyKey, Aspect> result = ArrayListMultimap.create();
Set<SkyKey> aspectKeys = new HashSet<>();
for (Dependency dep : deps) {
@@ -517,12 +520,12 @@
} catch (ConfiguredValueCreationException e) {
// The configured target should have been created in resolveConfiguredTargetDependencies()
throw new IllegalStateException(e);
- } catch (NoSuchThingException | AspectCreationException e) {
+ } catch (NoSuchThingException e) {
AspectFactory<?, ?, ?> depAspectFactory =
AspectFactory.Util.create(depAspect.getAspectFactory());
- throw new DependencyEvaluationException(new ConfiguredValueCreationException(
+ throw new AspectCreationException(
String.format("Evaluation of aspect %s on %s failed: %s",
- depAspectFactory.getDefinition().getName(), dep.getLabel(), e.toString())));
+ depAspectFactory.getDefinition().getName(), dep.getLabel(), e.toString()));
}
if (aspectValue == null) {
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/AspectTest.java b/src/test/java/com/google/devtools/build/lib/analysis/AspectTest.java
index 02b4f3e..b051db1 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/AspectTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/AspectTest.java
@@ -126,6 +126,29 @@
}
@Test
+ public void transitiveAspectInError() throws Exception {
+ setRules(new TestAspects.BaseRule(), new TestAspects.ErrorAspectRule(),
+ new TestAspects.SimpleRule());
+
+ pkg("a",
+ "error_aspect(name='a', foo=[':b'])",
+ "error_aspect(name='b', bar=[':c'])",
+ "error_aspect(name='c', bar=[':d'])",
+ "error_aspect(name='d')");
+
+ reporter.removeHandler(failFastHandler);
+ // getConfiguredTarget() uses a separate code path that does not hit
+ // SkyframeBuildView#configureTargets
+ try {
+ update("//a:a");
+ fail();
+ } catch (ViewCreationFailedException e) {
+ // expected
+ }
+ assertContainsEvent("Aspect error");
+ }
+
+ @Test
public void sameTargetInDifferentAttributes() throws Exception {
setRules(new TestAspects.BaseRule(), new TestAspects.AspectRequiringRule(),
new TestAspects.SimpleRule());
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/TestAspects.java b/src/test/java/com/google/devtools/build/lib/analysis/util/TestAspects.java
index 5f41e3d..c0ee911 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/TestAspects.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/TestAspects.java
@@ -237,6 +237,7 @@
}
private static final AspectDefinition ERROR_ASPECT = new AspectDefinition.Builder("error")
+ .attributeAspect("bar", ErrorAspect.class)
.build();
/**
@@ -367,6 +368,7 @@
return builder
.add(attr("foo", LABEL_LIST).allowedFileTypes(FileTypeSet.ANY_FILE)
.aspect(ErrorAspect.class))
+ .add(attr("bar", LABEL_LIST).allowedFileTypes(FileTypeSet.ANY_FILE))
.build();
}
