Create a mode to propagate AnalysisFailureInfo for rule errors instead of failing a build
This new functionality is tied to --experimental_allow_analysis_failures : This feature is designed to facilitate in-build (analysis-phase) testing rules.
Progress toward #6237.
RELNOTES: None.
PiperOrigin-RevId: 215820356
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java
index 1542591..8464e3b 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java
@@ -35,6 +35,8 @@
import com.google.devtools.build.lib.analysis.configuredtargets.OutputFileConfiguredTarget;
import com.google.devtools.build.lib.analysis.configuredtargets.PackageGroupConfiguredTarget;
import com.google.devtools.build.lib.analysis.skylark.SkylarkRuleConfiguredTargetUtil;
+import com.google.devtools.build.lib.analysis.test.AnalysisFailure;
+import com.google.devtools.build.lib.analysis.test.AnalysisFailureInfo;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
@@ -282,7 +284,7 @@
.setConstraintSemantics(ruleClassProvider.getConstraintSemantics())
.build();
if (ruleContext.hasErrors()) {
- return null;
+ return erroredConfiguredTarget(ruleContext);
}
ConfigurationFragmentPolicy configurationFragmentPolicy =
rule.getRuleClassObject().getConfigurationFragmentPolicy();
@@ -303,12 +305,14 @@
}
if (rule.getRuleClassObject().isSkylark()) {
// TODO(bazel-team): maybe merge with RuleConfiguredTargetBuilder?
- return SkylarkRuleConfiguredTargetUtil.buildRule(
+ ConfiguredTarget target = SkylarkRuleConfiguredTargetUtil.buildRule(
ruleContext,
rule.getRuleClassObject().getAdvertisedProviders(),
rule.getRuleClassObject().getConfiguredTargetFunction(),
rule.getLocation(),
env.getSkylarkSemantics());
+
+ return target != null ? target : erroredConfiguredTarget(ruleContext);
} else {
RuleClass.ConfiguredTargetFactory<ConfiguredTarget, RuleContext, ActionConflictException>
factory =
@@ -323,6 +327,36 @@
// propagated, as this would show a nasty stack trace to users, and only provide info
// on one specific failure with poor messaging. By returning null, the caller can
// inspect ruleContext for multiple errors and output thorough messaging on each.
+ return erroredConfiguredTarget(ruleContext);
+ }
+ }
+
+ /**
+ * Returns a {@link ConfiguredTarget} which indicates that an analysis error occurred in
+ * processing the target. In most cases, this returns null, which signals to callers that
+ * the target failed to build and thus the build should fail. However, if analysis failures
+ * are allowed in this build, this returns a stub {@link ConfiguredTarget} which contains
+ * information about the failure.
+ */
+ @Nullable
+ private ConfiguredTarget erroredConfiguredTarget(RuleContext ruleContext)
+ throws ActionConflictException {
+ if (ruleContext.getConfiguration().allowAnalysisFailures()) {
+ ImmutableList.Builder<AnalysisFailure> analysisFailures = ImmutableList.builder();
+
+ for (String errorMessage : ruleContext.getSuppressedErrorMessages()) {
+ analysisFailures.add(new AnalysisFailure(ruleContext.getLabel(), errorMessage));
+ }
+ RuleConfiguredTargetBuilder builder = new RuleConfiguredTargetBuilder(ruleContext);
+ builder.addNativeDeclaredProvider(
+ AnalysisFailureInfo.forAnalysisFailures(analysisFailures.build()));
+ builder.addProvider(RunfilesProvider.class, RunfilesProvider.simple(Runfiles.EMPTY));
+ return builder.build();
+ } else {
+ // Returning a null ConfiguredTarget is an indication a rule error occurred. Exceptions are
+ // not propagated, as this would show a nasty stack trace to users, and only provide info
+ // on one specific failure with poor messaging. By returning null, the caller can
+ // inspect ruleContext for multiple errors and output thorough messaging on each.
return null;
}
}
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 f1902f2..66d071e 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
@@ -85,7 +85,6 @@
import com.google.devtools.build.lib.packages.RequiredProviders;
import com.google.devtools.build.lib.packages.Rule;
import com.google.devtools.build.lib.packages.RuleClass;
-import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException;
import com.google.devtools.build.lib.packages.RuleErrorConsumer;
import com.google.devtools.build.lib.packages.Target;
import com.google.devtools.build.lib.packages.TargetUtils;
@@ -181,7 +180,7 @@
private final BuildConfiguration hostConfiguration;
private final ConfigurationFragmentPolicy configurationFragmentPolicy;
private final ImmutableList<Class<? extends BuildConfiguration.Fragment>> universalFragments;
- private final ErrorReporter reporter;
+ private final RuleErrorConsumer reporter;
@Nullable private final ToolchainContext toolchainContext;
private final ConstraintSemantics constraintSemantics;
@@ -294,6 +293,22 @@
}
/**
+ * If this target's configuration suppresses analysis failures, this returns a list
+ * of strings, where each string corresponds to a description of an error that occurred during
+ * processing this target.
+ *
+ * @throws IllegalStateException if this target's configuration does not suppress analysis
+ * failures (if {@code getConfiguration().allowAnalysisFailures()} is false)
+ */
+ public List<String> getSuppressedErrorMessages() {
+ Preconditions.checkState(getConfiguration().allowAnalysisFailures(),
+ "Error messages can only be retrieved via RuleContext if allow_analysis_failures is true");
+ Preconditions.checkState(reporter instanceof SuppressingErrorReporter,
+ "Unexpected error reporter");
+ return ((SuppressingErrorReporter) reporter).getErrorMessages();
+ }
+
+ /**
* If this <code>RuleContext</code> is for an aspect implementation, returns that aspect.
* (it is the last aspect in the list of aspects applied to a target; all other aspects
* are the ones main aspect sees as specified by its "required_aspect_providers")
@@ -357,13 +372,6 @@
return getAnalysisEnvironment().hasErrors();
}
- @Override
- public void assertNoErrors() throws RuleErrorException {
- if (hasErrors()) {
- throw new RuleErrorException();
- }
- }
-
/**
* Returns an immutable map from attribute name to list of configured targets for that attribute.
*/
@@ -502,12 +510,6 @@
reporter.ruleError(message);
}
- @Override
- public RuleErrorException throwWithRuleError(String message) throws RuleErrorException {
- reporter.ruleError(message);
- throw new RuleErrorException();
- }
-
/**
* Convenience function for subclasses to report non-attribute-specific
* warnings in the current rule.
@@ -529,13 +531,6 @@
reporter.attributeError(attrName, message);
}
- @Override
- public RuleErrorException throwWithAttributeError(String attrName, String message)
- throws RuleErrorException {
- reporter.attributeError(attrName, message);
- throw new RuleErrorException();
- }
-
/**
* Like attributeError, but does not mark the configured target as errored.
*
@@ -1436,7 +1431,7 @@
private final BuildConfiguration configuration;
private final BuildConfiguration hostConfiguration;
private final PrerequisiteValidator prerequisiteValidator;
- private final ErrorReporter reporter;
+ private final RuleErrorConsumer reporter;
private OrderedSetMultimap<Attribute, ConfiguredTargetAndData> prerequisiteMap;
private ImmutableMap<Label, ConfigMatchingProvider> configConditions;
private NestedSet<PackageGroupContents> visibility;
@@ -1461,7 +1456,11 @@
this.configuration = Preconditions.checkNotNull(configuration);
this.hostConfiguration = Preconditions.checkNotNull(hostConfiguration);
this.prerequisiteValidator = prerequisiteValidator;
- reporter = new ErrorReporter(env, target.getAssociatedRule(), getRuleClassNameForLogging());
+ if (configuration.allowAnalysisFailures()) {
+ reporter = new SuppressingErrorReporter();
+ } else {
+ reporter = new ErrorReporter(env, target.getAssociatedRule(), getRuleClassNameForLogging());
+ }
}
@VisibleForTesting
@@ -1694,26 +1693,10 @@
}
@Override
- public RuleErrorException throwWithRuleError(String message) throws RuleErrorException {
- throw reporter.throwWithRuleError(message);
- }
-
- @Override
- public RuleErrorException throwWithAttributeError(String attrName, String message)
- throws RuleErrorException {
- throw reporter.throwWithAttributeError(attrName, message);
- }
-
- @Override
public boolean hasErrors() {
return reporter.hasErrors();
}
- @Override
- public void assertNoErrors() throws RuleErrorException {
- reporter.assertNoErrors();
- }
-
private String badPrerequisiteMessage(
ConfiguredTargetAndData prerequisite, String reason, boolean isWarning) {
String msgReason = reason != null ? " (" + reason + ")" : "";
@@ -2026,6 +2009,41 @@
protected Location getAttributeLocation(String attrName) {
return rule.getAttributeLocation(attrName);
}
+ }
+ /**
+ * Implementation of an error consumer which does not post any events, saves rule and attribute
+ * errors for future consumption, and drops warnings.
+ */
+ public static final class SuppressingErrorReporter implements RuleErrorConsumer {
+ private final List<String> errorMessages = Lists.newArrayList();
+
+ @Override
+ public void ruleWarning(String message) {}
+
+ @Override
+ public void ruleError(String message) {
+ errorMessages.add(message);
+ }
+
+ @Override
+ public void attributeWarning(String attrName, String message) {}
+
+ @Override
+ public void attributeError(String attrName, String message) {
+ errorMessages.add(message);
+ }
+
+ @Override
+ public boolean hasErrors() {
+ return !errorMessages.isEmpty();
+ }
+
+ /**
+ * Returns the error message strings reported to this error consumer.
+ */
+ public List<String> getErrorMessages() {
+ return errorMessages;
+ }
}
}
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 cb0c927..5f1b52b 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
@@ -786,6 +786,19 @@
)
public boolean isHost;
+ // TODO(cparsons): Make this flag non-experimental when it is fully implemented.
+ @Option(
+ name = "experimental_allow_analysis_failures",
+ defaultValue = "false",
+ documentationCategory = OptionDocumentationCategory.TESTING,
+ effectTags = { OptionEffectTag.LOADING_AND_ANALYSIS },
+ metadataTags = { OptionMetadataTag.EXPERIMENTAL },
+ help = "If true, an analysis failure of a rule target results in the target's propagation "
+ + "of an instance of AnalysisFailureInfo containing the error description, instead of "
+ + "resulting in a build failure."
+ )
+ public boolean allowAnalysisFailures;
+
@Option(
name = "features",
allowMultiple = true,
@@ -1779,6 +1792,10 @@
return options.enforceConstraints;
}
+ public boolean allowAnalysisFailures() {
+ return options.allowAnalysisFailures;
+ }
+
public List<Label> getActionListeners() {
return options.actionListeners;
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/test/AnalysisFailureInfo.java b/src/main/java/com/google/devtools/build/lib/analysis/test/AnalysisFailureInfo.java
index 5ee6aea..ffbe86b 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/test/AnalysisFailureInfo.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/test/AnalysisFailureInfo.java
@@ -13,11 +13,12 @@
// limitations under the License.
package com.google.devtools.build.lib.analysis.test;
+import com.google.devtools.build.lib.collect.nestedset.NestedSet;
+import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
import com.google.devtools.build.lib.events.Location;
import com.google.devtools.build.lib.packages.BuiltinProvider;
import com.google.devtools.build.lib.packages.Info;
import com.google.devtools.build.lib.skylarkbuildapi.test.AnalysisFailureInfoApi;
-import com.google.devtools.build.lib.syntax.SkylarkNestedSet;
/**
* Implementation of {@link AnalysisFailureInfoApi}.
@@ -25,7 +26,7 @@
* Encapsulates information about analysis-phase errors which would have occurred during a
* build.
*/
-public class AnalysisFailureInfo extends Info implements AnalysisFailureInfoApi {
+public class AnalysisFailureInfo extends Info implements AnalysisFailureInfoApi<AnalysisFailure> {
/**
* Singleton provider instance for {@link AnalysisFailureInfo}.
@@ -33,16 +34,23 @@
public static final AnalysisFailureInfoProvider SKYLARK_CONSTRUCTOR =
new AnalysisFailureInfoProvider();
- private final SkylarkNestedSet causes;
+ private final NestedSet<AnalysisFailure> causes;
- public AnalysisFailureInfo(
- SkylarkNestedSet causes) {
+ private AnalysisFailureInfo(NestedSet<AnalysisFailure> causes) {
super(SKYLARK_CONSTRUCTOR, Location.BUILTIN);
this.causes = causes;
}
+ /**
+ * Constructs and returns an {@link AnalysisFailureInfo} object representing the given failures.
+ */
+ public static AnalysisFailureInfo forAnalysisFailures(Iterable<AnalysisFailure> failures) {
+ return new AnalysisFailureInfo(
+ NestedSetBuilder.<AnalysisFailure>stableOrder().addAll(failures).build());
+ }
+
@Override
- public SkylarkNestedSet getCauses() {
+ public NestedSet<AnalysisFailure> getCauses() {
return causes;
}
diff --git a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/test/AnalysisFailureInfoApi.java b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/test/AnalysisFailureInfoApi.java
index d2f776e..2b1ea62 100644
--- a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/test/AnalysisFailureInfoApi.java
+++ b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/test/AnalysisFailureInfoApi.java
@@ -14,11 +14,11 @@
package com.google.devtools.build.lib.skylarkbuildapi.test;
+import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.skylarkbuildapi.ProviderApi;
import com.google.devtools.build.lib.skylarkinterface.SkylarkCallable;
import com.google.devtools.build.lib.skylarkinterface.SkylarkModule;
import com.google.devtools.build.lib.skylarkinterface.SkylarkValue;
-import com.google.devtools.build.lib.syntax.SkylarkNestedSet;
import com.google.devtools.build.lib.syntax.SkylarkSemantics.FlagIdentifier;
/**
@@ -48,7 +48,8 @@
+ "<code>causes</code> equal to the union of the <code>causes</code> of the "
+ "dependencies.</li></ul>",
documented = false)
-public interface AnalysisFailureInfoApi extends SkylarkValue {
+public interface AnalysisFailureInfoApi<AnalysisFailureApiT extends AnalysisFailureApi>
+ extends SkylarkValue {
@SkylarkCallable(
name = "causes",
@@ -58,7 +59,7 @@
structField = true,
enableOnlyWithFlag = FlagIdentifier.EXPERIMENTAL_ANALYSIS_TESTING_IMPROVEMENTS
)
- public SkylarkNestedSet getCauses();
+ public NestedSet<AnalysisFailureApiT> getCauses();
/** Provider class for {@link AnalysisFailureInfoApi} objects. */
@SkylarkModule(name = "Provider", documented = false, doc = "")
diff --git a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/test/BUILD b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/test/BUILD
index 15c79f7..a030d48 100644
--- a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/test/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/test/BUILD
@@ -22,6 +22,7 @@
"//src/main/java/com/google/devtools/build/lib:skylarkinterface",
"//src/main/java/com/google/devtools/build/lib:syntax",
"//src/main/java/com/google/devtools/build/lib/cmdline",
+ "//src/main/java/com/google/devtools/build/lib/collect/nestedset",
"//src/main/java/com/google/devtools/build/lib/skylarkbuildapi",
"//third_party:guava",
"//third_party:jsr305",
