Skip to content

Commit

Permalink
Fix ParsedFlagsFunction to properly convert label-typed values.
Browse files Browse the repository at this point in the history
Fixes bazelbuild#23115.

Closes bazelbuild#23272.

PiperOrigin-RevId: 662599406
Change-Id: I12ee5ef4596d3ef9f22227bbeece86f0dc63a249
  • Loading branch information
katre authored and copybara-github committed Aug 13, 2024
1 parent 32e0a21 commit b257fee
Show file tree
Hide file tree
Showing 2 changed files with 92 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,12 @@ public SkyValue compute(SkyKey skyKey, Environment env)
starlarkFlags.add(flagSetting);
}
}
// The StarlarkOptionsParser needs a native options parser just to inject its Starlark flag
// values. It doesn't actually parse anything with the native parser.
OptionsParser fakeNativeParser = OptionsParser.builder().build();
// The StarlarkOptionsParser needs a native options parser to handle some forms of value
// conversion and as a place to inject the flag values.
// TODO: https://github.com/bazelbuild/bazel/issues/22365 - Clean this up as part of a general
// rewrite.
OptionsParser fakeNativeParser =
OptionsParser.builder().withConversionContext(key.packageContext()).build();
StarlarkOptionsParser starlarkFlagParser =
StarlarkOptionsParser.builder()
.buildSettingLoader(new SkyframeTargetLoader(env, key.packageContext()))
Expand Down
86 changes: 86 additions & 0 deletions src/test/shell/integration/platform_based_flags_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -580,4 +580,90 @@ EOF
done
}

function is_bazel() {
[ $TEST_WORKSPACE == "_main" ]
}

# Regression test for https://github.com/bazelbuild/bazel/issues/23115
function test_module_label_flag() {
# Only bazel supports modules.
is_bazel || return 0

local -r pkg="$FUNCNAME"
mkdir -p "$pkg"

# Create another module that defines a label flag.
mkdir -p "$pkg/flags"
cat > "$pkg/flags/MODULE.bazel" <<EOF
module(name = "flags")
EOF

write_flags "${pkg}/flags/pbf"
cat >>${pkg}/flags/rule.bzl <<EOF
load('//pbf:sample_flag.bzl', 'BuildSettingInfo')
def _impl(ctx):
value = ctx.attr.value
return BuildSettingInfo(value = value)
sample_value = rule(
implementation = _impl,
attrs = {
"value": attr.string(),
},
)
EOF
cat >>${pkg}/flags/BUILD <<EOF
load(":rule.bzl", "sample_value")
package(default_visibility = ["//visibility:public"])
label_flag(
name = "label_flag",
build_setting_default = ":default",
)
sample_value(
name = "default",
value = "default",
)
sample_value(
name = "alt",
value = "alt",
)
EOF

# Refer to the module from the base.
cat > MODULE.bazel <<EOF
module(name = "main")
bazel_dep(name = "flags")
local_path_override(
module_name = "flags",
path = "${pkg}/flags",
)
EOF

cat >> $pkg/BUILD <<EOF
load("@flags//pbf:sample_flag.bzl", "show_sample_flag")
platform(
name = "pbf_demo",
flags = [
# Test setting the value to a label in the module.
"--@flags//:label_flag=@flags//:alt",
],
)
show_sample_flag(
name = "show",
flag = "@flags//:label_flag",
)
EOF

bazel build --platforms="//$pkg:pbf_demo" //$pkg:show &> $TEST_log || fail "bazel failed"
expect_log "//${pkg}:show: value = \"alt\""
}

run_suite "Tests for platform based flags"

0 comments on commit b257fee

Please sign in to comment.