for_analysis_test mode on transition() function
This change enforces that for_analysis_test transitions occur only on attributes of rules with analysis_test=True. This restriction is separate from the whitelist restriction of non-analysis-test transitions.
Progress on #5574 and #6237
RELNOTES: None.
PiperOrigin-RevId: 217782561
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/StarlarkDefinedConfigTransition.java b/src/main/java/com/google/devtools/build/lib/analysis/config/StarlarkDefinedConfigTransition.java
index b9ee47a..925923a 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/config/StarlarkDefinedConfigTransition.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/StarlarkDefinedConfigTransition.java
@@ -25,9 +25,11 @@
public class StarlarkDefinedConfigTransition implements ConfigurationTransitionApi {
private final BaseFunction impl;
+ private final boolean forAnalysisTesting;
- public StarlarkDefinedConfigTransition(BaseFunction impl) {
+ public StarlarkDefinedConfigTransition(BaseFunction impl, boolean forAnalysisTesting) {
this.impl = impl;
+ this.forAnalysisTesting = forAnalysisTesting;
}
/**
@@ -40,6 +42,14 @@
return impl;
}
+ /**
+ * Returns true if this transition is for analysis testing. If true, then only attributes of
+ * rules with {@code analysis_test=true} may use this transition object.
+ */
+ public Boolean isForAnalysisTesting() {
+ return forAnalysisTesting;
+ }
+
@Override
public void repr(SkylarkPrinter printer) {
printer.append("<transition object>");
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 f59d7de..704f778 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
@@ -257,10 +257,16 @@
} else if (trans instanceof SplitTransitionProvider) {
builder.cfg((SplitTransitionProvider) trans);
} else if (trans instanceof StarlarkDefinedConfigTransition) {
- BaseFunction transImpl = ((StarlarkDefinedConfigTransition) trans).getImplementation();
- builder.hasFunctionTransition();
- builder.cfg(new FunctionSplitTransitionProvider(transImpl,
- env.getSemantics(), env.getEventHandler()));
+ StarlarkDefinedConfigTransition starlarkDefinedTransition =
+ (StarlarkDefinedConfigTransition) trans;
+ BaseFunction transImpl = starlarkDefinedTransition.getImplementation();
+ if (starlarkDefinedTransition.isForAnalysisTesting()) {
+ builder.hasAnalysisTestTransition();
+ } else {
+ builder.hasStarlarkDefinedTransition();
+ }
+ builder.cfg(new FunctionSplitTransitionProvider(
+ transImpl, env.getSemantics(), env.getEventHandler()));
} else if (!trans.equals("target")) {
throw new EvalException(ast.getLocation(),
"cfg must be either 'data', 'host', or 'target'.");
diff --git a/src/main/java/com/google/devtools/build/lib/packages/Attribute.java b/src/main/java/com/google/devtools/build/lib/packages/Attribute.java
index ef6496c..2f89d96 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/Attribute.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/Attribute.java
@@ -250,9 +250,16 @@
OUTPUT_LICENSES,
/**
- * Has a function-based split transition.
+ * Has a Starlark-defined configuration transition. Transitions for analysis testing are tracked
+ * separately: see {@link #HAS_ANALYSIS_TEST_TRANSITION}.
*/
- HAS_FUNCTION_TRANSITION,
+ HAS_STARLARK_DEFINED_TRANSITION,
+
+ /**
+ * Has a Starlark-defined configuration transition designed specifically for rules which
+ * run analysis tests.
+ */
+ HAS_ANALYSIS_TEST_TRANSITION,
}
// TODO(bazel-team): modify this interface to extend Predicate and have an extra error
@@ -598,11 +605,22 @@
}
/**
- * Indicate the attribute uses function-based split transition.
+ * Indicate the attribute uses uses a starlark-defined (non-analysis-test) configuration
+ * transition. Transitions for analysis testing are tracked separately: see
+ * {@link #hasAnalysisTestTransition()}.
*/
- public Builder<TYPE> hasFunctionTransition() {
- return setPropertyFlag(PropertyFlag.HAS_FUNCTION_TRANSITION,
- "function-based split transition");
+ public Builder<TYPE> hasStarlarkDefinedTransition() {
+ return setPropertyFlag(PropertyFlag.HAS_STARLARK_DEFINED_TRANSITION,
+ "starlark-defined split transition");
+ }
+
+ /**
+ * Indicate the attribute uses uses a starlark-defined analysis-test configuration transition.
+ * Such a configuration transition may only be applied on rules with {@code analysis_test=true}.
+ */
+ public Builder<TYPE> hasAnalysisTestTransition() {
+ return setPropertyFlag(PropertyFlag.HAS_ANALYSIS_TEST_TRANSITION,
+ "analysis-test split transition");
}
/**
@@ -2110,11 +2128,20 @@
}
/**
- * Returns true if this attribute uses a function-based split transition provider. See
- * {@link FunctionSplitTransitionProvider}.
+ * Returns true if this attribute uses a starlark-defined, non analysis-test configuration
+ * transition. See {@link FunctionSplitTransitionProvider}. Starlark-defined analysis-test
+ * configuration transitions are handled separately. See {@link #hasAnalysisTestTransition}.
*/
- public boolean hasFunctionTransition() {
- return getPropertyFlag(PropertyFlag.HAS_FUNCTION_TRANSITION);
+ public boolean hasStarlarkDefinedTransition() {
+ return getPropertyFlag(PropertyFlag.HAS_STARLARK_DEFINED_TRANSITION);
+ }
+
+ /**
+ * Returns true if this attributes uses Starlark-defined configuration transition designed
+ * specifically for rules which run analysis tests.
+ */
+ public boolean hasAnalysisTestTransition() {
+ return getPropertyFlag(PropertyFlag.HAS_ANALYSIS_TEST_TRANSITION);
}
/**
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 753b73e..8e9eb6c 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
@@ -768,7 +768,7 @@
.value(ImmutableList.of()));
}
if (skylark) {
- assertSkylarkRuleClassProperFunctionTransitionUsage();
+ assertRuleClassProperStarlarkDefinedTransitionUsage();
}
return new RuleClass(
@@ -826,14 +826,20 @@
type);
}
- private void assertSkylarkRuleClassProperFunctionTransitionUsage() {
- boolean hasFunctionTransitionAttribute =
- attributes.entrySet()
- .stream()
- .map(entry -> entry.getValue().hasFunctionTransition())
- .reduce(false, (a, b) -> a || b);
+ private void assertRuleClassProperStarlarkDefinedTransitionUsage() {
+ boolean hasStarlarkDefinedTransition = false;
+ boolean hasAnalysisTestTransitionAttribute = false;
+ for (Attribute attribute : attributes.values()) {
+ hasStarlarkDefinedTransition |= attribute.hasStarlarkDefinedTransition();
+ hasAnalysisTestTransitionAttribute |= attribute.hasAnalysisTestTransition();
+ }
- if (hasFunctionTransitionAttribute) {
+ if (hasAnalysisTestTransitionAttribute) {
+ Preconditions.checkState(isAnalysisTest,
+ "Only rule definitions with analysis_test=True may have attributes with "
+ + "for_analysis_testing=True");
+ }
+ if (hasStarlarkDefinedTransition) {
Preconditions.checkState(
hasFunctionTransitionWhitelist,
"Use of function based split transition without whitelist: %s %s",
diff --git a/src/main/java/com/google/devtools/build/lib/rules/config/ConfigGlobalLibrary.java b/src/main/java/com/google/devtools/build/lib/rules/config/ConfigGlobalLibrary.java
index 85f71a8..0b3c253 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/config/ConfigGlobalLibrary.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/config/ConfigGlobalLibrary.java
@@ -32,12 +32,13 @@
@Override
public ConfigurationTransitionApi transition(BaseFunction implementation, List<String> inputs,
- List<String> outputs, Location location, SkylarkSemantics semantics) throws EvalException {
+ List<String> outputs, Boolean forAnalysisTesting,
+ Location location, SkylarkSemantics semantics) throws EvalException {
if (!semantics.experimentalStarlarkConfigTransitions()) {
throw new EvalException(location, "transition() is experimental and disabled by default. "
+ "This API is in development and subject to change at any time. Use "
+ "--experimental_starlark_config_transitions to use this experimental API.");
}
- return new StarlarkDefinedConfigTransition(implementation);
+ return new StarlarkDefinedConfigTransition(implementation, forAnalysisTesting);
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/config/ConfigGlobalLibraryApi.java b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/config/ConfigGlobalLibraryApi.java
index 517b829..2be3563 100644
--- a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/config/ConfigGlobalLibraryApi.java
+++ b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/config/ConfigGlobalLibraryApi.java
@@ -78,6 +78,16 @@
named = true,
doc = "List of build settings that can be written by this transition. This must be "
+ "a superset of the key set of the dictionary returned by this transition."),
+ @Param(
+ name = "for_analysis_testing",
+ type = Boolean.class,
+ positional = false,
+ named = true,
+ defaultValue = "False",
+ // TODO(cparsons): In this doc string, link to more extensive documentation regarding
+ // analysis test rules.
+ doc = "If true, this transition may only be used on attributes of rules with "
+ + "analysis_test set to true."),
},
useLocation = true,
useSkylarkSemantics = true)
@@ -86,6 +96,7 @@
BaseFunction implementation,
List<String> inputs,
List<String> outputs,
+ Boolean forAnalysisTesting,
Location location,
SkylarkSemantics semantics)
throws EvalException;
diff --git a/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/config/FakeConfigGlobalLibrary.java b/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/config/FakeConfigGlobalLibrary.java
index 69fe591..c5c815c 100644
--- a/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/config/FakeConfigGlobalLibrary.java
+++ b/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/config/FakeConfigGlobalLibrary.java
@@ -18,6 +18,7 @@
import com.google.devtools.build.lib.skylarkbuildapi.config.ConfigGlobalLibraryApi;
import com.google.devtools.build.lib.skylarkbuildapi.config.ConfigurationTransitionApi;
import com.google.devtools.build.lib.syntax.BaseFunction;
+import com.google.devtools.build.lib.syntax.EvalException;
import com.google.devtools.build.lib.syntax.SkylarkSemantics;
import java.util.List;
@@ -28,7 +29,8 @@
@Override
public ConfigurationTransitionApi transition(BaseFunction implementation, List<String> inputs,
- List<String> outputs, Location location, SkylarkSemantics semantics) {
+ List<String> outputs, Boolean forAnalysisTesting, Location location,
+ SkylarkSemantics semantics) throws EvalException {
return new FakeConfigurationTransition();
}
}
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 d89f9fb..c1e7e22 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
@@ -2189,6 +2189,100 @@
// TODO(cparsons): Verify implicit action registration via AnalysisTestResultInfo.
}
+ @Test
+ public void testAnalysisTestTransitionOnAnalysisTest() throws Exception {
+ setSkylarkSemanticsOptions(
+ "--experimental_analysis_testing_improvements=true",
+ "--experimental_starlark_config_transitions=true");
+ useConfiguration("--strict_java_deps=OFF");
+
+ scratch.file(
+ "test/extension.bzl",
+ "MyInfo = provider()",
+ "MyDep = provider()",
+ "",
+ "def outer_rule_impl(ctx):",
+ " return [MyInfo(strict_java_deps = ctx.fragments.java.strict_java_deps),",
+ " MyDep(info = ctx.attr.dep[0][MyInfo])]",
+ "def inner_rule_impl(ctx):",
+ " return [MyInfo(strict_java_deps = ctx.fragments.java.strict_java_deps)]",
+ "",
+ "def transition_func(settings):",
+ " return { 'strict_java_deps' : 'WARN' }",
+ "my_transition = transition(",
+ " implementation = transition_func,",
+ " for_analysis_testing=True,",
+ " inputs = [],",
+ " outputs = [])",
+ "",
+ "inner_rule = rule(implementation = inner_rule_impl,",
+ " fragments = ['java'])",
+ "outer_rule = rule(",
+ " implementation = outer_rule_impl,",
+ " fragments = ['java'],",
+ " analysis_test = True,",
+ " attrs = {",
+ " 'dep': attr.label(cfg = my_transition),",
+ " })");
+
+ scratch.file(
+ "test/BUILD",
+ "load('//test:extension.bzl', 'inner_rule', 'outer_rule')",
+ "",
+ "inner_rule(name = 'inner')",
+ "outer_rule(name = 'r', dep = ':inner')");
+
+ SkylarkKey myInfoKey =
+ new SkylarkKey(Label.parseAbsolute("//test:extension.bzl", ImmutableMap.of()), "MyInfo");
+ SkylarkKey myDepKey =
+ new SkylarkKey(Label.parseAbsolute("//test:extension.bzl", ImmutableMap.of()), "MyDep");
+
+ ConfiguredTarget outerTarget = getConfiguredTarget("//test:r");
+ StructImpl outerInfo = (StructImpl) outerTarget.get(myInfoKey);
+ StructImpl outerDepInfo = (StructImpl) outerTarget.get(myDepKey);
+ StructImpl innerInfo = (StructImpl) outerDepInfo.getValue("info");
+
+ assertThat(outerInfo.getValue("strict_java_deps")).isEqualTo("off");
+ assertThat(innerInfo.getValue("strict_java_deps")).isEqualTo("warn");
+ }
+
+ @Test
+ public void testAnalysisTestTransitionOnNonAnalysisTest() throws Exception {
+ setSkylarkSemanticsOptions(
+ "--experimental_analysis_testing_improvements=true",
+ "--experimental_starlark_config_transitions=true");
+
+ scratch.file(
+ "test/extension.bzl",
+ "def custom_rule_impl(ctx):",
+ " return []",
+ "def transition_func(settings):",
+ " return settings",
+ "my_transition = transition(",
+ " implementation = transition_func,",
+ " for_analysis_testing=True,",
+ " inputs = [],",
+ " outputs = [])",
+ "",
+ "custom_rule = rule(",
+ " implementation = custom_rule_impl,",
+ " attrs = {",
+ " 'dep': attr.label(cfg = my_transition),",
+ " })");
+
+
+ scratch.file(
+ "test/BUILD",
+ "load('//test:extension.bzl', 'custom_rule')",
+ "",
+ "custom_rule(name = 'r')");
+
+ reporter.removeHandler(failFastHandler);
+ getConfiguredTarget("//test:r");
+ assertContainsEvent("Only rule definitions with analysis_test=True may have attributes "
+ + "with for_analysis_testing=True");
+ }
+
/**
* Skylark integration test that forces inlining.
*/
