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/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;
+ }
+ }
+}
