Return early if an empty dictionary is returned from starlark transitions (so it can't be misinterpreted as an empty dictionary of dictionaries).
PiperOrigin-RevId: 249097114
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 ee16fef..ff2056d 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
@@ -214,6 +214,13 @@
}
if (result instanceof SkylarkDict<?, ?>) {
+ // If we're recieving an empty dictionary, it's an error. Even if a
+ // transition function sometimes evaluates to a no-op, it needs to return the passed in
+ // settings. Return early for now since better error reporting will happen in
+ // {@link FunctionTransitionUtil#validateFunctionOutputsMatchesDeclaredOutputs}
+ if (((SkylarkDict) result).isEmpty()) {
+ return ImmutableList.of(ImmutableMap.of());
+ }
// TODO(bazel-team): integrate keys with ctx.split_attr. Currently ctx.split_attr always
// keys on cpu value - we should be able to key on the keys returned here.
try {
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/StarlarkRuleTransitionProviderTest.java b/src/test/java/com/google/devtools/build/lib/analysis/StarlarkRuleTransitionProviderTest.java
index fd1361f..89de7ca 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/StarlarkRuleTransitionProviderTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/StarlarkRuleTransitionProviderTest.java
@@ -908,4 +908,39 @@
assertThat(configuration.getOptions().get(TestOptions.class).testArguments)
.containsExactly("post-transition");
}
+
+ /**
+ * Regression test to ensure that an empty dict is not interpreted as a dict of dicts and
+ * generates the proper error message.
+ */
+ @Test
+ public void testEmptyReturnDict() throws Exception {
+ writeWhitelistFile();
+ scratch.file(
+ "test/transitions.bzl",
+ "def _impl(settings, attr):",
+ " return {}",
+ "my_transition = transition(implementation = _impl, inputs = [],",
+ " outputs = ['//command_line_option:test_arg'])");
+ scratch.file(
+ "test/rules.bzl",
+ "load('//test:transitions.bzl', 'my_transition')",
+ "def _impl(ctx):",
+ " return []",
+ "my_rule = rule(",
+ " implementation = _impl,",
+ " cfg = my_transition,",
+ " attrs = {",
+ " '_whitelist_function_transition': attr.label(",
+ " default = '//tools/whitelists/function_transition_whitelist',",
+ " ),",
+ " })");
+ scratch.file("test/BUILD", "load('//test:rules.bzl', 'my_rule')", "my_rule(name = 'test')");
+
+ reporter.removeHandler(failFastHandler);
+ getConfiguredTarget("//test");
+ assertContainsEvent(
+ "transition outputs [//command_line_option:test_arg] were "
+ + "not defined by transition function");
+ }
}
