Clean up RuleErrorException: stop allowing it to be called directly, require that it go through RuleErrorConsumer, and allow it to take a cause.
PiperOrigin-RevId: 296014466
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleContext.java b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleContext.java
index 0c4cae2..4cdbab6 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleContext.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleContext.java
@@ -266,7 +266,7 @@
try {
makeVariables = ruleContext.getConfigurationMakeVariableContext().collectMakeVariables();
} catch (ExpansionException e) {
- throw ruleContext.throwWithRuleError(e.getMessage());
+ throw ruleContext.throwWithRuleError(e);
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/ninja/actions/NinjaGraph.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/ninja/actions/NinjaGraph.java
index 930ba34..db819f5 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/ninja/actions/NinjaGraph.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/ninja/actions/NinjaGraph.java
@@ -69,7 +69,7 @@
public ConfiguredTarget create(RuleContext ruleContext)
throws InterruptedException, RuleErrorException, ActionConflictException {
if (!ruleContext.getAnalysisEnvironment().getSkylarkSemantics().experimentalNinjaActions()) {
- throw new RuleErrorException(
+ throw ruleContext.throwWithRuleError(
"Usage of ninja_graph is only allowed with --experimental_ninja_actions flag");
}
Artifact mainArtifact = ruleContext.getPrerequisiteArtifact("main", Mode.TARGET);
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/sh/ShBinary.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/sh/ShBinary.java
index e5d4fc3..8b142c2 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/sh/ShBinary.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/sh/ShBinary.java
@@ -135,10 +135,9 @@
return primaryOutput;
} else {
// If the extensions don't match, we should always respect mainFile's extension.
- ruleContext.ruleError(
+ throw ruleContext.throwWithRuleError(
"Source file is a Windows executable file,"
+ " target name extension should match source file extension");
- throw new RuleErrorException();
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java b/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java
index 1531c8a..301cc77 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java
@@ -210,14 +210,22 @@
* messaging should be done via {@link RuleErrorConsumer}; this exception only interrupts
* configured target creation in cases where it can no longer continue.
*/
- public static final class RuleErrorException extends Exception {
- public RuleErrorException() {
+ final class RuleErrorException extends Exception {
+ RuleErrorException() {
super();
}
- public RuleErrorException(String message) {
+ RuleErrorException(String message) {
super(message);
}
+
+ RuleErrorException(Throwable cause) {
+ super(cause);
+ }
+
+ RuleErrorException(String message, Throwable cause) {
+ super(message, cause);
+ }
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/packages/RuleErrorConsumer.java b/src/main/java/com/google/devtools/build/lib/packages/RuleErrorConsumer.java
index 68dc985..9f033b5 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/RuleErrorConsumer.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/RuleErrorConsumer.java
@@ -48,16 +48,29 @@
void attributeError(String attrName, String message);
/**
- * Convenience function to report non-attribute-specific errors in the current rule and then
- * throw a {@link RuleErrorException}, immediately exiting the build invocation. Alternatively,
- * invoke {@link #ruleError} instead to collect additional error information before ending the
- * invocation.
+ * Convenience function to report non-attribute-specific errors in the current rule and then throw
+ * a {@link RuleErrorException}, immediately exiting the current rule, and shutting down the
+ * invocation in a no-keep-going build. If multiple errors are present, invoke {@link #ruleError}
+ * to collect additional error information before calling this method.
*/
default RuleErrorException throwWithRuleError(String message) throws RuleErrorException {
ruleError(message);
throw new RuleErrorException(message);
}
+ /** See {@link #throwWithRuleError(String)}. */
+ default RuleErrorException throwWithRuleError(Throwable cause) throws RuleErrorException {
+ ruleError(cause.getMessage());
+ throw new RuleErrorException(cause);
+ }
+
+ /** See {@link #throwWithRuleError(String)}. */
+ default RuleErrorException throwWithRuleError(String message, Throwable cause)
+ throws RuleErrorException {
+ ruleError(message);
+ throw new RuleErrorException(message, cause);
+ }
+
/**
* Convenience function to report attribute-specific errors in the current rule, and then throw a
* {@link RuleErrorException}, immediately exiting the build invocation. Alternatively, invoke
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidFeatureFlagSetProvider.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidFeatureFlagSetProvider.java
index f1cf604..765ab64 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidFeatureFlagSetProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidFeatureFlagSetProvider.java
@@ -93,30 +93,34 @@
}
if (!ConfigFeatureFlag.isAvailable(ruleContext)) {
- ruleContext.attributeError(
+ throw ruleContext.throwWithAttributeError(
FEATURE_FLAG_ATTR,
String.format(
"the %s attribute is not available in package '%s'",
FEATURE_FLAG_ATTR, ruleContext.getLabel().getPackageIdentifier()));
- throw new RuleErrorException();
}
Iterable<? extends TransitiveInfoCollection> actualTargets =
ruleContext.getPrerequisites(FEATURE_FLAG_ATTR, Mode.TARGET);
- boolean aliasFound = false;
+ RuleErrorException exception = null;
for (TransitiveInfoCollection target : actualTargets) {
Label label = AliasProvider.getDependencyLabel(target);
if (!label.equals(target.getLabel())) {
- ruleContext.attributeError(
- FEATURE_FLAG_ATTR,
- String.format(
- "Feature flags must be named directly, not through aliases; use '%s', not '%s'",
- target.getLabel(), label));
- aliasFound = true;
+ try {
+ exception =
+ ruleContext.throwWithAttributeError(
+ FEATURE_FLAG_ATTR,
+ String.format(
+ "Feature flags must be named directly, not through aliases; use '%s', not"
+ + " '%s'",
+ target.getLabel(), label));
+ } catch (RuleErrorException e) {
+ exception = e;
+ }
}
}
- if (aliasFound) {
- throw new RuleErrorException();
+ if (exception != null) {
+ throw exception;
}
return Optional.of(ImmutableMap.copyOf(expectedValues));
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidInstrumentationTestBase.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidInstrumentationTestBase.java
index d2e5d91..7f78611 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidInstrumentationTestBase.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidInstrumentationTestBase.java
@@ -318,8 +318,8 @@
AndroidInstrumentationTestBase.class, TEST_SUITE_PROPERTY_NAME_FILE)
.trim();
} catch (IOException e) {
- ruleContext.throwWithRuleError("Cannot load test suite property name: " + e.getMessage());
- return null;
+ throw ruleContext.throwWithRuleError(
+ "Cannot load test suite property name: " + e.getMessage(), e);
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResources.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResources.java
index ff18b30..84a2382 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResources.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResources.java
@@ -211,14 +211,13 @@
throws RuleErrorException {
PathFragment resourceDir = findResourceDir(file);
if (resourceDir == null) {
- ruleErrorConsumer.attributeError(
+ throw ruleErrorConsumer.throwWithAttributeError(
resourcesAttr,
String.format(INCORRECT_RESOURCE_LAYOUT_MESSAGE, file.getRootRelativePath()));
- throw new RuleErrorException();
}
if (lastResourceDir != null && !resourceDir.equals(lastResourceDir)) {
- ruleErrorConsumer.attributeError(
+ throw ruleErrorConsumer.throwWithAttributeError(
resourcesAttr,
String.format(
"'%s' (generated by '%s') is not in the same directory '%s' (derived from %s)."
@@ -227,7 +226,6 @@
file.getOwnerLabel(),
lastResourceDir,
lastFile.getRootRelativePath()));
- throw new RuleErrorException();
}
PathFragment packageFragment = file.getOwnerLabel().getPackageIdentifier().getSourceRoot();
@@ -239,7 +237,7 @@
0,
path.segmentCount() - segmentCountAfterAncestor(resourceDir, packageRelativePath)));
} catch (IllegalArgumentException e) {
- ruleErrorConsumer.attributeError(
+ throw ruleErrorConsumer.throwWithAttributeError(
resourcesAttr,
String.format(
"'%s' (generated by '%s') is not under the directory '%s' (derived from %s).",
@@ -247,7 +245,6 @@
file.getOwnerLabel(),
packageRelativePath,
file.getRootRelativePath()));
- throw new RuleErrorException();
}
return resourceDir;
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/apple/XcodeConfig.java b/src/main/java/com/google/devtools/build/lib/rules/apple/XcodeConfig.java
index 5814866..adc0af1 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/apple/XcodeConfig.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/apple/XcodeConfig.java
@@ -228,7 +228,7 @@
try {
aliasesToVersionMap = aliasesToVersionMap(explicitVersions);
} catch (XcodeConfigException e) {
- ruleContext.throwWithRuleError(e.getMessage());
+ throw ruleContext.throwWithRuleError(e);
}
if (!Strings.isNullOrEmpty(versionOverrideFlag)) {
// The version override flag is not necessarily an actual version - it may be a version
@@ -271,7 +271,7 @@
localAliasesToVersionMap = aliasesToVersionMap(localVersions.getAvailableVersions());
remoteAliasesToVersionMap = aliasesToVersionMap(remoteVersions.getAvailableVersions());
} catch (XcodeConfigException e) {
- throw ruleContext.throwWithRuleError(e.getMessage());
+ throw ruleContext.throwWithRuleError(e);
}
// Mutually available Xcode versions are versions that are available both locally and remotely,
// but are referred to by the aliases listed in remote_xcodes.
diff --git a/src/main/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlag.java b/src/main/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlag.java
index 2a8ce3e..6417a4b 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlag.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/config/ConfigFeatureFlag.java
@@ -91,12 +91,11 @@
public ConfiguredTarget create(RuleContext ruleContext)
throws InterruptedException, RuleErrorException, ActionConflictException {
if (!ConfigFeatureFlag.isAvailable(ruleContext)) {
- ruleContext.ruleError(
+ throw ruleContext.throwWithRuleError(
String.format(
"the %s rule is not available in package '%s'",
ruleContext.getRuleClassNameForLogging(),
ruleContext.getLabel().getPackageIdentifier()));
- throw new RuleErrorException();
}
List<String> specifiedValues = ruleContext.attributes().get("allowed_values", STRING_LIST);
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java
index 7fba4fb..ad42202 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java
@@ -221,7 +221,7 @@
try {
builder.addTransitiveArtifacts(toolchain.getDynamicRuntimeLinkInputs(featureConfiguration));
} catch (EvalException e) {
- throw ruleContext.throwWithRuleError(e.getMessage());
+ throw ruleContext.throwWithRuleError(e);
}
}
if (linkCompileOutputSeparately) {
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java
index 92da26c..7c19b43 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java
@@ -347,7 +347,7 @@
ArtifactCategory.DYNAMIC_LIBRARY, ruleContext.getLabel().getName()));
targetBuilder.addOutputGroup(DEF_FILE_OUTPUT_GROUP_NAME, generatedDefFile);
} catch (EvalException e) {
- throw ruleContext.throwWithRuleError(e.getMessage());
+ throw ruleContext.throwWithRuleError(e);
}
}
linkingHelper.setDefFile(
@@ -447,7 +447,7 @@
builder.addTransitiveArtifacts(
ccToolchain.getDynamicRuntimeLinkInputs(featureConfiguration));
} catch (EvalException e) {
- throw ruleContext.throwWithRuleError(e.getMessage());
+ throw ruleContext.throwWithRuleError(e);
}
}
Runfiles runfiles = builder.build();
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLinkingHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLinkingHelper.java
index 0938d5f..6effe70 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLinkingHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLinkingHelper.java
@@ -699,7 +699,7 @@
ccToolchain.getDynamicRuntimeLinkMiddleman(ruleErrorConsumer, featureConfiguration),
ccToolchain.getDynamicRuntimeLinkInputs(featureConfiguration));
} catch (EvalException e) {
- throw ruleErrorConsumer.throwWithRuleError(e.getMessage());
+ throw ruleErrorConsumer.throwWithRuleError(e);
}
} else {
try {
@@ -708,7 +708,7 @@
ccToolchain.getStaticRuntimeLinkMiddleman(ruleErrorConsumer, featureConfiguration),
ccToolchain.getStaticRuntimeLinkInputs(featureConfiguration));
} catch (EvalException e) {
- throw ruleErrorConsumer.throwWithRuleError(e.getMessage());
+ throw ruleErrorConsumer.throwWithRuleError(e);
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProviderHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProviderHelper.java
index bdc62d4..3205910 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProviderHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProviderHelper.java
@@ -81,7 +81,7 @@
toolPaths = computeToolPaths(toolchainConfigInfo, toolsDirectory);
toolchainFeatures = new CcToolchainFeatures(toolchainConfigInfo, toolsDirectory);
} catch (EvalException e) {
- throw ruleContext.throwWithRuleError(e.getMessage());
+ throw ruleContext.throwWithRuleError(e);
}
FdoContext fdoContext =
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionBuilder.java
index 8c1d2d6..0205e1e 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionBuilder.java
@@ -208,18 +208,21 @@
* <p>This method may be called multiple times to create multiple compile actions (usually after
* calling some setters to modify the generated action).
*/
- public CppCompileAction buildOrThrowRuleError(RuleErrorConsumer ruleErrorConsumer)
+ CppCompileAction buildOrThrowRuleError(RuleErrorConsumer ruleErrorConsumer)
throws RuleErrorException {
List<String> errorMessages = new ArrayList<>();
CppCompileAction result =
buildAndVerify((String errorMessage) -> errorMessages.add(errorMessage));
if (!errorMessages.isEmpty()) {
- for (String errorMessage : errorMessages) {
- ruleErrorConsumer.ruleError(errorMessage);
+ RuleErrorException exception = null;
+ try {
+ exception = ruleErrorConsumer.throwWithRuleError(errorMessages.get(0));
+ } catch (RuleErrorException e) {
+ exception = e;
}
-
- throw new RuleErrorException(errorMessages.get(0));
+ errorMessages.stream().skip(1L).forEach(ruleErrorConsumer::ruleError);
+ throw exception;
}
return result;
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java
index bd0aded..397081f 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java
@@ -247,7 +247,7 @@
try {
return getDefaultCcToolchainDynamicRuntimeInputsFromStarlark(ruleContext);
} catch (EvalException e) {
- throw ruleContext.throwWithRuleError(e.getMessage());
+ throw ruleContext.throwWithRuleError(e);
}
}
@@ -287,7 +287,7 @@
try {
return defaultToolchain.getStaticRuntimeLinkInputs(featureConfiguration);
} catch (EvalException e) {
- throw ruleContext.throwWithRuleError(e.getMessage());
+ throw ruleContext.throwWithRuleError(e);
}
}
@@ -748,7 +748,7 @@
try {
return ImmutableList.copyOf(featureConfiguration.getCommandLine(actionName, variables));
} catch (ExpansionException e) {
- throw ruleErrorConsumer.throwWithRuleError(e.getMessage());
+ throw ruleErrorConsumer.throwWithRuleError(e);
}
}
@@ -761,7 +761,7 @@
try {
return featureConfiguration.getEnvironmentVariables(actionName, variables);
} catch (ExpansionException e) {
- throw ruleErrorConsumer.throwWithRuleError(e.getMessage());
+ throw ruleErrorConsumer.throwWithRuleError(e);
}
}
@@ -814,7 +814,7 @@
try {
return toolchain.getFeatures().getArtifactNameForCategory(category, outputName);
} catch (EvalException e) {
- ruleErrorConsumer.throwWithRuleError(e.getMessage());
+ ruleErrorConsumer.throwWithRuleError(e);
throw new IllegalStateException("Should not be reached");
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaBinary.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaBinary.java
index e51444d..1d45935 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaBinary.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaBinary.java
@@ -328,7 +328,7 @@
try {
dynamicRuntimeActionInputs = ccToolchain.getDynamicRuntimeLinkInputs(featureConfiguration);
} catch (EvalException e) {
- throw ruleContext.throwWithRuleError(e.getMessage());
+ throw ruleContext.throwWithRuleError(e);
}
collectDefaultRunfiles(
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/AppleBinary.java b/src/main/java/com/google/devtools/build/lib/rules/objc/AppleBinary.java
index 8a42747f..fb8e021 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/AppleBinary.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/AppleBinary.java
@@ -226,8 +226,7 @@
new AppleLoadableBundleBinaryInfo(outputArtifact, objcProvider);
break;
default:
- ruleContext.ruleError("Unhandled binary type " + getBinaryType(ruleContext));
- throw new RuleErrorException();
+ throw ruleContext.throwWithRuleError("Unhandled binary type " + getBinaryType(ruleContext));
}
AppleDebugOutputsInfo.Builder builder = AppleDebugOutputsInfo.Builder.create();
@@ -355,8 +354,7 @@
outputArtifact = loadableBundleProvider.getAppleLoadableBundleBinary();
break;
default:
- ruleContext.ruleError("Unhandled binary type " + getBinaryType(ruleContext));
- throw new RuleErrorException();
+ throw ruleContext.throwWithRuleError("Unhandled binary type " + getBinaryType(ruleContext));
}
NestedSetBuilder<Artifact> filesToBuild =
