Skip to content

Commit

Permalink
Aspect terminology update.
Browse files Browse the repository at this point in the history
Aspect => ConfiguredAspect
AspectWithParameters => Aspect

--
MOS_MIGRATED_REVID=107375211
  • Loading branch information
dslomov authored and damienmg committed Nov 10, 2015
1 parent c7d277f commit b487ac6
Show file tree
Hide file tree
Showing 31 changed files with 240 additions and 253 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,12 @@
* <p>Aspects are created alongside configured targets on request from dependents.
*/
@Immutable
public final class Aspect implements Iterable<TransitiveInfoProvider> {
public final class ConfiguredAspect implements Iterable<TransitiveInfoProvider> {
private final String name;
private final ImmutableMap<Class<? extends TransitiveInfoProvider>, TransitiveInfoProvider>
providers;

private Aspect(
private ConfiguredAspect(
String name,
ImmutableMap<Class<? extends TransitiveInfoProvider>, TransitiveInfoProvider> providers) {
this.name = name;
Expand Down Expand Up @@ -84,7 +84,7 @@ public UnmodifiableIterator<TransitiveInfoProvider> iterator() {
}

/**
* Builder for {@link Aspect}.
* Builder for {@link ConfiguredAspect}.
*/
public static class Builder {
private final Map<Class<? extends TransitiveInfoProvider>, TransitiveInfoProvider>
Expand Down Expand Up @@ -140,7 +140,7 @@ public Builder addSkylarkTransitiveInfo(String name, Object value, Location loc)
return this;
}

public Aspect build() {
public ConfiguredAspect build() {
if (!outputGroupBuilders.isEmpty()) {
ImmutableMap.Builder<String, NestedSet<Artifact>> outputGroups = ImmutableMap.builder();
for (Map.Entry<String, NestedSetBuilder<Artifact>> entry : outputGroupBuilders.entrySet()) {
Expand All @@ -159,7 +159,7 @@ public Aspect build() {
providers.put(SkylarkProviders.class, new SkylarkProviders(skylarkProvidersMap));
}

return new Aspect(name, ImmutableMap.copyOf(providers));
return new ConfiguredAspect(name, ImmutableMap.copyOf(providers));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,6 @@ public interface ConfiguredAspectFactory {
* @param parameters information from attributes of the rule that have requested this
* aspect
*/
Aspect create(ConfiguredTarget base, RuleContext context, AspectParameters parameters)
ConfiguredAspect create(ConfiguredTarget base, RuleContext context, AspectParameters parameters)
throws InterruptedException;
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@

package com.google.devtools.build.lib.analysis;

import com.google.devtools.build.lib.packages.AspectWithParameters;
import com.google.common.base.Joiner;
import com.google.common.base.Preconditions;
import com.google.common.collect.ListMultimap;
Expand All @@ -33,7 +32,7 @@
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.Aspect;
import com.google.devtools.build.lib.packages.Attribute;
import com.google.devtools.build.lib.packages.ConfigurationFragmentPolicy;
import com.google.devtools.build.lib.packages.ConfigurationFragmentPolicy.MissingFragmentPolicy;
Expand All @@ -56,7 +55,6 @@
import java.util.ArrayList;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;

import javax.annotation.Nullable;
Expand Down Expand Up @@ -284,36 +282,37 @@ private String missingFragmentError(
}

/**
* Constructs an {@link Aspect}. Returns null if an error occurs; in that case,
* Constructs an {@link ConfiguredAspect}. Returns null if an error occurs; in that case,
* {@code aspectFactory} should call one of the error reporting methods of {@link RuleContext}.
*/
public Aspect createAspect(
public ConfiguredAspect createAspect(
AnalysisEnvironment env,
RuleConfiguredTarget associatedTarget,
ConfiguredAspectFactory aspectFactory,
AspectWithParameters aspectWithParameters,
Aspect aspect,
ListMultimap<Attribute, ConfiguredTarget> prerequisiteMap,
Set<ConfigMatchingProvider> configConditions,
BuildConfiguration hostConfiguration)
throws InterruptedException {
throws InterruptedException {
RuleContext.Builder builder = new RuleContext.Builder(env,
associatedTarget.getTarget(),
associatedTarget.getConfiguration(),
hostConfiguration,
ruleClassProvider.getPrerequisiteValidator());
RuleContext ruleContext = builder
.setVisibility(convertVisibility(
prerequisiteMap, env.getEventHandler(), associatedTarget.getTarget(), null))
.setPrerequisites(prerequisiteMap)
.setAspectAttributes(aspectWithParameters.getDefinition().getAttributes())
.setConfigConditions(configConditions)
.build();
RuleContext ruleContext =
builder
.setVisibility(
convertVisibility(
prerequisiteMap, env.getEventHandler(), associatedTarget.getTarget(), null))
.setPrerequisites(prerequisiteMap)
.setAspectAttributes(aspect.getDefinition().getAttributes())
.setConfigConditions(configConditions)
.build();
if (ruleContext.hasErrors()) {
return null;
}

return aspectFactory.create(associatedTarget, ruleContext,
aspectWithParameters.getParameters());
return aspectFactory.create(associatedTarget, ruleContext, aspect.getParameters());
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,9 @@
import com.google.devtools.build.lib.analysis.config.ConfigMatchingProvider;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.collect.ImmutableSortedKeyListMultimap;
import com.google.devtools.build.lib.packages.Aspect;
import com.google.devtools.build.lib.packages.AspectClass;
import com.google.devtools.build.lib.packages.AspectDefinition;
import com.google.devtools.build.lib.packages.AspectWithParameters;
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;
Expand Down Expand Up @@ -102,15 +102,14 @@ public ConfiguredTargetKey apply(Dependency input) {
private final Attribute.Transition transition;

private final boolean hasStaticConfiguration;
private final ImmutableSet<AspectWithParameters> aspects;
private final ImmutableSet<Aspect> aspects;

/**
* Constructs a Dependency with a given configuration (suitable for static configuration
* builds).
*/
public Dependency(Label label,
@Nullable BuildConfiguration configuration,
ImmutableSet<AspectWithParameters> aspects) {
public Dependency(
Label label, @Nullable BuildConfiguration configuration, ImmutableSet<Aspect> aspects) {
this.label = Preconditions.checkNotNull(label);
this.configuration = configuration;
this.transition = null;
Expand All @@ -123,14 +122,13 @@ public Dependency(Label label,
* builds).
*/
public Dependency(Label label, @Nullable BuildConfiguration configuration) {
this(label, configuration, ImmutableSet.<AspectWithParameters>of());
this(label, configuration, ImmutableSet.<Aspect>of());
}

/**
* Constructs a Dependency with a given transition (suitable for dynamic configuration builds).
*/
public Dependency(Label label, Attribute.Transition transition,
ImmutableSet<AspectWithParameters> aspects) {
public Dependency(Label label, Attribute.Transition transition, ImmutableSet<Aspect> aspects) {
this.label = Preconditions.checkNotNull(label);
this.configuration = null;
this.transition = Preconditions.checkNotNull(transition);
Expand Down Expand Up @@ -167,7 +165,7 @@ public Attribute.Transition getTransition() {
return transition;
}

public ImmutableSet<AspectWithParameters> getAspects() {
public ImmutableSet<Aspect> getAspects() {
return aspects;
}

Expand Down Expand Up @@ -204,7 +202,7 @@ protected DependencyResolver() {
* represent those edges. Visibility attributes are only visited if {@code visitVisibility} is
* {@code true}.
*
* <p>If {@code aspectWithParameters} is null, returns the dependent nodes of the configured
* <p>If {@code aspect} is null, returns the dependent nodes of the configured
* target node representing the given target and configuration, otherwise that of the aspect
* node accompanying the aforementioned configured target node for the specified aspect.
*
Expand All @@ -220,7 +218,7 @@ protected DependencyResolver() {
public final ListMultimap<Attribute, Dependency> dependentNodeMap(
TargetAndConfiguration node,
BuildConfiguration hostConfig,
AspectWithParameters aspectWithParameters,
Aspect aspect,
Set<ConfigMatchingProvider> configConditions)
throws EvalException, InterruptedException {
Target target = node.getTarget();
Expand All @@ -242,11 +240,11 @@ public final ListMultimap<Attribute, Dependency> dependentNodeMap(
ListMultimap<Attribute, LabelAndConfiguration> labelMap =
resolveAttributes(
rule,
aspectWithParameters != null ? aspectWithParameters.getDefinition() : null,
aspect != null ? aspect.getDefinition() : null,
config,
hostConfig,
configConditions);
visitRule(rule, aspectWithParameters, labelMap, outgoingEdges);
visitRule(rule, aspect, labelMap, outgoingEdges);
} else if (target instanceof PackageGroup) {
visitPackageGroup(node, (PackageGroup) target, outgoingEdges.get(null));
} else {
Expand Down Expand Up @@ -555,20 +553,16 @@ private void visitPackageGroup(TargetAndConfiguration node, PackageGroup package
}
}

private ImmutableSet<AspectWithParameters> requiredAspects(
AspectWithParameters aspectWithParameters,
Attribute attribute,
Target target,
Rule originalRule) {
private ImmutableSet<Aspect> requiredAspects(
Aspect aspect, Attribute attribute, Target target, Rule originalRule) {
if (!(target instanceof Rule)) {
return ImmutableSet.of();
}

Set<AspectWithParameters> aspectCandidates =
extractAspectCandidates(aspectWithParameters, attribute, originalRule);
Set<Aspect> aspectCandidates = extractAspectCandidates(aspect, attribute, originalRule);
RuleClass ruleClass = ((Rule) target).getRuleClassObject();
ImmutableSet.Builder<AspectWithParameters> result = ImmutableSet.builder();
for (AspectWithParameters candidateClass : aspectCandidates) {
ImmutableSet.Builder<Aspect> result = ImmutableSet.builder();
for (Aspect candidateClass : aspectCandidates) {
if (Sets.difference(
candidateClass.getDefinition().getRequiredProviders(),
ruleClass.getAdvertisedProviders())
Expand All @@ -579,19 +573,18 @@ private ImmutableSet<AspectWithParameters> requiredAspects(
return result.build();
}

private static Set<AspectWithParameters> extractAspectCandidates(
AspectWithParameters aspectWithParameters, Attribute attribute, Rule originalRule) {
private static Set<Aspect> extractAspectCandidates(
Aspect aspectWithParameters, 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<AspectWithParameters> aspectCandidates = new LinkedHashSet<>();
aspectCandidates.addAll(attribute.getAspectsWithParameters(originalRule));
Set<Aspect> aspectCandidates = new LinkedHashSet<>();
aspectCandidates.addAll(attribute.getAspects(originalRule));
if (aspectWithParameters != null) {
for (AspectClass aspect :
aspectWithParameters.getDefinition().getAttributeAspects().get(attribute.getName())) {
aspectCandidates.add(
new AspectWithParameters(aspect, aspectWithParameters.getParameters()));
aspectCandidates.add(new Aspect(aspect, aspectWithParameters.getParameters()));
}
}
return aspectCandidates;
Expand All @@ -604,7 +597,7 @@ private void visitRule(Rule rule, ListMultimap<Attribute, LabelAndConfiguration>

private void visitRule(
Rule rule,
AspectWithParameters aspectWithParameters,
Aspect aspect,
ListMultimap<Attribute, LabelAndConfiguration> labelMap,
ListMultimap<Attribute, Dependency> outgoingEdges) {
Preconditions.checkNotNull(labelMap);
Expand Down Expand Up @@ -647,7 +640,7 @@ private void visitRule(
}
for (Dependency dependency :
transitionApplier.getDependencies(
label, requiredAspects(aspectWithParameters, attribute, toTarget, rule))) {
label, requiredAspects(aspect, attribute, toTarget, rule))) {
outgoingEdges.put(
entry.getKey(),
dependency);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ public enum Mode {
private final ImmutableMap<Class<? extends TransitiveInfoProvider>, Object> providers;
private final ImmutableList<Artifact> mandatoryStampFiles;
private final Set<ConfigMatchingProvider> configConditions;
private final ImmutableList<Aspect> aspects;
private final ImmutableList<ConfiguredAspect> configuredAspects;

RuleConfiguredTarget(RuleContext ruleContext,
ImmutableList<Artifact> mandatoryStampFiles,
Expand All @@ -80,7 +80,7 @@ public enum Mode {
this.providers = ImmutableMap.copyOf(providerBuilder);
this.mandatoryStampFiles = mandatoryStampFiles;
this.configConditions = ruleContext.getConfigConditions();
this.aspects = ImmutableList.of();
this.configuredAspects = ImmutableList.of();

// If this rule is the run_under target, then check that we have an executable; note that
// run_under is only set in the target configuration, and the target must also be analyzed for
Expand Down Expand Up @@ -108,7 +108,7 @@ public enum Mode {
* an input or an output file).
*/
public static ConfiguredTarget mergeAspects(
ConfiguredTarget base, Iterable<Aspect> aspects) {
ConfiguredTarget base, Iterable<ConfiguredAspect> aspects) {
if (Iterables.isEmpty(aspects)) {
// If there are no aspects, don't bother with creating a proxy object
return base;
Expand All @@ -122,7 +122,7 @@ public static ConfiguredTarget mergeAspects(
/**
* Creates an instance based on a configured target and a set of aspects.
*/
private RuleConfiguredTarget(RuleConfiguredTarget base, Iterable<Aspect> aspects) {
private RuleConfiguredTarget(RuleConfiguredTarget base, Iterable<ConfiguredAspect> aspects) {
super(base.getTarget(), base.getConfiguration());

Set<Class<? extends TransitiveInfoProvider>> providers = new HashSet<>();
Expand All @@ -138,8 +138,8 @@ private RuleConfiguredTarget(RuleConfiguredTarget base, Iterable<Aspect> aspects
SkylarkProviders.merge(getAllProviders(base, aspects, SkylarkProviders.class));

// Validate that all other providers are only provided once.
for (Aspect aspect : aspects) {
for (TransitiveInfoProvider aspectProvider : aspect) {
for (ConfiguredAspect configuredAspect : aspects) {
for (TransitiveInfoProvider aspectProvider : configuredAspect) {
Class<? extends TransitiveInfoProvider> aClass = aspectProvider.getClass();
if (OutputGroupProvider.class.equals(aClass)) {
continue;
Expand Down Expand Up @@ -178,21 +178,19 @@ private RuleConfiguredTarget(RuleConfiguredTarget base, Iterable<Aspect> aspects
}
this.mandatoryStampFiles = base.mandatoryStampFiles;
this.configConditions = base.configConditions;
this.aspects = ImmutableList.copyOf(aspects);
this.configuredAspects = ImmutableList.copyOf(aspects);
}

private static <T extends TransitiveInfoProvider> List<T> getAllProviders(
RuleConfiguredTarget base,
Iterable<Aspect> aspects,
Class<T> providerClass) {
RuleConfiguredTarget base, Iterable<ConfiguredAspect> aspects, Class<T> providerClass) {
T baseProvider = base.getProvider(providerClass);
List<T> providers = new ArrayList<>();
if (baseProvider != null) {
providers.add(baseProvider);
}

for (Aspect aspect : aspects) {
final T aspectProvider = aspect.getProvider(providerClass);
for (ConfiguredAspect configuredAspect : aspects) {
final T aspectProvider = configuredAspect.getProvider(providerClass);
if (aspectProvider == null) {
continue;
}
Expand All @@ -215,8 +213,8 @@ public <P extends TransitiveInfoProvider> P getProvider(Class<P> providerClass)
// class?
Object provider = providers.get(providerClass);
if (provider == null) {
for (Aspect aspect : aspects) {
provider = aspect.getProviders().get(providerClass);
for (ConfiguredAspect configuredAspect : configuredAspects) {
provider = configuredAspect.getProviders().get(providerClass);
if (provider != null) {
break;
}
Expand Down Expand Up @@ -247,8 +245,8 @@ public final Rule getTarget() {
public UnmodifiableIterator<TransitiveInfoProvider> iterator() {
Map<Class<? extends TransitiveInfoProvider>, TransitiveInfoProvider> allProviders =
new LinkedHashMap<>();
for (int i = aspects.size() - 1; i >= 0; i++) {
for (TransitiveInfoProvider tip : aspects.get(i)) {
for (int i = configuredAspects.size() - 1; i >= 0; i++) {
for (TransitiveInfoProvider tip : configuredAspects.get(i)) {
allProviders.put(tip.getClass(), tip);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,12 +134,11 @@ public static ArtifactsToBuild getAllArtifactsToBuild(TransitiveInfoCollection t

public static ArtifactsToBuild getAllArtifactsToBuild(
AspectValue aspectValue, TopLevelArtifactContext context) {
Aspect aspect = aspectValue.getAspect();
ConfiguredAspect configuredAspect = aspectValue.getConfiguredAspect();
return getAllArtifactsToBuild(
aspect.getProvider(OutputGroupProvider.class),
aspect.getProvider(FileProvider.class),
context
);
configuredAspect.getProvider(OutputGroupProvider.class),
configuredAspect.getProvider(FileProvider.class),
context);
}

public static ArtifactsToBuild getAllArtifactsToBuild(
Expand Down
Loading

0 comments on commit b487ac6

Please sign in to comment.