Skip to content

Commit

Permalink
bazel packages: avoid EvalException for errors unrelated to Starlark
Browse files Browse the repository at this point in the history
This changes introduces two more-specific exceptions:
  DependencyResolver.Failure
  ConfiguredAttributeMapper.ValidationException
and is another step toward removing the EvalException(Location) constructor.
More to follow.

PiperOrigin-RevId: 350774869
  • Loading branch information
adonovan authored and copybara-github committed Jan 8, 2021
1 parent e50edd8 commit 63e0392
Show file tree
Hide file tree
Showing 5 changed files with 66 additions and 51 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,7 @@
import java.util.List;
import java.util.Map;
import javax.annotation.Nullable;
import net.starlark.java.eval.EvalException;
import net.starlark.java.eval.Starlark;
import net.starlark.java.syntax.Location;

/**
* Resolver for dependencies between configured targets.
Expand Down Expand Up @@ -184,7 +183,7 @@ public final OrderedSetMultimap<DependencyKind, DependencyKey> dependentNodeMap(
@Nullable ToolchainCollection<ToolchainContext> toolchainContexts,
boolean useToolchainTransition,
@Nullable TransitionFactory<Rule> trimmingTransitionFactory)
throws EvalException, InterruptedException, InconsistentAspectOrderException {
throws Failure, InterruptedException, InconsistentAspectOrderException {
NestedSetBuilder<Cause> rootCauses = NestedSetBuilder.stableOrder();
OrderedSetMultimap<DependencyKind, DependencyKey> outgoingEdges =
dependentNodeMap(
Expand Down Expand Up @@ -242,7 +241,7 @@ public final OrderedSetMultimap<DependencyKind, DependencyKey> dependentNodeMap(
boolean useToolchainTransition,
NestedSetBuilder<Cause> rootCauses,
@Nullable TransitionFactory<Rule> trimmingTransitionFactory)
throws EvalException, InterruptedException, InconsistentAspectOrderException {
throws Failure, InterruptedException, InconsistentAspectOrderException {
Target target = node.getTarget();
BuildConfiguration config = node.getConfiguration();
OrderedSetMultimap<DependencyKind, Label> outgoingLabels = OrderedSetMultimap.create();
Expand Down Expand Up @@ -304,11 +303,11 @@ public final OrderedSetMultimap<DependencyKind, DependencyKey> dependentNodeMap(
partiallyResolveDependencies(
OrderedSetMultimap<DependencyKind, Label> outgoingLabels,
@Nullable Rule fromRule,
ConfiguredAttributeMapper attributeMap,
@Nullable ConfiguredAttributeMapper attributeMap,
@Nullable ToolchainCollection<ToolchainContext> toolchainContexts,
boolean useToolchainTransition,
Iterable<Aspect> aspects)
throws EvalException {
throws Failure {
OrderedSetMultimap<DependencyKind, PartiallyResolvedDependency> partiallyResolvedDeps =
OrderedSetMultimap.create();

Expand Down Expand Up @@ -394,15 +393,11 @@ public final OrderedSetMultimap<DependencyKind, DependencyKey> dependentNodeMap(
String execGroup =
((ExecutionTransitionFactory) attribute.getTransitionFactory()).getExecGroup();
if (!toolchainContexts.hasToolchainContext(execGroup)) {
String error =
throw new Failure(
fromRule != null ? fromRule.getLocation() : null,
String.format(
"Attr '%s' declares a transition for non-existent exec group '%s'",
attribute.getName(), execGroup);
if (fromRule != null) {
throw new EvalException(fromRule.getLocation(), error);
} else {
throw Starlark.errorf("%s", error);
}
attribute.getName(), execGroup));
}
if (toolchainContexts.getToolchainContext(execGroup).executionPlatform() != null) {
executionPlatformLabel =
Expand Down Expand Up @@ -477,19 +472,39 @@ private OrderedSetMultimap<DependencyKind, DependencyKey> fullyResolveDependenci
return outgoingEdges;
}

/** A DependencyResolver.Failure indicates a failure during dependency resolution. */
public static class Failure extends Exception {
@Nullable private final Location location;

private Failure(Location location, String message) {
super(message);
this.location = location;
}

/** Returns the location of the error, if known. */
@Nullable
public Location getLocation() {
return location;
}
}

