Report the first rule error as a root cause event
This is not fool-proof. In particular, it is missing all errors
generated in the ConfiguredTargetFunction. However, it should give
us significantly better coverage being a more generic mechanism.
We might want to consider either adding an index to the root cause
events so we can report multiple, or collecting all error messages into
a single root cause event for each target.
PiperOrigin-RevId: 268639983
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/EventHandlingErrorReporter.java b/src/main/java/com/google/devtools/build/lib/analysis/EventHandlingErrorReporter.java
index adfe113..8aab1c8 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/EventHandlingErrorReporter.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/EventHandlingErrorReporter.java
@@ -13,6 +13,7 @@
// limitations under the License.
package com.google.devtools.build.lib.analysis;
+import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.events.Location;
@@ -36,7 +37,14 @@
this.env = env;
}
- public void reportError(Location location, String message) {
+ private void reportError(Location location, String message) {
+ // TODO(ulfjack): Consider generating the error message from the root cause event rather than
+ // the other way round.
+ if (!hasErrors()) {
+ // We must not report duplicate events, so we only report the first one for now.
+ env.getEventHandler()
+ .post(new AnalysisRootCauseEvent(getConfiguration(), getLabel(), message));
+ }
env.getEventHandler().handle(Event.error(location, message));
}
@@ -98,6 +106,8 @@
protected abstract Label getLabel();
+ protected abstract BuildConfiguration getConfiguration();
+
protected abstract Location getRuleLocation();
protected abstract Location getAttributeLocation(String attrName);
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
index 5bcaf64..ded8ede 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
@@ -1607,7 +1607,9 @@
if (configuration.allowAnalysisFailures()) {
reporter = new SuppressingErrorReporter();
} else {
- reporter = new ErrorReporter(env, target.getAssociatedRule(), getRuleClassNameForLogging());
+ reporter =
+ new ErrorReporter(
+ env, target.getAssociatedRule(), configuration, getRuleClassNameForLogging());
}
}
@@ -2124,10 +2126,16 @@
private static final class ErrorReporter extends EventHandlingErrorReporter
implements RuleErrorConsumer {
private final Rule rule;
+ private final BuildConfiguration configuration;
- ErrorReporter(AnalysisEnvironment env, Rule rule, String ruleClassNameForLogging) {
+ ErrorReporter(
+ AnalysisEnvironment env,
+ Rule rule,
+ BuildConfiguration configuration,
+ String ruleClassNameForLogging) {
super(ruleClassNameForLogging, env);
this.rule = rule;
+ this.configuration = configuration;
}
@Override
@@ -2150,6 +2158,11 @@
}
@Override
+ protected BuildConfiguration getConfiguration() {
+ return configuration;
+ }
+
+ @Override
protected Location getRuleLocation() {
return rule.getLocation();
}
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelPrerequisiteValidator.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelPrerequisiteValidator.java
index 7c96058..3047d43 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelPrerequisiteValidator.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelPrerequisiteValidator.java
@@ -16,7 +16,6 @@
import com.google.devtools.build.lib.analysis.AliasProvider;
import com.google.devtools.build.lib.analysis.AliasProvider.TargetMode;
-import com.google.devtools.build.lib.analysis.AnalysisRootCauseEvent;
import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider;
import com.google.devtools.build.lib.analysis.RuleContext;
import com.google.devtools.build.lib.packages.Attribute;
@@ -75,10 +74,6 @@
rule.getLabel());
context.ruleError(errorMessage);
}
- // We can always post the visibility error as, regardless of the value of keep going,
- // that target will not be built.
- context.post(
- new AnalysisRootCauseEvent(context.getConfiguration(), rule.getLabel(), errorMessage));
}
if (prerequisiteTarget instanceof PackageGroup) {
@@ -130,8 +125,6 @@
context.ruleWarning(message);
} else {
context.ruleError(message);
- context.post(
- new AnalysisRootCauseEvent(context.getConfiguration(), rule.getLabel(), message));
}
}
}
