Skip to content

Commit

Permalink
Convert Starlark options with Starlark#fromJava in build_options
Browse files Browse the repository at this point in the history
`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
  • Loading branch information
iancha1992 authored and copybara-github committed Jul 20, 2023
1 parent a8e821b commit 81cf998
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ public Object buildOptions(ConfiguredTarget target) {

This comment has been minimized.

Copy link
@fmeum

fmeum Jul 21, 2023

Collaborator

@iancha1992 It doesn't really matter, just FYI that the import flow you used lost the commit author information.

// 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();
}
Expand Down
34 changes: 28 additions & 6 deletions src/test/shell/integration/configured_query_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
Expand All @@ -963,6 +972,7 @@ _dep_transition = transition(
inputs = [],
outputs = [
"//$pkg:myflag",
"//$pkg:mylistflag",
"//command_line_option:platform_suffix",
],
)
Expand All @@ -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"])
Expand All @@ -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"],
Expand All @@ -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"
Expand Down

0 comments on commit 81cf998

Please sign in to comment.