private void visitRule(
TargetAndConfiguration node,
BuildConfiguration hostConfig,
Iterable<Aspect> aspects,
ConfiguredAttributeMapper attributeMap,
@Nullable ToolchainCollection<ToolchainContext> toolchainContexts,
OrderedSetMultimap<DependencyKind, Label> outgoingLabels)
throws EvalException {
throws Failure {
Preconditions.checkArgument(node.getTarget() instanceof Rule, node);
BuildConfiguration ruleConfig = Preconditions.checkNotNull(node.getConfiguration(), node);
Rule rule = (Rule) node.getTarget();

attributeMap.validateAttributes();
try {
attributeMap.validateAttributes();
} catch (ConfiguredAttributeMapper.ValidationException ex) {
throw new Failure(rule.getLocation(), ex.getMessage());
}

visitTargetVisibility(node, outgoingLabels);
resolveAttributes(outgoingLabels, rule, attributeMap, aspects, ruleConfig, hostConfig);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
import java.util.Map;
import java.util.Set;
import javax.annotation.Nullable;
import net.starlark.java.eval.EvalException;

/**
* {@link AttributeMap} implementation that binds a rule's attribute as follows:
Expand Down Expand Up @@ -75,24 +74,30 @@ public static ConfiguredAttributeMapper of(
}

/**
* Checks that all attributes can be mapped to their configured values. This is
* useful for checking that the configuration space in a configured attribute doesn't
* contain unresolvable contradictions.
* Checks that all attributes can be mapped to their configured values. This is useful for
* checking that the configuration space in a configured attribute doesn't contain unresolvable
* contradictions.
*
* @throws EvalException if any attribute's value can't be resolved under this mapper
* @throws ValidationException if any attribute's value can't be resolved under this mapper
*/
public void validateAttributes() throws EvalException {
public void validateAttributes() throws ValidationException {
for (String attrName : getAttributeNames()) {
getAndValidate(attrName, getAttributeType(attrName));
}
}

/** ValidationException indicates an error during attribute validation. */
public static final class ValidationException extends Exception {
private ValidationException(String message) {
super(message);
}
}

/**
* Variation of {@link #get} that throws an informative exception if the attribute can't be
* resolved due to intrinsic contradictions in the configuration.
*/
@SuppressWarnings("unchecked")
private <T> T getAndValidate(String attributeName, Type<T> type) throws EvalException {
private <T> T getAndValidate(String attributeName, Type<T> type) throws ValidationException {
SelectorList<T> selectorList = getSelectorList(attributeName, type);
if (selectorList == null) {
// This is a normal attribute.
Expand All @@ -109,15 +114,16 @@ private <T> T getAndValidate(String attributeName, Type<T> type) throws EvalExce
// do that anyway, so that isn't a loss.
Attribute attr = getAttributeDefinition(attributeName);
if (attr.isMandatory()) {
throw new EvalException(
rule.getLocation(),
throw new ValidationException(
String.format(
"Mandatory attribute '%s' resolved to 'None' after evaluating 'select'"
+ " expression",
attributeName));
}
Verify.verify(attr.getCondition() == Predicates.<AttributeMap>alwaysTrue());
resolvedList.add((T) attr.getDefaultValue(null)); // unchecked cast
@SuppressWarnings("unchecked")
T defaultValue = (T) attr.getDefaultValue(null);
resolvedList.add(defaultValue);
} else {
resolvedList.add(resolvedPath.value);
}
Expand All @@ -139,7 +145,7 @@ private static class ConfigKeyAndValue<T> {
}

private <T> ConfigKeyAndValue<T> resolveSelector(String attributeName, Selector<T> selector)
throws EvalException {
throws ValidationException {
Map<Label, ConfigKeyAndValue<T>> matchingConditions = new LinkedHashMap<>();
Set<Label> conditionLabels = new LinkedHashSet<>();
ConfigKeyAndValue<T> matchingResult = null;
Expand Down Expand Up @@ -184,8 +190,7 @@ private <T> ConfigKeyAndValue<T> resolveSelector(String attributeName, Selector<
}

if (matchingConditions.size() > 1) {
throw new EvalException(
rule.getLocation(),
throw new ValidationException(
"Illegal ambiguous match on configurable attribute \""
+ attributeName
+ "\" in "
Expand All @@ -208,7 +213,7 @@ private <T> ConfigKeyAndValue<T> resolveSelector(String attributeName, Selector<
noMatchMessage += " (would a default condition help?).\nConditions checked:\n "
+ Joiner.on("\n ").join(conditionLabels);
}
throw new EvalException(rule.getLocation(), noMatchMessage);
throw new ValidationException(noMatchMessage);
}
matchingResult =
selector.hasDefault()
Expand All @@ -224,7 +229,7 @@ private <T> ConfigKeyAndValue<T> resolveSelector(String attributeName, Selector<
public <T> T get(String attributeName, Type<T> type) {
try {
return getAndValidate(attributeName, type);
} catch (EvalException e) {
} catch (ValidationException e) {
// Callers that reach this branch should explicitly validate the attribute through an
// appropriate call and handle the exception directly. This method assumes
// pre-validated attributes.
Expand All @@ -246,7 +251,7 @@ public boolean isAttributeValueExplicitlySpecified(String attributeName) {
if (selector.isValueSet(resolvedPath.configKey)) {
return true;
}
} catch (EvalException e) {
} catch (ValidationException unused) {
// This will trigger an error via any other call, so the actual return doesn't matter much.
return true;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@
import java.util.function.Function;
import java.util.stream.Collectors;
import javax.annotation.Nullable;
import net.starlark.java.eval.EvalException;

/**
* Output formatter that prints {@link ConfigurationTransition} information for rule configured
Expand Down Expand Up @@ -134,7 +133,7 @@ public void processOutput(Iterable<KeyedConfiguredTarget> partialResult)
toolchainContexts,
DependencyResolver.shouldUseToolchainTransition(config, target),
trimmingTransitionFactory);
} catch (EvalException | InconsistentAspectOrderException e) {
} catch (DependencyResolver.Failure | InconsistentAspectOrderException e) {
// This is an abuse of InterruptedException.
throw new InterruptedException(e.getMessage());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,6 @@
import java.util.concurrent.TimeUnit;
import java.util.stream.Collectors;
import javax.annotation.Nullable;
import net.starlark.java.eval.EvalException;

/**
* SkyFunction for {@link ConfiguredTargetValue}s.
Expand Down Expand Up @@ -656,13 +655,11 @@ static OrderedSetMultimap<DependencyKind, ConfiguredTargetAndData> computeDepend
useToolchainTransition,
transitiveRootCauses,
((ConfiguredRuleClassProvider) ruleClassProvider).getTrimmingTransitionFactory());
} catch (EvalException e) {
// EvalException can only be thrown by computed Starlark attributes in the current rule.
String msgWithStack = e.getMessageWithStack();
env.getListener().handle(Event.error(null, msgWithStack));
env.getListener().post(new AnalysisRootCauseEvent(configuration, label, msgWithStack));
} catch (DependencyResolver.Failure e) {
env.getListener().handle(Event.error(e.getLocation(), e.getMessage()));
env.getListener().post(new AnalysisRootCauseEvent(configuration, label, e.getMessage()));
throw new DependencyEvaluationException(
new ConfiguredValueCreationException(msgWithStack, label, configuration));
new ConfiguredValueCreationException(e.getMessage(), label, configuration));
} catch (InconsistentAspectOrderException e) {
env.getListener().handle(Event.error(e.getLocation(), e.getMessage()));
throw new DependencyEvaluationException(e);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,6 @@
import java.util.function.Function;
import java.util.stream.Collectors;
import javax.annotation.Nullable;
import net.starlark.java.eval.EvalException;
import net.starlark.java.eval.Mutability;

/**
Expand Down Expand Up @@ -225,7 +224,7 @@ public Collection<ConfiguredTarget> getDirectPrerequisitesForTesting(
ExtendedEventHandler eventHandler,
ConfiguredTarget ct,
BuildConfigurationCollection configurations)
throws EvalException, InvalidConfigurationException, InterruptedException,
throws DependencyResolver.Failure, InvalidConfigurationException, InterruptedException,
InconsistentAspectOrderException, StarlarkTransition.TransitionException {
return Collections2.transform(
getConfiguredTargetAndDataDirectPrerequisitesForTesting(eventHandler, ct, configurations),
Expand All @@ -238,7 +237,7 @@ public Collection<ConfiguredTarget> getDirectPrerequisitesForTesting(
ExtendedEventHandler eventHandler,
ConfiguredTarget ct,
BuildConfigurationCollection configurations)
throws EvalException, InvalidConfigurationException, InterruptedException,
throws DependencyResolver.Failure, InvalidConfigurationException, InterruptedException,
InconsistentAspectOrderException, StarlarkTransition.TransitionException {
return getConfiguredTargetAndDataDirectPrerequisitesForTesting(
eventHandler, ct, ct.getConfigurationKey(), configurations);
Expand All @@ -250,7 +249,7 @@ public Collection<ConfiguredTarget> getDirectPrerequisitesForTesting(
ExtendedEventHandler eventHandler,
ConfiguredTargetAndData ct,
BuildConfigurationCollection configurations)
throws EvalException, InvalidConfigurationException, InterruptedException,
throws DependencyResolver.Failure, InvalidConfigurationException, InterruptedException,
InconsistentAspectOrderException, StarlarkTransition.TransitionException {
return getConfiguredTargetAndDataDirectPrerequisitesForTesting(
eventHandler,
Expand All @@ -265,7 +264,7 @@ public Collection<ConfiguredTarget> getDirectPrerequisitesForTesting(
ConfiguredTarget ct,
BuildConfigurationValue.Key configuration,
BuildConfigurationCollection configurations)
throws EvalException, InvalidConfigurationException, InterruptedException,
throws DependencyResolver.Failure, InvalidConfigurationException, InterruptedException,
InconsistentAspectOrderException, StarlarkTransition.TransitionException {
return skyframeExecutor.getConfiguredTargetsForTesting(
eventHandler,
Expand All @@ -283,7 +282,7 @@ public Collection<ConfiguredTarget> getDirectPrerequisitesForTesting(
final ConfiguredTarget ct,
BuildConfigurationCollection configurations,
@Nullable ToolchainCollection<ToolchainContext> toolchainContexts)
throws EvalException, InterruptedException, InconsistentAspectOrderException,
throws DependencyResolver.Failure, InterruptedException, InconsistentAspectOrderException,
StarlarkTransition.TransitionException, InvalidConfigurationException {

Target target = null;
Expand Down Expand Up @@ -385,7 +384,7 @@ private OrderedSetMultimap<DependencyKind, ConfiguredTargetAndData> getPrerequis
ConfiguredTarget target,
BuildConfigurationCollection configurations,
@Nullable ToolchainCollection<ToolchainContext> toolchainContexts)
throws EvalException, InvalidConfigurationException, InterruptedException,
throws DependencyResolver.Failure, InvalidConfigurationException, InterruptedException,
InconsistentAspectOrderException, StarlarkTransition.TransitionException {
OrderedSetMultimap<DependencyKind, DependencyKey> depNodeNames =
getDirectPrerequisiteDependenciesForTesting(
Expand Down Expand Up @@ -469,7 +468,7 @@ public RuleContext getRuleContextForTesting(
ConfiguredTarget target,
StoredEventHandler eventHandler,
BuildConfigurationCollection configurations)
throws EvalException, InvalidConfigurationException, InterruptedException,
throws DependencyResolver.Failure, InvalidConfigurationException, InterruptedException,
InconsistentAspectOrderException, ToolchainException,
StarlarkTransition.TransitionException, InvalidExecGroupException {
BuildConfiguration targetConfig =
Expand Down Expand Up @@ -506,7 +505,7 @@ public RuleContext getRuleContextForTesting(
ConfiguredTarget configuredTarget,
AnalysisEnvironment env,
BuildConfigurationCollection configurations)
throws EvalException, InvalidConfigurationException, InterruptedException,
throws DependencyResolver.Failure, InvalidConfigurationException, InterruptedException,
InconsistentAspectOrderException, ToolchainException,
StarlarkTransition.TransitionException, InvalidExecGroupException {
BuildConfiguration targetConfig =
Expand Down Expand Up @@ -626,7 +625,7 @@ public ConfiguredTarget getPrerequisiteConfiguredTargetForTesting(
ConfiguredTarget dependentTarget,
Label desiredTarget,
BuildConfigurationCollection configurations)
throws EvalException, InvalidConfigurationException, InterruptedException,
throws DependencyResolver.Failure, InvalidConfigurationException, InterruptedException,
InconsistentAspectOrderException, StarlarkTransition.TransitionException {
Collection<ConfiguredTargetAndData> configuredTargets =
getPrerequisiteMapForTesting(
Expand Down

0 comments on commit 63e0392

Please sign in to comment.