tree 6869442a38559b8c17c015876a4248015d538a7c
parent 36853c432e3aba9055c96d4fec0a7127f4d6bb08
author janakr <janakr@google.com> 1585602481 -0700
committer Copybara-Service <copybara-worker@google.com> 1585602558 -0700

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