Skip to content

Commit

Permalink
Provide more detail on the failing targets during attribute map failu…
Browse files Browse the repository at this point in the history
…res.

PiperOrigin-RevId: 571036040
Change-Id: If188372f2f00cd26a6af51d261d6ed9c95dd5441
  • Loading branch information
katre authored and copybara-github committed Oct 5, 2023
1 parent 70fc6e4 commit 5b4e986
Show file tree
Hide file tree
Showing 6 changed files with 38 additions and 21 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,10 @@ public <T> T get(String attributeName, Type<T> type) {
} else {
Attribute attribute = aspectAttributes.get(attributeName);
if (attribute == null) {
throw new IllegalArgumentException(String.format(
"no attribute '%s' in either %s or its aspects",
attributeName, ruleAttributes.getLabel()));
throw new IllegalArgumentException(
String.format(
"no attribute '%s' in either %s or its aspects",
attributeName, ruleAttributes.describeRule()));
} else if (attribute.getType() != type) {
throw new IllegalArgumentException(String.format(
"attribute %s has type %s, not expected type %s",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,11 @@ protected AbstractAttributeMapper(Rule rule) {
this.rule = rule;
}

@Override
public String describeRule() {
return String.format("%s %s", this.rule.getRuleClass(), getLabel());
}

@Override
public Label getLabel() {
return ruleLabel;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1507,7 +1507,7 @@ public Object getDefault(AttributeMap rule) {
Preconditions.checkState(
lookupTable.containsKey(key),
"Error in rule '%s': precomputed value missing for dependencies: %s. Available keys: %s.",
rule.getLabel(),
rule.describeRule(),
Iterables.toString(key),
Iterables.toString(lookupTable.keySet()));
return lookupTable.get(key);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,11 @@
* its particular needs are.
*/
public interface AttributeMap {
/** Describe the underlying rule, for use in messages. */
default String describeRule() {
return getLabel().toString();
}

/**
* Returns the label of the rule.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,21 +58,6 @@
*/
public class ConfiguredAttributeMapper extends AbstractAttributeMapper {

private final Map<Label, ConfigMatchingProvider> configConditions;
private final String configHash;
private final boolean alwaysSucceed;

private ConfiguredAttributeMapper(
Rule rule,
ImmutableMap<Label, ConfigMatchingProvider> configConditions,
String configHash,
boolean alwaysSucceed) {
super(Preconditions.checkNotNull(rule));
this.configConditions = configConditions;
this.configHash = configHash;
this.alwaysSucceed = alwaysSucceed;
}

/**
* "Manual" constructor that requires the caller to pass the set of configurability conditions
* that trigger this rule's configurable attributes.
Expand Down Expand Up @@ -101,7 +86,27 @@ public static ConfiguredAttributeMapper of(
BuildConfigurationValue configuration) {
boolean alwaysSucceed =
configuration.getOptions().get(CoreOptions.class).debugSelectsAlwaysSucceed;
return of(rule, configConditions, configuration.checksum(), alwaysSucceed);
return of(rule, configConditions, configuration.shortId(), alwaysSucceed);
}

private final ImmutableMap<Label, ConfigMatchingProvider> configConditions;
private final String configHash;
private final boolean alwaysSucceed;

private ConfiguredAttributeMapper(
Rule rule,
ImmutableMap<Label, ConfigMatchingProvider> configConditions,
String configHash,
boolean alwaysSucceed) {
super(Preconditions.checkNotNull(rule));
this.configConditions = configConditions;
this.configHash = configHash;
this.alwaysSucceed = alwaysSucceed;
}

@Override
public String describeRule() {
return String.format("%s (%s)", super.describeRule(), this.configHash.substring(0, 6));
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,8 @@ public void getMissingAttributeValue() throws Exception {
assertThrows(IllegalArgumentException.class, () -> mapper.get("noexist", BuildType.LABEL));
assertThat(e)
.hasMessageThat()
.isEqualTo("no attribute 'noexist' in either //foo:myrule or its aspects");
.matches(
"no attribute 'noexist' in either cc_binary //foo:myrule \\([^)]+\\) or its aspects");
}

@Test
Expand Down

0 comments on commit 5b4e986

Please sign in to comment.