Skip to content

Commit 45762fc

Browse files
authored
refactor: read migrated native flags through a centralized accessor function (#3290)
This lets Python logic support either Starlark- or native-defined flags, based on Bazel support: - Pre-Bazel 9.0: always use native flags - Bazel 9.0+: use Starlark flags if [incompatible change](bazelbuild/bazel#27056) that disables `ctx.fragments.py` and `ctx.fragments.bazel_py` is set - Include a developer override to trigger Starlark versions while testing Starlarkification. Developers enable this by updating .bzl code in their local workspace. This can be removed as soon as Starlarkification is complete Also check in a Starlark version of `--experimental_python_import_all_repositories` for testing. **This is a no-op. New flag sources of truth don't take effect without a supporting bazel version that sets appropriate incompatible flags.** **Caveats:** - May not work with `configuration_field` as implemented. We can loosen the incompatible flag lockdown if that's an issue. For #3252 TODO list * [ ] Add docs to docs/api/rules_python/python/config_settings/index.md for starlarkified flags
1 parent fec87e4 commit 45762fc

File tree

4 files changed

+69
-9
lines changed

4 files changed

+69
-9
lines changed

python/config_settings/BUILD.bazel

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
load("@bazel_skylib//rules:common_settings.bzl", "string_flag")
1+
load("@bazel_skylib//rules:common_settings.bzl", "bool_flag", "string_flag")
22
load("@pythons_hub//:versions.bzl", "DEFAULT_PYTHON_VERSION", "MINOR_MAPPING", "PYTHON_VERSIONS")
33
load(
44
"//python/private:flags.bzl",
@@ -240,3 +240,9 @@ label_flag(
240240
# NOTE: Only public because it is used in pip hub repos.
241241
visibility = ["//visibility:public"],
242242
)
243+
244+
bool_flag(
245+
name = "experimental_python_import_all_repositories",
246+
build_setting_default = True,
247+
visibility = ["//visibility:public"],
248+
)

python/private/flags.bzl

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,59 @@ unnecessary files when all that are needed are flag definitions.
2121
load("@bazel_skylib//rules:common_settings.bzl", "BuildSettingInfo")
2222
load(":enum.bzl", "FlagEnum", "enum")
2323

24+
# Maps "--myflag" to a tuple of:
25+
#
26+
# - the flag's ctx.fragments native API accessor
27+
# -"native|starlark": which definition to use if the flag is available both
28+
# from ctx.fragments and Starlark
29+
#
30+
# Builds that set --incompatible_remove_ctx_py_fragment or
31+
# --incompatible_remove_ctx_bazel_py_fragment disable ctx.fragments. These
32+
# builds assume flags are solely defined in Starlark.
33+
#
34+
# The "native|starlark" override is only for devs who are testing flag
35+
# Starlarkification. If ctx.fragments.[py|bazel_py] is available and
36+
# a flag is set to "starlark", we exclusively read its starlark version.
37+
#
38+
# See https://github.com/bazel-contrib/rules_python/issues/3252.
39+
_POSSIBLY_NATIVE_FLAGS = {
40+
"build_python_zip": (lambda ctx: ctx.fragments.py.build_python_zip, "native"),
41+
"default_to_explicit_init_py": (lambda ctx: ctx.fragments.py.default_to_explicit_init_py, "native"),
42+
"disable_py2": (lambda ctx: ctx.fragments.py.disable_py2, "native"),
43+
"python_import_all_repositories": (lambda ctx: ctx.fragments.bazel_py.python_import_all_repositories, "native"),
44+
"python_path": (lambda ctx: ctx.fragments.bazel_py.python_path, "native"),
45+
}
46+
47+
def read_possibly_native_flag(ctx, flag_name):
48+
"""
49+
Canonical API for reading a Python build flag.
50+
51+
Flags might be defined in Starlark or native-Bazel. This function reasd flags
52+
from tbe correct source based on supporting Bazel version and --incompatible*
53+
flags that disable native references.
54+
55+
Args:
56+
ctx: Rule's configuration context.
57+
flag_name: Name of the flag to read, without preceding "--".
58+
59+
Returns:
60+
The flag's value.
61+
"""
62+
63+
# Bazel 9.0+ can disable these fragments with --incompatible_remove_ctx_py_fragment and
64+
# --incompatible_remove_ctx_bazel_py_fragment. Disabling them means bazel expects
65+
# Python to read Starlark flags.
66+
use_native_def = hasattr(ctx.fragments, "py") and hasattr(ctx.fragments, "bazel_py")
67+
68+
# Developer override to force the Starlark definition for testing.
69+
if _POSSIBLY_NATIVE_FLAGS[flag_name][1] == "starlark":
70+
use_native_def = False
71+
if use_native_def:
72+
return _POSSIBLY_NATIVE_FLAGS[flag_name][0](ctx)
73+
else:
74+
# Starlark definition of "--foo" is assumed to be a label dependency named "_foo".
75+
return getattr(ctx.attr, "_" + flag_name)[BuildSettingInfo].value
76+
2477
def _AddSrcsToRunfilesFlag_is_enabled(ctx):
2578
value = ctx.attr._add_srcs_to_runfiles_flag[BuildSettingInfo].value
2679
if value == AddSrcsToRunfilesFlag.AUTO:

python/private/py_executable.bzl

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ load(
5353
"target_platform_has_any_constraint",
5454
)
5555
load(":common_labels.bzl", "labels")
56-
load(":flags.bzl", "BootstrapImplFlag", "VenvsUseDeclareSymlinkFlag")
56+
load(":flags.bzl", "BootstrapImplFlag", "VenvsUseDeclareSymlinkFlag", "read_possibly_native_flag")
5757
load(":precompile.bzl", "maybe_precompile")
5858
load(":py_cc_link_params_info.bzl", "PyCcLinkParamsInfo")
5959
load(":py_executable_info.bzl", "PyExecutableInfo")
@@ -293,7 +293,7 @@ def _get_stamp_flag(ctx):
293293

294294
def _should_create_init_files(ctx):
295295
if ctx.attr.legacy_create_init == -1:
296-
return not ctx.fragments.py.default_to_explicit_init_py
296+
return not read_possibly_native_flag(ctx, "default_to_explicit_init_py")
297297
else:
298298
return bool(ctx.attr.legacy_create_init)
299299

@@ -381,7 +381,7 @@ def _create_executable(
381381
extra_files_to_build = []
382382

383383
# NOTE: --build_python_zip defaults to true on Windows
384-
build_zip_enabled = ctx.fragments.py.build_python_zip
384+
build_zip_enabled = read_possibly_native_flag(ctx, "build_python_zip")
385385

386386
# When --build_python_zip is enabled, then the zip file becomes
387387
# one of the default outputs.
@@ -587,7 +587,7 @@ def _create_venv(ctx, output_prefix, imports, runtime_details):
587587
output = site_init,
588588
substitutions = {
589589
"%coverage_tool%": _get_coverage_tool_runfiles_path(ctx, runtime),
590-
"%import_all%": "True" if ctx.fragments.bazel_py.python_import_all_repositories else "False",
590+
"%import_all%": "True" if read_possibly_native_flag(ctx, "python_import_all_repositories") else "False",
591591
"%site_init_runfiles_path%": "{}/{}".format(ctx.workspace_name, site_init.short_path),
592592
"%workspace_name%": ctx.workspace_name,
593593
},
@@ -668,7 +668,7 @@ def _create_stage2_bootstrap(
668668
output = output,
669669
substitutions = {
670670
"%coverage_tool%": _get_coverage_tool_runfiles_path(ctx, runtime),
671-
"%import_all%": "True" if ctx.fragments.bazel_py.python_import_all_repositories else "False",
671+
"%import_all%": "True" if read_possibly_native_flag(ctx, "python_import_all_repositories") else "False",
672672
"%imports%": ":".join(imports.to_list()),
673673
"%main%": main_py_path,
674674
"%main_module%": ctx.attr.main_module,
@@ -755,7 +755,7 @@ def _create_stage1_bootstrap(
755755
template = ctx.file._bootstrap_template
756756

757757
subs["%coverage_tool%"] = coverage_tool_runfiles_path
758-
subs["%import_all%"] = ("True" if ctx.fragments.bazel_py.python_import_all_repositories else "False")
758+
subs["%import_all%"] = ("True" if read_possibly_native_flag(ctx, "python_import_all_repositories") else "False")
759759
subs["%imports%"] = ":".join(imports.to_list())
760760
subs["%main%"] = "{}/{}".format(ctx.workspace_name, main_py.short_path)
761761

@@ -1135,7 +1135,7 @@ def _get_runtime_details(ctx, semantics):
11351135
#
11361136
# TOOD(bazelbuild/bazel#7901): Remove this once --python_path flag is removed.
11371137

1138-
flag_interpreter_path = ctx.fragments.bazel_py.python_path
1138+
flag_interpreter_path = read_possibly_native_flag(ctx, "python_path")
11391139
toolchain_runtime, effective_runtime = _maybe_get_runtime_from_ctx(ctx)
11401140
if not effective_runtime:
11411141
# Clear these just in case

python/private/py_runtime_pair_rule.bzl

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
load("@bazel_skylib//rules:common_settings.bzl", "BuildSettingInfo")
1818
load("//python:py_runtime_info.bzl", "PyRuntimeInfo")
1919
load(":common_labels.bzl", "labels")
20+
load(":flags.bzl", "read_possibly_native_flag")
2021
load(":reexports.bzl", "BuiltinPyRuntimeInfo")
2122
load(":util.bzl", "IS_BAZEL_7_OR_HIGHER")
2223

@@ -69,7 +70,7 @@ def _is_py2_disabled(ctx):
6970
# TODO: Remove this once all supported Balze versions have this flag.
7071
if not hasattr(ctx.fragments.py, "disable_py"):
7172
return False
72-
return ctx.fragments.py.disable_py2
73+
return read_possibly_native_flag(ctx, "disable_py2")
7374

7475
_MaybeBuiltinPyRuntimeInfo = [[BuiltinPyRuntimeInfo]] if BuiltinPyRuntimeInfo != None else []
7576

0 commit comments

Comments
 (0)