Adds cycle detection errors when top-level dynamic

configuration creation fails because transitive fragment
visitation hits these cycles.

This makes CircularDependencyTest pass with dynamic
configurations.

It's a little bit unfortunate that BuildViewTestCase
follows a different code path to create configured targets
than production (BuildView.getConfiguredTargetForTesting
vs. BuildView.update). As a result, doing an actual build over
the rules defined in CircularDependencyTest#testTwoCycles
correctly reports the cycle, while the test itself doesn't.
That means the test isn't 100% faithfully testing production
logic.

But I'm not interested in fixing the gap between
BuildView.update and BuildView.getConfiguredTargetForTesting
in this change. That's part of a larger refactoring effort
on the various forked ways of acccessing configured targets
and dependencies in BuildView.

--
MOS_MIGRATED_REVID=125118553

1 files changed, 4 insertions, 4 deletions

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 63486e66ad..d497c3065a 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
@@ -1201,10 +1201,7 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
 
     EvaluationResult<SkyValue> result = evaluateSkyKeys(eventHandler, skyKeys);
     for (Map.Entry<SkyKey, ErrorInfo> entry : result.errorMap().entrySet()) {
-      getCyclesReporter().reportCycles(
-          entry.getValue().getCycleInfo(),
-          entry.getKey(),
-          eventHandler);
+      reportCycles(eventHandler, entry.getValue().getCycleInfo(), entry.getKey());
     }
 
     ImmutableMap.Builder<Dependency, ConfiguredTarget> cts = ImmutableMap.builder();
@@ -1274,6 +1271,9 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
     }
     EvaluationResult<SkyValue> fragmentsResult = evaluateSkyKeys(
         eventHandler, transitiveFragmentSkyKeys, /*keepGoing=*/true);
+    for (Map.Entry<SkyKey, ErrorInfo> entry : fragmentsResult.errorMap().entrySet()) {
+      reportCycles(eventHandler, entry.getValue().getCycleInfo(), entry.getKey());
+    }
     for (Dependency key : keys) {
       if (!depsToEvaluate.contains(key)) {
         // No fragments to compute here.