Avoid trying to build a top-level target that has an action conflict in its transitive closure, even if that conflict happens in an action coming from an aspect.
This is more accurate than the previous scheme, which completely ignored aspects, but more tolerant in some cases: if the target that owns the conflicting action is actually not needed to build the top-level target, then the top-level target can successfully build, even if the target with the conflict is listed as a dep.
We also now detect action conflicts underneath top-level aspects instead of trying to build them and failing.
Overall, we will probably build less after this CL, since we will no longer try to build as much of the target as we can if an aspect caused a conflict. This may seem to violate the spirit of --keep_going, but that's already our behavior for target deps that caused conflicts, so our behavior is now more consistent.
In an effort to avoid code duplication and skew, the traversal starts with artifacts that are computed in CompletionFunction by calling the same utility method, so that the set of artifacts is always the same. To that end, I made the inheritance hierarchy of ConfiguredAspects and ConfiguredTargets more similar, as well as their completion keys. There's probably more convergence possible.
While writing this, caught that we didn't properly handle cycles underneath top-level aspects: fixed in the sequel, unknown commit.
This is effectively a rollback of the production side of https://github.com/bazelbuild/bazel/commit/46bfa2ed003fd6a550f89783efd2729e73236a7a, because we are detecting the conflict before the execution phase. The excellent test is kept and expanded on for the scenarios this fixes.
PiperOrigin-RevId: 303819009
diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionLookupValue.java b/src/main/java/com/google/devtools/build/lib/actions/ActionLookupValue.java
index 6af1c13..5e843aa 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/ActionLookupValue.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/ActionLookupValue.java
@@ -20,18 +20,18 @@
import com.google.devtools.build.skyframe.SkyValue;
import javax.annotation.Nullable;
-/** Base class for all values which can provide the generating action of an artifact. */
-public abstract class ActionLookupValue implements SkyValue {
+/** Base interface for all values which can provide the generating action of an artifact. */
+public interface ActionLookupValue extends SkyValue {
/** Returns a list of actions registered by this {@link SkyValue}. */
- public abstract ImmutableList<ActionAnalysisMetadata> getActions();
+ ImmutableList<ActionAnalysisMetadata> getActions();
/**
* Returns the {@link Action} with index {@code index} in this value. Never null. Should only be
* called during action execution by {@code ArtifactFunction} and {@code ActionExecutionFunction}
* -- after an action has executed, calling this with its index may crash.
*/
- public Action getAction(int index) {
+ default Action getAction(int index) {
ActionAnalysisMetadata result = getActions().get(index);
// Avoid Preconditions.checkState which would box the int arg.
if (!(result instanceof Action)) {
@@ -40,7 +40,7 @@
return (Action) result;
}
- public ActionTemplate<?> getActionTemplate(int index) {
+ default ActionTemplate<?> getActionTemplate(int index) {
ActionAnalysisMetadata result = getActions().get(index);
// Avoid Preconditions.checkState which would box the int arg.
if (!(result instanceof ActionTemplate)) {
@@ -51,13 +51,13 @@
}
/** Returns the number of {@link Action} objects present in this value. */
- public int getNumActions() {
+ default int getNumActions() {
return getActions().size();
}
/** Returns a source artifact if the underlying configured target is an input file. */
@Nullable
- public SourceArtifact getSourceArtifact() {
+ default SourceArtifact getSourceArtifact() {
return null;
}
@@ -66,7 +66,7 @@
* subclasses of ActionLookupKey. This allows callers to easily find the value key, while
* remaining agnostic to what ActionLookupValues actually exist.
*/
- public abstract static class ActionLookupKey implements ArtifactOwner, SkyKey {
+ abstract class ActionLookupKey implements ArtifactOwner, SkyKey {
@Override
public Label getLabel() {
return null;
diff --git a/src/main/java/com/google/devtools/build/lib/actions/BasicActionLookupValue.java b/src/main/java/com/google/devtools/build/lib/actions/BasicActionLookupValue.java
index 06d511a..dc3dd4a 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/BasicActionLookupValue.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/BasicActionLookupValue.java
@@ -19,10 +19,10 @@
import com.google.common.collect.ImmutableList;
/**
- * Basic implementation of {@link ActionLookupValue} where the value itself owns and maintains
- * the list of generating actions.
+ * Basic implementation of {@link ActionLookupValue} where the value itself owns and maintains the
+ * list of generating actions.
*/
-public class BasicActionLookupValue extends ActionLookupValue {
+public class BasicActionLookupValue implements ActionLookupValue {
protected final ImmutableList<ActionAnalysisMetadata> actions;
protected BasicActionLookupValue(ImmutableList<ActionAnalysisMetadata> actions) {
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 f2ff198..28040ba 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
@@ -406,6 +406,7 @@
topLevelCtKeys,
aspectKeys,
Suppliers.memoize(configurationLookupSupplier),
+ topLevelOptions,
eventBus,
keepGoing,
loadingPhaseThreads);
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 696e930..05e0321 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
@@ -16,7 +16,6 @@
import static com.google.devtools.build.lib.analysis.ExtraActionUtils.createExtraActionProvider;
-import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
@@ -38,7 +37,6 @@
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.packages.SkylarkProviderIdentifier;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
import com.google.devtools.build.lib.syntax.EvalException;
import java.util.Arrays;
@@ -63,7 +61,7 @@
*/
@Immutable
@AutoCodec
-public final class ConfiguredAspect {
+public final class ConfiguredAspect implements ProviderCollection {
private final AspectDescriptor descriptor;
private final ImmutableList<ActionAnalysisMetadata> actions;
private final TransitiveInfoProviderMap providers;
@@ -109,25 +107,19 @@
return providers;
}
+ @Override
@Nullable
- @VisibleForTesting
public <P extends TransitiveInfoProvider> P getProvider(Class<P> providerClass) {
AnalysisUtils.checkProvider(providerClass);
return providers.getProvider(providerClass);
}
- public Object getProvider(SkylarkProviderIdentifier id) {
- if (id.isLegacy()) {
- return get(id.getLegacyId());
- } else {
- return get(id.getKey());
- }
- }
-
+ @Override
public Info get(Provider.Key key) {
return providers.get(key);
}
+ @Override
public Object get(String legacyKey) {
if (OutputGroupInfo.SKYLARK_NAME.equals(legacyKey)) {
return get(OutputGroupInfo.SKYLARK_CONSTRUCTOR.getKey());
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 4e6c4eb..4d838a5 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
@@ -706,7 +706,7 @@
}
for (SkylarkProviderIdentifier providerId : advertisedProviders.getSkylarkProviders()) {
- if (configuredAspect.getProvider(providerId) == null) {
+ if (configuredAspect.get(providerId) == null) {
eventHandler.handle(Event.error(
target.getLocation(),
String.format(
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/OutputGroupInfo.java b/src/main/java/com/google/devtools/build/lib/analysis/OutputGroupInfo.java
index 41bdca1..a12e78b 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/OutputGroupInfo.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/OutputGroupInfo.java
@@ -144,16 +144,10 @@
}
@Nullable
- public static OutputGroupInfo get(TransitiveInfoCollection collection) {
- return collection.get(OutputGroupInfo.SKYLARK_CONSTRUCTOR);
+ public static OutputGroupInfo get(ProviderCollection collection) {
+ return collection.get(SKYLARK_CONSTRUCTOR);
}
- @Nullable
- public static OutputGroupInfo get(ConfiguredAspect aspect) {
- return (OutputGroupInfo) aspect.get(SKYLARK_CONSTRUCTOR.getKey());
- }
-
-
/** Return the artifacts in a particular output group.
*
* @return the artifacts in the output group with the given name. The return value is never null.
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 a924050..2ca4785 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
@@ -179,15 +179,15 @@
/**
* Returns all artifacts to build if this target is requested as a top-level target. The resulting
- * set includes the temps and either the files to compile, if
- * {@code context.compileOnly() == true}, or the files to run.
+ * set includes the temps and either the files to compile, if {@code context.compileOnly() ==
+ * true}, or the files to run.
*
* <p>Calls to this method should generally return quickly; however, the runfiles computation can
* be lazy, in which case it can be expensive on the first call. Subsequent calls may or may not
* return the same {@code Iterable} instance.
*/
- public static ArtifactsToBuild getAllArtifactsToBuild(TransitiveInfoCollection target,
- TopLevelArtifactContext context) {
+ public static ArtifactsToBuild getAllArtifactsToBuild(
+ ProviderCollection target, TopLevelArtifactContext context) {
return getAllArtifactsToBuild(
OutputGroupInfo.get(target),
target.getProvider(FileProvider.class),
@@ -197,11 +197,7 @@
public static ArtifactsToBuild getAllArtifactsToBuild(
AspectValue aspectValue, TopLevelArtifactContext context) {
- ConfiguredAspect configuredAspect = aspectValue.getConfiguredAspect();
- return getAllArtifactsToBuild(
- OutputGroupInfo.get(configuredAspect),
- configuredAspect.getProvider(FileProvider.class),
- context);
+ return getAllArtifactsToBuild(aspectValue.getConfiguredAspect(), context);
}
static ArtifactsToBuild getAllArtifactsToBuild(
diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionProgressReceiver.java b/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionProgressReceiver.java
index c2f9653..a8b4ddb 100644
--- a/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionProgressReceiver.java
+++ b/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionProgressReceiver.java
@@ -108,12 +108,11 @@
SkyFunctionName type = skyKey.functionName();
if (type.equals(SkyFunctions.TARGET_COMPLETION)) {
if (evaluationSuccessState.get().succeeded()) {
- builtTargets.add(
- ((TargetCompletionValue.TargetCompletionKey) skyKey).configuredTargetKey());
+ builtTargets.add(((TargetCompletionValue.TargetCompletionKey) skyKey).actionLookupKey());
}
} else if (type.equals(SkyFunctions.ASPECT_COMPLETION)) {
if (evaluationSuccessState.get().succeeded()) {
- builtAspects.add(((AspectCompletionValue.AspectCompletionKey) skyKey).aspectKey());
+ builtAspects.add(((AspectCompletionValue.AspectCompletionKey) skyKey).actionLookupKey());
}
} else if (type.equals(SkyFunctions.ACTION_EXECUTION)) {
// Remember all completed actions, even those in error, regardless of having been cached or
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionArtifactCycleReporter.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionArtifactCycleReporter.java
index 37e38e3..9efc6b8 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionArtifactCycleReporter.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionArtifactCycleReporter.java
@@ -59,7 +59,7 @@
return "action from: " + arg;
} else if (arg instanceof TargetCompletionKey
&& skyFunctionName.equals(SkyFunctions.TARGET_COMPLETION)) {
- return "configured target: " + ((TargetCompletionKey) arg).configuredTargetKey().getLabel();
+ return "configured target: " + ((TargetCompletionKey) arg).actionLookupKey().getLabel();
} else if (arg instanceof TestCompletionKey
&& skyFunctionName.equals(SkyFunctions.TEST_COMPLETION)) {
return "test target: " + ((TestCompletionKey) arg).configuredTargetKey().getLabel();
@@ -77,7 +77,7 @@
return ((ActionLookupData) arg).getLabel();
} else if (arg instanceof TargetCompletionKey
&& key.functionName().equals(SkyFunctions.TARGET_COMPLETION)) {
- return ((TargetCompletionKey) arg).configuredTargetKey().getLabel();
+ return ((TargetCompletionKey) arg).actionLookupKey().getLabel();
} else if (arg instanceof TestCompletionKey
&& key.functionName().equals(SkyFunctions.TEST_COMPLETION)) {
return ((TestCompletionKey) arg).configuredTargetKey().getLabel();
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionLookupConflictFindingFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionLookupConflictFindingFunction.java
new file mode 100644
index 0000000..fc52a2a
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionLookupConflictFindingFunction.java
@@ -0,0 +1,85 @@
+// Copyright 2014 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.google.devtools.build.lib.skyframe;
+
+import com.google.common.collect.ImmutableMap;
+import com.google.devtools.build.lib.actions.ActionAnalysisMetadata;
+import com.google.devtools.build.lib.actions.ActionLookupValue;
+import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.collect.compacthashset.CompactHashSet;
+import com.google.devtools.build.lib.collect.nestedset.NestedSet;
+import com.google.devtools.build.lib.skyframe.SkyframeActionExecutor.ConflictException;
+import com.google.devtools.build.skyframe.SkyFunction;
+import com.google.devtools.build.skyframe.SkyFunctionException;
+import com.google.devtools.build.skyframe.SkyKey;
+import com.google.devtools.build.skyframe.SkyValue;
+import java.util.Set;
+import java.util.stream.Stream;
+import javax.annotation.Nullable;
+
+/** Check all transitive actions of an {@link ActionLookupValue} for action conflicts. */
+public class ActionLookupConflictFindingFunction implements SkyFunction {
+ ActionLookupConflictFindingFunction() {}
+
+ @Nullable
+ @Override
+ public SkyValue compute(SkyKey skyKey, Environment env)
+ throws SkyFunctionException, InterruptedException {
+ ImmutableMap<ActionAnalysisMetadata, ConflictException> badActions =
+ PrecomputedValue.BAD_ACTIONS.get(env);
+ ActionLookupValue alValue =
+ (ActionLookupValue)
+ env.getValue(((ActionLookupConflictFindingValue.Key) skyKey).argument());
+ if (env.valuesMissing()) {
+ return null;
+ }
+
+ Set<ActionLookupConflictFindingValue.Key> depKeys = CompactHashSet.create();
+ for (ActionAnalysisMetadata action : alValue.getActions()) {
+ if (badActions.containsKey(action)) {
+ throw new ActionConflictFunctionException(badActions.get(action));
+ }
+ convertArtifacts(action.getInputs()).forEach(depKeys::add);
+ }
+ // Avoid silly cycles.
+ depKeys.remove(skyKey);
+
+ env.getValues(depKeys);
+ return env.valuesMissing() ? null : ActionLookupConflictFindingValue.INSTANCE;
+ }
+
+ static Stream<ActionLookupConflictFindingValue.Key> convertArtifacts(
+ NestedSet<Artifact> artifacts) {
+ return artifacts.toList().stream()
+ .filter(a -> !a.isSourceArtifact())
+ .map(a -> (Artifact.DerivedArtifact) a)
+ .map(
+ a ->
+ ActionLookupConflictFindingValue.key(
+ a.getGeneratingActionKey().getActionLookupKey()));
+ }
+
+ @Nullable
+ @Override
+ public String extractTag(SkyKey skyKey) {
+ return Label.print(((ConfiguredTargetKey) skyKey.argument()).getLabel());
+ }
+
+ private static class ActionConflictFunctionException extends SkyFunctionException {
+ ActionConflictFunctionException(ConflictException e) {
+ super(e, Transience.PERSISTENT);
+ }
+ }
+}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionLookupConflictFindingValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionLookupConflictFindingValue.java
new file mode 100644
index 0000000..8b03eb1
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionLookupConflictFindingValue.java
@@ -0,0 +1,67 @@
+// Copyright 2014 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.google.devtools.build.lib.skyframe;
+
+import static com.google.common.collect.ImmutableList.toImmutableList;
+import static com.google.common.collect.Streams.stream;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Interner;
+import com.google.devtools.build.lib.actions.ActionLookupValue;
+import com.google.devtools.build.lib.concurrent.BlazeInterners;
+import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
+import com.google.devtools.build.skyframe.AbstractSkyKey;
+import com.google.devtools.build.skyframe.SkyFunctionName;
+import com.google.devtools.build.skyframe.SkyKey;
+import com.google.devtools.build.skyframe.SkyValue;
+
+/**
+ * A marker for an {@link ActionLookupValue} which is known to be transitively error-free from
+ * action conflict issues.
+ */
+class ActionLookupConflictFindingValue implements SkyValue {
+ @AutoCodec
+ static final ActionLookupConflictFindingValue INSTANCE = new ActionLookupConflictFindingValue();
+
+ private ActionLookupConflictFindingValue() {}
+
+ public static ImmutableList<SkyKey> keys(Iterable<ActionLookupValue.ActionLookupKey> lookupKeys) {
+ return stream(lookupKeys).map(Key::create).collect(toImmutableList());
+ }
+
+ public static Key key(ActionLookupValue.ActionLookupKey lookupKey) {
+ return Key.create(lookupKey);
+ }
+
+ @AutoCodec.VisibleForSerialization
+ @AutoCodec
+ static class Key extends AbstractSkyKey<ActionLookupValue.ActionLookupKey> {
+ private static final Interner<Key> interner = BlazeInterners.newWeakInterner();
+
+ private Key(ActionLookupValue.ActionLookupKey arg) {
+ super(arg);
+ }
+
+ @AutoCodec.VisibleForSerialization
+ @AutoCodec.Instantiator
+ static Key create(ActionLookupValue.ActionLookupKey arg) {
+ return interner.intern(new Key(arg));
+ }
+
+ @Override
+ public SkyFunctionName functionName() {
+ return SkyFunctions.ACTION_LOOKUP_CONFLICT_FINDING;
+ }
+ }
+}
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 64066cf..a31bf58 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
@@ -17,6 +17,7 @@
import com.google.common.collect.Iterables;
import com.google.devtools.build.lib.analysis.TopLevelArtifactContext;
import com.google.devtools.build.lib.skyframe.AspectValue.AspectKey;
+import com.google.devtools.build.lib.skyframe.CompletionFunction.TopLevelActionLookupKey;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
import com.google.devtools.build.skyframe.SkyFunctionName;
import com.google.devtools.build.skyframe.SkyKey;
@@ -39,15 +40,15 @@
/** The key of an AspectCompletionValue. */
@AutoValue
- public abstract static class AspectCompletionKey implements SkyKey {
+ public abstract static class AspectCompletionKey implements TopLevelActionLookupKey {
public static AspectCompletionKey create(
AspectKey aspectKey, TopLevelArtifactContext topLevelArtifactContext) {
return new AutoValue_AspectCompletionValue_AspectCompletionKey(
- aspectKey, topLevelArtifactContext);
+ topLevelArtifactContext, aspectKey);
}
- public abstract AspectKey aspectKey();
- public abstract TopLevelArtifactContext topLevelArtifactContext();
+ @Override
+ public abstract AspectKey actionLookupKey();
@Override
public SkyFunctionName functionName() {
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/AspectValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/AspectValue.java
index d3b133f..9199533 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/AspectValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/AspectValue.java
@@ -21,6 +21,7 @@
import com.google.common.collect.Interner;
import com.google.devtools.build.lib.actions.BasicActionLookupValue;
import com.google.devtools.build.lib.analysis.ConfiguredAspect;
+import com.google.devtools.build.lib.analysis.ProviderCollection;
import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
@@ -38,7 +39,7 @@
import javax.annotation.Nullable;
/** An aspect in the context of the Skyframe graph. */
-public final class AspectValue extends BasicActionLookupValue {
+public final class AspectValue extends BasicActionLookupValue implements ConfiguredObjectValue {
/**
* A base class for keys that have AspectValue as a Sky value.
*/
@@ -481,7 +482,8 @@
return Preconditions.checkNotNull(aspect);
}
- void clear(boolean clearEverything) {
+ @Override
+ public void clear(boolean clearEverything) {
Preconditions.checkNotNull(label, this);
Preconditions.checkNotNull(aspect, this);
Preconditions.checkNotNull(location, this);
@@ -497,12 +499,7 @@
transitivePackagesForPackageRootResolution = null;
}
- /**
- * Returns the set of packages transitively loaded by this value. Must only be used for
- * constructing the package -> source root map needed for some builds. If the caller has not
- * specified that this map needs to be constructed (via the constructor argument in {@link
- * AspectFunction#AspectFunction}), calling this will crash.
- */
+ @Override
public NestedSet<Package> getTransitivePackagesForPackageRootResolution() {
return Preconditions.checkNotNull(transitivePackagesForPackageRootResolution);
}
@@ -518,6 +515,11 @@
.toString();
}
+ @Override
+ public ProviderCollection getConfiguredObject() {
+ return getConfiguredAspect();
+ }
+
public static AspectKey createAspectKey(
Label label,
BuildConfiguration baseConfiguration,
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 a0bf2e6..ff58a54 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
@@ -18,6 +18,7 @@
import com.google.common.collect.Streams;
import com.google.devtools.build.lib.actions.ActionExecutionException;
import com.google.devtools.build.lib.actions.ActionInputMap;
+import com.google.devtools.build.lib.actions.ActionLookupValue.ActionLookupKey;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.CompletionContext;
import com.google.devtools.build.lib.actions.CompletionContext.PathResolverFactory;
@@ -39,9 +40,8 @@
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.events.ExtendedEventHandler;
import com.google.devtools.build.lib.skyframe.ArtifactFunction.MissingFileArtifactValue;
-import com.google.devtools.build.lib.skyframe.AspectCompletionValue.AspectCompletionKey;
-import com.google.devtools.build.lib.skyframe.AspectValue.AspectKey;
import com.google.devtools.build.lib.skyframe.TargetCompletionValue.TargetCompletionKey;
+import com.google.devtools.build.lib.util.Pair;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.skyframe.SkyFunction;
import com.google.devtools.build.skyframe.SkyFunctionException;
@@ -57,14 +57,12 @@
import javax.annotation.Nullable;
/** CompletionFunction builds the artifactsToBuild collection of a {@link ConfiguredTarget}. */
-public final class CompletionFunction<TValue extends SkyValue, TResult extends SkyValue>
+public final class CompletionFunction<
+ ValueT extends ConfiguredObjectValue, ResultT extends SkyValue>
implements SkyFunction {
/** A strategy for completing the build. */
- interface Completor<TValue, TResult extends SkyValue> {
-
- /** Obtains an analysis result value from environment. */
- TValue getValueFromSkyKey(SkyKey skyKey, Environment env) throws InterruptedException;
+ interface Completor<ValueT, ResultT extends SkyValue> {
/**
* Returns the options which determine the artifacts to build for the top-level targets.
@@ -78,25 +76,21 @@
* earlier runs, instead of causing an eager invalidation were the TopLevelArtifactContext
* modeled as a separate SkyKey.
*/
- TopLevelArtifactContext getTopLevelArtifactContext(SkyKey skyKey);
-
- /** Returns all artifacts that need to be built to complete the {@code value} */
- ArtifactsToBuild getAllArtifactsToBuild(TValue value, TopLevelArtifactContext context);
/** Creates an event reporting an absent input artifact. */
- Event getRootCauseError(TValue value, Cause rootCause, Environment env)
+ Event getRootCauseError(ValueT value, Cause rootCause, Environment env)
throws InterruptedException;
/** Creates an error message reporting {@code missingCount} missing input files. */
MissingInputFileException getMissingFilesException(
- TValue value, int missingCount, Environment env) throws InterruptedException;
+ ValueT value, int missingCount, Environment env) throws InterruptedException;
/** Provides a successful completion value. */
- TResult getResult();
+ ResultT getResult();
/** Creates a failed completion value. */
ExtendedEventHandler.Postable createFailed(
- TValue value,
+ ValueT value,
NestedSet<Cause> rootCauses,
NestedSet<ArtifactsInOutputGroup> outputs,
Environment env,
@@ -106,39 +100,21 @@
/** Creates a succeeded completion value. */
ExtendedEventHandler.Postable createSucceeded(
SkyKey skyKey,
- TValue value,
+ ValueT value,
CompletionContext completionContext,
- TopLevelArtifactContext topLevelArtifactContext,
+ ArtifactsToBuild artifactsToBuild,
Environment env)
throws InterruptedException;
+ }
- /** Extracts a tag given the {@link SkyKey}. */
- String extractTag(SkyKey skyKey);
+ interface TopLevelActionLookupKey extends SkyKey {
+ ActionLookupKey actionLookupKey();
+
+ TopLevelArtifactContext topLevelArtifactContext();
}
private static class TargetCompletor
implements Completor<ConfiguredTargetValue, TargetCompletionValue> {
-
- @Override
- public ConfiguredTargetValue getValueFromSkyKey(SkyKey skyKey, Environment env)
- throws InterruptedException {
- TargetCompletionKey tcKey = (TargetCompletionKey) skyKey.argument();
- return (ConfiguredTargetValue) env.getValue(tcKey.configuredTargetKey());
- }
-
- @Override
- public TopLevelArtifactContext getTopLevelArtifactContext(SkyKey skyKey) {
- TargetCompletionKey tcKey = (TargetCompletionKey) skyKey.argument();
- return tcKey.topLevelArtifactContext();
- }
-
- @Override
- public ArtifactsToBuild getAllArtifactsToBuild(
- ConfiguredTargetValue value, TopLevelArtifactContext topLevelContext) {
- return TopLevelArtifactHelper.getAllArtifactsToBuild(
- value.getConfiguredTarget(), topLevelContext);
- }
-
@Override
public Event getRootCauseError(ConfiguredTargetValue ctValue, Cause rootCause, Environment env)
throws InterruptedException {
@@ -195,18 +171,12 @@
}
@Override
- public String extractTag(SkyKey skyKey) {
- return Label.print(
- ((TargetCompletionKey) skyKey.argument()).configuredTargetKey().getLabel());
- }
-
- @Override
@Nullable
public ExtendedEventHandler.Postable createSucceeded(
SkyKey skyKey,
ConfiguredTargetValue value,
CompletionContext completionContext,
- TopLevelArtifactContext topLevelArtifactContext,
+ ArtifactsToBuild artifactsToBuild,
Environment env)
throws InterruptedException {
ConfiguredTarget target = value.getConfiguredTarget();
@@ -215,7 +185,6 @@
if (configuredTargetAndData == null) {
return null;
}
- ArtifactsToBuild artifactsToBuild = getAllArtifactsToBuild(value, topLevelArtifactContext);
if (((TargetCompletionKey) skyKey.argument()).willTest()) {
return TargetCompleteEvent.successfulBuildSchedulingTest(
configuredTargetAndData,
@@ -232,26 +201,6 @@
private static class AspectCompletor implements Completor<AspectValue, AspectCompletionValue> {
@Override
- public AspectValue getValueFromSkyKey(SkyKey skyKey, Environment env)
- throws InterruptedException {
- AspectCompletionKey acKey = (AspectCompletionKey) skyKey.argument();
- AspectKey aspectKey = acKey.aspectKey();
- return (AspectValue) env.getValue(aspectKey);
- }
-
- @Override
- public TopLevelArtifactContext getTopLevelArtifactContext(SkyKey skyKey) {
- AspectCompletionKey acKey = (AspectCompletionKey) skyKey.argument();
- return acKey.topLevelArtifactContext();
- }
-
- @Override
- public ArtifactsToBuild getAllArtifactsToBuild(
- AspectValue value, TopLevelArtifactContext topLevelArtifactContext) {
- return TopLevelArtifactHelper.getAllArtifactsToBuild(value, topLevelArtifactContext);
- }
-
- @Override
public Event getRootCauseError(AspectValue value, Cause rootCause, Environment env) {
return Event.error(
value.getLocation(),
@@ -292,11 +241,6 @@
return AspectCompleteEvent.createFailed(value, rootCauses, configurationEventId, outputs);
}
- @Override
- public String extractTag(SkyKey skyKey) {
- return Label.print(((AspectCompletionKey) skyKey.argument()).aspectKey().getLabel());
- }
-
@Nullable
private BuildEventId getConfigurationEventIdFromAspectValue(AspectValue value, Environment env)
throws InterruptedException {
@@ -318,18 +262,19 @@
SkyKey skyKey,
AspectValue value,
CompletionContext completionContext,
- TopLevelArtifactContext topLevelArtifactContext,
+ ArtifactsToBuild artifactsToBuild,
Environment env)
throws InterruptedException {
- ArtifactsToBuild artifacts = getAllArtifactsToBuild(value, topLevelArtifactContext);
-
BuildEventId configurationEventId = getConfigurationEventIdFromAspectValue(value, env);
if (configurationEventId == null) {
return null;
}
return AspectCompleteEvent.createSuccessful(
- value, completionContext, artifacts.getAllArtifactsByOutputGroup(), configurationEventId);
+ value,
+ completionContext,
+ artifactsToBuild.getAllArtifactsByOutputGroup(),
+ configurationEventId);
}
}
@@ -373,12 +318,12 @@
}
private final PathResolverFactory pathResolverFactory;
- private final Completor<TValue, TResult> completor;
+ private final Completor<ValueT, ResultT> completor;
private final Supplier<Path> execRootSupplier;
private CompletionFunction(
PathResolverFactory pathResolverFactory,
- Completor<TValue, TResult> completor,
+ Completor<ValueT, ResultT> completor,
Supplier<Path> execRootSupplier) {
this.pathResolverFactory = pathResolverFactory;
this.completor = completor;
@@ -395,14 +340,15 @@
return null;
}
- TValue value = completor.getValueFromSkyKey(skyKey, env);
- TopLevelArtifactContext topLevelContext = completor.getTopLevelArtifactContext(skyKey);
+ TopLevelActionLookupKey key = (TopLevelActionLookupKey) skyKey;
+ Pair<ValueT, ArtifactsToBuild> valueAndArtifactsToBuild = getValueAndArtifactsToBuild(key, env);
if (env.valuesMissing()) {
return null;
}
+ ValueT value = valueAndArtifactsToBuild.first;
+ ArtifactsToBuild artifactsToBuild = valueAndArtifactsToBuild.second;
// Avoid iterating over nested set twice.
- ArtifactsToBuild artifactsToBuild = completor.getAllArtifactsToBuild(value, topLevelContext);
ImmutableList<Artifact> allArtifacts = artifactsToBuild.getAllArtifacts().toList();
Map<SkyKey, ValueOrException<ActionExecutionException>> inputDeps =
env.getValuesOrThrow(Artifact.keys(allArtifacts), ActionExecutionException.class);
@@ -469,7 +415,8 @@
builtArtifactsBuilder.build(), artifactsToBuild);
ExtendedEventHandler.Postable postable =
- completor.createFailed(value, rootCauses, builtOutputs, env, topLevelContext);
+ completor.createFailed(
+ value, rootCauses, builtOutputs, env, key.topLevelArtifactContext());
if (postable == null) {
return null;
}
@@ -494,7 +441,7 @@
CompletionContext.create(
expandedArtifacts,
expandedFilesets,
- topLevelContext.expandFilesets(),
+ key.topLevelArtifactContext().expandFilesets(),
inputMap,
pathResolverFactory,
execRootSupplier.get(),
@@ -504,7 +451,7 @@
}
ExtendedEventHandler.Postable postable =
- completor.createSucceeded(skyKey, value, ctx, topLevelContext, env);
+ completor.createSucceeded(key, value, ctx, artifactsToBuild, env);
if (postable == null) {
return null;
}
@@ -512,9 +459,25 @@
return completor.getResult();
}
+ @Nullable
+ static <ValueT extends ConfiguredObjectValue>
+ Pair<ValueT, ArtifactsToBuild> getValueAndArtifactsToBuild(
+ TopLevelActionLookupKey key, Environment env) throws InterruptedException {
+ @SuppressWarnings("unchecked")
+ ValueT value = (ValueT) env.getValue(key.actionLookupKey());
+ if (env.valuesMissing()) {
+ return null;
+ }
+
+ TopLevelArtifactContext topLevelContext = key.topLevelArtifactContext();
+ ArtifactsToBuild artifactsToBuild =
+ TopLevelArtifactHelper.getAllArtifactsToBuild(value.getConfiguredObject(), topLevelContext);
+ return Pair.of(value, artifactsToBuild);
+ }
+
@Override
public String extractTag(SkyKey skyKey) {
- return completor.extractTag(skyKey);
+ return Label.print(((TopLevelActionLookupKey) skyKey).actionLookupKey().getLabel());
}
private static final class CompletionFunctionException extends SkyFunctionException {
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredObjectValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredObjectValue.java
new file mode 100644
index 0000000..5318221
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredObjectValue.java
@@ -0,0 +1,44 @@
+// Copyright 2020 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.google.devtools.build.lib.skyframe;
+
+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.skyframe.NotComparableSkyValue;
+
+/** Super-interface for {@link ConfiguredTargetValue} and {@link AspectValue}. */
+interface ConfiguredObjectValue extends ActionLookupValue, NotComparableSkyValue {
+ /** Returns the configured target/aspect for this value. */
+ ProviderCollection getConfiguredObject();
+
+ /**
+ * Returns the set of packages transitively loaded by this value. Must only be used for
+ * constructing the package -> source root map needed for some builds. If the caller has not
+ * specified that this map needs to be constructed (via the constructor argument in {@link
+ * ConfiguredTargetFunction#ConfiguredTargetFunction} or {@link AspectFunction#AspectFunction}),
+ * calling this will crash.
+ */
+ NestedSet<Package> getTransitivePackagesForPackageRootResolution();
+
+ /**
+ * Clears provider data from this value, leaving only the artifact->generating action map.
+ *
+ * <p>Should only be used when user specifies --discard_analysis_cache. Must be called at most
+ * once per value, after which {@link #getConfiguredObject} and {@link #getActions} cannot be
+ * called.
+ */
+ void clear(boolean clearEverything);
+}
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 ab43534..3512403 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,52 +14,23 @@
package com.google.devtools.build.lib.skyframe;
-import com.google.common.collect.ImmutableList;
-import com.google.devtools.build.lib.actions.Action;
-import com.google.devtools.build.lib.actions.ActionAnalysisMetadata;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
import com.google.devtools.build.lib.cmdline.Label;
-import com.google.devtools.build.lib.collect.nestedset.NestedSet;
-import com.google.devtools.build.lib.packages.Package;
-import com.google.devtools.build.skyframe.NotComparableSkyValue;
import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.SkyValue;
/** A {@link SkyValue} for a {@link ConfiguredTarget}. */
-public interface ConfiguredTargetValue extends NotComparableSkyValue {
+public interface ConfiguredTargetValue extends ConfiguredObjectValue {
static SkyKey key(Label label, BuildConfiguration configuration) {
return ConfiguredTargetKey.of(label, configuration);
}
- /**
- * Returns the configured target for this value.
- */
+ /** Returns the configured target for this value. */
ConfiguredTarget getConfiguredTarget();
- /**
- * Returns the set of packages transitively loaded by this value. Must only be used for
- * constructing the package -> source root map needed for some builds. If the caller has not
- * specified that this map needs to be constructed (via the constructor argument in {@link
- * ConfiguredTargetFunction#ConfiguredTargetFunction}), calling this will crash.
- */
- NestedSet<Package> getTransitivePackagesForPackageRootResolution();
-
- /** Returns the actions registered by the configured target for this value. */
- ImmutableList<ActionAnalysisMetadata> getActions();
-
- /**
- * Returns the number of {@link Action} objects present in this value.
- */
- int getNumActions();
-
- /**
- * Clears configured target data from this value, leaving only the artifact->generating action
- * map.
- *
- * <p>Should only be used when user specifies --discard_analysis_cache. Must be called at most
- * once per value, after which {@link #getConfiguredTarget} and {@link #getActions} cannot be
- * called.
- */
- void clear(boolean clearEverything);
+ @Override
+ default ConfiguredTarget getConfiguredObject() {
+ return getConfiguredTarget();
+ }
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PostConfiguredTargetFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/PostConfiguredTargetFunction.java
deleted file mode 100644
index 82138c6..0000000
--- a/src/main/java/com/google/devtools/build/lib/skyframe/PostConfiguredTargetFunction.java
+++ /dev/null
@@ -1,230 +0,0 @@
-// Copyright 2014 The Bazel Authors. All rights reserved.
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-package com.google.devtools.build.lib.skyframe;
-
-import static com.google.devtools.build.lib.skyframe.ConfiguredTargetFunction.getExecutionPlatformConstraints;
-
-import com.google.common.base.Function;
-import com.google.common.base.Preconditions;
-import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.Iterables;
-import com.google.devtools.build.lib.actions.ActionAnalysisMetadata;
-import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider;
-import com.google.devtools.build.lib.analysis.ConfiguredTarget;
-import com.google.devtools.build.lib.analysis.Dependency;
-import com.google.devtools.build.lib.analysis.DependencyResolver.DependencyKind;
-import com.google.devtools.build.lib.analysis.DependencyResolver.InconsistentAspectOrderException;
-import com.google.devtools.build.lib.analysis.PlatformConfiguration;
-import com.google.devtools.build.lib.analysis.TargetAndConfiguration;
-import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
-import com.google.devtools.build.lib.analysis.config.BuildOptions;
-import com.google.devtools.build.lib.analysis.config.ConfigMatchingProvider;
-import com.google.devtools.build.lib.analysis.config.ConfigurationResolver;
-import com.google.devtools.build.lib.cmdline.Label;
-import com.google.devtools.build.lib.packages.Attribute;
-import com.google.devtools.build.lib.packages.BuildType;
-import com.google.devtools.build.lib.packages.RawAttributeMapper;
-import com.google.devtools.build.lib.packages.Rule;
-import com.google.devtools.build.lib.packages.Target;
-import com.google.devtools.build.lib.skyframe.SkyframeActionExecutor.ConflictException;
-import com.google.devtools.build.lib.syntax.EvalException;
-import com.google.devtools.build.lib.util.OrderedSetMultimap;
-import com.google.devtools.build.skyframe.SkyFunction;
-import com.google.devtools.build.skyframe.SkyFunctionException;
-import com.google.devtools.build.skyframe.SkyKey;
-import com.google.devtools.build.skyframe.SkyValue;
-import java.util.LinkedHashMap;
-import java.util.LinkedHashSet;
-import java.util.Map;
-import java.util.Set;
-import javax.annotation.Nullable;
-
-/**
- * Build a post-processed ConfiguredTarget, vetting it for action conflict issues.
- */
-public class PostConfiguredTargetFunction implements SkyFunction {
- private static final Function<Dependency, SkyKey> TO_KEYS =
- new Function<Dependency, SkyKey>() {
- @Override
- public SkyKey apply(Dependency input) {
- return PostConfiguredTargetValue.key(
- ConfiguredTargetKey.of(input.getLabel(), input.getConfiguration()));
- }
- };
-
- private final SkyframeExecutor.BuildViewProvider buildViewProvider;
- private final ConfiguredRuleClassProvider ruleClassProvider;
- private final BuildOptions defaultBuildOptions;
-
- public PostConfiguredTargetFunction(
- SkyframeExecutor.BuildViewProvider buildViewProvider,
- ConfiguredRuleClassProvider ruleClassProvider,
- BuildOptions defaultBuildOptions) {
- this.buildViewProvider = Preconditions.checkNotNull(buildViewProvider);
- this.ruleClassProvider = ruleClassProvider;
- this.defaultBuildOptions = defaultBuildOptions;
- }
-
- @Nullable
- @Override
- public SkyValue compute(SkyKey skyKey, Environment env)
- throws SkyFunctionException, InterruptedException {
- ImmutableMap<ActionAnalysisMetadata, ConflictException> badActions =
- PrecomputedValue.BAD_ACTIONS.get(env);
- ConfiguredTargetValue ctValue =
- (ConfiguredTargetValue) env.getValue((ConfiguredTargetKey) skyKey.argument());
- if (env.valuesMissing()) {
- return null;
- }
-
- for (ActionAnalysisMetadata action : ctValue.getActions()) {
- if (badActions.containsKey(action)) {
- throw new ActionConflictFunctionException(badActions.get(action));
- }
- }
-
- ConfiguredTarget ct = ctValue.getConfiguredTarget();
- ConfiguredTargetAndData configuredTargetAndData =
- ConfiguredTargetAndData.fromConfiguredTargetInSkyframe(ct, env);
- if (configuredTargetAndData == null) {
- return null;
- }
- Target target = configuredTargetAndData.getTarget();
- BuildConfiguration configuration = configuredTargetAndData.getConfiguration();
- TargetAndConfiguration ctgValue = new TargetAndConfiguration(target, configuration);
-
- ImmutableMap<Label, ConfigMatchingProvider> configConditions =
- getConfigurableAttributeConditions(ctgValue, env);
- if (configConditions == null) {
- return null;
- }
-
- // Determine what toolchains are needed by this target.
- UnloadedToolchainContext unloadedToolchainContext = null;
- try {
- if (target instanceof Rule) {
- Rule rule = ((Rule) target);
- if (rule.getRuleClassObject().useToolchainResolution()) {
- ImmutableSet<Label> requiredToolchains =
- rule.getRuleClassObject().getRequiredToolchains();
-
- // Collect local (target, rule) constraints for filtering out execution platforms.
- ImmutableSet<Label> execConstraintLabels =
- getExecutionPlatformConstraints(
- rule, configuration.getFragment(PlatformConfiguration.class));
- unloadedToolchainContext =
- (UnloadedToolchainContext)
- env.getValueOrThrow(
- UnloadedToolchainContext.key()
- .configurationKey(BuildConfigurationValue.key(configuration))
- .requiredToolchainTypeLabels(requiredToolchains)
- .execConstraintLabels(execConstraintLabels)
- .build(),
- ToolchainException.class);
- if (env.valuesMissing()) {
- return null;
- }
- }
- }
- } catch (ToolchainException e) {
- throw new PostConfiguredTargetFunctionException(e.asConfiguredValueCreationException());
- }
-
- OrderedSetMultimap<DependencyKind, Dependency> deps;
- try {
- BuildConfiguration hostConfiguration =
- buildViewProvider.getSkyframeBuildView().getHostConfiguration(configuration);
- SkyframeDependencyResolver resolver =
- buildViewProvider.getSkyframeBuildView().createDependencyResolver(env);
- // We don't track root causes here - this function is only invoked for successfully analyzed
- // targets - as long as we redo the exact same steps here as in ConfiguredTargetFunction, this
- // can never fail.
- deps =
- resolver.dependentNodeMap(
- ctgValue,
- hostConfiguration,
- /*aspect=*/ null,
- configConditions,
- unloadedToolchainContext,
- ruleClassProvider.getTrimmingTransitionFactory());
- deps =
- ConfigurationResolver.resolveConfigurations(
- env, ctgValue, deps, hostConfiguration, ruleClassProvider, defaultBuildOptions);
- } catch (EvalException
- | ConfiguredTargetFunction.DependencyEvaluationException
- | InconsistentAspectOrderException e) {
- throw new PostConfiguredTargetFunctionException(e);
- }
-
- env.getValues(Iterables.transform(deps.values(), TO_KEYS));
- if (env.valuesMissing()) {
- return null;
- }
-
- return new PostConfiguredTargetValue(ct);
- }
-
- /**
- * Returns the configurable attribute conditions necessary to evaluate the given configured
- * target, or null if not all dependencies have yet been SkyFrame-evaluated.
- */
- @Nullable
- private static ImmutableMap<Label, ConfigMatchingProvider> getConfigurableAttributeConditions(
- TargetAndConfiguration ctg, Environment env) throws InterruptedException {
- if (!(ctg.getTarget() instanceof Rule)) {
- return ImmutableMap.of();
- }
- Rule rule = (Rule) ctg.getTarget();
- RawAttributeMapper mapper = RawAttributeMapper.of(rule);
- Set<SkyKey> depKeys = new LinkedHashSet<>();
- for (Attribute attribute : rule.getAttributes()) {
- for (Label label : mapper.getConfigurabilityKeys(attribute.getName(), attribute.getType())) {
- if (!BuildType.Selector.isReservedLabel(label)) {
- depKeys.add(ConfiguredTargetValue.key(label, ctg.getConfiguration()));
- }
- }
- }
- Map<SkyKey, SkyValue> cts = env.getValues(depKeys);
- if (env.valuesMissing()) {
- return null;
- }
- Map<Label, ConfigMatchingProvider> conditions = new LinkedHashMap<>();
- for (Map.Entry<SkyKey, SkyValue> entry : cts.entrySet()) {
- Label label = ((ConfiguredTargetKey) entry.getKey().argument()).getLabel();
- ConfiguredTarget ct = ((ConfiguredTargetValue) entry.getValue()).getConfiguredTarget();
- conditions.put(label, Preconditions.checkNotNull(
- ct.getProvider(ConfigMatchingProvider.class)));
- }
- return ImmutableMap.copyOf(conditions);
- }
-
- @Nullable
- @Override
- public String extractTag(SkyKey skyKey) {
- return Label.print(((ConfiguredTargetKey) skyKey.argument()).getLabel());
- }
-
- private static class ActionConflictFunctionException extends SkyFunctionException {
- public ActionConflictFunctionException(ConflictException e) {
- super(e, Transience.PERSISTENT);
- }
- }
-
- private static class PostConfiguredTargetFunctionException extends SkyFunctionException {
- public PostConfiguredTargetFunctionException(Exception e) {
- super(e, Transience.PERSISTENT);
- }
- }
-}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PostConfiguredTargetValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/PostConfiguredTargetValue.java
deleted file mode 100644
index 4e80bd6..0000000
--- a/src/main/java/com/google/devtools/build/lib/skyframe/PostConfiguredTargetValue.java
+++ /dev/null
@@ -1,77 +0,0 @@
-// Copyright 2014 The Bazel Authors. All rights reserved.
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-package com.google.devtools.build.lib.skyframe;
-
-import com.google.common.base.Preconditions;
-import com.google.common.collect.ImmutableList;
-import com.google.common.collect.Interner;
-import com.google.devtools.build.lib.analysis.ConfiguredTarget;
-import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget;
-import com.google.devtools.build.lib.concurrent.BlazeInterners;
-import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
-import com.google.devtools.build.skyframe.AbstractSkyKey;
-import com.google.devtools.build.skyframe.SkyFunctionName;
-import com.google.devtools.build.skyframe.SkyKey;
-import com.google.devtools.build.skyframe.SkyValue;
-
-/**
- * A post-processed ConfiguredTarget which is known to be transitively error-free from action
- * conflict issues.
- */
-@AutoCodec(explicitlyAllowClass = RuleConfiguredTarget.class)
-class PostConfiguredTargetValue implements SkyValue {
-
- private final ConfiguredTarget ct;
-
- PostConfiguredTargetValue(ConfiguredTarget ct) {
- this.ct = Preconditions.checkNotNull(ct);
- }
-
- public static ImmutableList<SkyKey> keys(Iterable<ConfiguredTargetKey> lacs) {
- ImmutableList.Builder<SkyKey> keys = ImmutableList.builder();
- for (ConfiguredTargetKey lac : lacs) {
- keys.add(key(lac));
- }
- return keys.build();
- }
-
- public static Key key(ConfiguredTargetKey lac) {
- return Key.create(lac);
- }
-
- @AutoCodec.VisibleForSerialization
- @AutoCodec
- static class Key extends AbstractSkyKey<ConfiguredTargetKey> {
- private static final Interner<Key> interner = BlazeInterners.newWeakInterner();
-
- private Key(ConfiguredTargetKey arg) {
- super(arg);
- }
-
- @AutoCodec.VisibleForSerialization
- @AutoCodec.Instantiator
- static Key create(ConfiguredTargetKey arg) {
- return interner.intern(new Key(arg));
- }
-
- @Override
- public SkyFunctionName functionName() {
- return SkyFunctions.POST_CONFIGURED_TARGET;
- }
- }
-
- public ConfiguredTarget getCt() {
- return ct;
- }
-}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/RuleConfiguredTargetValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/RuleConfiguredTargetValue.java
index 4c9bdf7..a9619c0 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/RuleConfiguredTargetValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/RuleConfiguredTargetValue.java
@@ -31,8 +31,7 @@
@Immutable
@ThreadSafe
@AutoCodec(explicitlyAllowClass = RuleConfiguredTarget.class)
-public final class RuleConfiguredTargetValue extends ActionLookupValue
- implements ConfiguredTargetValue {
+public final class RuleConfiguredTargetValue implements ActionLookupValue, ConfiguredTargetValue {
// This variable is non-final because it may be clear()ed to save memory. It is null only after
// clear(true) is called.
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctions.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctions.java
index d8f9a1f..4ae5dae 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctions.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctions.java
@@ -88,8 +88,10 @@
SkyFunctionName.createHermetic("TRANSITIVE_TARGET");
public static final SkyFunctionName CONFIGURED_TARGET =
SkyFunctionName.createHermetic("CONFIGURED_TARGET");
- public static final SkyFunctionName POST_CONFIGURED_TARGET =
- SkyFunctionName.createHermetic("POST_CONFIGURED_TARGET");
+ static final SkyFunctionName ACTION_LOOKUP_CONFLICT_FINDING =
+ SkyFunctionName.createHermetic("ACTION_LOOKUP_CONFLICT_DETECTION");
+ static final SkyFunctionName TOP_LEVEL_ACTION_LOOKUP_CONFLICT_FINDING =
+ SkyFunctionName.createHermetic("TOP_LEVEL_ACTION_LOOKUP_CONFLICT_DETECTION");
public static final SkyFunctionName ASPECT = SkyFunctionName.createHermetic("ASPECT");
static final SkyFunctionName LOAD_SKYLARK_ASPECT =
SkyFunctionName.createHermetic("LOAD_SKYLARK_ASPECT");
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java
index 6f14413..ccee6c3 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java
@@ -521,15 +521,7 @@
*/
@Nullable
ActionExecutionState probeActionExecution(Action action) {
- ActionExecutionState state =
- buildActionMap.get(new OwnerlessArtifactWrapper(action.getPrimaryOutput()));
- // Prior to sharing execution state between two actions, ensure that no conflict was detected.
- // This can happen with actions owned by aspects, because unlike actions owned by configured
- // targets, we don't proactively prune them from the graph when a conflict is detected.
- if (state == null || badActionMap.containsKey(action)) {
- return null;
- }
- return state;
+ return buildActionMap.get(new OwnerlessArtifactWrapper(action.getPrimaryOutput()));
}
boolean probeCompletedAndReset(Action action) {
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 4564b33..28d775c 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
@@ -44,6 +44,7 @@
import com.google.devtools.build.lib.analysis.ConfiguredTargetFactory;
import com.google.devtools.build.lib.analysis.DependencyResolver.DependencyKind;
import com.google.devtools.build.lib.analysis.ResolvedToolchainContext;
+import com.google.devtools.build.lib.analysis.TopLevelArtifactContext;
import com.google.devtools.build.lib.analysis.ViewCreationFailedException;
import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
import com.google.devtools.build.lib.analysis.config.BuildConfigurationCollection;
@@ -375,6 +376,7 @@
List<ConfiguredTargetKey> ctKeys,
List<AspectValueKey> aspectKeys,
Supplier<Map<BuildConfigurationValue.Key, BuildConfiguration>> configurationLookupSupplier,
+ TopLevelArtifactContext topLevelArtifactContextForConflictPruning,
EventBus eventBus,
boolean keepGoing,
int numThreads)
@@ -493,23 +495,43 @@
if (!badActions.isEmpty()) {
// In order to determine the set of configured targets transitively error free from action
// conflict issues, we run a post-processing update() that uses the bad action map.
- EvaluationResult<PostConfiguredTargetValue> actionConflictResult;
+ EvaluationResult<ActionLookupConflictFindingValue> actionConflictResult;
enableAnalysis(true);
try {
actionConflictResult =
- skyframeExecutor.postConfigureTargets(eventHandler, ctKeys, keepGoing, badActions);
+ skyframeExecutor.filterActionConflicts(
+ eventHandler,
+ Iterables.concat(ctKeys, aspectKeys),
+ keepGoing,
+ badActions,
+ topLevelArtifactContextForConflictPruning);
} finally {
enableAnalysis(false);
}
- cts = Lists.newArrayListWithCapacity(ctKeys.size());
- for (ConfiguredTargetKey value : ctKeys) {
- PostConfiguredTargetValue postCt =
- actionConflictResult.get(PostConfiguredTargetValue.key(value));
- if (postCt != null) {
- cts.add(postCt.getCt());
+ ImmutableList.Builder<ConfiguredTarget> ctBuilder = ImmutableList.builder();
+
+ for (ConfiguredTargetKey key : ctKeys) {
+ TopLevelActionLookupConflictFindingFunction.Key conflictKey =
+ TopLevelActionLookupConflictFindingFunction.Key.create(
+ key, topLevelArtifactContextForConflictPruning);
+ if (actionConflictResult.get(conflictKey) != null) {
+ ctBuilder.add(
+ Preconditions.checkNotNull((ConfiguredTargetValue) result.get(key), key)
+ .getConfiguredTarget());
}
}
+ cts = ctBuilder.build();
+
+ ImmutableList.Builder<AspectValue> aspectBuilder = ImmutableList.builder();
+
+ for (AspectValueKey key : aspectKeys) {
+ if (actionConflictResult.get(ActionLookupConflictFindingValue.key(key)) != null) {
+ aspectBuilder.add(Preconditions.checkNotNull((AspectValue) result.get(key), key));
+ }
+ }
+
+ aspects = aspectBuilder.build();
}
return new SkyframeAnalysisResult(
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 ca32009..66e4dd9 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
@@ -550,10 +550,10 @@
map.put(
SkyFunctions.LOAD_SKYLARK_ASPECT,
new ToplevelSkylarkAspectFunction(skylarkImportLookupFunctionForInlining));
+ map.put(SkyFunctions.ACTION_LOOKUP_CONFLICT_FINDING, new ActionLookupConflictFindingFunction());
map.put(
- SkyFunctions.POST_CONFIGURED_TARGET,
- new PostConfiguredTargetFunction(
- new BuildViewProvider(), ruleClassProvider, defaultBuildOptions));
+ SkyFunctions.TOP_LEVEL_ACTION_LOOKUP_CONFLICT_FINDING,
+ new TopLevelActionLookupConflictFindingFunction());
map.put(
SkyFunctions.BUILD_CONFIGURATION,
new BuildConfigurationFunction(directories, ruleClassProvider, defaultBuildOptions));
@@ -2411,25 +2411,29 @@
* Post-process the targets. Values in the EvaluationResult are known to be transitively
* error-free from action conflicts.
*/
- public EvaluationResult<PostConfiguredTargetValue> postConfigureTargets(
+ EvaluationResult<ActionLookupConflictFindingValue> filterActionConflicts(
ExtendedEventHandler eventHandler,
- List<ConfiguredTargetKey> values,
+ Iterable<ActionLookupValue.ActionLookupKey> keys,
boolean keepGoing,
- ImmutableMap<ActionAnalysisMetadata, SkyframeActionExecutor.ConflictException> badActions)
+ ImmutableMap<ActionAnalysisMetadata, SkyframeActionExecutor.ConflictException> badActions,
+ TopLevelArtifactContext topLevelArtifactContext)
throws InterruptedException {
checkActive();
PrecomputedValue.BAD_ACTIONS.set(injectable(), badActions);
// Make sure to not run too many analysis threads. This can cause memory thrashing.
- EvaluationResult<PostConfiguredTargetValue> result =
+ EvaluationResult<ActionLookupConflictFindingValue> result =
evaluate(
- PostConfiguredTargetValue.keys(values),
+ TopLevelActionLookupConflictFindingFunction.keys(keys, topLevelArtifactContext),
keepGoing,
/*numThreads=*/ ResourceUsage.getAvailableProcessors(),
eventHandler);
- // Remove all post-configured target values immediately for memory efficiency. We are OK with
+ // Remove all action-conflict detection values immediately for memory efficiency. We are OK with
// this mini-phase being non-incremental as the failure mode of action conflict is rare.
- memoizingEvaluator.delete(SkyFunctionName.functionIs(SkyFunctions.POST_CONFIGURED_TARGET));
+ memoizingEvaluator.delete(
+ SkyFunctionName.functionIs(SkyFunctions.ACTION_LOOKUP_CONFLICT_FINDING));
+ memoizingEvaluator.delete(
+ SkyFunctionName.functionIs(SkyFunctions.TOP_LEVEL_ACTION_LOOKUP_CONFLICT_FINDING));
return result;
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TargetCompletionValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/TargetCompletionValue.java
index c0d64c6..e60cf44 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/TargetCompletionValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/TargetCompletionValue.java
@@ -55,25 +55,25 @@
/** {@link SkyKey} for {@link TargetCompletionValue}. */
@AutoCodec
@AutoValue
- public abstract static class TargetCompletionKey implements SkyKey {
+ public abstract static class TargetCompletionKey
+ implements CompletionFunction.TopLevelActionLookupKey {
@AutoCodec.Instantiator
static TargetCompletionKey create(
- ConfiguredTargetKey configuredTargetKey,
+ ConfiguredTargetKey actionLookupKey,
TopLevelArtifactContext topLevelArtifactContext,
boolean willTest) {
return new AutoValue_TargetCompletionValue_TargetCompletionKey(
- configuredTargetKey, topLevelArtifactContext, willTest);
+ topLevelArtifactContext, actionLookupKey, willTest);
}
@Override
+ public abstract ConfiguredTargetKey actionLookupKey();
+
+ @Override
public SkyFunctionName functionName() {
return SkyFunctions.TARGET_COMPLETION;
}
- public abstract ConfiguredTargetKey configuredTargetKey();
-
- abstract TopLevelArtifactContext topLevelArtifactContext();
-
abstract boolean willTest();
}
}
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
new file mode 100644
index 0000000..7f77c62
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/TopLevelActionLookupConflictFindingFunction.java
@@ -0,0 +1,83 @@
+// Copyright 2020 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.google.devtools.build.lib.skyframe;
+
+import com.google.auto.value.AutoValue;
+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.TopLevelArtifactContext;
+import com.google.devtools.build.lib.analysis.TopLevelArtifactHelper;
+import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
+import com.google.devtools.build.lib.util.Pair;
+import com.google.devtools.build.skyframe.SkyFunction;
+import com.google.devtools.build.skyframe.SkyFunctionException;
+import com.google.devtools.build.skyframe.SkyFunctionName;
+import com.google.devtools.build.skyframe.SkyKey;
+import com.google.devtools.build.skyframe.SkyValue;
+import java.util.stream.Collectors;
+import javax.annotation.Nullable;
+
+/**
+ * Checks to see if any artifacts to be built by this {@link ActionLookupKey} transitively depend on
+ * actions from an {@link ActionLookupValue} that has an action in conflict with another. If so,
+ * none of this key's artifacts will be built.
+ */
+class TopLevelActionLookupConflictFindingFunction implements SkyFunction {
+ @Nullable
+ @Override
+ public SkyValue compute(SkyKey skyKey, Environment env)
+ throws SkyFunctionException, InterruptedException {
+ Key key = (Key) skyKey;
+ Pair<ConfiguredObjectValue, TopLevelArtifactHelper.ArtifactsToBuild> valueAndArtifactsToBuild =
+ CompletionFunction.getValueAndArtifactsToBuild(key, env);
+ if (env.valuesMissing()) {
+ return null;
+ }
+
+ env.getValues(
+ ActionLookupConflictFindingFunction.convertArtifacts(
+ valueAndArtifactsToBuild.second.getAllArtifacts())
+ .collect(Collectors.toList()));
+ return env.valuesMissing() ? null : ActionLookupConflictFindingValue.INSTANCE;
+ }
+
+ @Nullable
+ @Override
+ public String extractTag(SkyKey skyKey) {
+ return Label.print(((Key) skyKey).actionLookupKey().getLabel());
+ }
+
+ static Iterable<Key> keys(
+ Iterable<ActionLookupKey> keys, TopLevelArtifactContext topLevelArtifactContext) {
+ return Iterables.transform(keys, k -> Key.create(k, topLevelArtifactContext));
+ }
+
+ @AutoCodec
+ @AutoValue
+ abstract static class Key implements CompletionFunction.TopLevelActionLookupKey {
+ @AutoCodec.Instantiator
+ static Key create(
+ ActionLookupKey actionLookupKey, TopLevelArtifactContext topLevelArtifactContext) {
+ return new AutoValue_TopLevelActionLookupConflictFindingFunction_Key(
+ actionLookupKey, topLevelArtifactContext);
+ }
+
+ @Override
+ public SkyFunctionName functionName() {
+ return SkyFunctions.TOP_LEVEL_ACTION_LOOKUP_CONFLICT_FINDING;
+ }
+ }
+}
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java b/src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java
index 991a1a4..9df6b28 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java
@@ -46,6 +46,7 @@
import com.google.devtools.build.lib.packages.Rule;
import com.google.devtools.build.lib.packages.Type;
import com.google.devtools.build.lib.pkgcache.LoadingFailureEvent;
+import com.google.devtools.build.lib.skyframe.ActionLookupConflictFindingFunction;
import com.google.devtools.build.lib.skyframe.ConfiguredTargetAndData;
import com.google.devtools.build.lib.testutil.Suite;
import com.google.devtools.build.lib.testutil.TestConstants;
@@ -916,8 +917,7 @@
/**
* Tests that rules with configurable attributes can be accessed through {@link
- * com.google.devtools.build.lib.skyframe.PostConfiguredTargetFunction}.
- * This is a regression test for a Bazel crash.
+ * ActionLookupConflictFindingFunction}. This is a regression test for a Bazel crash.
*/
@Test
public void testPostProcessedConfigurableAttributes() throws Exception {
