Only apply deps limit to analysis tests using transitions
Previously, a transitive-dependency count limit was applied to all analysis_test rules for simplicity, since there was not deemed a use case for an analysis test that was "large". However, relaxing this restriction to only apply to rules using transitions facilitates a build_test-like Starlark rule which verifies an existing real target analyzes correctly without executing any actions.
Progress toward #6237
RELNOTES: None.
PiperOrigin-RevId: 235580276
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java
index af2f5d2..dfba1a7 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java
@@ -162,6 +162,27 @@
addProvider(new DepCountInfo(transitiveDepCount()));
}
+ if (ruleContext.getRule().hasAnalysisTestTransition()) {
+ int depCount = transitiveDepCount();
+ if (depCount > ruleContext.getConfiguration().analysisTestingDepsLimit()) {
+ ruleContext.ruleError(
+ String.format(
+ "analysis test rule excedeed maximum dependency edge count. "
+ + "Count: %s. Limit is %s. This limit is imposed on analysis test rules which "
+ + "use analysis_test_transition attribute transitions. Exceeding this limit "
+ + "indicates either the analysis_test has too many dependencies, or the "
+ + "underlying toolchains may be large. Try decreasing the number of test "
+ + "dependencies, (Analysis tests should not be very large!) or, if possible, "
+ + "try not using configuration transitions. If underlying toolchain size is "
+ + "to blame, it might be worth considering increasing "
+ + "--analysis_testing_deps_limit. (Beware, however, that large values of "
+ + "this flag can lead to no safeguards against giant "
+ + "test suites that can lead to Out Of Memory exceptions in the build server.)",
+ depCount, ruleContext.getConfiguration().analysisTestingDepsLimit()));
+ return null;
+ }
+ }
+
TransitiveInfoProviderMap providers = providersBuilder.build();
if (ruleContext.getRule().isAnalysisTest()) {
@@ -177,22 +198,6 @@
}
AnalysisTestActionBuilder.writeAnalysisTestAction(ruleContext, testResultInfo);
-
- int depCount = transitiveDepCount();
- if (depCount > ruleContext.getConfiguration().analysisTestingDepsLimit()) {
- ruleContext.ruleError(
- String.format(
- "analysis test rule excedeed maximum dependency edge count. "
- + "Count: %s. Limit is %s. This indicates either the analysis_test has too "
- + "many dependencies, or the underlying toolchains may be large. Try "
- + "decreasing the number of test dependencies. (Analysis tests should not be "
- + "very large!) If underlying toolchain size is to blame, it might be worth "
- + "considering increasing --analysis_testing_deps_limit. (Beware, however, "
- + "that large values of this flag can lead to no safeguards against giant "
- + "test suites that can lead to Out Of Memory exceptions in the build server.)",
- depCount, ruleContext.getConfiguration().analysisTestingDepsLimit()));
- return null;
- }
}
AnalysisEnvironment analysisEnvironment = ruleContext.getAnalysisEnvironment();
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleClassFunctions.java b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleClassFunctions.java
index 4639d41..24e1241 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleClassFunctions.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleClassFunctions.java
@@ -677,11 +677,14 @@
Attribute attr = descriptor.build(name);
hasStarlarkDefinedTransition |= attr.hasStarlarkDefinedTransition();
- if (attr.hasAnalysisTestTransition() && !builder.isAnalysisTest()) {
- throw new EvalException(
- location,
- "Only rule definitions with analysis_test=True may have attributes with "
- + "analysis_test_transition transitions");
+ if (attr.hasAnalysisTestTransition()) {
+ if (!builder.isAnalysisTest()) {
+ throw new EvalException(
+ location,
+ "Only rule definitions with analysis_test=True may have attributes with "
+ + "analysis_test_transition transitions");
+ }
+ builder.setHasAnalysisTestTransition();
}
// Check for existence of the function transition whitelist attribute.
// TODO(b/121385274): remove when we stop whitelisting starlark transitions
diff --git a/src/main/java/com/google/devtools/build/lib/packages/Rule.java b/src/main/java/com/google/devtools/build/lib/packages/Rule.java
index ae01bd2..c3245b4 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/Rule.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/Rule.java
@@ -182,6 +182,14 @@
}
/**
+ * Returns true if this rule has at least one attribute with an analysis test transition. (A
+ * starlark-defined transition using analysis_test_transition()).
+ */
+ public boolean hasAnalysisTestTransition() {
+ return ruleClass.hasAnalysisTestTransition();
+ }
+
+ /**
* Returns true iff there were errors while constructing this rule, such as
* attributes with missing values or values of the wrong type.
*/
diff --git a/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java b/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java
index d61cc4a..7f96fc4 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java
@@ -645,6 +645,7 @@
private boolean workspaceOnly = false;
private boolean isExecutableSkylark = false;
private boolean isAnalysisTest = false;
+ private boolean hasAnalysisTestTransition = false;
private boolean isConfigMatcher = false;
private boolean hasFunctionTransitionWhitelist = false;
private boolean ignorePackageLicenses = false;
@@ -808,6 +809,7 @@
workspaceOnly,
isExecutableSkylark,
isAnalysisTest,
+ hasAnalysisTestTransition,
hasFunctionTransitionWhitelist,
ignorePackageLicenses,
implicitOutputsFunction,
@@ -1195,6 +1197,19 @@
}
/**
+ * This rule class has at least one attribute with an analysis test transition. (A
+ * starlark-defined transition using analysis_test_transition()).
+ */
+ public Builder setHasAnalysisTestTransition() {
+ this.hasAnalysisTestTransition = true;
+ return this;
+ }
+
+ public boolean hasAnalysisTestTransition() {
+ return this.hasAnalysisTestTransition;
+ }
+
+ /**
* This rule class has the _whitelist_function_transition attribute. Intended only for Skylark
* rules.
*/
@@ -1390,6 +1405,7 @@
private final boolean workspaceOnly;
private final boolean isExecutableSkylark;
private final boolean isAnalysisTest;
+ private final boolean hasAnalysisTestTransition;
private final boolean isConfigMatcher;
private final boolean hasFunctionTransitionWhitelist;
private final boolean ignorePackageLicenses;
@@ -1519,6 +1535,7 @@
boolean workspaceOnly,
boolean isExecutableSkylark,
boolean isAnalysisTest,
+ boolean hasAnalysisTestTransition,
boolean hasFunctionTransitionWhitelist,
boolean ignorePackageLicenses,
ImplicitOutputsFunction implicitOutputsFunction,
@@ -1569,6 +1586,7 @@
this.workspaceOnly = workspaceOnly;
this.isExecutableSkylark = isExecutableSkylark;
this.isAnalysisTest = isAnalysisTest;
+ this.hasAnalysisTestTransition = hasAnalysisTestTransition;
this.hasFunctionTransitionWhitelist = hasFunctionTransitionWhitelist;
this.ignorePackageLicenses = ignorePackageLicenses;
this.configurationFragmentPolicy = configurationFragmentPolicy;
@@ -2424,6 +2442,14 @@
}
/**
+ * Returns true if this rule class has at least one attribute with an analysis test transition. (A
+ * starlark-defined transition using analysis_test_transition()).
+ */
+ public boolean hasAnalysisTestTransition() {
+ return hasAnalysisTestTransition;
+ }
+
+ /**
* Returns true if this rule class has the _whitelist_function_transition attribute.
*/
public boolean hasFunctionTransitionWhitelist() {
diff --git a/src/test/java/com/google/devtools/build/lib/packages/RuleClassTest.java b/src/test/java/com/google/devtools/build/lib/packages/RuleClassTest.java
index 8c70e2b..4cbb2fe 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/RuleClassTest.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/RuleClassTest.java
@@ -888,6 +888,7 @@
workspaceOnly,
outputsDefaultExecutable,
isAnalysisTest,
+ /* hasAnalysisTestTransition=*/ false,
/* hasFunctionTransitionWhitelist=*/ false,
/* ignorePackageLicenses=*/ false,
implicitOutputsFunction,
diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkIntegrationTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkIntegrationTest.java
index 25ec752..be0ff46 100644
--- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkIntegrationTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkIntegrationTest.java
@@ -2417,7 +2417,7 @@
@Test
public void testAnalysisTestOverDepsLimit() throws Exception {
- setupAnalysisTestDepsLimitTest(10, 12);
+ setupAnalysisTestDepsLimitTest(10, 12, true);
reporter.removeHandler(failFastHandler);
getConfiguredTarget("//test:r");
@@ -2427,14 +2427,31 @@
@Test
public void testAnalysisTestUnderDepsLimit() throws Exception {
- setupAnalysisTestDepsLimitTest(10, 8);
+ setupAnalysisTestDepsLimitTest(10, 8, true);
assertThat(getConfiguredTarget("//test:r")).isNotNull();
}
- private void setupAnalysisTestDepsLimitTest(int limit, int dependencyChainSize) throws Exception {
+ @Test
+ public void testAnalysisDepsLimitOnlyForTransition() throws Exception {
+ setupAnalysisTestDepsLimitTest(3, 10, false);
+
+ assertThat(getConfiguredTarget("//test:r")).isNotNull();
+ }
+
+ private void setupAnalysisTestDepsLimitTest(
+ int limit, int dependencyChainSize, boolean useTransition) throws Exception {
useConfiguration("--analysis_testing_deps_limit=" + limit);
+ String transitionDefinition;
+ if (useTransition) {
+ transitionDefinition =
+ "my_transition = analysis_test_transition("
+ + "settings = {'//command_line_option:test_arg' : ['yeehaw'] })";
+ } else {
+ transitionDefinition = "my_transition = None";
+ }
+
scratch.file(
"test/extension.bzl",
"",
@@ -2443,10 +2460,8 @@
"def dep_rule_impl(ctx):",
" return []",
"",
- "my_transition = analysis_test_transition(",
- " settings = {",
- " '//command_line_option:test_arg' : ['yeehaw'] }",
- ")",
+ transitionDefinition,
+ "",
"dep_rule = rule(",
" implementation = dep_rule_impl,",
" attrs = {'dep': attr.label()}",
