Skip to content

Commit

Permalink
Rollback bazelbuild@8174262
Browse files Browse the repository at this point in the history 
PiperOrigin-RevId: 425940910
  • Loading branch information
@brandjon @copybara-github
brandjon authored and copybara-github committed Feb 2, 2022
1 parent 2fd886b commit 45ce545
Show file tree
Hide file tree
Showing 7 changed files with 176 additions and 142 deletions.
168 changes: 95 additions & 73 deletions src/main/java/com/google/devtools/build/lib/analysis/AspectAwareAttributeMapper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,78 +17,80 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.packages.AbstractAttributeMapper;
import com.google.devtools.build.lib.packages.Attribute;
import com.google.devtools.build.lib.packages.Attribute.ComputedDefault;
import com.google.devtools.build.lib.packages.AttributeMap;
import com.google.devtools.build.lib.packages.DependencyFilter;
import com.google.devtools.build.lib.packages.Rule;
import com.google.devtools.build.lib.packages.Type;
import java.util.Map;
import java.util.function.BiConsumer;
import java.util.function.Consumer;

/**
* An {@link AttributeMap} that supports queries on both a rule and its aspects.
* An {@link AttributeMap} that supports attribute type queries on both a rule and its aspects and
* attribute value queries on the rule.
*
* <p>When both the rule and aspect declare the same attribute, the aspect's value takes precedence.
* This is because aspects expect access to the merged rule/attribute data (including possible
* aspect overrides) while rules evaluate before aspects are attached.
* <p>An attribute type query is anything accessible from {@link Attribute} (i.e. anything about how
* the attribute is integrated into the {@link com.google.devtools.build.lib.packages.RuleClass}).
* An attribute value query is anything related to the actual value an attribute takes.
*
* <p>Note that public aspect attributes must be strings that inherit the values of the equivalent
* rule attributes. Only private (implicit) attributes can have different values.
* <p>For example, given {@code deps = [":adep"]}, checking that {@code deps} exists or that it's
* type is {@link com.google.devtools.build.lib.packages.BuildType#LABEL_LIST} are type queries.
* Checking that its value is explicitly set in the BUILD File or that its value {@code [":adep"]}
* are value queries..
*
* <p>Value queries on aspect attributes trigger {@link UnsupportedOperationException}.
*/
final class AspectAwareAttributeMapper extends AbstractAttributeMapper {
private final AbstractAttributeMapper ruleAttributes;
// Attribute name -> definition.
class AspectAwareAttributeMapper implements AttributeMap {
private final AttributeMap ruleAttributes;
private final ImmutableMap<String, Attribute> aspectAttributes;
// Attribute name -> value. null values (which are valid) are excluded from this map. Use
// aspectAttributes to check attribute existence.
private final ImmutableMap<String, Object> aspectAttributeValues;

public AspectAwareAttributeMapper(
Rule rule,
AbstractAttributeMapper ruleAttributes,
public AspectAwareAttributeMapper(AttributeMap ruleAttributes,
ImmutableMap<String, Attribute> aspectAttributes) {
super(rule);
this.ruleAttributes = ruleAttributes;
this.aspectAttributes = aspectAttributes;
ImmutableMap.Builder<String, Object> valueBuilder = new ImmutableMap.Builder<>();
for (Map.Entry<String, Attribute> aspectAttribute : aspectAttributes.entrySet()) {
Attribute attribute = aspectAttribute.getValue();
Object defaultValue = attribute.getDefaultValue(rule);
Object attributeValue =
attribute
.getType()
.cast(
defaultValue instanceof ComputedDefault
? ((ComputedDefault) defaultValue).getDefault(ruleAttributes)
: defaultValue);
if (attributeValue != null) {
valueBuilder.put(aspectAttribute.getKey(), attributeValue);
}
}
this.aspectAttributeValues = valueBuilder.buildOrThrow();
}

@Override
public String getName() {
return ruleAttributes.getName();
}

@Override
public String getRuleClassName() {
return ruleAttributes.getRuleClassName();
}

@Override
public Label getLabel() {
return ruleAttributes.getLabel();
}

@Override
public <T> T get(String attributeName, Type<T> type) {
Attribute aspectAttribute = aspectAttributes.get(attributeName);
if (aspectAttribute != null) {
if (aspectAttribute.getType() != type) {
throw new IllegalArgumentException(
String.format(
"attribute %s has type %s, not expected type %s",
attributeName, aspectAttribute.getType(), type));
}
return type.cast(aspectAttributeValues.get(attributeName));
}
if (ruleAttributes.has(attributeName, type)) {
return ruleAttributes.get(attributeName, type);
}
throw new IllegalArgumentException(
String.format(
} 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()));
} else if (attribute.getType() != type) {
throw new IllegalArgumentException(String.format(
"attribute %s has type %s, not expected type %s",
attributeName, attribute.getType(), type));
} else {
throw new UnsupportedOperationException(
String.format(
"Attribute '%s' comes from an aspect. "
+ "Value retrieval for aspect attributes is not supported.",
attributeName));
}
}
}

