From 81cf998fc43e4ac8f9f3fed4269757f280f3439e Mon Sep 17 00:00:00 2001
From: "Ian (Hee) Cha" <heec@google.com>
Date: Thu, 20 Jul 2023 07:45:11 -0700
Subject: [PATCH] Convert Starlark options with `Starlark#fromJava` in
 `build_options`

`cquery`'s `build_options` function now ensures that Starlark option values are converted to Starlark types before being added to the `dict` of all options, fixing a crash observed for list-typed build settings specified on the command line.

Fixes #18977

Closes #18988.

PiperOrigin-RevId: 549627675
Change-Id: I1209ad4794e8668134bebeb571b484c4cffd83c7
---
 .../StarlarkOutputFormatterCallback.java      |  2 +-
 .../integration/configured_query_test.sh      | 34 +++++++++++++++----
 2 files changed, 29 insertions(+), 7 deletions(-)

diff --git a/src/main/java/com/google/devtools/build/lib/query2/cquery/StarlarkOutputFormatterCallback.java b/src/main/java/com/google/devtools/build/lib/query2/cquery/StarlarkOutputFormatterCallback.java
index 9acbb60f9bc397..752b27fca6a242 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/cquery/StarlarkOutputFormatterCallback.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/cquery/StarlarkOutputFormatterCallback.java
@@ -107,7 +107,7 @@ public Object buildOptions(ConfiguredTarget target) {
 
       // Add Starlark options.
       for (Map.Entry<Label, Object> e : buildOptions.getStarlarkOptions().entrySet()) {
-        result.put(e.getKey().toString(), e.getValue());
+        result.put(e.getKey().toString(), Starlark.fromJava(e.getValue(), null));
       }
       return result.buildOrThrow();
     }
diff --git a/src/test/shell/integration/configured_query_test.sh b/src/test/shell/integration/configured_query_test.sh
index 250cf4fa74599b..4ddac2aefca188 100755
--- a/src/test/shell/integration/configured_query_test.sh
+++ b/src/test/shell/integration/configured_query_test.sh
@@ -952,9 +952,18 @@ bool_flag = rule(
     build_setting = config.bool(flag = True),
 )
 
+def _list_flag_impl(ctx):
+    return BuildSettingInfo(value = ctx.build_setting_value)
+
+list_flag = rule(
+    implementation = _list_flag_impl,
+    build_setting = config.string_list(flag = True),
+)
+
 def _dep_transition_impl(settings, attr):
     return {
         "//$pkg:myflag": True,
+        "//$pkg:mylistflag": ["a", "b"],
         "//command_line_option:platform_suffix": "blah"
     }
 
@@ -963,6 +972,7 @@ _dep_transition = transition(
     inputs = [],
     outputs = [
         "//$pkg:myflag",
+        "//$pkg:mylistflag",
         "//command_line_option:platform_suffix",
     ],
 )
@@ -980,7 +990,7 @@ root_rule = rule(
 EOF
 
   cat > $pkg/BUILD <<'EOF'
-load(":rules.bzl", "bool_flag", "root_rule")
+load(":rules.bzl", "bool_flag", "list_flag", "root_rule")
 
 exports_files(["rules.bzl"])
 
@@ -989,6 +999,11 @@ bool_flag(
     build_setting_default = False,
 )
 
+list_flag(
+    name = "mylistflag",
+    build_setting_default = ["c"],
+)
+
 py_library(
     name = "bar",
     srcs = ["pylib.py"],
@@ -1008,24 +1023,31 @@ def format(target):
     return str(target.label) + '%None'
   first = str(bo['//command_line_option:platform_suffix'])
   second = str(('//$pkg:myflag' in bo) and bo['//$pkg:myflag'])
-  return str(target.label) + '%' + first + '%' + second
+  third = str(bo['//$pkg:mylistflag'] if '//$pkg:mylistflag' in bo else None)
+  return str(target.label) + '%' + first + '%' + second + '%' + third
 EOF
 
   bazel cquery "//$pkg:bar" --output=starlark \
     --starlark:file=$pkg/expr.star > output 2>"$TEST_log" || fail "Expected success"
 
-  assert_contains "//$pkg:bar%None%False" output
+  assert_contains "//$pkg:bar%None%False%None" output
+
+  bazel cquery "//$pkg:bar" --output=starlark \
+    --//$pkg:myflag=True --//$pkg:mylistflag=c,d \
+    --starlark:file=$pkg/expr.star > output 2>"$TEST_log" || fail "Expected success"
+
+  assert_contains "//$pkg:bar%None%True%\\[\"c\", \"d\"]" output
 
   bazel cquery "//$pkg:foo" --output=starlark \
     --starlark:file=$pkg/expr.star > output 2>"$TEST_log" || fail "Expected success"
 
-  assert_contains "//$pkg:foo%None%False" output
+  assert_contains "//$pkg:foo%None%False%None" output
 
   bazel cquery "kind(rule, deps(//$pkg:foo))" --output=starlark \
     --starlark:file=$pkg/expr.star > output 2>"$TEST_log" || fail "Expected success"
 
-  assert_contains "//$pkg:foo%None%False" output
-  assert_contains "//$pkg:bar%blah%True" output
+  assert_contains "//$pkg:foo%None%False%None" output
+  assert_contains "//$pkg:bar%blah%True%\\[\"a\", \"b\"]" output
 
   bazel cquery "//$pkg:rules.bzl" --output=starlark \
     --starlark:file=$pkg/expr.star > output 2>"$TEST_log" || fail "Expected success"