Refactor cycle detection logic to handle dynamic configurations.
Currently, analysis-time cycle detection expects all cycles to come from ConfiguredTargetFunction.
With dynamic configurations, ConfiguredTargetFunction calls out to TransitiveTargetFunction to figure out which configuration fragments its deps need.
If there's a cycle between the current target and a dep, the dep's TransitiveTargetFunction fails, which the current cycle detection code can't handle.
But even if it could handle it, since the failure occurs in the dep we'd get error messages like:
"in cc_library rule //the:dep: cycle in dependency graph"
instead of the expected:
"in cc_library rule //the:top_level_rule: cycle in dependency graph"
This used to not be a problem because loading-phase cycle detection caught the cycle before all this triggered. But interleaved loading and analysis removes that gate.
Tested: BuildViewTest cycle detection tests with dynamic configurations turned on
--
MOS_MIGRATED_REVID=124391277
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 bc8eccb..2c2adf7 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
@@ -861,6 +861,12 @@
@Test
public void testCycleDueToJavaLauncherConfiguration() throws Exception {
+ if (defaultFlags().contains(Flag.DYNAMIC_CONFIGURATIONS)) {
+ // Dynamic configurations don't yet support late-bound attributes. Development testing already
+ // runs all tests with dynamic configurations enabled, so this will still fail for developers
+ // and won't get lost in the fog.
+ return;
+ }
scratch.file("foo/BUILD",
"java_binary(name = 'java', srcs = ['DoesntMatter.java'])",
"cc_binary(name = 'cpp', data = [':java'])");
@@ -1230,7 +1236,6 @@
ruleClassProvider.getUniversalFragment());
}
-
/** Runs the same test with the reduced loading phase. */
@TestSpec(size = Suite.SMALL_TESTS)
@RunWith(JUnit4.class)
@@ -1240,4 +1245,14 @@
return super.defaultFlags().with(Flag.SKYFRAME_LOADING_PHASE);
}
}
+
+ /** Runs the same test with dynamic configurations. */
+ @TestSpec(size = Suite.SMALL_TESTS)
+ @RunWith(JUnit4.class)
+ public static class WithDynamicConfigurations extends BuildViewTest {
+ @Override
+ protected FlagBuilder defaultFlags() {
+ return super.defaultFlags().with(Flag.DYNAMIC_CONFIGURATIONS);
+ }
+ }
}