@Override
public boolean isConfigurable(String attributeName) {
return ruleAttributes.isConfigurable(attributeName);
}

@Override
Expand All @@ -101,48 +103,68 @@ public Iterable<String> getAttributeNames() {

@Override
public Type<?> getAttributeType(String attrName) {
Attribute aspectAttribute = aspectAttributes.get(attrName);
if (aspectAttribute != null) {
return aspectAttribute.getType();
Type<?> type = ruleAttributes.getAttributeType(attrName);
if (type != null) {
return type;
} else {
Attribute attribute = aspectAttributes.get(attrName);
return attribute != null ? attribute.getType() : null;
}
return ruleAttributes.getAttributeType(attrName);
}

@Override
public Attribute getAttributeDefinition(String attrName) {
Attribute aspectAttribute = aspectAttributes.get(attrName);
if (aspectAttribute != null) {
return aspectAttribute;
Attribute attribute = ruleAttributes.getAttributeDefinition(attrName);
if (attribute != null) {
return attribute;
} else {
return aspectAttributes.get(attrName);
}
return ruleAttributes.getAttributeDefinition(attrName);
}

@Override
public boolean isAttributeValueExplicitlySpecified(String attributeName) {
return ruleAttributes.isAttributeValueExplicitlySpecified(attributeName);
}

@Override
public void visitAllLabels(BiConsumer<Attribute, Label> consumer) {
throw new UnsupportedOperationException("rule + aspects label visition is not supported");
}

@Override
public void visitLabels(Attribute attribute, Consumer<Label> consumer) {
throw new UnsupportedOperationException("rule + aspects label visition is not supported");
}

@Override
public void visitLabels(DependencyFilter filter, BiConsumer<Attribute, Label> consumer) {
ImmutableList.Builder<Attribute> combined = ImmutableList.builder();
combined.addAll(rule.getAttributes());
aspectAttributes.values().forEach(combined::add);
visitLabels(combined.build(), filter, consumer);
throw new UnsupportedOperationException("rule + aspects label visition is not supported");
}

@Override
public <T> void visitLabels(Attribute attribute, Type<T> type, Type.LabelVisitor visitor) {
Attribute aspectAttr = aspectAttributes.get(attribute.getName());
if (aspectAttr != null) {
T value = type.cast(aspectAttributeValues.get(attribute.getName()));
if (value != null) { // null values are particularly possible for computed defaults.
type.visitLabels(visitor, value, attribute);
}
return; // If both the aspect and rule have this attribute, the aspect instance overrides.
}
if (ruleAttributes.has(attribute.getName(), type)) {
ruleAttributes.visitLabels(attribute, type, visitor);
}
public String getPackageDefaultHdrsCheck() {
return ruleAttributes.getPackageDefaultHdrsCheck();
}

@Override
public boolean isPackageDefaultHdrsCheckSet() {
return ruleAttributes.isPackageDefaultHdrsCheckSet();
}

@Override
public Boolean getPackageDefaultTestOnly() {
return ruleAttributes.getPackageDefaultTestOnly();
}

@Override
public String getPackageDefaultDeprecation() {
return ruleAttributes.getPackageDefaultDeprecation();
}

@Override
public ImmutableList<String> getPackageDefaultCopts() {
return ruleAttributes.getPackageDefaultCopts();
}

@Override
Expand Down
84 changes: 59 additions & 25 deletions src/main/java/com/google/devtools/build/lib/analysis/DependencyResolver.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
import com.google.devtools.build.lib.packages.Aspect;
import com.google.devtools.build.lib.packages.AspectClass;
import com.google.devtools.build.lib.packages.Attribute;
import com.google.devtools.build.lib.packages.Attribute.ComputedDefault;
import com.google.devtools.build.lib.packages.Attribute.LateBoundDefault;
import com.google.devtools.build.lib.packages.AttributeMap;
import com.google.devtools.build.lib.packages.AttributeTransitionData;
Expand All @@ -58,7 +59,6 @@
import com.google.devtools.build.lib.packages.RuleTransitionData;
import com.google.devtools.build.lib.packages.Target;
import com.google.devtools.build.lib.packages.Type;
import com.google.devtools.build.lib.packages.Type.LabelVisitor;
import com.google.devtools.build.lib.util.OrderedSetMultimap;
import java.util.ArrayList;
import java.util.Collection;
Expand Down Expand Up @@ -591,11 +591,11 @@ private void resolveAttributes(
Iterable<AttributeDependencyKind> attributeDependencyKinds,
OrderedSetMultimap<DependencyKind, Label> outgoingLabels,
Rule rule,
ConfiguredAttributeMapper ruleAttributes,
ConfiguredAttributeMapper attributeMap,
BuildConfigurationValue ruleConfig) {
for (AttributeDependencyKind dependencyKind : attributeDependencyKinds) {
Attribute attribute = dependencyKind.getAttribute();
if (!attribute.getCondition().apply(ruleAttributes)
if (!attribute.getCondition().apply(attributeMap)
// Not only is resolving CONFIG_SETTING_DEPS_ATTRIBUTE deps here wasteful, since the only
// place they're used is in ConfiguredTargetFunction.getConfigConditions, but it actually
// breaks trimming as shown by
Expand All @@ -604,36 +604,70 @@ private void resolveAttributes(
// part of an unchosen select() branch.
|| attribute.getName().equals(RuleClass.CONFIG_SETTING_DEPS_ATTRIBUTE)) {
continue;
} else if (attribute.isLateBound()) {
// Latebound attributes: compute their values here instead of from the rule / aspect attrs.
addLateBoundDefault(
resolveLateBoundDefault(rule, ruleAttributes, attribute, ruleConfig),
attribute.getType(),
(depLabel, ctx) -> outgoingLabels.put(dependencyKind, depLabel));
} else {
// Everything else: read the rule / aspect attribute settings.
AspectAwareAttributeMapper combinedAttributes =
new AspectAwareAttributeMapper(
rule,
ruleAttributes,
dependencyKind.getOwningAspect() != null
? ImmutableMap.of(attribute.getName(), attribute)
: ImmutableMap.of());
combinedAttributes.visitLabels(
attribute, (depLabel) -> outgoingLabels.put(dependencyKind, depLabel));
}

Type<?> type = attribute.getType();
if (type == BuildType.OUTPUT
|| type == BuildType.OUTPUT_LIST
|| type == BuildType.NODEP_LABEL
|| type == BuildType.NODEP_LABEL_LIST) {
// These types invoke visitLabels() so that they are reported in "bazel query" but do not
// create a dependency. Maybe it's better to remove that, but then the labels() query
// function would need to be rethought.
continue;
}

resolveAttribute(
attribute, type, dependencyKind, outgoingLabels, rule, attributeMap, ruleConfig);
}
}

private <T> void addLateBoundDefault(
@Nullable Object value, Type<T> type, LabelVisitor labelVisitor) {
if (value != null) {
type.visitLabels(labelVisitor, type.cast(value), /*context=*/ null);
private <T> void resolveAttribute(
Attribute attribute,
Type<T> type,
AttributeDependencyKind dependencyKind,
OrderedSetMultimap<DependencyKind, Label> outgoingLabels,
Rule rule,
ConfiguredAttributeMapper attributeMap,
BuildConfigurationValue ruleConfig) {
T attributeValue = null;
if (attribute.isImplicit()) {
// Since the attributes that come from aspects do not appear in attributeMap, we have to get
// their values from somewhere else. This incidentally means that aspects attributes are not
// configurable. It would be nice if that wasn't the case, but we'd have to revamp how
// attribute mapping works, which is a large chunk of work.
if (dependencyKind.getOwningAspect() == null) {
attributeValue = attributeMap.get(attribute.getName(), type);
} else {
Object defaultValue = attribute.getDefaultValue(rule);
attributeValue =
type.cast(
defaultValue instanceof ComputedDefault
? ((ComputedDefault) defaultValue).getDefault(attributeMap)
: defaultValue);
}
} else if (attribute.isLateBound()) {
attributeValue =
type.cast(resolveLateBoundDefault(rule, attributeMap, attribute, ruleConfig));
} else if (attributeMap.has(attribute.getName())) {
// This condition is false for aspect attributes that do not give rise to dependencies because
// attributes that come from aspects do not appear in attributeMap (see the comment in the
// case that handles implicit attributes).
attributeValue = attributeMap.get(attribute.getName(), type);
}

if (attributeValue == null) {
return;
}

type.visitLabels(
(depLabel, ctx) -> outgoingLabels.put(dependencyKind, depLabel),
attributeValue,
/*context=*/ null);
}

@VisibleForTesting(/* used to test LateBoundDefaults' default values */ )
static <FragmentT> Object resolveLateBoundDefault(
public static <FragmentT> Object resolveLateBoundDefault(
Rule rule,
AttributeMap attributeMap,
Attribute attribute,
Expand Down
4 changes: 1 addition & 3 deletions src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -209,9 +209,7 @@ private RuleContext(
this.ruleClassProvider = builder.ruleClassProvider;
this.targetMap = targetMap;
this.configConditions = builder.configConditions.asProviders();
this.attributes =
new AspectAwareAttributeMapper(
rule, (ConfiguredAttributeMapper) attributes, builder.aspectAttributes);
this.attributes = new AspectAwareAttributeMapper(attributes, builder.aspectAttributes);
Set<String> allEnabledFeatures = new HashSet<>();
Set<String> allDisabledFeatures = new HashSet<>();
getAllFeatures(allEnabledFeatures, allDisabledFeatures);
Expand Down
Loading

0 comments on commit 45ce545

Please sign in to comment.