Skip to content

Commit

Permalink
Command line aspect-on-aspect
Browse files Browse the repository at this point in the history
This CL supports aspect-on-aspect for command line aspects. Command line aspects specified via `--aspects` option will support a top-level aspect requiring aspect providers via `required_aspect_providers` to get their values from other top-level aspects advertising it that come before it in the `--aspects` list.

PiperOrigin-RevId: 381862973
  • Loading branch information
mai93 authored and copybara-github committed Jun 28, 2021
1 parent 92ae4e3 commit 943c83a
Show file tree
Hide file tree
Showing 14 changed files with 1,753 additions and 233 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
import com.google.devtools.build.lib.causes.Cause;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.skyframe.AspectValueKey;
import com.google.devtools.build.lib.skyframe.AspectValueKey.AspectKey;
import com.google.devtools.build.lib.skyframe.ConfiguredTargetKey;
import java.util.Collection;
import javax.annotation.Nullable;
Expand All @@ -40,20 +40,20 @@
* target cannot be completed because of an error in one of its dependencies.
*/
public class AnalysisFailureEvent implements BuildEvent {
@Nullable private final AspectValueKey failedAspect;
@Nullable private final AspectKey failedAspect;
private final ConfiguredTargetKey failedTarget;
private final BuildEventId configuration;
private final NestedSet<Cause> rootCauses;

public AnalysisFailureEvent(
ActionLookupKey failedTarget, BuildEventId configuration, NestedSet<Cause> rootCauses) {
Preconditions.checkArgument(
failedTarget instanceof ConfiguredTargetKey || failedTarget instanceof AspectValueKey);
failedTarget instanceof ConfiguredTargetKey || failedTarget instanceof AspectKey);
if (failedTarget instanceof ConfiguredTargetKey) {
this.failedAspect = null;
this.failedTarget = (ConfiguredTargetKey) failedTarget;
} else {
this.failedAspect = (AspectValueKey) failedTarget;
this.failedAspect = (AspectKey) failedTarget;
this.failedTarget = failedAspect.getBaseConfiguredTargetKey();
}
if (configuration != null) {
Expand All @@ -65,7 +65,7 @@ public AnalysisFailureEvent(
}

public AnalysisFailureEvent(
AspectValueKey failedAspect, BuildEventId configuration, NestedSet<Cause> rootCauses) {
AspectKey failedAspect, BuildEventId configuration, NestedSet<Cause> rootCauses) {
this.failedAspect = failedAspect;
this.failedTarget = failedAspect.getBaseConfiguredTargetKey();
if (configuration != null) {
Expand Down
59 changes: 25 additions & 34 deletions src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
Original file line number Diff line number Diff line change
Expand Up @@ -55,13 +55,12 @@
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.events.ExtendedEventHandler;
import com.google.devtools.build.lib.packages.AspectClass;
import com.google.devtools.build.lib.packages.AspectDescriptor;
import com.google.devtools.build.lib.packages.AspectParameters;
import com.google.devtools.build.lib.packages.Attribute;
import com.google.devtools.build.lib.packages.NativeAspectClass;
import com.google.devtools.build.lib.packages.NoSuchPackageException;
import com.google.devtools.build.lib.packages.NoSuchTargetException;
import com.google.devtools.build.lib.packages.Rule;
import com.google.devtools.build.lib.packages.StarlarkAspectClass;
import com.google.devtools.build.lib.packages.Target;
import com.google.devtools.build.lib.packages.TargetUtils;
import com.google.devtools.build.lib.pkgcache.PackageManager;
Expand All @@ -75,6 +74,7 @@
import com.google.devtools.build.lib.server.FailureDetails.TargetPatterns.Code;
import com.google.devtools.build.lib.skyframe.AspectValueKey;
import com.google.devtools.build.lib.skyframe.AspectValueKey.AspectKey;
import com.google.devtools.build.lib.skyframe.AspectValueKey.TopLevelAspectsKey;
import com.google.devtools.build.lib.skyframe.BuildConfigurationValue;
import com.google.devtools.build.lib.skyframe.ConfiguredTargetKey;
import com.google.devtools.build.lib.skyframe.CoverageReportValue;
Expand All @@ -86,7 +86,6 @@
import com.google.devtools.build.lib.util.Pair;
import com.google.devtools.build.lib.util.RegexFilter;
import com.google.devtools.build.skyframe.WalkableGraph;
import java.util.ArrayList;
import java.util.Collection;
import java.util.HashMap;
import java.util.HashSet;
Expand Down Expand Up @@ -275,10 +274,7 @@ public AnalysisResult update(
.map(TargetAndConfiguration::getConfiguredTargetKey)
.collect(Collectors.toList());

Multimap<Pair<Label, String>, BuildConfiguration> aspectConfigurations =
ArrayListMultimap.create();

List<AspectValueKey> aspectKeys = new ArrayList<>();
ImmutableList.Builder<AspectClass> aspectClassesBuilder = ImmutableList.builder();
for (String aspect : aspects) {
// Syntax: label%aspect
int delimiterPosition = aspect.indexOf('%');
Expand Down Expand Up @@ -318,38 +314,14 @@ public AnalysisResult update(
createFailureDetail(errorMessage, Analysis.Code.ASPECT_LABEL_SYNTAX_ERROR),
e);
}

String starlarkFunctionName = aspect.substring(delimiterPosition + 1);
for (TargetAndConfiguration targetSpec : topLevelTargetsWithConfigs) {
aspectConfigurations.put(
Pair.of(targetSpec.getLabel(), aspect), targetSpec.getConfiguration());
aspectKeys.add(
AspectValueKey.createStarlarkAspectKey(
targetSpec.getLabel(),
// For invoking top-level aspects, use the top-level configuration for both the
// aspect and the base target while the top-level configuration is untrimmed.
targetSpec.getConfiguration(),
targetSpec.getConfiguration(),
starlarkFileLabel,
starlarkFunctionName));
}
aspectClassesBuilder.add(new StarlarkAspectClass(starlarkFileLabel, starlarkFunctionName));
} else {
final NativeAspectClass aspectFactoryClass =
ruleClassProvider.getNativeAspectClassMap().get(aspect);

if (aspectFactoryClass != null) {
for (TargetAndConfiguration targetSpec : topLevelTargetsWithConfigs) {
// For invoking top-level aspects, use the top-level configuration for both the
// aspect and the base target while the top-level configuration is untrimmed.
BuildConfiguration configuration = targetSpec.getConfiguration();
aspectConfigurations.put(Pair.of(targetSpec.getLabel(), aspect), configuration);
aspectKeys.add(
AspectValueKey.createAspectKey(
targetSpec.getLabel(),
configuration,
new AspectDescriptor(aspectFactoryClass, AspectParameters.EMPTY),
configuration));
}
aspectClassesBuilder.add(aspectFactoryClass);
} else {
String errorMessage = "Aspect '" + aspect + "' is unknown";
throw new ViewCreationFailedException(
Expand All @@ -358,6 +330,25 @@ public AnalysisResult update(
}
}

Multimap<Pair<Label, String>, BuildConfiguration> aspectConfigurations =
ArrayListMultimap.create();
ImmutableList<AspectClass> aspectClasses = aspectClassesBuilder.build();
ImmutableList.Builder<TopLevelAspectsKey> aspectsKeys = ImmutableList.builder();
for (TargetAndConfiguration targetSpec : topLevelTargetsWithConfigs) {
BuildConfiguration configuration = targetSpec.getConfiguration();
for (AspectClass aspectClass : aspectClasses) {
aspectConfigurations.put(
Pair.of(targetSpec.getLabel(), aspectClass.getName()), configuration);
}
// For invoking top-level aspects, use the top-level configuration for both the
// aspect and the base target while the top-level configuration is untrimmed.
if (!aspectClasses.isEmpty()) {
aspectsKeys.add(
AspectValueKey.createTopLevelAspectsKey(
aspectClasses, targetSpec.getLabel(), configuration));
}
}

for (Pair<Label, String> target : aspectConfigurations.keys()) {
eventBus.post(
new AspectConfiguredEvent(
Expand All @@ -382,7 +373,7 @@ public AnalysisResult update(
skyframeBuildView.configureTargets(
eventHandler,
topLevelCtKeys,
aspectKeys,
aspectsKeys.build(),
Suppliers.memoize(configurationLookupSupplier),
topLevelOptions,
eventBus,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -297,11 +297,18 @@ public class BuildRequestOptions extends OptionsBase {
effectTags = {OptionEffectTag.UNKNOWN},
allowMultiple = true,
help =
"Comma-separated list of aspects to be applied to top-level targets. All aspects "
+ "are applied to all top-level targets independently. Aspects are specified in "
+ "the form <bzl-file-label>%<aspect_name>, "
+ "for example '//tools:my_def.bzl%my_aspect', where 'my_aspect' is a top-level "
+ "value from from a file tools/my_def.bzl")
"Comma-separated list of aspects to be applied to top-level targets. All aspects are"
+ " applied to all top-level targets. If aspect <code>some_aspect</code> specifies"
+ " required aspect providers via <code>required_aspect_providers</code>,"
+ " <code>some_aspect</code> will run after every aspect that was mentioned before it"
+ " in the aspects list and whose advertised providers satisfy"
+ " <code>some_aspect</code> required aspect providers. <code>some_aspect</code> will"
+ " then have access to the values of those aspects' providers. Aspects that do not"
+ " have such dependency will run independently on the top-level targets."
+ ""
+ " Aspects are specified in the form <bzl-file-label>%<aspect_name>, for example"
+ " '//tools:my_def.bzl%my_aspect', where 'my_aspect' is a top-level value from a"
+ " file tools/my_def.bzl")
public List<String> aspects;

public BuildRequestOptions() throws OptionsParsingException {}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,4 +64,9 @@ public final boolean equals(Object o) {
public final int hashCode() {
return Objects.hash(getExtensionLabel(), getExportedName());
}

@Override
public String toString() {
return getName();
}
}
Loading

0 comments on commit 943c83a

Please sign in to comment.