diff options
23 files changed, 459 insertions, 117 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/AspectWithParameters.java b/src/main/java/com/google/devtools/build/lib/analysis/AspectWithParameters.java new file mode 100644 index 0000000000..0cffc4c388 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/analysis/AspectWithParameters.java @@ -0,0 +1,79 @@ +// Copyright 2015 Google Inc. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +package com.google.devtools.build.lib.analysis; + +import com.google.devtools.build.lib.packages.AspectParameters; + +import java.util.Objects; + +/** + * Wrapper around {@link ConfiguredAspectFactory} class and {@link AspectParameters}. Aspects are + * created with help of aspect factory instances and parameters are used to configure them, so we + * have to keep them together. + */ +public final class AspectWithParameters { + // TODO(bazel-team): class objects are not really hashable or comparable for equality other than + // by reference. We should identify the aspect here in a way that does not rely on comparison + // by reference so that keys can be serialized and deserialized properly. + private final Class<? extends ConfiguredAspectFactory> aspectFactory; + private final AspectParameters parameters; + + public AspectWithParameters( + Class<? extends ConfiguredAspectFactory> aspect, AspectParameters parameters) { + this.aspectFactory = aspect; + this.parameters = parameters; + } + + public AspectWithParameters(Class<? extends ConfiguredAspectFactory> aspect) { + this.aspectFactory = aspect; + this.parameters = AspectParameters.EMPTY; + } + + /** + * Returns the aspectFactory required for building the aspect. + */ + public Class<? extends ConfiguredAspectFactory> getAspectFactory() { + return aspectFactory; + } + + /** + * Returns parameters for evaluation of the aspect. + */ + public AspectParameters getParameters() { + return parameters; + } + + @Override + public boolean equals(Object other) { + if (this == other) { + return true; + } + if (!(other instanceof AspectWithParameters)) { + return false; + } + AspectWithParameters that = (AspectWithParameters) other; + return Objects.equals(this.aspectFactory, that.aspectFactory) + && Objects.equals(this.parameters, that.parameters); + } + + @Override + public int hashCode() { + return Objects.hash(aspectFactory, parameters); + } + + @Override + public String toString() { + return String.format("AspectWithParameters %s(%s)", aspectFactory, parameters); + } +} diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java index b27c2f362f..a7fff98a90 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java @@ -877,8 +877,8 @@ public class BuildView { TargetAndConfiguration ctNode = new TargetAndConfiguration(target); ListMultimap<Attribute, Dependency> depNodeNames; try { - depNodeNames = resolver.dependentNodeMap(ctNode, configurations.getHostConfiguration(), null, - getConfigurableAttributeKeys(ctNode)); + depNodeNames = resolver.dependentNodeMap(ctNode, configurations.getHostConfiguration(), + /*aspect=*/null, /*aspectParameters=*/null, getConfigurableAttributeKeys(ctNode)); } catch (EvalException e) { throw new IllegalStateException(e); } diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java index f193298c3e..9cce2f37a9 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java @@ -31,6 +31,7 @@ import com.google.devtools.build.lib.collect.nestedset.Order; import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe; import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.events.EventHandler; +import com.google.devtools.build.lib.packages.AspectParameters; import com.google.devtools.build.lib.packages.Attribute; import com.google.devtools.build.lib.packages.ConstantRuleVisibility; import com.google.devtools.build.lib.packages.InputFile; @@ -280,8 +281,10 @@ public final class ConfiguredTargetFactory { public Aspect createAspect( AnalysisEnvironment env, RuleConfiguredTarget associatedTarget, ConfiguredAspectFactory aspectFactory, + AspectParameters aspectParameters, ListMultimap<Attribute, ConfiguredTarget> prerequisiteMap, - Set<ConfigMatchingProvider> configConditions, BuildConfiguration hostConfiguration) { + Set<ConfigMatchingProvider> configConditions, + BuildConfiguration hostConfiguration) { RuleContext.Builder builder = new RuleContext.Builder(env, associatedTarget.getTarget(), associatedTarget.getConfiguration(), @@ -297,7 +300,7 @@ public final class ConfiguredTargetFactory { return null; } - return aspectFactory.create(associatedTarget, ruleContext); + return aspectFactory.create(associatedTarget, ruleContext, aspectParameters); } /** diff --git a/src/main/java/com/google/devtools/build/lib/analysis/DependencyResolver.java b/src/main/java/com/google/devtools/build/lib/analysis/DependencyResolver.java index 9b6ea93ef7..f8ec20a445 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/DependencyResolver.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/DependencyResolver.java @@ -26,6 +26,7 @@ import com.google.devtools.build.lib.analysis.config.ConfigMatchingProvider; import com.google.devtools.build.lib.collect.ImmutableSortedKeyListMultimap; import com.google.devtools.build.lib.packages.AspectDefinition; import com.google.devtools.build.lib.packages.AspectFactory; +import com.google.devtools.build.lib.packages.AspectParameters; import com.google.devtools.build.lib.packages.Attribute; import com.google.devtools.build.lib.packages.Attribute.LateBoundDefault; import com.google.devtools.build.lib.packages.Attribute.SplitTransition; @@ -100,7 +101,7 @@ public abstract class DependencyResolver { private final Attribute.Transition transition; private final boolean hasStaticConfiguration; - private final ImmutableSet<Class<? extends ConfiguredAspectFactory>> aspects; + private final ImmutableSet<AspectWithParameters> aspects; /** * Constructs a Dependency with a given configuration (suitable for static configuration @@ -108,7 +109,7 @@ public abstract class DependencyResolver { */ public Dependency(Label label, @Nullable BuildConfiguration configuration, - ImmutableSet<Class<? extends ConfiguredAspectFactory>> aspects) { + ImmutableSet<AspectWithParameters> aspects) { this.label = Preconditions.checkNotNull(label); this.configuration = configuration; this.transition = null; @@ -121,14 +122,14 @@ public abstract class DependencyResolver { * builds). */ public Dependency(Label label, @Nullable BuildConfiguration configuration) { - this(label, configuration, ImmutableSet.<Class<? extends ConfiguredAspectFactory>>of()); + this(label, configuration, ImmutableSet.<AspectWithParameters>of()); } /** * Constructs a Dependency with a given transition (suitable for dynamic configuration builds). */ public Dependency(Label label, Attribute.Transition transition, - ImmutableSet<Class<? extends ConfiguredAspectFactory>> aspects) { + ImmutableSet<AspectWithParameters> aspects) { this.label = Preconditions.checkNotNull(label); this.configuration = null; this.transition = Preconditions.checkNotNull(transition); @@ -165,7 +166,7 @@ public abstract class DependencyResolver { return transition; } - public ImmutableSet<Class<? extends ConfiguredAspectFactory>> getAspects() { + public ImmutableSet<AspectWithParameters> getAspects() { return aspects; } @@ -217,7 +218,7 @@ public abstract class DependencyResolver { */ public final ListMultimap<Attribute, Dependency> dependentNodeMap( TargetAndConfiguration node, BuildConfiguration hostConfig, AspectDefinition aspect, - Set<ConfigMatchingProvider> configConditions) + AspectParameters aspectParameters, Set<ConfigMatchingProvider> configConditions) throws EvalException { Target target = node.getTarget(); BuildConfiguration config = node.getConfiguration(); @@ -237,7 +238,7 @@ public abstract class DependencyResolver { Rule rule = (Rule) target; ListMultimap<Attribute, LabelAndConfiguration> labelMap = resolveAttributes(rule, aspect, config, hostConfig, configConditions); - visitRule(rule, aspect, labelMap, outgoingEdges); + visitRule(rule, aspect, aspectParameters, labelMap, outgoingEdges); } else if (target instanceof PackageGroup) { visitPackageGroup(node, (PackageGroup) target, outgoingEdges.get(null)); } else { @@ -502,8 +503,8 @@ public abstract class DependencyResolver { TargetAndConfiguration node, BuildConfiguration hostConfig, Set<ConfigMatchingProvider> configConditions) { try { - return ImmutableSet.copyOf( - dependentNodeMap(node, hostConfig, null, configConditions).values()); + return ImmutableSet.copyOf(dependentNodeMap(node, hostConfig, /*aspect=*/null, + /*aspectParameters=*/null, configConditions).values()); } catch (EvalException e) { throw new IllegalStateException(e); } @@ -516,12 +517,12 @@ public abstract class DependencyResolver { * @throws IllegalArgumentException if the {@code node} does not refer to a {@link Rule} instance */ public final Collection<Dependency> resolveRuleLabels( - TargetAndConfiguration node, AspectDefinition aspect, ListMultimap<Attribute, + TargetAndConfiguration node, ListMultimap<Attribute, LabelAndConfiguration> labelMap) { Preconditions.checkArgument(node.getTarget() instanceof Rule); Rule rule = (Rule) node.getTarget(); ListMultimap<Attribute, Dependency> outgoingEdges = ArrayListMultimap.create(); - visitRule(rule, aspect, labelMap, outgoingEdges); + visitRule(rule, labelMap, outgoingEdges); return outgoingEdges.values(); } @@ -548,39 +549,59 @@ public abstract class DependencyResolver { } } - private ImmutableSet<Class<? extends ConfiguredAspectFactory>> requiredAspects( - AspectDefinition aspect, Attribute attribute, Target target) { + private ImmutableSet<AspectWithParameters> requiredAspects(AspectDefinition aspectDefinition, + AspectParameters aspectParameters, Attribute attribute, Target target, Rule originalRule) { if (!(target instanceof Rule)) { return ImmutableSet.of(); } + Set<AspectWithParameters> aspectCandidates = + extractAspectCandidates(aspectDefinition, aspectParameters, attribute, originalRule); RuleClass ruleClass = ((Rule) target).getRuleClassObject(); + ImmutableSet.Builder<AspectWithParameters> result = ImmutableSet.builder(); + for (AspectWithParameters candidateClass : aspectCandidates) { + ConfiguredAspectFactory candidate = + (ConfiguredAspectFactory) AspectFactory.Util.create(candidateClass.getAspectFactory()); + if (Sets.difference( + candidate.getDefinition().getRequiredProviders(), + ruleClass.getAdvertisedProviders()).isEmpty()) { + result.add(candidateClass); + } + } + return result.build(); + } + private static Set<AspectWithParameters> extractAspectCandidates( + AspectDefinition aspectDefinition, AspectParameters aspectParameters, + Attribute attribute, Rule originalRule) { // The order of this set will be deterministic. This is necessary because this order eventually // influences the order in which aspects are merged into the main configured target, which in // turn influences which aspect takes precedence if two emit the same provider (maybe this // should be an error) - Set<Class<? extends AspectFactory<?, ?, ?>>> aspectCandidates = new LinkedHashSet<>(); - aspectCandidates.addAll(attribute.getAspects()); - if (aspect != null) { - aspectCandidates.addAll(aspect.getAttributeAspects().get(attribute.getName())); + Set<AspectWithParameters> aspectCandidates = new LinkedHashSet<>(); + for (Map.Entry<Class<? extends AspectFactory<?, ?, ?>>, AspectParameters> aspectWithParameters : + attribute.getAspectsWithParameters(originalRule).entrySet()) { + aspectCandidates.add(new AspectWithParameters( + aspectWithParameters.getKey().asSubclass(ConfiguredAspectFactory.class), + aspectWithParameters.getValue())); } - - ImmutableSet.Builder<Class<? extends ConfiguredAspectFactory>> result = ImmutableSet.builder(); - for (Class<? extends AspectFactory<?, ?, ?>> candidateClass : aspectCandidates) { - ConfiguredAspectFactory candidate = - (ConfiguredAspectFactory) AspectFactory.Util.create(candidateClass); - if (Sets.difference( - candidate.getDefinition().getRequiredProviders(), - ruleClass.getAdvertisedProviders()).isEmpty()) { - result.add(candidateClass.asSubclass(ConfiguredAspectFactory.class)); + if (aspectDefinition != null) { + for (Class<? extends AspectFactory<?, ?, ?>> aspect : + aspectDefinition.getAttributeAspects().get(attribute.getName())) { + aspectCandidates.add(new AspectWithParameters( + aspect.asSubclass(ConfiguredAspectFactory.class), + aspectParameters)); } } + return aspectCandidates; + } - return result.build(); + private void visitRule(Rule rule, ListMultimap<Attribute, LabelAndConfiguration> labelMap, + ListMultimap<Attribute, Dependency> outgoingEdges) { + visitRule(rule, /*aspect=*/null, /*aspectParameters=*/null, labelMap, outgoingEdges); } - private void visitRule(Rule rule, AspectDefinition aspect, + private void visitRule(Rule rule, AspectDefinition aspect, AspectParameters aspectParameters, ListMultimap<Attribute, LabelAndConfiguration> labelMap, ListMultimap<Attribute, Dependency> outgoingEdges) { Preconditions.checkNotNull(labelMap); @@ -622,7 +643,7 @@ public abstract class DependencyResolver { config.evaluateTransition(rule, attribute, toTarget, transitionApplier); } for (Dependency dependency : transitionApplier.getDependencies(label, - requiredAspects(aspect, attribute, toTarget))) { + requiredAspects(aspect, aspectParameters, attribute, toTarget, rule))) { outgoingEdges.put( entry.getKey(), dependency); diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java index 29b5f4be1d..485786fc53 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java @@ -32,8 +32,8 @@ import com.google.common.collect.MutableClassToInstanceMap; import com.google.devtools.build.lib.actions.ArtifactFactory; import com.google.devtools.build.lib.actions.PackageRootResolver; import com.google.devtools.build.lib.actions.Root; +import com.google.devtools.build.lib.analysis.AspectWithParameters; import com.google.devtools.build.lib.analysis.BlazeDirectories; -import com.google.devtools.build.lib.analysis.ConfiguredAspectFactory; import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider; import com.google.devtools.build.lib.analysis.DependencyResolver; import com.google.devtools.build.lib.analysis.ViewCreationFailedException; @@ -1397,7 +1397,7 @@ public final class BuildConfiguration { * TODO(bazel-team): this is a really ugly reverse dependency: factor this away. */ Iterable<DependencyResolver.Dependency> getDependencies(Label label, - ImmutableSet<Class<? extends ConfiguredAspectFactory>> aspects); + ImmutableSet<AspectWithParameters> aspects); } /** @@ -1471,7 +1471,7 @@ public final class BuildConfiguration { @Override public Iterable<DependencyResolver.Dependency> getDependencies(Label label, - ImmutableSet<Class<? extends ConfiguredAspectFactory>> aspects) { + ImmutableSet<AspectWithParameters> aspects) { return ImmutableList.of( new DependencyResolver.Dependency(label, currentConfiguration, aspects)); } @@ -1572,7 +1572,7 @@ public final class BuildConfiguration { @Override public Iterable<DependencyResolver.Dependency> getDependencies(Label label, - ImmutableSet<Class<? extends ConfiguredAspectFactory>> aspects) { + ImmutableSet<AspectWithParameters> aspects) { return ImmutableList.of(new DependencyResolver.Dependency(label, transition, aspects)); } } @@ -1639,7 +1639,7 @@ public final class BuildConfiguration { @Override public Iterable<DependencyResolver.Dependency> getDependencies(Label label, - ImmutableSet<Class<? extends ConfiguredAspectFactory>> aspects) { + ImmutableSet<AspectWithParameters> aspects) { ImmutableList.Builder<DependencyResolver.Dependency> builder = ImmutableList.builder(); for (TransitionApplier applier : appliers) { builder.addAll(applier.getDependencies(label, aspects)); diff --git a/src/main/java/com/google/devtools/build/lib/ideinfo/AndroidStudioInfoAspect.java b/src/main/java/com/google/devtools/build/lib/ideinfo/AndroidStudioInfoAspect.java index 6316f1ad94..8defc6638c 100644 --- a/src/main/java/com/google/devtools/build/lib/ideinfo/AndroidStudioInfoAspect.java +++ b/src/main/java/com/google/devtools/build/lib/ideinfo/AndroidStudioInfoAspect.java @@ -31,6 +31,7 @@ import com.google.devtools.build.lib.ideinfo.androidstudio.AndroidStudioIdeInfo. import com.google.devtools.build.lib.ideinfo.androidstudio.AndroidStudioIdeInfo.LibraryArtifact; import com.google.devtools.build.lib.ideinfo.androidstudio.AndroidStudioIdeInfo.RuleIdeInfo; import com.google.devtools.build.lib.packages.AspectDefinition; +import com.google.devtools.build.lib.packages.AspectParameters; import com.google.devtools.build.lib.packages.Rule; import com.google.devtools.build.lib.rules.java.JavaRuleOutputJarsProvider; import com.google.devtools.build.lib.rules.java.JavaSourceInfoProvider; @@ -58,7 +59,8 @@ public class AndroidStudioInfoAspect implements ConfiguredAspectFactory { } @Override - public Aspect create(ConfiguredTarget base, RuleContext ruleContext) { + public Aspect create(ConfiguredTarget base, RuleContext ruleContext, + AspectParameters parameters) { NestedSetBuilder<Artifact> ideBuildFilesBuilder = NestedSetBuilder.stableOrder(); Iterable<AndroidStudioInfoFilesProvider> deps = ruleContext.getPrerequisites("deps", Mode.TARGET, AndroidStudioInfoFilesProvider.class); diff --git a/src/main/java/com/google/devtools/build/lib/packages/AspectFactory.java b/src/main/java/com/google/devtools/build/lib/packages/AspectFactory.java index b375dd9b00..a92084bb6e 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/AspectFactory.java +++ b/src/main/java/com/google/devtools/build/lib/packages/AspectFactory.java @@ -26,8 +26,10 @@ public interface AspectFactory<TConfiguredTarget, TRuleContext, TAspect> { * @param base the configured target of the associated rule * @param context the context of the associated configured target plus all the attributes the * aspect itself has defined + * @param parameters information from attributes of the rule that have requested this + * aspect */ - TAspect create(TConfiguredTarget base, TRuleContext context); + TAspect create(TConfiguredTarget base, TRuleContext context, AspectParameters parameters); /** * Returns the definition of the aspect. diff --git a/src/main/java/com/google/devtools/build/lib/packages/AspectParameters.java b/src/main/java/com/google/devtools/build/lib/packages/AspectParameters.java new file mode 100644 index 0000000000..69f82ceca1 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/packages/AspectParameters.java @@ -0,0 +1,93 @@ +// Copyright 2015 Google Inc. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +package com.google.devtools.build.lib.packages; + +import com.google.common.collect.ArrayListMultimap; +import com.google.common.collect.ImmutableCollection; +import com.google.common.collect.ImmutableMultimap; +import com.google.common.collect.Multimap; + +import java.util.Objects; + +import javax.annotation.Nullable; + +/** + * Objects of this class contain values of some attributes of rules. Used for passing this + * information to the aspects. + */ +public final class AspectParameters { + private final ImmutableMultimap<String, String> attributes; + + private AspectParameters(Multimap<String, String> attributes) { + this.attributes = ImmutableMultimap.copyOf(attributes); + } + + public static final AspectParameters EMPTY = new AspectParameters.Builder().build(); + + /** + * A builder for @{link {@link AspectParameters} class. + */ + public static class Builder { + private final Multimap<String, String> attributes = ArrayListMultimap.create(); + + /** + * Adds a new pair of attribute-value. + */ + public Builder addAttribute(String name, String value) { + attributes.put(name, value); + return this; + } + + /** + * Creates a new instance of {@link AspectParameters} class. + */ + public AspectParameters build() { + return new AspectParameters(attributes); + } + } + + /** + * Returns collection of values for specified key, or null if key is missing. + */ + @Nullable + public ImmutableCollection<String> getAttribute(String key) { + return attributes.get(key); + } + + public boolean isEmpty() { + return this.equals(AspectParameters.EMPTY); + } + + @Override + public boolean equals(Object other) { + if (this == other) { + return true; + } + if (!(other instanceof AspectParameters)) { + return false; + } + AspectParameters that = (AspectParameters) other; + return Objects.equals(this.attributes, that.attributes); + } + + @Override + public int hashCode() { + return Objects.hashCode(attributes); + } + + @Override + public String toString() { + return attributes.toString(); + } +} diff --git a/src/main/java/com/google/devtools/build/lib/packages/Attribute.java b/src/main/java/com/google/devtools/build/lib/packages/Attribute.java index 7d06129e6b..391a1da7b6 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/Attribute.java +++ b/src/main/java/com/google/devtools/build/lib/packages/Attribute.java @@ -15,10 +15,12 @@ package com.google.devtools.build.lib.packages; import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Function; import com.google.common.base.Preconditions; import com.google.common.base.Predicate; import com.google.common.base.Predicates; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; import com.google.common.collect.Sets; @@ -57,6 +59,26 @@ public final class Attribute implements Comparable<Attribute> { public static final Predicate<RuleClass> NO_RULE = Predicates.alwaysFalse(); + private static final class RuleAspect { + private final Class<? extends AspectFactory<?, ?, ?>> aspectFactory; + private final Function<Rule, AspectParameters> parametersExtractor; + + RuleAspect( + Class<? extends AspectFactory<?, ?, ?>> aspectFactory, + Function<Rule, AspectParameters> parametersExtractor) { + this.aspectFactory = aspectFactory; + this.parametersExtractor = parametersExtractor; + } + + Class<? extends AspectFactory<?, ?, ?>> getAspectFactory() { + return aspectFactory; + } + + Function<Rule, AspectParameters> getParametersExtractor() { + return parametersExtractor; + } + } + /** * A configuration transition. */ @@ -274,7 +296,7 @@ public final class Attribute implements Comparable<Attribute> { private Set<PropertyFlag> propertyFlags = EnumSet.noneOf(PropertyFlag.class); private PredicateWithMessage<Object> allowedValues = null; private ImmutableSet<String> mandatoryProviders = ImmutableSet.<String>of(); - private Set<Class<? extends AspectFactory<?, ?, ?>>> aspects = new LinkedHashSet<>(); + private Set<RuleAspect> aspects = new LinkedHashSet<>(); /** * Creates an attribute builder with given name and type. This attribute is optional, uses @@ -641,13 +663,31 @@ public final class Attribute implements Comparable<Attribute> { } /** - * Asserts that a particular aspect needs to be computed for all direct dependencies through - * this attribute. + * Asserts that a particular aspect probably needs to be computed for all direct dependencies + * through this attribute. */ public Builder<TYPE> aspect(Class<? extends AspectFactory<?, ?, ?>> aspect) { - this.aspects.add(aspect); + Function<Rule, AspectParameters> noParameters = new Function<Rule, AspectParameters>() { + @Override + public AspectParameters apply(Rule input) { + return AspectParameters.EMPTY; + } + }; + return this.aspect(aspect, noParameters); + } + + /** + * Asserts that a particular parameterized aspect probably needs to be computed for all direct + * dependencies through this attribute. + * + * @param evaluator function that extracts aspect parameters from rule. + */ + public Builder<TYPE> aspect(Class<? extends AspectFactory<?, ?, ?>> aspect, + Function<Rule, AspectParameters> evaluator) { + this.aspects.add(new RuleAspect(aspect, evaluator)); return this; } + /** * Sets the predicate-like edge validity checker. */ @@ -1001,7 +1041,7 @@ public final class Attribute implements Comparable<Attribute> { private final ImmutableSet<String> mandatoryProviders; - private final ImmutableSet<Class<? extends AspectFactory<?, ?, ?>>> aspects; + private final ImmutableSet<RuleAspect> aspects; /** * Constructs a rule attribute with the specified name, type and default @@ -1028,7 +1068,7 @@ public final class Attribute implements Comparable<Attribute> { Predicate<AttributeMap> condition, PredicateWithMessage<Object> allowedValues, ImmutableSet<String> mandatoryProviders, - ImmutableSet<Class<? extends AspectFactory<?, ?, ?>>> aspects) { + ImmutableSet<RuleAspect> aspects) { Preconditions.checkNotNull(configTransition); Preconditions.checkArgument( (configTransition == ConfigurationTransition.NONE && configurator == null) @@ -1252,7 +1292,24 @@ public final class Attribute implements Comparable<Attribute> { * Returns the set of aspects required for dependencies through this attribute. */ public ImmutableSet<Class<? extends AspectFactory<?, ?, ?>>> getAspects() { - return aspects; + ImmutableSet.Builder<Class<? extends AspectFactory<?, ?, ?>>> builder = ImmutableSet.builder(); + for (RuleAspect aspect : aspects) { + builder.add(aspect.getAspectFactory()); + } + return builder.build(); + } + + /** + * Returns set of pairs of aspect factories and corresponding aspect parameters. + */ + public ImmutableMap<Class<? extends AspectFactory<?, ?, ?>>, AspectParameters> + getAspectsWithParameters(Rule rule) { + ImmutableMap.Builder<Class<? extends AspectFactory<?, ?, ?>>, AspectParameters> builder = + ImmutableMap.builder(); + for (RuleAspect aspect : aspects) { + builder.put(aspect.getAspectFactory(), aspect.getParametersExtractor().apply(rule)); + } + return builder.build(); } /** diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidNeverlinkAspect.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidNeverlinkAspect.java index 92207e8734..3179e45ab1 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidNeverlinkAspect.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidNeverlinkAspect.java @@ -21,6 +21,7 @@ import com.google.devtools.build.lib.analysis.RuleConfiguredTarget.Mode; import com.google.devtools.build.lib.analysis.RuleContext; import com.google.devtools.build.lib.analysis.TransitiveInfoCollection; import com.google.devtools.build.lib.packages.AspectDefinition; +import com.google.devtools.build.lib.packages.AspectParameters; import com.google.devtools.build.lib.packages.Type; import com.google.devtools.build.lib.rules.java.JavaCommon; import com.google.devtools.build.lib.rules.java.JavaCompilationArgsProvider; @@ -44,7 +45,8 @@ public class AndroidNeverlinkAspect implements ConfiguredAspectFactory { "deps", "exports", "runtime_deps", "binary_under_test", "$instrumentation_test_runner"); @Override - public Aspect create(ConfiguredTarget base, RuleContext ruleContext) { + public Aspect create(ConfiguredTarget base, RuleContext ruleContext, + AspectParameters parameters) { if (!JavaCommon.getConstraints(ruleContext).contains("android") && !ruleContext.getRule().getRuleClass().startsWith("android_")) { return new Aspect.Builder(NAME).build(); diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/J2ObjcAspect.java b/src/main/java/com/google/devtools/build/lib/rules/objc/J2ObjcAspect.java index 2a922eacde..130be53c92 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/J2ObjcAspect.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/J2ObjcAspect.java @@ -35,6 +35,7 @@ import com.google.devtools.build.lib.analysis.actions.SpawnAction; import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; import com.google.devtools.build.lib.packages.AspectDefinition; +import com.google.devtools.build.lib.packages.AspectParameters; import com.google.devtools.build.lib.packages.Type; import com.google.devtools.build.lib.rules.java.J2ObjcConfiguration; import com.google.devtools.build.lib.rules.java.JavaCompilationArgsProvider; @@ -88,7 +89,8 @@ public class J2ObjcAspect implements ConfiguredAspectFactory { } @Override - public Aspect create(ConfiguredTarget base, RuleContext ruleContext) { + public Aspect create(ConfiguredTarget base, RuleContext ruleContext, + AspectParameters parameters) { Aspect.Builder builder = new Aspect.Builder(NAME); JavaCompilationArgsProvider compilationArgsProvider = diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java index bcb6d3d688..eb3b934018 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java @@ -118,8 +118,8 @@ public final class AspectFunction implements SkyFunction { ListMultimap<Attribute, ConfiguredTarget> depValueMap = ConfiguredTargetFunction.computeDependencies(env, resolver, ctgValue, - aspectFactory.getDefinition(), configConditions, ruleClassProvider, - view.getHostConfiguration(ctgValue.getConfiguration())); + aspectFactory.getDefinition(), key.getParameters(), configConditions, + ruleClassProvider, view.getHostConfiguration(ctgValue.getConfiguration())); return createAspect(env, key, associatedTarget, configConditions, depValueMap); } catch (DependencyEvaluationException e) { @@ -143,8 +143,9 @@ public final class AspectFunction implements SkyFunction { ConfiguredAspectFactory aspectFactory = (ConfiguredAspectFactory) AspectFactory.Util.create(key.getAspect()); - Aspect aspect = view.createAspect(analysisEnvironment, associatedTarget, aspectFactory, - directDeps, configConditions); + Aspect aspect = view.createAspect( + analysisEnvironment, associatedTarget, aspectFactory, directDeps, configConditions, + key.getParameters()); events.replayOn(env.getListener()); if (events.hasErrors()) { diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/AspectValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/AspectValue.java index 2657caa438..f1260ba3c7 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/AspectValue.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/AspectValue.java @@ -15,15 +15,20 @@ package com.google.devtools.build.lib.skyframe; import com.google.common.base.Objects; +import com.google.common.base.Preconditions; import com.google.devtools.build.lib.actions.Action; import com.google.devtools.build.lib.analysis.Aspect; +import com.google.devtools.build.lib.analysis.AspectWithParameters; import com.google.devtools.build.lib.analysis.ConfiguredAspectFactory; import com.google.devtools.build.lib.analysis.config.BuildConfiguration; import com.google.devtools.build.lib.events.Location; +import com.google.devtools.build.lib.packages.AspectParameters; import com.google.devtools.build.lib.syntax.Label; import com.google.devtools.build.skyframe.SkyFunctionName; import com.google.devtools.build.skyframe.SkyKey; +import javax.annotation.Nullable; + /** * An aspect in the context of the Skyframe graph. */ @@ -34,16 +39,15 @@ public final class AspectValue extends ActionLookupValue { public static final class AspectKey extends ActionLookupKey { private final Label label; private final BuildConfiguration configuration; - // TODO(bazel-team): class objects are not really hashable or comparable for equality other than - // by reference. We should identify the aspect here in a way that does not rely on comparison - // by reference so that keys can be serialized and deserialized properly. - private final Class<? extends ConfiguredAspectFactory> aspectFactory; + private final AspectWithParameters aspect; private AspectKey(Label label, BuildConfiguration configuration, - Class<? extends ConfiguredAspectFactory> aspectFactory) { + Class<? extends ConfiguredAspectFactory> aspectFactory, + AspectParameters parameters) { + Preconditions.checkNotNull(parameters); this.label = label; this.configuration = configuration; - this.aspectFactory = aspectFactory; + this.aspect = new AspectWithParameters(aspectFactory, parameters); } @Override @@ -56,7 +60,12 @@ public final class AspectValue extends ActionLookupValue { } public Class<? extends ConfiguredAspectFactory> getAspect() { - return aspectFactory; + return aspect.getAspectFactory(); + } + + @Nullable + public AspectParameters getParameters() { + return aspect.getParameters(); } @Override @@ -66,7 +75,7 @@ public final class AspectValue extends ActionLookupValue { @Override public int hashCode() { - return Objects.hashCode(label, configuration, aspectFactory); + return Objects.hashCode(label, configuration, aspect); } @Override @@ -82,13 +91,14 @@ public final class AspectValue extends ActionLookupValue { AspectKey that = (AspectKey) other; return Objects.equal(label, that.label) && Objects.equal(configuration, that.configuration) - && Objects.equal(aspectFactory, that.aspectFactory); + && Objects.equal(aspect, that.aspect); } @Override public String toString() { - return label + "#" + aspectFactory.getSimpleName() + " " - + (configuration == null ? "null" : configuration.checksum()); + return label + "#" + aspect.getAspectFactory().getSimpleName() + " " + + (configuration == null ? "null" : configuration.checksum()) + " " + + aspect.getParameters(); } } @@ -123,8 +133,10 @@ public final class AspectValue extends ActionLookupValue { } public static SkyKey key(Label label, BuildConfiguration configuration, - Class<? extends ConfiguredAspectFactory> aspectFactory) { - return new SkyKey(SkyFunctions.ASPECT, new AspectKey(label, configuration, aspectFactory)); + Class<? extends ConfiguredAspectFactory> aspectFactory, + AspectParameters additionalConfiguration) { + return new SkyKey(SkyFunctions.ASPECT, + new AspectKey(label, configuration, aspectFactory, additionalConfiguration)); } public static SkyKey key(AspectKey aspectKey) { @@ -135,6 +147,6 @@ public final class AspectValue extends ActionLookupValue { Label label, BuildConfiguration configuration, Class<? extends ConfiguredAspectFactory> aspectFactory) { - return new AspectKey(label, configuration, aspectFactory); + return new AspectKey(label, configuration, aspectFactory, AspectParameters.EMPTY); } } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java index 5df28bdedb..b565f1e900 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java @@ -27,6 +27,7 @@ import com.google.devtools.build.lib.actions.Actions; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException; import com.google.devtools.build.lib.analysis.Aspect; +import com.google.devtools.build.lib.analysis.AspectWithParameters; import com.google.devtools.build.lib.analysis.CachingAnalysisEnvironment; import com.google.devtools.build.lib.analysis.ConfiguredAspectFactory; import com.google.devtools.build.lib.analysis.ConfiguredTarget; @@ -45,6 +46,7 @@ import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.events.StoredEventHandler; import com.google.devtools.build.lib.packages.AspectDefinition; import com.google.devtools.build.lib.packages.AspectFactory; +import com.google.devtools.build.lib.packages.AspectParameters; import com.google.devtools.build.lib.packages.Attribute; import com.google.devtools.build.lib.packages.InputFile; import com.google.devtools.build.lib.packages.NoSuchPackageException; @@ -181,7 +183,7 @@ final class ConfiguredTargetFunction implements SkyFunction { } ListMultimap<Attribute, ConfiguredTarget> depValueMap = computeDependencies(env, resolver, - ctgValue, null, configConditions, ruleClassProvider, + ctgValue, null, AspectParameters.EMPTY, configConditions, ruleClassProvider, view.getHostConfiguration(configuration)); ConfiguredTargetValue ans = createConfiguredTarget( view, env, target, configuration, depValueMap, configConditions); @@ -202,7 +204,8 @@ final class ConfiguredTargetFunction implements SkyFunction { * @param resolver The dependency resolver * @param ctgValue The label and the configuration of the node * @param aspectDefinition the aspect of the node (if null, the node is a configured target, - * otherwise it's an asect) + * otherwise it's an aspect) + * @param aspectParameters additional parameters for aspect construction * @param configConditions the configuration conditions for evaluating the attributes of the node * @param ruleClassProvider rule class provider for determining the right configuration fragments * to apply to deps @@ -215,15 +218,16 @@ final class ConfiguredTargetFunction implements SkyFunction { @Nullable static ListMultimap<Attribute, ConfiguredTarget> computeDependencies( Environment env, SkyframeDependencyResolver resolver, TargetAndConfiguration ctgValue, - AspectDefinition aspectDefinition, Set<ConfigMatchingProvider> configConditions, - RuleClassProvider ruleClassProvider, BuildConfiguration hostConfiguration) + AspectDefinition aspectDefinition, AspectParameters aspectParameters, + Set<ConfigMatchingProvider> configConditions, RuleClassProvider ruleClassProvider, + BuildConfiguration hostConfiguration) throws DependencyEvaluationException { // Create the map from attributes to list of (target, configuration) pairs. ListMultimap<Attribute, Dependency> depValueNames; try { depValueNames = resolver.dependentNodeMap(ctgValue, hostConfiguration, aspectDefinition, - configConditions); + aspectParameters, configConditions); } catch (EvalException e) { env.getListener().handle(Event.error(e.getLocation(), e.getMessage())); throw new DependencyEvaluationException(new ConfiguredValueCreationException(e.print())); @@ -479,8 +483,8 @@ final class ConfiguredTargetFunction implements SkyFunction { ListMultimap<SkyKey, Aspect> result = ArrayListMultimap.create(); Set<SkyKey> aspectKeys = new HashSet<>(); for (Dependency dep : deps) { - for (Class<? extends ConfiguredAspectFactory> depAspect : dep.getAspects()) { - aspectKeys.add(AspectValue.key(dep.getLabel(), dep.getConfiguration(), depAspect)); + for (AspectWithParameters depAspect : dep.getAspects()) { + aspectKeys.add(createAspectKey(dep.getLabel(), dep.getConfiguration(), depAspect)); } } @@ -497,12 +501,12 @@ final class ConfiguredTargetFunction implements SkyFunction { continue; } ConfiguredTarget depConfiguredTarget = configuredTargetMap.get(depKey); - for (Class<? extends ConfiguredAspectFactory> depAspect : dep.getAspects()) { - if (!aspectMatchesConfiguredTarget(depConfiguredTarget, depAspect)) { + for (AspectWithParameters depAspect : dep.getAspects()) { + if (!aspectMatchesConfiguredTarget(depConfiguredTarget, depAspect.getAspectFactory())) { continue; } - SkyKey aspectKey = AspectValue.key(dep.getLabel(), dep.getConfiguration(), depAspect); + SkyKey aspectKey = createAspectKey(dep.getLabel(), dep.getConfiguration(), depAspect); AspectValue aspectValue = null; try { aspectValue = (AspectValue) depAspects.get(aspectKey).get(); @@ -510,7 +514,8 @@ final class ConfiguredTargetFunction implements SkyFunction { // The configured target should have been created in resolveConfiguredTargetDependencies() throw new IllegalStateException(e); } catch (NoSuchThingException | AspectCreationException e) { - AspectFactory<?, ?, ?> depAspectFactory = AspectFactory.Util.create(depAspect); + AspectFactory<?, ?, ?> depAspectFactory = + AspectFactory.Util.create(depAspect.getAspectFactory()); throw new DependencyEvaluationException(new ConfiguredValueCreationException( String.format("Evaluation of aspect %s on %s failed: %s", depAspectFactory.getDefinition().getName(), dep.getLabel(), e.toString()))); @@ -523,10 +528,17 @@ final class ConfiguredTargetFunction implements SkyFunction { result.put(depKey, aspectValue.getAspect()); } } - return result; } + public static SkyKey createAspectKey(Label label, BuildConfiguration buildConfiguration, + AspectWithParameters depAspect) { + return AspectValue.key(label, + buildConfiguration, + depAspect.getAspectFactory(), + depAspect.getParameters()); + } + private static boolean aspectMatchesConfiguredTarget(ConfiguredTarget dep, Class<? extends ConfiguredAspectFactory> aspectFactory) { AspectDefinition aspectDefinition = AspectFactory.Util.create(aspectFactory).getDefinition(); @@ -573,8 +585,7 @@ final class ConfiguredTargetFunction implements SkyFunction { // Collect the corresponding Skyframe configured target values. Abort early if they haven't // been computed yet. - Collection<Dependency> configValueNames = - resolver.resolveRuleLabels(ctgValue, null, configLabelMap); + Collection<Dependency> configValueNames = resolver.resolveRuleLabels(ctgValue, configLabelMap); // No need to get new configs from Skyframe - config_setting rules always use the current // target's config. diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PostConfiguredTargetFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/PostConfiguredTargetFunction.java index 5f5d538956..d14bf46aa0 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/PostConfiguredTargetFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/PostConfiguredTargetFunction.java @@ -100,7 +100,8 @@ public class PostConfiguredTargetFunction implements SkyFunction { try { BuildConfiguration hostConfiguration = buildViewProvider.getSkyframeBuildView().getHostConfiguration(ct.getConfiguration()); - deps = resolver.dependentNodeMap(ctgValue, hostConfiguration, null, configConditions); + deps = resolver.dependentNodeMap(ctgValue, hostConfiguration, /*aspect=*/null, + /*aspectParameters=*/null, configConditions); if (ct.getConfiguration() != null && ct.getConfiguration().useDynamicConfigurations()) { deps = ConfiguredTargetFunction.trimConfigurations(env, ctgValue, deps, hostConfiguration, ruleClassProvider); diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java index 1eecdb71f1..0e6e4197ff 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java @@ -48,6 +48,7 @@ import com.google.devtools.build.lib.analysis.config.BuildConfiguration; import com.google.devtools.build.lib.analysis.config.ConfigMatchingProvider; import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.events.EventHandler; +import com.google.devtools.build.lib.packages.AspectParameters; import com.google.devtools.build.lib.packages.Attribute; import com.google.devtools.build.lib.packages.RuleClassProvider; import com.google.devtools.build.lib.packages.Target; @@ -459,9 +460,11 @@ public final class SkyframeBuildView { AnalysisEnvironment env, RuleConfiguredTarget associatedTarget, ConfiguredAspectFactory aspectFactory, ListMultimap<Attribute, ConfiguredTarget> prerequisiteMap, - Set<ConfigMatchingProvider> configConditions) { - return factory.createAspect(env, associatedTarget, aspectFactory, prerequisiteMap, - configConditions, getHostConfiguration(associatedTarget.getConfiguration())); + Set<ConfigMatchingProvider> configConditions, + AspectParameters aspectParameters) { + return factory.createAspect(env, associatedTarget, aspectFactory, aspectParameters, + prerequisiteMap, configConditions, + getHostConfiguration(associatedTarget.getConfiguration())); } @Nullable diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java index e5c6cf1497..22bd506670 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java @@ -46,9 +46,9 @@ import com.google.devtools.build.lib.actions.PackageRootResolutionException; import com.google.devtools.build.lib.actions.ResourceManager; import com.google.devtools.build.lib.actions.Root; import com.google.devtools.build.lib.analysis.Aspect; +import com.google.devtools.build.lib.analysis.AspectWithParameters; import com.google.devtools.build.lib.analysis.BlazeDirectories; import com.google.devtools.build.lib.analysis.BuildView.Options; -import com.google.devtools.build.lib.analysis.ConfiguredAspectFactory; import com.google.devtools.build.lib.analysis.ConfiguredTarget; import com.google.devtools.build.lib.analysis.DependencyResolver.Dependency; import com.google.devtools.build.lib.analysis.RuleConfiguredTarget; @@ -1110,8 +1110,9 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory { final List<SkyKey> skyKeys = new ArrayList<>(); for (Dependency key : keys) { skyKeys.add(ConfiguredTargetValue.key(key.getLabel(), configs.get(key))); - for (Class<? extends ConfiguredAspectFactory> aspect : key.getAspects()) { - skyKeys.add(AspectValue.key(key.getLabel(), configs.get(key), aspect)); + for (AspectWithParameters aspect : key.getAspects()) { + skyKeys.add( + ConfiguredTargetFunction.createAspectKey(key.getLabel(), configs.get(key), aspect)); } } @@ -1130,8 +1131,9 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory { ((ConfiguredTargetValue) result.get(configuredTargetKey)).getConfiguredTarget(); List<Aspect> aspects = new ArrayList<>(); - for (Class<? extends ConfiguredAspectFactory> aspect : key.getAspects()) { - SkyKey aspectKey = AspectValue.key(key.getLabel(), configs.get(key), aspect); + for (AspectWithParameters aspect : key.getAspects()) { + SkyKey aspectKey = + ConfiguredTargetFunction.createAspectKey(key.getLabel(), configs.get(key), aspect); if (result.get(aspectKey) == null) { continue DependentNodeLoop; } diff --git a/src/test/java/com/google/devtools/build/lib/analysis/AspectDefinitionTest.java b/src/test/java/com/google/devtools/build/lib/analysis/AspectDefinitionTest.java index b4b055aee0..461738eb7f 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/AspectDefinitionTest.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/AspectDefinitionTest.java @@ -17,6 +17,7 @@ import static com.google.devtools.build.lib.packages.Attribute.attr; import static org.junit.Assert.fail; import com.google.devtools.build.lib.packages.AspectDefinition; +import com.google.devtools.build.lib.packages.AspectParameters; import com.google.devtools.build.lib.packages.Type; import com.google.devtools.build.lib.syntax.Label; @@ -46,7 +47,7 @@ public class AspectDefinitionTest { } @Override - public Aspect create(ConfiguredTarget base, RuleContext context) { + public Aspect create(ConfiguredTarget base, RuleContext context, AspectParameters parameters) { throw new IllegalStateException(); } diff --git a/src/test/java/com/google/devtools/build/lib/analysis/AspectTest.java b/src/test/java/com/google/devtools/build/lib/analysis/AspectTest.java index 7d17a98cb5..02b4f3e1dc 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/AspectTest.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/AspectTest.java @@ -137,4 +137,18 @@ public class AspectTest extends AnalysisTestCase { assertThat(a.getProvider(TestAspects.RuleInfo.class).getData()) .containsExactly("aspect //a:b", "rule //a:a"); } + + @Test + public void informationFromBaseRulePassedToAspect() throws Exception { + setRules(new TestAspects.BaseRule(), new TestAspects.HonestRule(), + new TestAspects.AspectRequiringProviderRule()); + + pkg("a", + "aspect_requiring_provider(name='a', foo=[':b'], baz='hello')", + "honest(name='b', foo=[])"); + + ConfiguredTarget a = getConfiguredTarget("//a:a"); + assertThat(a.getProvider(TestAspects.RuleInfo.class).getData()) + .containsExactly("rule //a:a", "aspect //a:b data hello"); + } } diff --git a/src/test/java/com/google/devtools/build/lib/analysis/AspectValueTest.java b/src/test/java/com/google/devtools/build/lib/analysis/AspectValueTest.java index 5365728bea..2906961e7b 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/AspectValueTest.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/AspectValueTest.java @@ -17,6 +17,7 @@ import com.google.common.testing.EqualsTester; import com.google.devtools.build.lib.analysis.config.BuildConfiguration; import com.google.devtools.build.lib.analysis.util.AnalysisTestCase; import com.google.devtools.build.lib.analysis.util.TestAspects; +import com.google.devtools.build.lib.packages.AspectParameters; import com.google.devtools.build.lib.skyframe.AspectValue; import com.google.devtools.build.lib.syntax.Label; @@ -51,18 +52,26 @@ public class AspectValueTest extends AnalysisTestCase { Label l1 = Label.parseAbsolute("//a:l1"); Label l1b = Label.parseAbsolute("//a:l1"); Label l2 = Label.parseAbsolute("//a:l2"); + AspectParameters i1 = new AspectParameters.Builder() + .addAttribute("foo", "bar") + .build(); + AspectParameters i2 = new AspectParameters.Builder() + .addAttribute("foo", "baz") + .build(); Class<? extends ConfiguredAspectFactory> a1 = TestAspects.AttributeAspect.class; Class<? extends ConfiguredAspectFactory> a2 = TestAspects.ExtraAttributeAspect.class; new EqualsTester() - .addEqualityGroup(AspectValue.key(l1, c1, a1), AspectValue.key(l1b, c1, a1)) - .addEqualityGroup(AspectValue.key(l2, c1, a1)) - .addEqualityGroup(AspectValue.key(l1, c2, a1)) - .addEqualityGroup(AspectValue.key(l2, c2, a1)) - .addEqualityGroup(AspectValue.key(l1, c1, a2)) - .addEqualityGroup(AspectValue.key(l2, c1, a2)) - .addEqualityGroup(AspectValue.key(l1, c2, a2)) - .addEqualityGroup(AspectValue.key(l2, c2, a2)) + .addEqualityGroup(AspectValue.key(l1, c1, a1, null), AspectValue.key(l1b, c1, a1, null)) + .addEqualityGroup(AspectValue.key(l1, c1, a1, i1)) + .addEqualityGroup(AspectValue.key(l1, c1, a1, i2)) + .addEqualityGroup(AspectValue.key(l2, c1, a1, null)) + .addEqualityGroup(AspectValue.key(l1, c2, a1, null)) + .addEqualityGroup(AspectValue.key(l2, c2, a1, null)) + .addEqualityGroup(AspectValue.key(l1, c1, a2, null)) + .addEqualityGroup(AspectValue.key(l2, c1, a2, null)) + .addEqualityGroup(AspectValue.key(l1, c2, a2, null)) + .addEqualityGroup(AspectValue.key(l2, c2, a2, null)) .addEqualityGroup(l1) // A random object .testEquals(); } diff --git a/src/test/java/com/google/devtools/build/lib/analysis/DependencyResolverTest.java b/src/test/java/com/google/devtools/build/lib/analysis/DependencyResolverTest.java index fc5b156dca..e149cb8693 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/DependencyResolverTest.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/DependencyResolverTest.java @@ -116,7 +116,7 @@ public class DependencyResolverTest extends AnalysisTestCase { new TargetAndConfiguration(target, getTargetConfiguration()), getHostConfiguration(), aspectDefinition, - ImmutableSet.<ConfigMatchingProvider>of()); + null, ImmutableSet.<ConfigMatchingProvider>of()); } @SafeVarargs @@ -199,15 +199,15 @@ public class DependencyResolverTest extends AnalysisTestCase { BuildConfiguration host = getHostConfiguration(); BuildConfiguration target = getTargetConfiguration(); - ImmutableSet<Class<? extends ConfiguredAspectFactory>> twoAspects = - ImmutableSet.<Class<? extends ConfiguredAspectFactory>>of( - TestAspects.SimpleAspect.class, TestAspects.AttributeAspect.class); - ImmutableSet<Class<? extends ConfiguredAspectFactory>> inverseAspects = - ImmutableSet.<Class<? extends ConfiguredAspectFactory>>of( - TestAspects.AttributeAspect.class, TestAspects.SimpleAspect.class); - ImmutableSet<Class<? extends ConfiguredAspectFactory>> differentAspects = - ImmutableSet.<Class<? extends ConfiguredAspectFactory>>of( - TestAspects.AttributeAspect.class, TestAspects.ErrorAspect.class); + ImmutableSet<AspectWithParameters> twoAspects = ImmutableSet.of( + new AspectWithParameters(TestAspects.SimpleAspect.class), + new AspectWithParameters(TestAspects.AttributeAspect.class)); + ImmutableSet<AspectWithParameters> inverseAspects = ImmutableSet.of( + new AspectWithParameters(TestAspects.AttributeAspect.class), + new AspectWithParameters(TestAspects.SimpleAspect.class)); + ImmutableSet<AspectWithParameters> differentAspects = ImmutableSet.of( + new AspectWithParameters(TestAspects.AttributeAspect.class), + new AspectWithParameters(TestAspects.ErrorAspect.class)); new EqualsTester() .addEqualityGroup( diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java index 7f7d05b052..7068278951 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java @@ -84,6 +84,7 @@ import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.events.EventHandler; import com.google.devtools.build.lib.events.StoredEventHandler; import com.google.devtools.build.lib.exec.ExecutionOptions; +import com.google.devtools.build.lib.packages.AspectParameters; import com.google.devtools.build.lib.packages.Attribute.ConfigurationTransition; import com.google.devtools.build.lib.packages.AttributeMap; import com.google.devtools.build.lib.packages.ConstantRuleVisibility; @@ -861,9 +862,9 @@ public abstract class BuildViewTestCase extends FoundationTestCase { * Gets a derived Artifact for testing in the subdirectory of the {@link * BuildConfiguration#getBinDirectory()} corresponding to the package of {@code owner}, * where the given artifact belongs to the given ConfiguredTarget together with the given Aspect. - * So to specify a file foo/foo.o owned by target //foo:foo with an apsect from FooAspect, + * So to specify a file foo/foo.o owned by target //foo:foo with an aspect from FooAspect, * {@code packageRelativePath} should just be "foo.o", and aspectOfOwner should be - * FooAspect.class. This method is necessary when an Apsect of the target, not the target itself, + * FooAspect.class. This method is necessary when an Aspect of the target, not the target itself, * is creating an Artifact. */ protected Artifact getBinArtifact(String packageRelativePath, ConfiguredTarget owner, @@ -871,7 +872,8 @@ public abstract class BuildViewTestCase extends FoundationTestCase { return getPackageRelativeDerivedArtifact(packageRelativePath, owner.getConfiguration().getBinDirectory(), (AspectValue.AspectKey) AspectValue.key( - owner.getLabel(), owner.getConfiguration(), creatingAspectFactory).argument()); + owner.getLabel(), owner.getConfiguration(), creatingAspectFactory, + AspectParameters.EMPTY).argument()); } /** @@ -930,7 +932,8 @@ public abstract class BuildViewTestCase extends FoundationTestCase { return getPackageRelativeDerivedArtifact(packageRelativePath, owner.getConfiguration().getGenfilesDirectory(), (AspectValue.AspectKey) AspectValue.key( - owner.getLabel(), owner.getConfiguration(), creatingAspectFactory).argument()); + owner.getLabel(), owner.getConfiguration(), creatingAspectFactory, + AspectParameters.EMPTY).argument()); } /** diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/TestAspects.java b/src/test/java/com/google/devtools/build/lib/analysis/util/TestAspects.java index bf564fb225..ffea8bcc9f 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/util/TestAspects.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/util/TestAspects.java @@ -22,6 +22,8 @@ import static com.google.devtools.build.lib.packages.Type.NODEP_LABEL_LIST; import static com.google.devtools.build.lib.packages.Type.STRING; import static com.google.devtools.build.lib.packages.Type.STRING_LIST; +import com.google.common.base.Function; +import com.google.common.collect.Iterables; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.analysis.Aspect; import com.google.devtools.build.lib.analysis.ConfiguredAspectFactory; @@ -39,6 +41,8 @@ import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; import com.google.devtools.build.lib.collect.nestedset.Order; import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; import com.google.devtools.build.lib.packages.AspectDefinition; +import com.google.devtools.build.lib.packages.AspectParameters; +import com.google.devtools.build.lib.packages.Rule; import com.google.devtools.build.lib.packages.RuleClass; import com.google.devtools.build.lib.packages.RuleClass.Builder; import com.google.devtools.build.lib.rules.RuleConfiguredTargetFactory; @@ -131,11 +135,16 @@ public class TestAspects { */ public abstract static class BaseAspect implements ConfiguredAspectFactory { @Override - public Aspect create(ConfiguredTarget base, RuleContext ruleContext) { + public Aspect create(ConfiguredTarget base, RuleContext ruleContext, + AspectParameters parameters) { + String information = parameters.isEmpty() + ? "" + : " data " + Iterables.getFirst(parameters.getAttribute("baz"), null); return new Aspect.Builder(getClass().getName()) .addProvider( AspectInfo.class, - new AspectInfo(collectAspectData("aspect " + ruleContext.getLabel(), ruleContext))) + new AspectInfo(collectAspectData("aspect " + ruleContext.getLabel() + information, + ruleContext))) .build(); } } @@ -215,7 +224,8 @@ public class TestAspects { */ public static class ErrorAspect implements ConfiguredAspectFactory { @Override - public Aspect create(ConfiguredTarget base, RuleContext ruleContext) { + public Aspect create(ConfiguredTarget base, RuleContext ruleContext, + AspectParameters parameters) { ruleContext.ruleError("Aspect error"); return null; } @@ -290,11 +300,25 @@ public class TestAspects { * A rule that defines an {@link AspectRequiringProvider} on one of its attributes. */ public static class AspectRequiringProviderRule implements RuleDefinition { + + private static final class TestAspectParametersExtractor implements + Function<Rule, AspectParameters> { + @Override + public AspectParameters apply(Rule rule) { + if (rule.isAttrDefined("baz", STRING)) { + return new AspectParameters.Builder().addAttribute("baz", + rule.getAttributeContainer().getAttr("baz").toString()).build(); + } + return AspectParameters.EMPTY; + } + } + @Override public RuleClass build(Builder builder, RuleDefinitionEnvironment environment) { return builder .add(attr("foo", LABEL_LIST).allowedFileTypes(FileTypeSet.ANY_FILE) - .aspect(AspectRequiringProvider.class)) + .aspect(AspectRequiringProvider.class, new TestAspectParametersExtractor())) + .add(attr("baz", STRING)) .build(); } |