Skip to content

Commit

Permalink
Make SkylarkErrorReporter proxy ruleContext error consumer
Browse files Browse the repository at this point in the history
Fixes #8234

RELNOTES: None.
PiperOrigin-RevId: 246571018
  • Loading branch information
c-parsons authored and copybara-github committed May 3, 2019
1 parent b501813 commit 4929441
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 62 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,10 @@ public ActionConstructionContext getActionConstructionContext() {
return ruleContext;
}

public RuleContext getRuleContext() {
return ruleContext;
}

@Override
public void runShell(
SkylarkList outputs,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,80 +13,65 @@
// limitations under the License
package com.google.devtools.build.lib.analysis.skylark;

import com.google.devtools.build.lib.analysis.AnalysisEnvironment;
import com.google.devtools.build.lib.analysis.EventHandlingErrorReporter;
import com.google.devtools.build.lib.analysis.actions.ActionConstructionContext;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.events.Location;
import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException;
import com.google.devtools.build.lib.packages.RuleErrorConsumer;
import com.google.devtools.build.lib.skylarkinterface.SkylarkCallable;
import com.google.devtools.build.lib.syntax.Environment;
import com.google.devtools.build.lib.syntax.EvalException;

/**
* {@link RuleErrorConsumer} for Native implementations of Skylark APIs.
*
* <p>This class largely reproduces the functionality of the error consumer in {@link
* com.google.devtools.build.lib.analysis.RuleContext}, but should be easy to create and use from
* methods annotated with {@link com.google.devtools.build.lib.skylarkinterface.SkylarkCallable}.
* (Environment and Location are automatically provided to those methods be specifying the {@link
* SkylarkCallable#useLocation()} and {@link SkylarkCallable#useEnvironment()} fields in the
* annotation.
* <p>This class proxies reported errors and warnings to a proxy {@link RuleErrorConsumer}, except
* that it suppresses all cases of actually throwing exceptions until this reporter is closed.
*
* <p>This class is AutoClosable, to ensure that {@link RuleErrorException} are checked and handled
* before leaving native code. The {@link #close()} method will only throw {@link EvalException},
* properly wrapping any {@link RuleErrorException} instances if needed.
*/
public class SkylarkErrorReporter extends EventHandlingErrorReporter implements AutoCloseable {
public class SkylarkErrorReporter implements AutoCloseable, RuleErrorConsumer {
private final RuleErrorConsumer ruleErrorConsumer;
private final Location location;
private final Label label;

public static SkylarkErrorReporter from(
ActionConstructionContext context, Location location, Environment env) {
return new SkylarkErrorReporter(
context.getActionOwner().getTargetKind(),
context.getAnalysisEnvironment(),
location,
env.getCallerLabel());
public static SkylarkErrorReporter from(RuleErrorConsumer ruleErrorConsumer, Location location) {
return new SkylarkErrorReporter(ruleErrorConsumer, location);
}

private SkylarkErrorReporter(
String ruleClassNameForLogging,
AnalysisEnvironment analysisEnvironment,
Location location,
Label label) {
super(ruleClassNameForLogging, analysisEnvironment);
this.label = label;
private SkylarkErrorReporter(RuleErrorConsumer ruleErrorConsumer, Location location) {
this.ruleErrorConsumer = ruleErrorConsumer;
this.location = location;
}

@Override
protected String getMacroMessageAppendix(String attrName) {
return "";
public void close() throws EvalException {
try {
assertNoErrors();
} catch (RuleErrorException e) {
throw new EvalException(location, "error occurred while evaluating builtin function", e);
}
}

@Override
protected Label getLabel() {
return label;
public void ruleWarning(String message) {
ruleErrorConsumer.ruleWarning(message);
}

@Override
protected Location getRuleLocation() {
return location;
public void ruleError(String message) {
ruleErrorConsumer.ruleError(message);
}

@Override
protected Location getAttributeLocation(String attrName) {
return location;
public void attributeWarning(String attrName, String message) {
ruleErrorConsumer.attributeWarning(attrName, message);
}

@Override
public void close() throws EvalException {
try {
assertNoErrors();
} catch (RuleErrorException e) {
throw new EvalException(location, e);
}
public void attributeError(String attrName, String message) {
ruleErrorConsumer.attributeError(attrName, message);
}

@Override
public boolean hasErrors() {
return ruleErrorConsumer.hasErrors();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget.Mode;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.packages.ImplicitOutputsFunction.SafeImplicitOutputsFunction;
import com.google.devtools.build.lib.packages.RuleErrorConsumer;
import com.google.devtools.build.lib.skylarkbuildapi.android.AndroidDataContextApi;
import com.google.devtools.build.lib.vfs.PathFragment;

Expand All @@ -42,7 +43,7 @@
public class AndroidDataContext implements AndroidDataContextApi {

private final Label label;
private final ActionConstructionContext actionConstructionContext;
private final RuleContext ruleContext;
private final FilesToRunProvider busybox;
private final AndroidSdkProvider sdk;
private final boolean persistentBusyboxToolsEnabled;
Expand Down Expand Up @@ -76,15 +77,15 @@ private static boolean shouldThrowOnResourceConflictFromWhitelist(RuleContext ru

protected AndroidDataContext(
Label label,
ActionConstructionContext actionConstructionContext,
RuleContext ruleContext,
FilesToRunProvider busybox,
boolean persistentBusyboxToolsEnabled,
AndroidSdkProvider sdk,
boolean throwOnResourceConflict,
boolean useDataBindingV2) {
this.label = label;
this.persistentBusyboxToolsEnabled = persistentBusyboxToolsEnabled;
this.actionConstructionContext = actionConstructionContext;
this.ruleContext = ruleContext;
this.busybox = busybox;
this.sdk = sdk;
this.throwOnResourceConflict = throwOnResourceConflict;
Expand All @@ -96,7 +97,11 @@ public Label getLabel() {
}

public ActionConstructionContext getActionConstructionContext() {
return actionConstructionContext;
return ruleContext;
}

public RuleErrorConsumer getRuleErrorConsumer() {
return ruleContext;
}

public FilesToRunProvider getBusybox() {
Expand All @@ -113,41 +118,41 @@ public AndroidSdkProvider getSdk() {

/** Builds and registers a {@link SpawnAction.Builder}. */
public void registerAction(SpawnAction.Builder spawnActionBuilder) {
registerAction(spawnActionBuilder.build(actionConstructionContext));
registerAction(spawnActionBuilder.build(ruleContext));
}

/** Registers one or more actions. */
public void registerAction(ActionAnalysisMetadata... actions) {
actionConstructionContext.registerAction(actions);
ruleContext.registerAction(actions);
}

public Artifact createOutputArtifact(SafeImplicitOutputsFunction function)
throws InterruptedException {
return actionConstructionContext.getImplicitOutputArtifact(function);
return ruleContext.getImplicitOutputArtifact(function);
}

public Artifact getUniqueDirectoryArtifact(String uniqueDirectorySuffix, String relative) {
return actionConstructionContext.getUniqueDirectoryArtifact(uniqueDirectorySuffix, relative);
return ruleContext.getUniqueDirectoryArtifact(uniqueDirectorySuffix, relative);
}

public Artifact getUniqueDirectoryArtifact(String uniqueDirectorySuffix, PathFragment relative) {
return actionConstructionContext.getUniqueDirectoryArtifact(uniqueDirectorySuffix, relative);
return ruleContext.getUniqueDirectoryArtifact(uniqueDirectorySuffix, relative);
}

public PathFragment getUniqueDirectory(PathFragment fragment) {
return actionConstructionContext.getUniqueDirectory(fragment);
return ruleContext.getUniqueDirectory(fragment);
}

public ArtifactRoot getBinOrGenfilesDirectory() {
return actionConstructionContext.getBinOrGenfilesDirectory();
return ruleContext.getBinOrGenfilesDirectory();
}

public PathFragment getPackageDirectory() {
return actionConstructionContext.getPackageDirectory();
return ruleContext.getPackageDirectory();
}

public AndroidConfiguration getAndroidConfig() {
return actionConstructionContext.getConfiguration().getFragment(AndroidConfiguration.class);
return ruleContext.getConfiguration().getFragment(AndroidConfiguration.class);
}

/** Indicates whether Busybox actions should be passed the "--debug" flag */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ public AndroidResourcesInfo resourcesFromDeps(
Environment env)
throws InterruptedException, EvalException {
try (SkylarkErrorReporter errorReporter =
SkylarkErrorReporter.from(ctx.getActionConstructionContext(), location, env)) {
SkylarkErrorReporter.from(ctx.getRuleErrorConsumer(), location)) {
String pkg = fromNoneable(customPackage, String.class);
if (pkg == null) {
pkg =
Expand Down Expand Up @@ -113,7 +113,7 @@ public AndroidManifestInfo stampAndroidManifest(
throws InterruptedException, EvalException {
String pkg = fromNoneable(customPackage, String.class);
try (SkylarkErrorReporter errorReporter =
SkylarkErrorReporter.from(ctx.getActionConstructionContext(), location, env)) {
SkylarkErrorReporter.from(ctx.getRuleErrorConsumer(), location)) {
return AndroidManifest.from(
ctx,
errorReporter,
Expand All @@ -137,7 +137,7 @@ public AndroidAssetsInfo mergeAssets(
Environment env)
throws EvalException, InterruptedException {
try (SkylarkErrorReporter errorReporter =
SkylarkErrorReporter.from(ctx.getActionConstructionContext(), location, env)) {
SkylarkErrorReporter.from(ctx.getRuleErrorConsumer(), location)) {
return AndroidAssets.from(
errorReporter,
listFromNoneable(assets, ConfiguredTarget.class),
Expand All @@ -164,7 +164,7 @@ public ValidatedAndroidResources mergeRes(
Environment env)
throws EvalException, InterruptedException {
try (SkylarkErrorReporter errorReporter =
SkylarkErrorReporter.from(ctx.getActionConstructionContext(), location, env)) {
SkylarkErrorReporter.from(ctx.getRuleErrorConsumer(), location)) {
AndroidAaptVersion aaptVersion =
ctx.getSdk().getAapt2() != null ? AndroidAaptVersion.AAPT2 : AndroidAaptVersion.AAPT;
return AndroidResources.from(errorReporter, getFileProviders(resources), "resources")
Expand Down Expand Up @@ -320,7 +320,7 @@ public SkylarkDict<Provider, NativeInfo> processLocalTestData(
Environment env)
throws InterruptedException, EvalException {
try (SkylarkErrorReporter errorReporter =
SkylarkErrorReporter.from(ctx.getActionConstructionContext(), location, env)) {
SkylarkErrorReporter.from(ctx.getRuleErrorConsumer(), location)) {

AndroidManifest rawManifest =
AndroidManifest.from(
Expand Down Expand Up @@ -382,7 +382,7 @@ public BinaryDataSettings makeBinarySettings(

AndroidAaptVersion aaptVersion;
try (SkylarkErrorReporter errorReporter =
SkylarkErrorReporter.from(ctx.getActionConstructionContext(), location, env)) {
SkylarkErrorReporter.from(ctx.getRuleErrorConsumer(), location)) {
aaptVersion =
AndroidAaptVersion.chooseTargetAaptVersion(ctx, errorReporter, aaptVersionString);
} catch (RuleErrorException e) {
Expand Down Expand Up @@ -455,7 +455,7 @@ public AndroidBinaryDataInfo processBinaryData(
Environment env)
throws InterruptedException, EvalException {
try (SkylarkErrorReporter errorReporter =
SkylarkErrorReporter.from(ctx.getActionConstructionContext(), location, env)) {
SkylarkErrorReporter.from(ctx.getRuleErrorConsumer(), location)) {

BinaryDataSettings settings =
fromNoneableOrDefault(
Expand Down

0 comments on commit 4929441

Please sign in to comment.