Command line options stored as Java List types should default to empt…

…y when given 'None' Starlark value during Bazel transitions
bazelbuild · Apr 1, 2021 · 88ec1e6 · 88ec1e6
1 parent 551fabd
commit 88ec1e6
Show file tree

Hide file tree

Showing 2 changed files with 34 additions and 0 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/starlark/FunctionTransitionUtil.java b/src/main/java/com/google/devtools/build/lib/analysis/starlark/FunctionTransitionUtil.java
@@ -332,6 +332,8 @@ private static BuildOptions applyTransition(
                       .convert(
                           optionValueAsList.stream().map(Object::toString).collect(joining(",")));
             }
+          } else if (def.getType() == List.class && optionValue == null) {
+            convertedValue = def.getDefaultValue();
           } else if (optionValue == null || def.getType().isInstance(optionValue)) {
             convertedValue = optionValue;
           } else if (def.getType().equals(boolean.class) && optionValue instanceof Boolean) {

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
@@ -1328,6 +1328,38 @@ public void successfulTypeConversionOfNativeListOptionEmptyList() throws Excepti
     assertThat(getConfiguration(ct).getOptions().get(CppOptions.class).fissionModes).isEmpty();
   }
 
+  @Test
+  public void successfulTypeConversionOfNativeListOptionNone() throws Exception {
+    writeAllowlistFile();
+    scratch.file(
+        "test/transitions.bzl",
+        "def _impl(settings, attr):",
+        "  return {'//command_line_option:copt': None}",
+        "my_transition = transition(implementation = _impl, inputs = [],",
+        "  outputs = ['//command_line_option:copt'])");
+    scratch.file(
+        "test/rules.bzl",
+        "load('//test:transitions.bzl', 'my_transition')",
+        "def _impl(ctx):",
+        "  return []",
+        "my_rule = rule(",
+        "  implementation = _impl,",
+        "  cfg = my_transition,",
+        "  attrs = {",
+        "    '_allowlist_function_transition': attr.label(",
+        "        default = '//tools/allowlists/function_transition_allowlist',",
+        "    ),",
+        "  })");
+    scratch.file(
+        "test/BUILD",
+        "load('//test:rules.bzl', 'my_rule')",
+        "my_rule(name = 'test')");
+
+    ConfiguredTarget ct = getConfiguredTarget("//test");
+    assertNoEvents();
+    assertThat(getConfiguration(ct).getOptions().get(CppOptions.class).coptList).isEmpty();
+  }
+
   @Test
   public void starlarkPatchTransitionRequiredFragments() throws Exception {
     // All Starlark rule transitions are patch transitions, while all Starlark attribute transitions