Restrict the number of transitive dependency edges traversable through attributes which use for_analysis_testing transitions.
Any cases exceeding this limit will result in a rule error being thrown on the analysis_test rule.
The limit is configurable via new flag --analysis_testing_deps_limit with a default limit of 500. The feature itself is still guarded behind --experimental_analysis_testing_improvements, so the former flag has no non-experimental meaning.
Progress toward #5574 and #6237.
RELNOTES: None.
PiperOrigin-RevId: 220144957
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 222125c..181f6d5 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
@@ -25,6 +25,7 @@
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException;
import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget;
+import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget.Mode;
import com.google.devtools.build.lib.analysis.constraints.ConstraintSemantics;
import com.google.devtools.build.lib.analysis.constraints.EnvironmentCollection;
import com.google.devtools.build.lib.analysis.constraints.SupportedEnvironments;
@@ -49,6 +50,7 @@
import com.google.devtools.build.lib.packages.TargetUtils;
import com.google.devtools.build.lib.syntax.EvalException;
import com.google.devtools.build.lib.syntax.Type;
+import com.google.devtools.build.lib.syntax.Type.LabelClass;
import java.util.LinkedHashMap;
import java.util.Map;
import java.util.TreeMap;
@@ -148,6 +150,18 @@
addNativeDeclaredProvider(outputGroupInfo);
}
+ if (ruleContext.getConfiguration().evaluatingForAnalysisTest()) {
+ if (ruleContext.getRule().isAnalysisTest()) {
+ ruleContext.ruleError(
+ String.format(
+ "analysis_test rule '%s' cannot be transitively "
+ + "depended on by another analysis test rule",
+ ruleContext.getLabel()));
+ return null;
+ }
+ addProvider(new DepCountInfo(transitiveDepCount()));
+ }
+
TransitiveInfoProviderMap providers = providersBuilder.build();
if (ruleContext.getRule().isAnalysisTest()) {
@@ -163,7 +177,24 @@
}
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();
GeneratingActions generatingActions = Actions.filterSharedActionsAndThrowActionConflict(
analysisEnvironment.getActionKeyContext(), analysisEnvironment.getRegisteredActions());
@@ -174,6 +205,22 @@
generatingActions.getGeneratingActionIndex());
}
+ private int transitiveDepCount() {
+ int depCount = 0;
+
+ for (String attributeName : ruleContext.attributes().getAttributeNames()) {
+ Type<?> attributeType =
+ ruleContext.attributes().getAttributeDefinition(attributeName).getType();
+ if (attributeType.getLabelClass() == LabelClass.DEPENDENCY) {
+ for (DepCountInfo depCountInfo :
+ ruleContext.getPrerequisites(attributeName, Mode.DONT_CHECK, DepCountInfo.class)) {
+ depCount += depCountInfo.getNumDepEdges() + 1;
+ }
+ }
+ }
+ return depCount;
+ }
+
/**
* Compute the artifacts to put into the {@link FilesToRunProvider} for this target. These are the
* filesToBuild, any artifacts added by the rule with {@link #addFilesToRun}, and the runfiles'
@@ -440,4 +487,22 @@
this.actionsWithoutExtraAction = actions;
return this;
}
+
+ /**
+ * Contains a count of transitive dependency edges traversed by the target which propagated this
+ * object.
+ *
+ * <p>This is automatically provided by all targets which are being evaluated in analysis testing.
+ */
+ private static class DepCountInfo implements TransitiveInfoProvider {
+ private final int numDepEdges;
+
+ public DepCountInfo(int numDepEdges) {
+ this.numDepEdges = numDepEdges;
+ }
+
+ public int getNumDepEdges() {
+ return numDepEdges;
+ }
+ }
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java
index 66dea6b..54d0f5f 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java
@@ -800,6 +800,29 @@
public boolean allowAnalysisFailures;
@Option(
+ name = "evaluating for analysis test",
+ defaultValue = "false",
+ documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
+ effectTags = {OptionEffectTag.BAZEL_INTERNAL_CONFIGURATION},
+ metadataTags = {OptionMetadataTag.INTERNAL},
+ help =
+ "If true, targets in the current configuration are being analyzed only for purposes "
+ + "of an analysis test. This, for example, imposes the restriction described by "
+ + "--analysis_testing_deps_limit.")
+ public boolean evaluatingForAnalysisTest;
+
+ @Option(
+ name = "analysis_testing_deps_limit",
+ defaultValue = "500",
+ documentationCategory = OptionDocumentationCategory.TESTING,
+ effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS},
+ help =
+ "Sets the maximum number of transitive dependencies through a rule attribute with "
+ + "a for_analysis_testing configuration transition. "
+ + "Exceeding this limit will result in a rule error.")
+ public int analysisTestingDepsLimit;
+
+ @Option(
name = "features",
allowMultiple = true,
defaultValue = "",
@@ -1791,6 +1814,14 @@
return options.allowAnalysisFailures;
}
+ public boolean evaluatingForAnalysisTest() {
+ return options.evaluatingForAnalysisTest;
+ }
+
+ public int analysisTestingDepsLimit() {
+ return options.analysisTestingDepsLimit;
+ }
+
public List<Label> getActionListeners() {
return options.actionListeners;
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/skylark/FunctionSplitTransitionProvider.java b/src/main/java/com/google/devtools/build/lib/analysis/skylark/FunctionSplitTransitionProvider.java
index bbfee98..4d11de7 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/skylark/FunctionSplitTransitionProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/skylark/FunctionSplitTransitionProvider.java
@@ -63,21 +63,27 @@
private final EventHandler eventHandler;
private final List<String> inputs;
private final List<String> expectedOutputs;
+ private final boolean isForAnalysisTesting;
- public FunctionSplitTransitionProvider(BaseFunction transitionFunction,
- SkylarkSemantics semantics, EventHandler eventHandler,
- List<String> inputs, List<String> expectedOutputs) {
+ public FunctionSplitTransitionProvider(
+ BaseFunction transitionFunction,
+ SkylarkSemantics semantics,
+ EventHandler eventHandler,
+ List<String> inputs,
+ List<String> expectedOutputs,
+ boolean isForAnalysisTesting) {
this.transitionFunction = transitionFunction;
this.semantics = semantics;
this.eventHandler = eventHandler;
this.inputs = inputs;
this.expectedOutputs = expectedOutputs;
+ this.isForAnalysisTesting = isForAnalysisTesting;
}
@Override
public SplitTransition apply(AttributeMap attributeMap) {
- return new FunctionSplitTransition(transitionFunction, semantics, eventHandler, inputs,
- expectedOutputs);
+ return new FunctionSplitTransition(
+ transitionFunction, semantics, eventHandler, inputs, expectedOutputs, isForAnalysisTesting);
}
private static class FunctionSplitTransition implements SplitTransition {
@@ -86,14 +92,21 @@
private final EventHandler eventHandler;
private final List<String> inputs;
private final List<String> expectedOutputs;
+ private final boolean isForAnalysisTesting;
- public FunctionSplitTransition(BaseFunction transitionFunction, SkylarkSemantics semantics,
- EventHandler eventHandler, List<String> inputs, List<String> expectedOutputs) {
+ public FunctionSplitTransition(
+ BaseFunction transitionFunction,
+ SkylarkSemantics semantics,
+ EventHandler eventHandler,
+ List<String> inputs,
+ List<String> expectedOutputs,
+ boolean isForAnalysisTesting) {
this.transitionFunction = transitionFunction;
this.semantics = semantics;
this.eventHandler = eventHandler;
this.inputs = inputs;
this.expectedOutputs = expectedOutputs;
+ this.isForAnalysisTesting = isForAnalysisTesting;
}
@Override
@@ -359,6 +372,10 @@
BuildConfiguration.Options buildConfigOptions;
buildConfigOptions = buildOptions.get(BuildConfiguration.Options.class);
+
+ if (isForAnalysisTesting) {
+ buildConfigOptions.evaluatingForAnalysisTest = true;
+ }
updateOutputDirectoryNameFragment(buildConfigOptions, transition);
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkAttr.java b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkAttr.java
index 8fea7ed..2163e5f 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkAttr.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkAttr.java
@@ -265,9 +265,14 @@
} else {
builder.hasStarlarkDefinedTransition();
}
- builder.cfg(new FunctionSplitTransitionProvider(
- transImpl, env.getSemantics(), env.getEventHandler(),
- starlarkDefinedTransition.getInputs(), starlarkDefinedTransition.getOutputs()));
+ builder.cfg(
+ new FunctionSplitTransitionProvider(
+ transImpl,
+ env.getSemantics(),
+ env.getEventHandler(),
+ starlarkDefinedTransition.getInputs(),
+ starlarkDefinedTransition.getOutputs(),
+ starlarkDefinedTransition.isForAnalysisTesting()));
} else if (!trans.equals("target")) {
throw new EvalException(ast.getLocation(),
"cfg must be either 'data', 'host', or 'target'.");
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 3dc8d44..7d8d298 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
@@ -2282,7 +2282,6 @@
+ "with for_analysis_testing=True");
}
-
@Test
public void testBuildSettingRule_flag() throws Exception {
setSkylarkSemanticsOptions("--experimental_build_setting_api=true");
@@ -2386,6 +2385,127 @@
+ "by specifying --experimental_build_setting_api");
}
+ @Test
+ public void testAnalysisTestCannotDependOnAnalysisTest() throws Exception {
+ setSkylarkSemanticsOptions(
+ "--experimental_analysis_testing_improvements=true",
+ "--experimental_starlark_config_transitions=true");
+
+ scratch.file(
+ "test/extension.bzl",
+ "",
+ "def analysis_test_rule_impl(ctx):",
+ " return [AnalysisTestResultInfo(success = True, message = 'message contents')]",
+ "def middle_rule_impl(ctx):",
+ " return []",
+ "def inner_rule_impl(ctx):",
+ " return [AnalysisTestResultInfo(success = True, message = 'message contents')]",
+ "",
+ "def transition_func(settings):",
+ " return { '//command_line_option:strict_java_deps' : 'WARN' }",
+ "my_transition = transition(",
+ " implementation = transition_func,",
+ " for_analysis_testing=True,",
+ " inputs = [],",
+ " outputs = ['//command_line_option:strict_java_deps'])",
+ "",
+ "inner_rule_test = rule(",
+ " implementation = analysis_test_rule_impl,",
+ " analysis_test = True,",
+ ")",
+ "middle_rule = rule(",
+ " implementation = middle_rule_impl,",
+ " attrs = {'dep': attr.label()}",
+ ")",
+ "outer_rule_test = rule(",
+ " implementation = analysis_test_rule_impl,",
+ " analysis_test = True,",
+ " attrs = {",
+ " 'dep': attr.label(cfg = my_transition),",
+ " })");
+
+ scratch.file(
+ "test/BUILD",
+ "load('//test:extension.bzl', 'outer_rule_test', 'middle_rule', 'inner_rule_test')",
+ "",
+ "outer_rule_test(name = 'outer', dep = ':middle')",
+ "middle_rule(name = 'middle', dep = ':inner')",
+ "inner_rule_test(name = 'inner')");
+
+ reporter.removeHandler(failFastHandler);
+ getConfiguredTarget("//test:outer");
+ assertContainsEvent(
+ "analysis_test rule '//test:inner' cannot be transitively "
+ + "depended on by another analysis test rule");
+ }
+
+ @Test
+ public void testAnalysisTestOverDepsLimit() throws Exception {
+ setupAnalysisTestDepsLimitTest(10, 12);
+
+ reporter.removeHandler(failFastHandler);
+ getConfiguredTarget("//test:r");
+ assertContainsEvent(
+ "analysis test rule excedeed maximum dependency edge count. " + "Count: 13. Limit is 10.");
+ }
+
+ @Test
+ public void testAnalysisTestUnderDepsLimit() throws Exception {
+ setupAnalysisTestDepsLimitTest(10, 8);
+
+ assertThat(getConfiguredTarget("//test:r")).isNotNull();
+ }
+
+ private void setupAnalysisTestDepsLimitTest(int limit, int dependencyChainSize) throws Exception {
+ setSkylarkSemanticsOptions(
+ "--experimental_analysis_testing_improvements=true",
+ "--experimental_starlark_config_transitions=true");
+ useConfiguration("--analysis_testing_deps_limit=" + limit);
+
+ scratch.file(
+ "test/extension.bzl",
+ "",
+ "def outer_rule_impl(ctx):",
+ " return [AnalysisTestResultInfo(success = True, message = 'message contents')]",
+ "def dep_rule_impl(ctx):",
+ " return []",
+ "",
+ "def transition_func(settings):",
+ " return { '//command_line_option:strict_java_deps' : 'WARN' }",
+ "my_transition = transition(",
+ " implementation = transition_func,",
+ " for_analysis_testing=True,",
+ " inputs = [],",
+ " outputs = ['//command_line_option:strict_java_deps'])",
+ "",
+ "dep_rule = rule(",
+ " implementation = dep_rule_impl,",
+ " attrs = {'dep': attr.label()}",
+ ")",
+ "outer_rule_test = rule(",
+ " implementation = outer_rule_impl,",
+ " fragments = ['java'],",
+ " analysis_test = True,",
+ " attrs = {",
+ " 'dep': attr.label(cfg = my_transition),",
+ " })");
+
+ // Create a chain of targets where 'innerN' depends on 'inner{N+1}' until the max length.
+ StringBuilder dependingRulesChain = new StringBuilder();
+ for (int i = 0; i < dependencyChainSize; i++) {
+ dependingRulesChain.append(
+ String.format("dep_rule(name = 'inner%s', dep = ':inner%s')\n", i, (i + 1)));
+ }
+ dependingRulesChain.append(String.format("dep_rule(name = 'inner%s')", dependencyChainSize));
+
+ scratch.file(
+ "test/BUILD",
+ "load('//test:extension.bzl', 'dep_rule', 'outer_rule_test')",
+ "",
+ "outer_rule_test(name = 'r', dep = ':inner0')",
+ dependingRulesChain.toString());
+ }
+
/**
* Skylark integration test that forces inlining.
*/
