Try to clean up some of the excess baggage Aspects are carting around, mainly from a code cleanliness perspective, but with some minor memory benefits:
1. Use AspectKey instead of AspectValue wherever possible at the top-level, since keys are what callers should be using, not values.
2. Remove the freestanding label field from AspectKey. It is always equal to its ConfiguredTargetKey field's label. Having two fields with redundant data is confusing.
3. Remove the freestanding label field from AspectValue. Same reason: it was always the AspectKey's label.
4. Remove AspectDescriptor from ConfiguredAspect: it comes from the AspectKey, so just use the key.
This is also good because I think maximizing correspondence between ConfiguredAspect and ConfiguredTarget is a goal. Having an AspectValue muddies the waters. Note that, for historical reasons, ConfiguredTarget has much more data in it than we'd want a ConfiguredAspect to have (ideally, both would just have actions and providers).
In a follow-up, I may wrap AspectValue together with an AspectKey in TopLevelSkylarkAspectFunction#compute's return value. I think that, plus some tolerable casting, would remove the only remaining real need for AspectValue to have an AspectKey inside it.
PiperOrigin-RevId: 307877866
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/AnalysisResult.java b/src/main/java/com/google/devtools/build/lib/analysis/AnalysisResult.java
index 60edd8f..ae03826 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/AnalysisResult.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/AnalysisResult.java
@@ -15,12 +15,14 @@
package com.google.devtools.build.lib.analysis;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ImmutableSortedSet;
import com.google.common.collect.Sets;
import com.google.devtools.build.lib.actions.ActionGraph;
import com.google.devtools.build.lib.actions.PackageRoots;
import com.google.devtools.build.lib.analysis.config.BuildConfigurationCollection;
+import com.google.devtools.build.lib.skyframe.AspectValueKey.AspectKey;
import java.util.Collection;
import javax.annotation.Nullable;
@@ -38,7 +40,7 @@
private final ImmutableSet<ConfiguredTarget> parallelTests;
private final ImmutableSet<ConfiguredTarget> exclusiveTests;
@Nullable private final TopLevelArtifactContext topLevelContext;
- private final ImmutableSet<AspectValue> aspects;
+ private final ImmutableMap<AspectKey, ConfiguredAspect> aspects;
private final PackageRoots packageRoots;
private final String workspaceName;
private final Collection<TargetAndConfiguration> topLevelTargetsWithConfigs;
@@ -47,7 +49,7 @@
AnalysisResult(
BuildConfigurationCollection configurations,
ImmutableSet<ConfiguredTarget> targetsToBuild,
- ImmutableSet<AspectValue> aspects,
+ ImmutableMap<AspectKey, ConfiguredAspect> aspects,
@Nullable ImmutableList<ConfiguredTarget> targetsToTest,
ImmutableSet<ConfiguredTarget> targetsToSkip,
@Nullable String error,
@@ -93,13 +95,8 @@
return packageRoots;
}
- /**
- * Returns aspects of configured targets to build.
- *
- * <p>If this list is empty, build the targets returned by {@code getTargetsToBuild()}.
- * Otherwise, only build these aspects of the targets returned by {@code getTargetsToBuild()}.
- */
- public ImmutableSet<AspectValue> getAspects() {
+ /** Returns aspects to build. */
+ public ImmutableMap<AspectKey, ConfiguredAspect> getAspectsMap() {
return aspects;
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/AspectCompleteEvent.java b/src/main/java/com/google/devtools/build/lib/analysis/AspectCompleteEvent.java
index 650a841..3d42896 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/AspectCompleteEvent.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/AspectCompleteEvent.java
@@ -109,7 +109,7 @@
@Override
public BuildEventId getEventId() {
return BuildEventIdUtil.aspectCompleted(
- aspectValue.getLabel(),
+ aspectValue.getKey().getLabel(),
configurationEventId,
aspectValue.getAspect().getDescriptor().getDescription());
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/AspectValue.java b/src/main/java/com/google/devtools/build/lib/analysis/AspectValue.java
index 57a80c3..6735b6c 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/AspectValue.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/AspectValue.java
@@ -16,23 +16,22 @@
import com.google.common.base.Preconditions;
import com.google.devtools.build.lib.actions.BasicActionLookupValue;
-import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.packages.Aspect;
import com.google.devtools.build.lib.packages.Package;
import com.google.devtools.build.lib.skyframe.AspectValueKey.AspectKey;
-import com.google.devtools.build.lib.skyframe.ConfiguredObjectValue;
import com.google.devtools.build.lib.syntax.Location;
import javax.annotation.Nullable;
/** An aspect in the context of the Skyframe graph. */
public final class AspectValue extends BasicActionLookupValue implements ConfiguredObjectValue {
-
// These variables are only non-final because they may be clear()ed to save memory. They are null
// only after they are cleared except for transitivePackagesForPackageRootResolution.
- @Nullable private Label label;
@Nullable private Aspect aspect;
@Nullable private Location location;
+ // Normally the key used to evaluate this value in AspectFunction#compute. But in the case of a
+ // top-level SkylarkAspectKey, the AspectValue will be this value but the key will be the
+ // associated aspect key from SkylarkAspectKey#toAspectkey.
@Nullable private AspectKey key;
@Nullable private ConfiguredAspect configuredAspect;
// May be null either after clearing or because transitive packages are not tracked.
@@ -41,16 +40,14 @@
public AspectValue(
AspectKey key,
Aspect aspect,
- Label label,
Location location,
ConfiguredAspect configuredAspect,
NestedSet<Package> transitivePackagesForPackageRootResolution) {
super(configuredAspect.getActions());
- this.label = Preconditions.checkNotNull(label, actions);
- this.aspect = Preconditions.checkNotNull(aspect, label);
- this.location = Preconditions.checkNotNull(location, label);
- this.key = Preconditions.checkNotNull(key, label);
- this.configuredAspect = Preconditions.checkNotNull(configuredAspect, label);
+ this.key = key;
+ this.aspect = Preconditions.checkNotNull(aspect, location);
+ this.location = Preconditions.checkNotNull(location, aspect);
+ this.configuredAspect = Preconditions.checkNotNull(configuredAspect, location);
this.transitivePackagesForPackageRootResolution = transitivePackagesForPackageRootResolution;
}
@@ -58,10 +55,6 @@
return Preconditions.checkNotNull(configuredAspect);
}
- public Label getLabel() {
- return Preconditions.checkNotNull(label);
- }
-
public Location getLocation() {
return Preconditions.checkNotNull(location);
}
@@ -76,13 +69,11 @@
@Override
public void clear(boolean clearEverything) {
- Preconditions.checkNotNull(label, this);
Preconditions.checkNotNull(aspect, this);
Preconditions.checkNotNull(location, this);
Preconditions.checkNotNull(key, this);
Preconditions.checkNotNull(configuredAspect, this);
if (clearEverything) {
- label = null;
aspect = null;
location = null;
key = null;
@@ -99,7 +90,6 @@
@Override
public String toString() {
return getStringHelper()
- .add("label", label)
.add("key", key)
.add("location", location)
.add("aspect", aspect)
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 90bdded..2e4a23b 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
@@ -460,7 +460,7 @@
Set<Label> testsToRun = loadingResult.getTestsToRunLabels();
Set<ConfiguredTarget> configuredTargets =
Sets.newLinkedHashSet(skyframeAnalysisResult.getConfiguredTargets());
- ImmutableSet<AspectValue> aspects = ImmutableSet.copyOf(skyframeAnalysisResult.getAspects());
+ ImmutableMap<AspectKey, ConfiguredAspect> aspects = skyframeAnalysisResult.getAspects();
Set<ConfiguredTarget> allTargetsToTest = null;
if (testsToRun != null) {
@@ -617,7 +617,7 @@
private void addExtraActionsIfRequested(
AnalysisOptions viewOptions,
Collection<ConfiguredTarget> configuredTargets,
- Collection<AspectValue> aspects,
+ ImmutableMap<AspectKey, ConfiguredAspect> aspects,
ArtifactsToOwnerLabels.Builder artifactsToTopLevelLabelsMap,
ExtendedEventHandler eventHandler) {
RegexFilter filter = viewOptions.extraActionFilter;
@@ -659,21 +659,21 @@
}
}
}
- for (AspectValue aspect : aspects) {
+ for (Map.Entry<AspectKey, ConfiguredAspect> aspectEntry : aspects.entrySet()) {
ExtraActionArtifactsProvider provider =
- aspect.getConfiguredAspect().getProvider(ExtraActionArtifactsProvider.class);
+ aspectEntry.getValue().getProvider(ExtraActionArtifactsProvider.class);
if (provider != null) {
if (viewOptions.extraActionTopLevelOnly) {
TopLevelArtifactHelper.addArtifactsWithOwnerLabel(
provider.getExtraActionArtifacts(),
filter,
- aspect.getLabel(),
+ aspectEntry.getKey().getLabel(),
artifactsToTopLevelLabelsMap);
} else {
TopLevelArtifactHelper.addArtifactsWithOwnerLabel(
provider.getTransitiveExtraActionArtifacts(),
filter,
- aspect.getLabel(),
+ aspectEntry.getKey().getLabel(),
artifactsToTopLevelLabelsMap);
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredAspect.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredAspect.java
index 29fc277..d7b6386 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredAspect.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredAspect.java
@@ -31,14 +31,9 @@
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
-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.Info;
import com.google.devtools.build.lib.packages.Provider;
-import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
import com.google.devtools.build.lib.syntax.EvalException;
-import com.google.devtools.build.lib.syntax.Location;
import java.util.Arrays;
import java.util.Map;
import java.util.TreeMap;
@@ -60,18 +55,12 @@
* @see com.google.devtools.build.lib.packages.AspectClass
*/
@Immutable
-@AutoCodec
public final class ConfiguredAspect implements ProviderCollection {
- private final AspectDescriptor descriptor;
private final ImmutableList<ActionAnalysisMetadata> actions;
private final TransitiveInfoProviderMap providers;
- @AutoCodec.VisibleForSerialization
- ConfiguredAspect(
- AspectDescriptor descriptor,
- ImmutableList<ActionAnalysisMetadata> actions,
- TransitiveInfoProviderMap providers) {
- this.descriptor = descriptor;
+ private ConfiguredAspect(
+ ImmutableList<ActionAnalysisMetadata> actions, TransitiveInfoProviderMap providers) {
this.actions = actions;
this.providers = providers;
@@ -84,20 +73,6 @@
}
}
- /**
- * Returns the aspect name.
- */
- public String getName() {
- return descriptor.getAspectClass().getName();
- }
-
- /**
- * The aspect descriptor originating this ConfiguredAspect.
- */
- public AspectDescriptor getDescriptor() {
- return descriptor;
- }
-
public ImmutableList<ActionAnalysisMetadata> getActions() {
return actions;
}
@@ -128,19 +103,17 @@
}
public static ConfiguredAspect forAlias(ConfiguredAspect real) {
- return new ConfiguredAspect(real.descriptor, real.getActions(), real.getProviders());
+ return new ConfiguredAspect(real.getActions(), real.getProviders());
}
- public static ConfiguredAspect forNonapplicableTarget(AspectDescriptor descriptor) {
+ public static ConfiguredAspect forNonapplicableTarget() {
return new ConfiguredAspect(
- descriptor,
ImmutableList.of(),
new TransitiveInfoProviderMapBuilder().add().build());
}
- public static Builder builder(
- AspectClass aspectClass, AspectParameters parameters, RuleContext ruleContext) {
- return new Builder(aspectClass, parameters, ruleContext);
+ public static Builder builder(RuleContext ruleContext) {
+ return new Builder(ruleContext);
}
/**
@@ -151,17 +124,8 @@
new TransitiveInfoProviderMapBuilder();
private final Map<String, NestedSetBuilder<Artifact>> outputGroupBuilders = new TreeMap<>();
private final RuleContext ruleContext;
- private final AspectDescriptor descriptor;
- public Builder(
- AspectClass aspectClass,
- AspectParameters parameters,
- RuleContext context) {
- this(new AspectDescriptor(aspectClass, parameters), context);
- }
-
- public Builder(AspectDescriptor descriptor, RuleContext ruleContext) {
- this.descriptor = descriptor;
+ public Builder(RuleContext ruleContext) {
this.ruleContext = ruleContext;
}
@@ -221,12 +185,6 @@
return this;
}
- public Builder addSkylarkTransitiveInfo(String name, Object value, Location loc)
- throws EvalException {
- providers.put(name, value);
- return this;
- }
-
public Builder addSkylarkDeclaredProvider(Info declaredProvider) throws EvalException {
Provider constructor = declaredProvider.getProvider();
if (!constructor.isExported()) {
@@ -288,7 +246,6 @@
}
return new ConfiguredAspect(
- descriptor,
generatingActions.getActions(),
providers.build());
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredObjectValue.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredObjectValue.java
similarity index 88%
rename from src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredObjectValue.java
rename to src/main/java/com/google/devtools/build/lib/analysis/ConfiguredObjectValue.java
index 0141683..6b15159 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredObjectValue.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredObjectValue.java
@@ -11,19 +11,20 @@
// 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.skyframe;
+package com.google.devtools.build.lib.analysis;
import com.google.devtools.build.lib.actions.ActionLookupValue;
-import com.google.devtools.build.lib.analysis.ProviderCollection;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.packages.Package;
+import com.google.devtools.build.lib.skyframe.AspectFunction;
+import com.google.devtools.build.lib.skyframe.ConfiguredTargetFunction;
+import com.google.devtools.build.lib.skyframe.ConfiguredTargetValue;
import com.google.devtools.build.skyframe.NotComparableSkyValue;
/**
* Super-interface for {@link ConfiguredTargetValue} and {@link
* com.google.devtools.build.lib.analysis.AspectValue}.
*/
-// TODO(janakr): Move this to analysis.
public interface ConfiguredObjectValue extends ActionLookupValue, NotComparableSkyValue {
/** Returns the configured target/aspect for this value. */
ProviderCollection getConfiguredObject();
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 efa2699..59c5526 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
@@ -20,7 +20,6 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
-import com.google.devtools.build.lib.actions.ActionLookupValue;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.Artifact.SourceArtifact;
import com.google.devtools.build.lib.actions.ArtifactFactory;
@@ -65,6 +64,7 @@
import com.google.devtools.build.lib.packages.StarlarkProviderIdentifier;
import com.google.devtools.build.lib.packages.Target;
import com.google.devtools.build.lib.profiler.memory.CurrentRuleTracker;
+import com.google.devtools.build.lib.skyframe.AspectValueKey;
import com.google.devtools.build.lib.skyframe.ConfiguredTargetAndData;
import com.google.devtools.build.lib.skyframe.ConfiguredTargetKey;
import com.google.devtools.build.lib.util.ClassName;
@@ -599,7 +599,7 @@
@Nullable ResolvedToolchainContext toolchainContext,
BuildConfiguration aspectConfiguration,
BuildConfiguration hostConfiguration,
- ActionLookupValue.ActionLookupKey aspectKey)
+ AspectValueKey.AspectKey aspectKey)
throws InterruptedException, ActionConflictException {
RuleContext.Builder builder =
@@ -649,10 +649,11 @@
ruleClassProvider.getToolsRepository());
if (configuredAspect != null) {
validateAdvertisedProviders(
- configuredAspect, aspect.getDefinition().getAdvertisedProviders(),
+ configuredAspect,
+ aspectKey,
+ aspect.getDefinition().getAdvertisedProviders(),
associatedTarget.getTarget(),
- env.getEventHandler()
- );
+ env.getEventHandler());
}
return configuredAspect;
}
@@ -680,34 +681,34 @@
private void validateAdvertisedProviders(
ConfiguredAspect configuredAspect,
- AdvertisedProviderSet advertisedProviders, Target target,
+ AspectValueKey.AspectKey aspectKey,
+ AdvertisedProviderSet advertisedProviders,
+ Target target,
EventHandler eventHandler) {
if (advertisedProviders.canHaveAnyProvider()) {
return;
}
for (Class<?> aClass : advertisedProviders.getNativeProviders()) {
if (configuredAspect.getProvider(aClass.asSubclass(TransitiveInfoProvider.class)) == null) {
- eventHandler.handle(Event.error(
- target.getLocation(),
- String.format(
- "Aspect '%s', applied to '%s', does not provide advertised provider '%s'",
- configuredAspect.getName(),
- target.getLabel(),
- aClass.getSimpleName()
- )));
+ eventHandler.handle(
+ Event.error(
+ target.getLocation(),
+ String.format(
+ "Aspect '%s', applied to '%s', does not provide advertised provider '%s'",
+ aspectKey.getAspectClass().getName(),
+ target.getLabel(),
+ aClass.getSimpleName())));
}
}
for (StarlarkProviderIdentifier providerId : advertisedProviders.getSkylarkProviders()) {
if (configuredAspect.get(providerId) == null) {
- eventHandler.handle(Event.error(
- target.getLocation(),
- String.format(
- "Aspect '%s', applied to '%s', does not provide advertised provider '%s'",
- configuredAspect.getName(),
- target.getLabel(),
- providerId
- )));
+ eventHandler.handle(
+ Event.error(
+ target.getLocation(),
+ String.format(
+ "Aspect '%s', applied to '%s', does not provide advertised provider '%s'",
+ aspectKey.getAspectClass().getName(), target.getLabel(), providerId)));
}
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/TopLevelArtifactHelper.java b/src/main/java/com/google/devtools/build/lib/analysis/TopLevelArtifactHelper.java
index 981166a..6cd33a3 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/TopLevelArtifactHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/TopLevelArtifactHelper.java
@@ -25,9 +25,11 @@
import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
import com.google.devtools.build.lib.profiler.AutoProfiler;
import com.google.devtools.build.lib.profiler.GoogleAutoProfilerUtils;
+import com.google.devtools.build.lib.skyframe.AspectValueKey.AspectKey;
import com.google.devtools.build.lib.util.RegexFilter;
import java.time.Duration;
import java.util.Collection;
+import java.util.Map;
import javax.annotation.Nullable;
/**
@@ -122,7 +124,7 @@
@VisibleForTesting
public static ArtifactsToOwnerLabels makeTopLevelArtifactsToOwnerLabels(
- AnalysisResult analysisResult, Iterable<AspectValue> aspects) {
+ AnalysisResult analysisResult) {
try (AutoProfiler ignored =
GoogleAutoProfilerUtils.logged("assigning owner labels", MIN_LOGGING)) {
ArtifactsToOwnerLabels.Builder artifactsToOwnerLabelsBuilder =
@@ -135,11 +137,12 @@
target.getLabel(),
artifactsToOwnerLabelsBuilder);
}
- for (AspectValue aspect : aspects) {
+ for (Map.Entry<AspectKey, ConfiguredAspect> aspectEntry :
+ analysisResult.getAspectsMap().entrySet()) {
addArtifactsWithOwnerLabel(
- getAllArtifactsToBuild(aspect, artifactContext).getAllArtifacts(),
+ getAllArtifactsToBuild(aspectEntry.getValue(), artifactContext).getAllArtifacts(),
null,
- aspect.getLabel(),
+ aspectEntry.getKey().getLabel(),
artifactsToOwnerLabelsBuilder);
}
if (analysisResult.getTargetsToTest() != null) {
@@ -195,11 +198,6 @@
);
}
- public static ArtifactsToBuild getAllArtifactsToBuild(
- AspectValue aspectValue, TopLevelArtifactContext context) {
- return getAllArtifactsToBuild(aspectValue.getConfiguredAspect(), context);
- }
-
static ArtifactsToBuild getAllArtifactsToBuild(
@Nullable OutputGroupInfo outputGroupInfo,
@Nullable FileProvider fileProvider,
diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/AnalysisPhaseRunner.java b/src/main/java/com/google/devtools/build/lib/buildtool/AnalysisPhaseRunner.java
index 25816fe..4772624 100644
--- a/src/main/java/com/google/devtools/build/lib/buildtool/AnalysisPhaseRunner.java
+++ b/src/main/java/com/google/devtools/build/lib/buildtool/AnalysisPhaseRunner.java
@@ -126,12 +126,7 @@
}
for (BlazeModule module : env.getRuntime().getBlazeModules()) {
- module.afterAnalysis(
- env,
- request,
- buildOptions,
- analysisResult.getTargetsToBuild(),
- analysisResult.getAspects());
+ module.afterAnalysis(env, request, buildOptions, analysisResult.getTargetsToBuild());
}
reportTargets(env, analysisResult);
diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/BuildResult.java b/src/main/java/com/google/devtools/build/lib/buildtool/BuildResult.java
index 77dcc8b..22f4672 100644
--- a/src/main/java/com/google/devtools/build/lib/buildtool/BuildResult.java
+++ b/src/main/java/com/google/devtools/build/lib/buildtool/BuildResult.java
@@ -17,15 +17,16 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.MoreObjects;
import com.google.common.base.Preconditions;
+import com.google.common.collect.ImmutableSet;
import com.google.common.util.concurrent.Futures;
import com.google.common.util.concurrent.ListenableFuture;
-import com.google.devtools.build.lib.analysis.AspectValue;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
import com.google.devtools.build.lib.analysis.config.BuildConfigurationCollection;
import com.google.devtools.build.lib.buildeventstream.BuildEvent.LocalFile.LocalFileCompression;
import com.google.devtools.build.lib.buildeventstream.BuildEvent.LocalFile.LocalFileType;
import com.google.devtools.build.lib.buildeventstream.BuildToolLogs;
import com.google.devtools.build.lib.buildeventstream.BuildToolLogs.LogFileEntry;
+import com.google.devtools.build.lib.skyframe.AspectValueKey.AspectKey;
import com.google.devtools.build.lib.util.DetailedExitCode;
import com.google.devtools.build.lib.util.ExitCode;
import com.google.devtools.build.lib.util.Pair;
@@ -58,7 +59,7 @@
private Collection<ConfiguredTarget> testTargets;
private Collection<ConfiguredTarget> successfulTargets;
private Collection<ConfiguredTarget> skippedTargets;
- private Collection<AspectValue> successfulAspects;
+ private ImmutableSet<AspectKey> successfulAspects;
private final BuildToolLogCollection buildToolLogCollection = new BuildToolLogCollection();
@@ -222,8 +223,8 @@
this.successfulTargets = successfulTargets;
}
- /** @see #getSuccessfulAspects */
- void setSuccessfulAspects(Collection<AspectValue> successfulAspects) {
+ /** See #getSuccessfulAspects */
+ void setSuccessfulAspects(ImmutableSet<AspectKey> successfulAspects) {
this.successfulAspects = successfulAspects;
}
@@ -247,7 +248,7 @@
* null if the execution phase was not attempted, as may happen if there are errors in the loading
* phase, for example.
*/
- public Collection<AspectValue> getSuccessfulAspects() {
+ ImmutableSet<AspectKey> getSuccessfulAspects() {
return successfulAspects;
}
diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/BuildResultPrinter.java b/src/main/java/com/google/devtools/build/lib/buildtool/BuildResultPrinter.java
index 2a140ff..d4a0e89 100644
--- a/src/main/java/com/google/devtools/build/lib/buildtool/BuildResultPrinter.java
+++ b/src/main/java/com/google/devtools/build/lib/buildtool/BuildResultPrinter.java
@@ -15,8 +15,10 @@
package com.google.devtools.build.lib.buildtool;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.actions.Artifact;
-import com.google.devtools.build.lib.analysis.AspectValue;
+import com.google.devtools.build.lib.analysis.ConfiguredAspect;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
import com.google.devtools.build.lib.analysis.FileProvider;
import com.google.devtools.build.lib.analysis.OutputGroupInfo;
@@ -33,6 +35,7 @@
import com.google.devtools.build.lib.exec.ExecutionOptions;
import com.google.devtools.build.lib.runtime.BlazeRuntime;
import com.google.devtools.build.lib.runtime.CommandEnvironment;
+import com.google.devtools.build.lib.skyframe.AspectValueKey.AspectKey;
import com.google.devtools.build.lib.util.io.OutErr;
import java.util.ArrayList;
import java.util.Collection;
@@ -48,17 +51,17 @@
}
/**
- * Shows the result of the build. Information includes the list of up-to-date
- * and failed targets and list of output artifacts for successful targets
+ * Shows the result of the build. Information includes the list of up-to-date and failed targets
+ * and list of output artifacts for successful targets
*
* <p>This corresponds to the --show_result flag.
*/
- public void showBuildResult(
+ void showBuildResult(
BuildRequest request,
BuildResult result,
Collection<ConfiguredTarget> configuredTargets,
Collection<ConfiguredTarget> configuredTargetsToSkip,
- Collection<AspectValue> aspects) {
+ ImmutableMap<AspectKey, ConfiguredAspect> aspects) {
// NOTE: be careful what you print! We don't want to create a consistency
// problem where the summary message and the exit code disagree. The logic
// here is already complex.
@@ -77,9 +80,8 @@
request.getBuildOptions().experimentalNoProductNameOutSymlink);
OutErr outErr = request.getOutErr();
Collection<ConfiguredTarget> targetsToPrint = filterTargetsToPrint(configuredTargets);
- Collection<AspectValue> aspectsToPrint = filterAspectsToPrint(aspects);
final boolean success;
- if (aspectsToPrint.isEmpty()) {
+ if (aspects.isEmpty()) {
// Suppress summary if --show_result value is exceeded:
if (targetsToPrint.size() > request.getBuildOptions().maxResultTargets) {
return;
@@ -135,23 +137,25 @@
success = failed.isEmpty();
} else {
// Suppress summary if --show_result value is exceeded:
- if (aspectsToPrint.size() > request.getBuildOptions().maxResultTargets) {
+ if (aspects.size() > request.getBuildOptions().maxResultTargets) {
return;
}
// Filter the targets we care about into two buckets:
- Collection<AspectValue> succeeded = new ArrayList<>();
- Collection<AspectValue> failed = new ArrayList<>();
- Collection<AspectValue> successfulAspects = result.getSuccessfulAspects();
- for (AspectValue aspect : aspectsToPrint) {
+ Collection<AspectKey> succeeded = new ArrayList<>();
+ Collection<AspectKey> failed = new ArrayList<>();
+ ImmutableSet<AspectKey> successfulAspects = result.getSuccessfulAspects();
+ for (AspectKey aspect : aspects.keySet()) {
(successfulAspects.contains(aspect) ? succeeded : failed).add(aspect);
}
TopLevelArtifactContext context = request.getTopLevelArtifactContext();
- for (AspectValue aspect : succeeded) {
+ for (AspectKey aspect : succeeded) {
Label label = aspect.getLabel();
- String aspectName = aspect.getConfiguredAspect().getName();
+ ConfiguredAspect configuredAspect = aspects.get(aspect);
+ String aspectName = aspect.getAspectClass().getName();
boolean headerFlag = true;
NestedSet<Artifact> importantArtifacts =
- TopLevelArtifactHelper.getAllArtifactsToBuild(aspect, context).getImportantArtifacts();
+ TopLevelArtifactHelper.getAllArtifactsToBuild(configuredAspect, context)
+ .getImportantArtifacts();
for (Artifact importantArtifact : importantArtifacts.toList()) {
if (headerFlag) {
outErr.printErr("Aspect " + aspectName + " of " + label + " up-to-date:\n");
@@ -166,9 +170,9 @@
"Aspect " + aspectName + " of " + label + " up-to-date (nothing to build)\n");
}
}
- for (AspectValue aspect : failed) {
+ for (AspectKey aspect : failed) {
Label label = aspect.getLabel();
- String aspectName = aspect.getConfiguredAspect().getName();
+ String aspectName = aspect.getAspectClass().getName();
outErr.printErr("Aspect " + aspectName + " of " + label + " failed to build\n");
}
success = failed.isEmpty();
@@ -191,25 +195,24 @@
*
* <p>This corresponds to the --experimental_show_artifacts flag.
*/
- public void showArtifacts(
+ void showArtifacts(
BuildRequest request,
Collection<ConfiguredTarget> configuredTargets,
- Collection<AspectValue> aspects) {
+ Collection<ConfiguredAspect> aspects) {
TopLevelArtifactContext context = request.getTopLevelArtifactContext();
Collection<ConfiguredTarget> targetsToPrint = filterTargetsToPrint(configuredTargets);
- Collection<AspectValue> aspectsToPrint = filterAspectsToPrint(aspects);
NestedSetBuilder<Artifact> artifactsBuilder = NestedSetBuilder.stableOrder();
- for (ConfiguredTarget target : targetsToPrint) {
- artifactsBuilder.addTransitive(
- TopLevelArtifactHelper.getAllArtifactsToBuild(target, context).getImportantArtifacts());
- }
+ targetsToPrint.forEach(
+ t ->
+ artifactsBuilder.addTransitive(
+ TopLevelArtifactHelper.getAllArtifactsToBuild(t, context).getImportantArtifacts()));
- for (AspectValue aspect : aspectsToPrint) {
- artifactsBuilder.addTransitive(
- TopLevelArtifactHelper.getAllArtifactsToBuild(aspect, context).getImportantArtifacts());
- }
+ aspects.forEach(
+ a ->
+ artifactsBuilder.addTransitive(
+ TopLevelArtifactHelper.getAllArtifactsToBuild(a, context).getImportantArtifacts()));
OutErr outErr = request.getOutErr();
outErr.printErrLn("Build artifacts:");
@@ -263,8 +266,4 @@
}
return result.build();
}
-
- private Collection<AspectValue> filterAspectsToPrint(Collection<AspectValue> aspects) {
- return aspects;
- }
}
diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java b/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java
index 7d172c0..0fce14c 100644
--- a/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java
+++ b/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java
@@ -40,7 +40,6 @@
import com.google.devtools.build.lib.actions.cache.ActionCache;
import com.google.devtools.build.lib.actions.cache.Protos.ActionCacheStatistics;
import com.google.devtools.build.lib.analysis.AnalysisResult;
-import com.google.devtools.build.lib.analysis.AspectValue;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
import com.google.devtools.build.lib.analysis.TopLevelArtifactContext;
import com.google.devtools.build.lib.analysis.TopLevelArtifactHelper;
@@ -320,7 +319,6 @@
Set<ConfiguredTargetKey> builtTargets = new HashSet<>();
Set<AspectKey> builtAspects = new HashSet<>();
- Collection<AspectValue> aspects = analysisResult.getAspects();
if (request.isRunningInEmacs()) {
// The syntax of this message is tightly constrained by lisp/progmodes/compile.el in emacs
@@ -337,7 +335,8 @@
// Free memory by removing cache entries that aren't going to be needed.
try (SilentCloseable c = Profiler.instance().profile("clearAnalysisCache")) {
env.getSkyframeBuildView()
- .clearAnalysisCache(analysisResult.getTargetsToBuild(), analysisResult.getAspects());
+ .clearAnalysisCache(
+ analysisResult.getTargetsToBuild(), analysisResult.getAspectsMap().keySet());
}
}
@@ -348,9 +347,7 @@
actionGraph,
// If this supplier is ever consumed by more than one ActionContextProvider, it can be
// pulled out of the loop and made a memoizing supplier.
- () ->
- TopLevelArtifactHelper.makeTopLevelArtifactsToOwnerLabels(
- analysisResult, aspects));
+ () -> TopLevelArtifactHelper.makeTopLevelArtifactsToOwnerLabels(analysisResult));
}
}
skyframeExecutor.drainChangedFiles();
@@ -371,7 +368,7 @@
analysisResult.getExclusiveTests(),
analysisResult.getTargetsToBuild(),
analysisResult.getTargetsToSkip(),
- analysisResult.getAspects(),
+ analysisResult.getAspectsMap().keySet(),
executor,
builtTargets,
builtAspects,
@@ -421,7 +418,8 @@
try (SilentCloseable c = Profiler.instance().profile("Show results")) {
buildResult.setSuccessfulTargets(
determineSuccessfulTargets(configuredTargets, builtTargets));
- buildResult.setSuccessfulAspects(determineSuccessfulAspects(aspects, builtAspects));
+ buildResult.setSuccessfulAspects(
+ determineSuccessfulAspects(analysisResult.getAspectsMap().keySet(), builtAspects));
buildResult.setSkippedTargets(analysisResult.getTargetsToSkip());
BuildResultPrinter buildResultPrinter = new BuildResultPrinter(env);
buildResultPrinter.showBuildResult(
@@ -429,13 +427,14 @@
buildResult,
configuredTargets,
analysisResult.getTargetsToSkip(),
- analysisResult.getAspects());
+ analysisResult.getAspectsMap());
}
try (SilentCloseable c = Profiler.instance().profile("Show artifacts")) {
if (request.getBuildOptions().showArtifacts) {
BuildResultPrinter buildResultPrinter = new BuildResultPrinter(env);
- buildResultPrinter.showArtifacts(request, configuredTargets, analysisResult.getAspects());
+ buildResultPrinter.showArtifacts(
+ request, configuredTargets, analysisResult.getAspectsMap().values());
}
}
@@ -692,17 +691,10 @@
return successfulTargets;
}
- private Collection<AspectValue> determineSuccessfulAspects(
- Collection<AspectValue> aspects, Set<AspectKey> builtAspects) {
- // Maintain the ordering by copying builtTargets into a LinkedHashSet in the same iteration
- // order as configuredTargets.
- Collection<AspectValue> successfulAspects = new LinkedHashSet<>();
- for (AspectValue aspect : aspects) {
- if (builtAspects.contains(aspect.getKey())) {
- successfulAspects.add(aspect);
- }
- }
- return successfulAspects;
+ private static ImmutableSet<AspectKey> determineSuccessfulAspects(
+ ImmutableSet<AspectKey> aspects, Set<AspectKey> builtAspects) {
+ // Maintain the ordering.
+ return aspects.stream().filter(builtAspects::contains).collect(ImmutableSet.toImmutableSet());
}
/** Get action cache if present or reload it from the on-disk cache. */
diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/SkyframeBuilder.java b/src/main/java/com/google/devtools/build/lib/buildtool/SkyframeBuilder.java
index 17d92cc..2b8f90e 100644
--- a/src/main/java/com/google/devtools/build/lib/buildtool/SkyframeBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/buildtool/SkyframeBuilder.java
@@ -15,6 +15,7 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
+import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Range;
import com.google.common.collect.Sets;
import com.google.devtools.build.lib.actions.ActionCacheChecker;
@@ -28,7 +29,6 @@
import com.google.devtools.build.lib.actions.MissingInputFileException;
import com.google.devtools.build.lib.actions.ResourceManager;
import com.google.devtools.build.lib.actions.TestExecException;
-import com.google.devtools.build.lib.analysis.AspectValue;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
import com.google.devtools.build.lib.analysis.TopLevelArtifactContext;
import com.google.devtools.build.lib.analysis.test.TestProvider;
@@ -57,7 +57,6 @@
import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.common.options.OptionsProvider;
import java.util.ArrayList;
-import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.Map;
@@ -106,7 +105,7 @@
Set<ConfiguredTarget> exclusiveTests,
Set<ConfiguredTarget> targetsToBuild,
Set<ConfiguredTarget> targetsToSkip,
- Collection<AspectValue> aspects,
+ ImmutableSet<AspectKey> aspects,
Executor executor,
Set<ConfiguredTargetKey> builtTargets,
Set<AspectKey> builtAspects,
diff --git a/src/main/java/com/google/devtools/build/lib/query2/ConfiguredTargetValueAccessor.java b/src/main/java/com/google/devtools/build/lib/query2/ConfiguredTargetValueAccessor.java
index 5abefc9..081f576 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/ConfiguredTargetValueAccessor.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/ConfiguredTargetValueAccessor.java
@@ -16,6 +16,7 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterables;
import com.google.devtools.build.lib.analysis.AspectValue;
+import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.packages.Rule;
import com.google.devtools.build.lib.packages.Target;
import com.google.devtools.build.lib.packages.TargetUtils;
@@ -25,6 +26,7 @@
import com.google.devtools.build.lib.query2.engine.QueryException;
import com.google.devtools.build.lib.query2.engine.QueryExpression;
import com.google.devtools.build.lib.query2.engine.QueryVisibility;
+import com.google.devtools.build.lib.skyframe.AspectValueKey.AspectKey;
import com.google.devtools.build.lib.skyframe.ConfiguredTargetKey;
import com.google.devtools.build.lib.skyframe.ConfiguredTargetValue;
import com.google.devtools.build.lib.skyframe.SkyFunctions;
@@ -143,11 +145,12 @@
SkyKey skyKey = configuredTargetKeyExtractor.extractKey(configuredTargetValue);
Iterable<SkyKey> revDeps =
Iterables.concat(walkableGraph.getReverseDeps(ImmutableList.of(skyKey)).values());
+ Label label = configuredTargetValue.getConfiguredTarget().getLabel();
for (SkyKey revDep : revDeps) {
SkyFunctionName skyFunctionName = revDep.functionName();
if (SkyFunctions.ASPECT.equals(skyFunctionName)) {
AspectValue aspectValue = (AspectValue) walkableGraph.getValue(revDep);
- if (aspectValue.getLabel().equals(configuredTargetValue.getConfiguredTarget().getLabel())) {
+ if (((AspectKey) revDep).getLabel().equals(label)) {
result.add(aspectValue);
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java
index 1c345c8..ee2107e 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java
@@ -27,7 +27,6 @@
import com.google.common.util.concurrent.MoreExecutors;
import com.google.devtools.build.lib.actions.ActionInput;
import com.google.devtools.build.lib.actions.Artifact;
-import com.google.devtools.build.lib.analysis.AspectValue;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
import com.google.devtools.build.lib.analysis.FilesToRunProvider;
import com.google.devtools.build.lib.analysis.RunfilesSupport;
@@ -500,8 +499,7 @@
CommandEnvironment env,
BuildRequest request,
BuildOptions buildOptions,
- Iterable<ConfiguredTarget> configuredTargets,
- ImmutableSet<AspectValue> aspects) {
+ Iterable<ConfiguredTarget> configuredTargets) {
if (remoteOutputsMode != null && remoteOutputsMode.downloadToplevelOutputsOnly()) {
Preconditions.checkState(actionContextProvider != null, "actionContextProvider was null");
boolean isTestCommand = env.getCommandName().equals("test");
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 dd8d6f0..2edb3e8 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
@@ -56,7 +56,7 @@
throws ActionConflictException {
if (!JavaCommon.getConstraints(ruleContext).contains("android")
&& !ruleContext.getRule().getRuleClass().startsWith("android_")) {
- return new ConfiguredAspect.Builder(this, parameters, ruleContext).build();
+ return new ConfiguredAspect.Builder(ruleContext).build();
}
List<TransitiveInfoCollection> deps = new ArrayList<>();
@@ -79,7 +79,7 @@
if (provider != null) {
runtimeJars.addTransitive(provider.getResourceClassJars());
}
- return new ConfiguredAspect.Builder(this, parameters, ruleContext)
+ return new ConfiguredAspect.Builder(ruleContext)
.addProvider(
AndroidNeverLinkLibrariesProvider.create(
AndroidCommon.collectTransitiveNeverlinkLibraries(
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/DexArchiveAspect.java b/src/main/java/com/google/devtools/build/lib/rules/android/DexArchiveAspect.java
index 01e5bad..498d7c8 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/DexArchiveAspect.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/DexArchiveAspect.java
@@ -186,7 +186,7 @@
AspectParameters params,
String toolsRepository)
throws InterruptedException, ActionConflictException {
- ConfiguredAspect.Builder result = new ConfiguredAspect.Builder(this, params, ruleContext);
+ ConfiguredAspect.Builder result = new ConfiguredAspect.Builder(ruleContext);
Function<Artifact, Artifact> desugaredJars =
desugarJarsIfRequested(ctadBase.getConfiguredTarget(), ruleContext, result);
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/GraphNodeAspect.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/GraphNodeAspect.java
index 9ad3a7a..6d0aac1 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/GraphNodeAspect.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/GraphNodeAspect.java
@@ -69,7 +69,7 @@
AnalysisUtils.getProviders(
ruleContext.getPrerequisites("deps", TransitionMode.TARGET), GraphNodeInfo.class));
}
- return new ConfiguredAspect.Builder(this, params, ruleContext)
+ return new ConfiguredAspect.Builder(ruleContext)
.addProvider(
GraphNodeInfo.class, new GraphNodeInfo(ruleContext.getLabel(), children.build()))
.build();
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoAspect.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoAspect.java
index 84ec348..0ddd3ca 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoAspect.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoAspect.java
@@ -108,7 +108,7 @@
ProtoInfo protoInfo = checkNotNull(ctadBase.getConfiguredTarget().get(ProtoInfo.PROVIDER));
try {
- ConfiguredAspect.Builder result = new ConfiguredAspect.Builder(this, parameters, ruleContext);
+ ConfiguredAspect.Builder result = new ConfiguredAspect.Builder(ruleContext);
new Impl(ruleContext, protoInfo, cppSemantics, ccToolchainType).addProviders(result);
return result.build();
} catch (RuleErrorException e) {
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaLiteProtoAspect.java b/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaLiteProtoAspect.java
index cb6af8e..f124669 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaLiteProtoAspect.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaLiteProtoAspect.java
@@ -89,7 +89,7 @@
AspectParameters parameters,
String toolsRepository)
throws InterruptedException, ActionConflictException {
- ConfiguredAspect.Builder aspect = new ConfiguredAspect.Builder(this, parameters, ruleContext);
+ ConfiguredAspect.Builder aspect = new ConfiguredAspect.Builder(ruleContext);
ProtoInfo protoInfo = ctadBase.getConfiguredTarget().get(ProtoInfo.PROVIDER);
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaProtoAspect.java b/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaProtoAspect.java
index 3777d14..33167de 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaProtoAspect.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaProtoAspect.java
@@ -95,7 +95,7 @@
AspectParameters parameters,
String toolsRepository)
throws InterruptedException, ActionConflictException {
- ConfiguredAspect.Builder aspect = new ConfiguredAspect.Builder(this, parameters, ruleContext);
+ ConfiguredAspect.Builder aspect = new ConfiguredAspect.Builder(ruleContext);
if (!rpcSupport.checkAttributes(ruleContext, parameters)) {
return aspect.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 0bb0c71..169d08c 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
@@ -244,22 +244,21 @@
throws InterruptedException, ActionConflictException {
ConfiguredTarget base = ctadBase.getConfiguredTarget();
if (isProtoRule(base)) {
- return proto(base, ruleContext, parameters);
+ return proto(base, ruleContext);
} else {
- return java(base, ruleContext, parameters);
+ return java(base, ruleContext);
}
}
private ConfiguredAspect buildAspect(
ConfiguredTarget base,
RuleContext ruleContext,
- AspectParameters parameters,
J2ObjcSource j2ObjcSource,
J2ObjcMappingFileProvider directJ2ObjcMappingFileProvider,
List<Attribute> depAttributes,
List<TransitiveInfoCollection> otherDeps)
throws InterruptedException, ActionConflictException {
- ConfiguredAspect.Builder builder = new ConfiguredAspect.Builder(this, parameters, ruleContext);
+ ConfiguredAspect.Builder builder = new ConfiguredAspect.Builder(ruleContext);
ObjcCommon common;
ObjcProvider objcProvider = null;
@@ -320,8 +319,7 @@
.build();
}
- private ConfiguredAspect java(
- ConfiguredTarget base, RuleContext ruleContext, AspectParameters parameters)
+ private ConfiguredAspect java(ConfiguredTarget base, RuleContext ruleContext)
throws InterruptedException, ActionConflictException {
JavaCompilationArgsProvider compilationArgsProvider =
JavaInfo.getProvider(JavaCompilationArgsProvider.class, base);
@@ -363,15 +361,13 @@
return buildAspect(
base,
ruleContext,
- parameters,
j2ObjcSource,
directJ2ObjcMappingFileProvider,
JAVA_DEPENDENT_ATTRIBUTES,
ImmutableList.of());
}
- private ConfiguredAspect proto(
- ConfiguredTarget base, RuleContext ruleContext, AspectParameters parameters)
+ private ConfiguredAspect proto(ConfiguredTarget base, RuleContext ruleContext)
throws InterruptedException, ActionConflictException {
ProtoInfo protoInfo = base.get(ProtoInfo.PROVIDER);
ImmutableList<Artifact> protoSources = protoInfo.getDirectProtoSources();
@@ -399,7 +395,6 @@
return buildAspect(
base,
ruleContext,
- parameters,
j2ObjcSource,
directJ2ObjcMappingFileProvider,
PROTO_DEPENDENT_ATTRIBUTES,
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProtoAspect.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProtoAspect.java
index 31aa172..fceff47 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProtoAspect.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProtoAspect.java
@@ -56,8 +56,7 @@
AspectParameters parameters,
String toolsRepository)
throws InterruptedException, ActionConflictException {
- ConfiguredAspect.Builder aspectBuilder = new ConfiguredAspect.Builder(
- this, parameters, ruleContext);
+ ConfiguredAspect.Builder aspectBuilder = new ConfiguredAspect.Builder(ruleContext);
ObjcProtoProvider.Builder aspectObjcProtoProvider = new ObjcProtoProvider.Builder();
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeModule.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeModule.java
index 163ea67..a9644e7 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeModule.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeModule.java
@@ -15,10 +15,8 @@
import com.google.auto.value.AutoValue;
import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableSet;
import com.google.common.eventbus.SubscriberExceptionHandler;
import com.google.devtools.build.lib.actions.ExecutorInitException;
-import com.google.devtools.build.lib.analysis.AspectValue;
import com.google.devtools.build.lib.analysis.BlazeDirectories;
import com.google.devtools.build.lib.analysis.BlazeVersionInfo;
import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider;
@@ -290,8 +288,7 @@
CommandEnvironment env,
BuildRequest request,
BuildOptions buildOptions,
- Iterable<ConfiguredTarget> configuredTargets,
- ImmutableSet<AspectValue> aspects)
+ Iterable<ConfiguredTarget> configuredTargets)
throws InterruptedException, ViewCreationFailedException {}
/**
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/AspectCompletionValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/AspectCompletionValue.java
index e793177..1e7b315 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/AspectCompletionValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/AspectCompletionValue.java
@@ -15,7 +15,6 @@
import com.google.auto.value.AutoValue;
import com.google.common.collect.Iterables;
-import com.google.devtools.build.lib.analysis.AspectValue;
import com.google.devtools.build.lib.analysis.TopLevelArtifactContext;
import com.google.devtools.build.lib.skyframe.AspectValueKey.AspectKey;
import com.google.devtools.build.lib.skyframe.CompletionFunction.TopLevelActionLookupKey;
@@ -33,10 +32,8 @@
private AspectCompletionValue() {}
- public static Iterable<SkyKey> keys(
- Collection<AspectValue> targets, final TopLevelArtifactContext ctx) {
- return Iterables.transform(
- targets, aspectValue -> AspectCompletionKey.create(aspectValue.getKey(), ctx));
+ public static Iterable<SkyKey> keys(Collection<AspectKey> keys, TopLevelArtifactContext ctx) {
+ return Iterables.transform(keys, k -> AspectCompletionKey.create(k, ctx));
}
/** The key of an AspectCompletionValue. */
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/AspectCompletor.java b/src/main/java/com/google/devtools/build/lib/skyframe/AspectCompletor.java
index b3c6455..2f5a251 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/AspectCompletor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/AspectCompletor.java
@@ -18,7 +18,6 @@
import com.google.devtools.build.lib.actions.MissingInputFileException;
import com.google.devtools.build.lib.analysis.AspectCompleteEvent;
import com.google.devtools.build.lib.analysis.AspectValue;
-import com.google.devtools.build.lib.analysis.TopLevelArtifactContext;
import com.google.devtools.build.lib.analysis.TopLevelArtifactHelper.ArtifactsInOutputGroup;
import com.google.devtools.build.lib.analysis.TopLevelArtifactHelper.ArtifactsToBuild;
import com.google.devtools.build.lib.buildeventstream.BuildEventIdUtil;
@@ -27,38 +26,43 @@
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.events.ExtendedEventHandler;
+import com.google.devtools.build.lib.skyframe.AspectCompletionValue.AspectCompletionKey;
+import com.google.devtools.build.lib.skyframe.AspectValueKey.AspectKey;
import com.google.devtools.build.lib.skyframe.CompletionFunction.Completor;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.skyframe.SkyFunction;
import com.google.devtools.build.skyframe.SkyFunction.Environment;
-import com.google.devtools.build.skyframe.SkyKey;
import java.util.function.Supplier;
import javax.annotation.Nullable;
/** Manages completing builds for aspects. */
-class AspectCompletor implements Completor<AspectValue, AspectCompletionValue> {
+class AspectCompletor
+ implements Completor<AspectValue, AspectCompletionValue, AspectCompletionKey> {
- public static SkyFunction aspectCompletionFunction(
+ static SkyFunction aspectCompletionFunction(
PathResolverFactory pathResolverFactory, Supplier<Path> execRootSupplier) {
return new CompletionFunction<>(pathResolverFactory, new AspectCompletor(), execRootSupplier);
}
@Override
- public Event getRootCauseError(AspectValue value, Cause rootCause, Environment env) {
+ public Event getRootCauseError(
+ AspectValue value, AspectCompletionKey key, Cause rootCause, Environment env) {
+ AspectKey aspectKey = key.actionLookupKey();
return Event.error(
value.getLocation(),
String.format(
"%s, aspect %s: missing input file '%s'",
- value.getLabel(), value.getConfiguredAspect().getName(), rootCause));
+ aspectKey.getLabel(), aspectKey.getAspectClass().getName(), rootCause));
}
@Override
public MissingInputFileException getMissingFilesException(
- AspectValue value, int missingCount, Environment env) {
+ AspectValue value, AspectCompletionKey key, int missingCount, Environment env) {
+ AspectKey aspectKey = key.actionLookupKey();
return new MissingInputFileException(
- value.getLabel()
+ aspectKey.getLabel()
+ ", aspect "
- + value.getConfiguredAspect().getName()
+ + aspectKey.getAspectClass().getName()
+ missingCount
+ " input file(s) do not exist",
value.getLocation());
@@ -75,9 +79,10 @@
NestedSet<Cause> rootCauses,
NestedSet<ArtifactsInOutputGroup> outputs,
Environment env,
- TopLevelArtifactContext topLevelArtifactContext)
+ AspectCompletionKey key)
throws InterruptedException {
- BuildEventId configurationEventId = getConfigurationEventIdFromAspectValue(value, env);
+ BuildEventId configurationEventId =
+ getConfigurationEventIdFromAspectKey(key.actionLookupKey(), env);
if (configurationEventId == null) {
return null;
}
@@ -85,14 +90,14 @@
}
@Nullable
- private BuildEventId getConfigurationEventIdFromAspectValue(AspectValue value, Environment env)
+ private BuildEventId getConfigurationEventIdFromAspectKey(AspectKey aspectKey, Environment env)
throws InterruptedException {
- if (value.getKey().getBaseConfiguredTargetKey().getConfigurationKey() == null) {
+ if (aspectKey.getBaseConfiguredTargetKey().getConfigurationKey() == null) {
return BuildEventIdUtil.nullConfigurationId();
} else {
BuildConfigurationValue buildConfigurationValue =
(BuildConfigurationValue)
- env.getValue(value.getKey().getBaseConfiguredTargetKey().getConfigurationKey());
+ env.getValue(aspectKey.getBaseConfiguredTargetKey().getConfigurationKey());
if (buildConfigurationValue == null) {
return null;
}
@@ -102,13 +107,14 @@
@Override
public ExtendedEventHandler.Postable createSucceeded(
- SkyKey skyKey,
+ AspectCompletionKey skyKey,
AspectValue value,
CompletionContext completionContext,
ArtifactsToBuild artifactsToBuild,
Environment env)
throws InterruptedException {
- BuildEventId configurationEventId = getConfigurationEventIdFromAspectValue(value, env);
+ AspectKey aspectKey = skyKey.actionLookupKey();
+ BuildEventId configurationEventId = getConfigurationEventIdFromAspectKey(aspectKey, env);
if (configurationEventId == null) {
return null;
}
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 6dbeaa0..a15c5a1 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
@@ -588,7 +588,6 @@
return new AspectValue(
originalKey,
aspect,
- originalTarget.getLabel(),
originalTarget.getLocation(),
ConfiguredAspect.forAlias(real.getConfiguredAspect()),
transitivePackagesForPackageRootResolution);
@@ -649,7 +648,7 @@
CurrentRuleTracker.endConfiguredAspect();
}
} else {
- configuredAspect = ConfiguredAspect.forNonapplicableTarget(aspect.getDescriptor());
+ configuredAspect = ConfiguredAspect.forNonapplicableTarget();
}
events.replayOn(env.getListener());
@@ -672,7 +671,6 @@
return new AspectValue(
key,
aspect,
- associatedTarget.getTarget().getLabel(),
associatedTarget.getTarget().getLocation(),
configuredAspect,
transitivePackagesForPackageRootResolution == null
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/AspectValueKey.java b/src/main/java/com/google/devtools/build/lib/skyframe/AspectValueKey.java
index 77ee261..bfd5266 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/AspectValueKey.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/AspectValueKey.java
@@ -51,7 +51,6 @@
BuildConfiguration aspectConfiguration) {
KeyAndHost aspectKeyAndHost = ConfiguredTargetKey.keyFromConfiguration(aspectConfiguration);
return AspectKey.createAspectKey(
- label,
ConfiguredTargetKey.of(label, baseConfiguration),
baseKeys,
aspectDescriptor,
@@ -66,7 +65,6 @@
BuildConfiguration aspectConfiguration) {
KeyAndHost aspectKeyAndHost = ConfiguredTargetKey.keyFromConfiguration(aspectConfiguration);
return AspectKey.createAspectKey(
- label,
ConfiguredTargetKey.of(label, baseConfiguration),
ImmutableList.of(),
aspectDescriptor,
@@ -104,7 +102,6 @@
/** A base class for a key representing an aspect applied to a particular target. */
@AutoCodec
public static class AspectKey extends AspectValueKey {
- private final Label label;
private final ImmutableList<AspectKey> baseKeys;
private final BuildConfigurationValue.Key aspectConfigurationKey;
private final ConfiguredTargetKey baseConfiguredTargetKey;
@@ -112,13 +109,11 @@
private int hashCode;
private AspectKey(
- Label label,
BuildConfigurationValue.Key aspectConfigurationKey,
ConfiguredTargetKey baseConfiguredTargetKey,
ImmutableList<AspectKey> baseKeys,
AspectDescriptor aspectDescriptor) {
this.baseKeys = baseKeys;
- this.label = label;
this.aspectConfigurationKey = aspectConfigurationKey;
this.baseConfiguredTargetKey = baseConfiguredTargetKey;
this.aspectDescriptor = aspectDescriptor;
@@ -127,7 +122,6 @@
@AutoCodec.VisibleForSerialization
@AutoCodec.Instantiator
static AspectKey createAspectKey(
- Label label,
ConfiguredTargetKey baseConfiguredTargetKey,
ImmutableList<AspectKey> baseKeys,
AspectDescriptor aspectDescriptor,
@@ -136,17 +130,9 @@
return aspectKeyInterner.intern(
aspectConfigurationIsHost
? new HostAspectKey(
- label,
- aspectConfigurationKey,
- baseConfiguredTargetKey,
- baseKeys,
- aspectDescriptor)
+ aspectConfigurationKey, baseConfiguredTargetKey, baseKeys, aspectDescriptor)
: new AspectKey(
- label,
- aspectConfigurationKey,
- baseConfiguredTargetKey,
- baseKeys,
- aspectDescriptor));
+ aspectConfigurationKey, baseConfiguredTargetKey, baseKeys, aspectDescriptor));
}
@Override
@@ -157,7 +143,7 @@
@Override
public Label getLabel() {
- return label;
+ return baseConfiguredTargetKey.getLabel();
}
public AspectClass getAspectClass() {
@@ -249,7 +235,7 @@
private int computeHashCode() {
return Objects.hashCode(
- label, baseKeys, aspectConfigurationKey, baseConfiguredTargetKey, aspectDescriptor);
+ baseKeys, aspectConfigurationKey, baseConfiguredTargetKey, aspectDescriptor);
}
@Override
@@ -263,15 +249,14 @@
}
AspectKey that = (AspectKey) other;
- return Objects.equal(label, that.label)
- && Objects.equal(baseKeys, that.baseKeys)
+ return Objects.equal(baseKeys, that.baseKeys)
&& Objects.equal(aspectConfigurationKey, that.aspectConfigurationKey)
&& Objects.equal(baseConfiguredTargetKey, that.baseConfiguredTargetKey)
&& Objects.equal(aspectDescriptor, that.aspectDescriptor);
}
public String prettyPrint() {
- if (label == null) {
+ if (getLabel() == null) {
return "null";
}
@@ -281,7 +266,7 @@
: String.format(" (over %s)", baseKeys.toString());
return String.format(
"%s with aspect %s%s%s",
- label.toString(),
+ getLabel().toString(),
aspectDescriptor.getAspectClass().getName(),
(aspectConfigurationKey != null && aspectConfigurationIsHost()) ? "(host) " : "",
baseKeysString);
@@ -289,7 +274,7 @@
@Override
public String toString() {
- return (baseKeys == null ? label : baseKeys.toString())
+ return (baseKeys == null ? getLabel() : baseKeys.toString())
+ "#"
+ aspectDescriptor
+ " "
@@ -308,7 +293,6 @@
}
return createAspectKey(
- label,
ConfiguredTargetKey.of(
label,
baseConfiguredTargetKey.getConfigurationKey(),
@@ -323,12 +307,11 @@
/** An {@link AspectKey} for an aspect in the host configuration. */
static class HostAspectKey extends AspectKey {
private HostAspectKey(
- Label label,
BuildConfigurationValue.Key aspectConfigurationKey,
ConfiguredTargetKey baseConfiguredTargetKey,
ImmutableList<AspectKey> baseKeys,
AspectDescriptor aspectDescriptor) {
- super(label, aspectConfigurationKey, baseConfiguredTargetKey, baseKeys, aspectDescriptor);
+ super(aspectConfigurationKey, baseConfiguredTargetKey, baseKeys, aspectDescriptor);
}
@Override
@@ -453,7 +436,6 @@
AspectKey toAspectKey(AspectClass aspectClass) {
return AspectKey.createAspectKey(
- targetLabel,
baseConfiguredTargetKey,
ImmutableList.of(),
new AspectDescriptor(aspectClass, AspectParameters.EMPTY),
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/Builder.java b/src/main/java/com/google/devtools/build/lib/skyframe/Builder.java
index 753f611..0fb5828 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/Builder.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/Builder.java
@@ -14,12 +14,12 @@
package com.google.devtools.build.lib.skyframe;
+import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Range;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.BuildFailedException;
import com.google.devtools.build.lib.actions.Executor;
import com.google.devtools.build.lib.actions.TestExecException;
-import com.google.devtools.build.lib.analysis.AspectValue;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
import com.google.devtools.build.lib.analysis.TopLevelArtifactContext;
import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadCompatible;
@@ -27,7 +27,6 @@
import com.google.devtools.build.lib.skyframe.AspectValueKey.AspectKey;
import com.google.devtools.build.lib.util.AbruptExitException;
import com.google.devtools.common.options.OptionsProvider;
-import java.util.Collection;
import java.util.Set;
import javax.annotation.Nullable;
@@ -86,7 +85,7 @@
Set<ConfiguredTarget> exclusiveTests,
Set<ConfiguredTarget> targetsToBuild,
Set<ConfiguredTarget> targetsToSkip,
- Collection<AspectValue> aspects,
+ ImmutableSet<AspectKey> aspects,
Executor executor,
Set<ConfiguredTargetKey> builtTargets,
Set<AspectKey> builtAspects,
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/CompletionFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/CompletionFunction.java
index b1539d5..3002c8b 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/CompletionFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/CompletionFunction.java
@@ -24,6 +24,7 @@
import com.google.devtools.build.lib.actions.CompletionContext.PathResolverFactory;
import com.google.devtools.build.lib.actions.FilesetOutputSymlink;
import com.google.devtools.build.lib.actions.MissingInputFileException;
+import com.google.devtools.build.lib.analysis.ConfiguredObjectValue;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
import com.google.devtools.build.lib.analysis.TopLevelArtifactContext;
import com.google.devtools.build.lib.analysis.TopLevelArtifactHelper;
@@ -54,11 +55,13 @@
/** CompletionFunction builds the artifactsToBuild collection of a {@link ConfiguredTarget}. */
public final class CompletionFunction<
- ValueT extends ConfiguredObjectValue, ResultT extends SkyValue>
+ ValueT extends ConfiguredObjectValue,
+ ResultT extends SkyValue,
+ KeyT extends CompletionFunction.TopLevelActionLookupKey>
implements SkyFunction {
/** A strategy for completing the build. */
- interface Completor<ValueT, ResultT extends SkyValue> {
+ interface Completor<ValueT, ResultT extends SkyValue, KeyT extends TopLevelActionLookupKey> {
/**
* Returns the options which determine the artifacts to build for the top-level targets.
@@ -74,12 +77,12 @@
*/
/** Creates an event reporting an absent input artifact. */
- Event getRootCauseError(ValueT value, Cause rootCause, Environment env)
+ Event getRootCauseError(ValueT value, KeyT key, Cause rootCause, Environment env)
throws InterruptedException;
/** Creates an error message reporting {@code missingCount} missing input files. */
MissingInputFileException getMissingFilesException(
- ValueT value, int missingCount, Environment env) throws InterruptedException;
+ ValueT value, KeyT key, int missingCount, Environment env) throws InterruptedException;
/** Provides a successful completion value. */
ResultT getResult();
@@ -90,12 +93,12 @@
NestedSet<Cause> rootCauses,
NestedSet<ArtifactsInOutputGroup> outputs,
Environment env,
- TopLevelArtifactContext topLevelArtifactContext)
+ KeyT key)
throws InterruptedException;
/** Creates a succeeded completion value. */
ExtendedEventHandler.Postable createSucceeded(
- SkyKey skyKey,
+ KeyT skyKey,
ValueT value,
CompletionContext completionContext,
ArtifactsToBuild artifactsToBuild,
@@ -139,18 +142,19 @@
}
private final PathResolverFactory pathResolverFactory;
- private final Completor<ValueT, ResultT> completor;
+ private final Completor<ValueT, ResultT, KeyT> completor;
private final Supplier<Path> execRootSupplier;
CompletionFunction(
PathResolverFactory pathResolverFactory,
- Completor<ValueT, ResultT> completor,
+ Completor<ValueT, ResultT, KeyT> completor,
Supplier<Path> execRootSupplier) {
this.pathResolverFactory = pathResolverFactory;
this.completor = completor;
this.execRootSupplier = execRootSupplier;
}
+ @SuppressWarnings("unchecked") // Cast to KeyT
@Nullable
@Override
public SkyValue compute(SkyKey skyKey, Environment env)
@@ -161,7 +165,7 @@
return null;
}
- TopLevelActionLookupKey key = (TopLevelActionLookupKey) skyKey;
+ KeyT key = (KeyT) skyKey;
Pair<ValueT, ArtifactsToBuild> valueAndArtifactsToBuild = getValueAndArtifactsToBuild(key, env);
if (env.valuesMissing()) {
return null;
@@ -197,7 +201,7 @@
env.getListener().handle(Event.error(e.getLocation(), e.getMessage()));
Cause cause = new LabelCause(inputOwner, e.getMessage());
rootCausesBuilder.add(cause);
- env.getListener().handle(completor.getRootCauseError(value, cause, env));
+ env.getListener().handle(completor.getRootCauseError(value, key, cause, env));
}
} else {
builtArtifactsBuilder.add(input);
@@ -223,7 +227,7 @@
expandedFilesets.putAll(topLevelFilesets);
if (missingCount > 0) {
- missingInputException = completor.getMissingFilesException(value, missingCount, env);
+ missingInputException = completor.getMissingFilesException(value, key, missingCount, env);
if (missingInputException == null) {
return null;
}
@@ -236,8 +240,7 @@
builtArtifactsBuilder.build(), artifactsToBuild);
ExtendedEventHandler.Postable postable =
- completor.createFailed(
- value, rootCauses, builtOutputs, env, key.topLevelArtifactContext());
+ completor.createFailed(value, rootCauses, builtOutputs, env, key);
if (postable == null) {
return null;
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetValue.java
index b65aa13..c0f8f77 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetValue.java
@@ -14,6 +14,7 @@
package com.google.devtools.build.lib.skyframe;
+import com.google.devtools.build.lib.analysis.ConfiguredObjectValue;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
/**
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java
index f138d57..cbec840 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java
@@ -1116,12 +1116,10 @@
return Preconditions.checkNotNull(
(GlobValue) valueOrException.get(), "%s should not be missing", globKey)
.getMatches();
- } catch (BuildFileNotFoundException e) {
+ } catch (BuildFileNotFoundException | IOException e) {
// Legacy package loading is only able to handle an IOException, so a rethrow here is the
// best we can do.
throw new SkyframeGlobbingIOException(e);
- } catch (IOException e) {
- throw new SkyframeGlobbingIOException(e);
}
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java
index 22397e4..c74cde7 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java
@@ -671,7 +671,7 @@
@Override
public void clearAnalysisCache(
- Collection<ConfiguredTarget> topLevelTargets, Collection<AspectValue> topLevelAspects) {
+ Collection<ConfiguredTarget> topLevelTargets, ImmutableSet<AspectKey> topLevelAspects) {
discardPreExecutionCache(
topLevelTargets,
topLevelAspects,
@@ -810,7 +810,7 @@
actionGraphDump.dumpConfiguredTarget((ConfiguredTargetValue) skyValue);
} else if (functionName.equals(SkyFunctions.ASPECT)) {
AspectValue aspectValue = (AspectValue) skyValue;
- AspectKey aspectKey = aspectValue.getKey();
+ AspectKey aspectKey = (AspectKey) key;
ConfiguredTargetValue configuredTargetValue =
(ConfiguredTargetValue)
memoizingEvaluator.getExistingValue(aspectKey.getBaseConfiguredTargetKey());
@@ -842,7 +842,7 @@
actionGraphDump.dumpConfiguredTarget((ConfiguredTargetValue) skyValue);
} else if (functionName.equals(SkyFunctions.ASPECT)) {
AspectValue aspectValue = (AspectValue) skyValue;
- AspectKey aspectKey = aspectValue.getKey();
+ AspectKey aspectKey = (AspectKey) key;
ConfiguredTargetValue configuredTargetValue =
(ConfiguredTargetValue)
memoizingEvaluator.getExistingValue(aspectKey.getBaseConfiguredTargetKey());
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeAnalysisResult.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeAnalysisResult.java
index 75bda41..47e8806 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeAnalysisResult.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeAnalysisResult.java
@@ -14,11 +14,12 @@
package com.google.devtools.build.lib.skyframe;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
import com.google.devtools.build.lib.actions.PackageRoots;
-import com.google.devtools.build.lib.analysis.AspectValue;
+import com.google.devtools.build.lib.analysis.ConfiguredAspect;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
+import com.google.devtools.build.lib.skyframe.AspectValueKey.AspectKey;
import com.google.devtools.build.skyframe.WalkableGraph;
-import java.util.Collection;
/**
* Encapsulates the raw analysis result of top level targets and aspects coming from Skyframe.
@@ -29,7 +30,7 @@
private final boolean hasActionConflicts;
private final ImmutableList<ConfiguredTarget> configuredTargets;
private final WalkableGraph walkableGraph;
- private final ImmutableList<AspectValue> aspects;
+ private final ImmutableMap<AspectKey, ConfiguredAspect> aspects;
private final PackageRoots packageRoots;
SkyframeAnalysisResult(
@@ -38,7 +39,7 @@
boolean hasActionConflicts,
ImmutableList<ConfiguredTarget> configuredTargets,
WalkableGraph walkableGraph,
- ImmutableList<AspectValue> aspects,
+ ImmutableMap<AspectKey, ConfiguredAspect> aspects,
PackageRoots packageRoots) {
this.hasLoadingError = hasLoadingError;
this.hasAnalysisError = hasAnalysisError;
@@ -74,7 +75,7 @@
return walkableGraph;
}
- public Collection<AspectValue> getAspects() {
+ public ImmutableMap<AspectKey, ConfiguredAspect> getAspects() {
return aspects;
}
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 1fe6951..98bc456 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
@@ -40,6 +40,7 @@
import com.google.devtools.build.lib.analysis.AspectValue;
import com.google.devtools.build.lib.analysis.BlazeDirectories;
import com.google.devtools.build.lib.analysis.CachingAnalysisEnvironment;
+import com.google.devtools.build.lib.analysis.ConfiguredAspect;
import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
import com.google.devtools.build.lib.analysis.ConfiguredTargetFactory;
@@ -76,6 +77,7 @@
import com.google.devtools.build.lib.profiler.Profiler;
import com.google.devtools.build.lib.profiler.SilentCloseable;
import com.google.devtools.build.lib.skyframe.ArtifactConflictFinder.ConflictException;
+import com.google.devtools.build.lib.skyframe.AspectValueKey.AspectKey;
import com.google.devtools.build.lib.util.OrderedSetMultimap;
import com.google.devtools.build.lib.util.Pair;
import com.google.devtools.build.lib.vfs.Root;
@@ -359,7 +361,7 @@
* @see com.google.devtools.build.lib.analysis.AnalysisOptions#discardAnalysisCache
*/
public void clearAnalysisCache(
- Collection<ConfiguredTarget> topLevelTargets, Collection<AspectValue> topLevelAspects) {
+ Collection<ConfiguredTarget> topLevelTargets, ImmutableSet<AspectKey> topLevelAspects) {
// TODO(bazel-team): Consider clearing packages too to save more memory.
skyframeAnalysisWasDiscarded = true;
skyframeExecutor.clearAnalysisCache(topLevelTargets, topLevelAspects);
@@ -391,7 +393,7 @@
enableAnalysis(false);
}
- Collection<AspectValue> aspects = Lists.newArrayListWithCapacity(aspectKeys.size());
+ Map<AspectKey, ConfiguredAspect> aspects = Maps.newHashMapWithExpectedSize(aspectKeys.size());
Root singleSourceRoot = skyframeExecutor.getForcedSingleSourceRootIfNoExecrootSymlinkCreation();
NestedSetBuilder<Package> packages =
singleSourceRoot == null ? NestedSetBuilder.stableOrder() : null;
@@ -401,7 +403,7 @@
// Skip aspects that couldn't be applied to targets.
continue;
}
- aspects.add(value);
+ aspects.put(value.getKey(), value.getConfiguredAspect());
if (packages != null) {
packages.addTransitive(value.getTransitivePackagesForPackageRootResolution());
}
@@ -453,7 +455,7 @@
foundActionConflict,
ImmutableList.copyOf(cts),
result.getWalkableGraph(),
- ImmutableList.copyOf(aspects),
+ ImmutableMap.copyOf(aspects),
packageRoots);
}
@@ -525,8 +527,11 @@
aspects =
aspectKeys.stream()
.filter(errorFreePredicate)
- .map(k -> Preconditions.checkNotNull((AspectValue) result.get(k), k))
- .collect(toImmutableList());
+ .map(result::get)
+ .map(AspectValue.class::cast)
+ .collect(
+ ImmutableMap.toImmutableMap(
+ AspectValue::getKey, AspectValue::getConfiguredAspect));
}
return new SkyframeAnalysisResult(
@@ -535,7 +540,7 @@
foundActionConflict,
ImmutableList.copyOf(cts),
result.getWalkableGraph(),
- ImmutableList.copyOf(aspects),
+ ImmutableMap.copyOf(aspects),
packageRoots);
}
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 9d53ba4..915f58a 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
@@ -142,6 +142,7 @@
import com.google.devtools.build.lib.runtime.KeepGoingOption;
import com.google.devtools.build.lib.server.FailureDetails;
import com.google.devtools.build.lib.server.FailureDetails.FailureDetail;
+import com.google.devtools.build.lib.skyframe.AspectValueKey.AspectKey;
import com.google.devtools.build.lib.skyframe.DirtinessCheckerUtils.FileDirtinessChecker;
import com.google.devtools.build.lib.skyframe.ExternalFilesHelper.ExternalFileAction;
import com.google.devtools.build.lib.skyframe.FileFunction.NonexistentFileReceiver;
@@ -1013,7 +1014,7 @@
*/
protected void discardPreExecutionCache(
Collection<ConfiguredTarget> topLevelTargets,
- Collection<AspectValue> topLevelAspects,
+ ImmutableSet<AspectKey> topLevelAspects,
DiscardType discardType) {
if (discardType.discardsAnalysis()) {
topLevelTargets = ImmutableSet.copyOf(topLevelTargets);
@@ -1076,7 +1077,7 @@
}
// value may be null if target was not successfully analyzed.
if (aspectValue != null) {
- aspectValue.clear(!topLevelAspects.contains(aspectValue));
+ aspectValue.clear(!topLevelAspects.contains(key));
}
}
}
@@ -1090,7 +1091,7 @@
* graph.
*/
public abstract void clearAnalysisCache(
- Collection<ConfiguredTarget> topLevelTargets, Collection<AspectValue> topLevelAspects);
+ Collection<ConfiguredTarget> topLevelTargets, ImmutableSet<AspectKey> topLevelAspects);
protected abstract void dropConfiguredTargetsNow(final ExtendedEventHandler eventHandler);
@@ -1564,7 +1565,7 @@
Executor executor,
Set<Artifact> artifactsToBuild,
Collection<ConfiguredTarget> targetsToBuild,
- Collection<AspectValue> aspects,
+ ImmutableSet<AspectKey> aspects,
Set<ConfiguredTarget> parallelTests,
Set<ConfiguredTarget> exclusiveTests,
OptionsProvider options,
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkAspectFactory.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkAspectFactory.java
index 09cc299..1d152ec 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkAspectFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkAspectFactory.java
@@ -45,10 +45,9 @@
/** A factory for aspects that are defined in Starlark. */
public class SkylarkAspectFactory implements ConfiguredAspectFactory {
-
private final SkylarkDefinedAspect skylarkAspect;
- public SkylarkAspectFactory(SkylarkDefinedAspect skylarkAspect) {
+ SkylarkAspectFactory(SkylarkDefinedAspect skylarkAspect) {
this.skylarkAspect = skylarkAspect;
}
@@ -111,7 +110,7 @@
EvalUtils.getDataTypeName(aspectSkylarkObject)));
return null;
}
- return createAspect(aspectSkylarkObject, aspectDescriptor, ruleContext);
+ return createAspect(aspectSkylarkObject, ruleContext);
} catch (EvalException e) {
addAspectToStackTrace(ctadBase.getTarget(), e);
ruleContext.ruleError("\n" + e.print());
@@ -124,11 +123,10 @@
}
}
- private ConfiguredAspect createAspect(
- Object aspectSkylarkObject, AspectDescriptor aspectDescriptor, RuleContext ruleContext)
+ private static ConfiguredAspect createAspect(Object aspectSkylarkObject, RuleContext ruleContext)
throws EvalException, ActionConflictException {
- ConfiguredAspect.Builder builder = new ConfiguredAspect.Builder(aspectDescriptor, ruleContext);
+ ConfiguredAspect.Builder builder = new ConfiguredAspect.Builder(ruleContext);
if (aspectSkylarkObject instanceof Iterable) {
addDeclaredProviders(builder, (Iterable) aspectSkylarkObject);
@@ -165,7 +163,7 @@
return configuredAspect;
}
- private void addDeclaredProviders(
+ private static void addDeclaredProviders(
ConfiguredAspect.Builder builder, Iterable<?> aspectSkylarkObject) throws EvalException {
int i = 0;
for (Object o : aspectSkylarkObject) {
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TargetCompletor.java b/src/main/java/com/google/devtools/build/lib/skyframe/TargetCompletor.java
index 26611d9..76ad043 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/TargetCompletor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/TargetCompletor.java
@@ -18,7 +18,6 @@
import com.google.devtools.build.lib.actions.MissingInputFileException;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
import com.google.devtools.build.lib.analysis.TargetCompleteEvent;
-import com.google.devtools.build.lib.analysis.TopLevelArtifactContext;
import com.google.devtools.build.lib.analysis.TopLevelArtifactHelper.ArtifactsInOutputGroup;
import com.google.devtools.build.lib.analysis.TopLevelArtifactHelper.ArtifactsToBuild;
import com.google.devtools.build.lib.causes.Cause;
@@ -30,33 +29,33 @@
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.skyframe.SkyFunction;
import com.google.devtools.build.skyframe.SkyFunction.Environment;
-import com.google.devtools.build.skyframe.SkyKey;
import java.util.function.Supplier;
import javax.annotation.Nullable;
/** Manages completing builds for configured targets. */
-class TargetCompletor implements Completor<ConfiguredTargetValue, TargetCompletionValue> {
-
- public static SkyFunction targetCompletionFunction(
+class TargetCompletor
+ implements Completor<ConfiguredTargetValue, TargetCompletionValue, TargetCompletionKey> {
+ static SkyFunction targetCompletionFunction(
PathResolverFactory pathResolverFactory, Supplier<Path> execRootSupplier) {
return new CompletionFunction<>(pathResolverFactory, new TargetCompletor(), execRootSupplier);
}
@Override
- public Event getRootCauseError(ConfiguredTargetValue ctValue, Cause rootCause, Environment env)
+ public Event getRootCauseError(
+ ConfiguredTargetValue ctValue, TargetCompletionKey key, Cause rootCause, Environment env)
throws InterruptedException {
ConfiguredTargetAndData configuredTargetAndData =
ConfiguredTargetAndData.fromConfiguredTargetInSkyframe(ctValue.getConfiguredTarget(), env);
return Event.error(
configuredTargetAndData == null ? null : configuredTargetAndData.getTarget().getLocation(),
- String.format(
- "%s: missing input file '%s'", ctValue.getConfiguredTarget().getLabel(), rootCause));
+ String.format("%s: missing input file '%s'", key.actionLookupKey().getLabel(), rootCause));
}
@Override
@Nullable
public MissingInputFileException getMissingFilesException(
- ConfiguredTargetValue value, int missingCount, Environment env) throws InterruptedException {
+ ConfiguredTargetValue value, TargetCompletionKey key, int missingCount, Environment env)
+ throws InterruptedException {
ConfiguredTargetAndData configuredTargetAndData =
ConfiguredTargetAndData.fromConfiguredTargetInSkyframe(value.getConfiguredTarget(), env);
if (configuredTargetAndData == null) {
@@ -82,7 +81,7 @@
NestedSet<Cause> rootCauses,
NestedSet<ArtifactsInOutputGroup> outputs,
Environment env,
- TopLevelArtifactContext topLevelArtifactContext)
+ TargetCompletionKey key)
throws InterruptedException {
ConfiguredTarget target = value.getConfiguredTarget();
ConfiguredTargetAndData configuredTargetAndData =
@@ -96,7 +95,7 @@
@Override
@Nullable
public ExtendedEventHandler.Postable createSucceeded(
- SkyKey skyKey,
+ TargetCompletionKey skyKey,
ConfiguredTargetValue value,
CompletionContext completionContext,
ArtifactsToBuild artifactsToBuild,
@@ -108,7 +107,7 @@
if (configuredTargetAndData == null) {
return null;
}
- if (((TargetCompletionKey) skyKey.argument()).willTest()) {
+ if (skyKey.willTest()) {
return TargetCompleteEvent.successfulBuildSchedulingTest(
configuredTargetAndData,
completionContext,
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TopLevelActionLookupConflictFindingFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/TopLevelActionLookupConflictFindingFunction.java
index 7f77c62..3ff17d9 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/TopLevelActionLookupConflictFindingFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/TopLevelActionLookupConflictFindingFunction.java
@@ -17,6 +17,7 @@
import com.google.common.collect.Iterables;
import com.google.devtools.build.lib.actions.ActionLookupValue;
import com.google.devtools.build.lib.actions.ActionLookupValue.ActionLookupKey;
+import com.google.devtools.build.lib.analysis.ConfiguredObjectValue;
import com.google.devtools.build.lib.analysis.TopLevelArtifactContext;
import com.google.devtools.build.lib.analysis.TopLevelArtifactHelper;
import com.google.devtools.build.lib.cmdline.Label;
