Skip to content

Commit f4780f7

Browse files
rickeylevaignas
rickeylev
and
aignas
authored
fix: fixes to prepare for making bootstrap=script the default for Linux (bazel-contrib#2760)
Various cleanup and prep work to switch bootstrap=script to be the default. * Change `bootstrap_impl` to always be disabled for windows. This allows setting it to true in a bazelrc without worrying about the target platform. This is done by using FeatureFlagInfo to force the value to disabled for windows. This allows any downstream usages of the flag to Just Work and not have to add selects() for windows themselves. * Switch pip_repository_annotations test to `import python.runfiles`. The script bootstrap doesn't add the runfiles root to sys.path, so `import rules_python` stops working. * Switch gazelle workspace to using the runtime-env toolchain. It was previously implicitly using the deprecated one built into bazel, which doesn't provide various necessary provider fields. * Make the local toolchain use `sys._base_executable` instead of `sys.executable` when finding the interpreter. Otherwise, it might find a venv interpreter or not properly handle wrapper scripts like pyenv. * Adds a toolchain attribute/field to indicate if the toolchain supports a build-time created venv. This is due to the runtime_env toolchain. See PR comments for details, but in short: if we don't know the python interpreter path and version at build time, the venv may not properly activate or find site-packages. If it isn't supported, then the stage1 bootstrap creates a temporary venv, similar to how the zip case is handled. Unfortunately, this requires invoking Python itself as part of program startup, but I don't see a way around that -- note this is only triggered by the runtime-env toolchain. * Make the runtime-env toolchain better support virtualenvs. Because it's a wrapper that re-invokes Python, Python can't automatically detect its in a venv. Two tricks are used (`exec -a` and PYTHONEXECUTABLE) to help address this (but they aren't guaranteed to work, hence the "recreate at runtime" logic). * Fix a subtle issue where `sys._base_executable` isn't set correctly due to `home` missing in the pyvenv.cfg file. This mostly only affected the creation of venvs from within the bazel-created venv. * Change the bazel site init to always add the build-time created site-packages (if it exists) as a site directory. This matches the system_python bootstrap behavior a bit better, which just shoved everything onto sys.path using PYTHONPATH. * Skip running runtime_env_toolchains tests on RBE. RBE's system python is 3.6, but the script bootstrap uses 3.9 features. (Running it on RBE is questionable anyways). Along the way... * Ignore gazelle convenience symlinks * Switch pip_repository_annotations test to use non-legacy_external_runfiles based paths. The legacy behavior is disabled in Bazel 8+ by default. * Also document why the script bootstrap doesn't add the runfiles root to sys.path. Work towards bazel-contrib#2521 --------- Co-authored-by: Ignas Anikevicius <240938+aignas@users.noreply.github.com>
1 parent aaf8ce8 commit f4780f7

22 files changed

+393
-56
lines changed

‎.bazelignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ examples/pip_parse/bazel-pip_parse
2525
examples/pip_parse_vendored/bazel-pip_parse_vendored
2626
examples/pip_repository_annotations/bazel-pip_repository_annotations
2727
examples/py_proto_library/bazel-py_proto_library
28+
gazelle/bazel-gazelle
2829
tests/integration/compile_pip_requirements/bazel-compile_pip_requirements
2930
tests/integration/ignore_root_user_error/bazel-ignore_root_user_error
3031
tests/integration/local_toolchains/bazel-local_toolchains

‎CHANGELOG.md

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,13 +54,21 @@ END_UNRELEASED_TEMPLATE
5454

5555
{#v0-0-0-changed}
5656
### Changed
57-
* Nothing changed.
57+
* (rules) On Windows, {obj}`--bootstrap_impl=system_python` is forced. This
58+
allows setting `--bootstrap_impl=script` in bazelrc for mixed-platform
59+
environments.
5860

5961
{#v0-0-0-fixed}
6062
### Fixed
63+
6164
* (rules) PyInfo provider is now advertised by py_test, py_binary, and py_library;
6265
this allows aspects using required_providers to function correctly.
6366
([#2506](https://github.com/bazel-contrib/rules_python/issues/2506)).
67+
* Fixes when using {obj}`--bootstrap_impl=script`:
68+
* `compile_pip_requirements` now works with it
69+
* The `sys._base_executable` value will reflect the underlying interpreter,
70+
not venv interpreter.
71+
* The {obj}`//python/runtime_env_toolchains:all` toolchain now works with it.
6472

6573
{#v0-0-0-added}
6674
### Added

‎examples/pip_repository_annotations/.bazelrc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,4 +5,5 @@ try-import %workspace%/user.bazelrc
55
# is in examples/bzlmod as the `whl_mods` feature.
66
common --noenable_bzlmod
77
common --enable_workspace
8+
common --legacy_external_runfiles=false
89
common --incompatible_python_disallow_native_rules

‎examples/pip_repository_annotations/pip_repository_annotations_test.py

Lines changed: 7 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
import unittest
2222
from pathlib import Path
2323

24-
from rules_python.python.runfiles import runfiles
24+
from python.runfiles import runfiles
2525

2626

2727
class PipRepositoryAnnotationsTest(unittest.TestCase):
@@ -34,11 +34,7 @@ def wheel_pkg_dir(self) -> str:
3434

3535
def test_build_content_and_data(self):
3636
r = runfiles.Create()
37-
rpath = r.Rlocation(
38-
"pip_repository_annotations_example/external/{}/generated_file.txt".format(
39-
self.wheel_pkg_dir()
40-
)
41-
)
37+
rpath = r.Rlocation("{}/generated_file.txt".format(self.wheel_pkg_dir()))
4238
generated_file = Path(rpath)
4339
self.assertTrue(generated_file.exists())
4440

@@ -47,11 +43,7 @@ def test_build_content_and_data(self):
4743

4844
def test_copy_files(self):
4945
r = runfiles.Create()
50-
rpath = r.Rlocation(
51-
"pip_repository_annotations_example/external/{}/copied_content/file.txt".format(
52-
self.wheel_pkg_dir()
53-
)
54-
)
46+
rpath = r.Rlocation("{}/copied_content/file.txt".format(self.wheel_pkg_dir()))
5547
copied_file = Path(rpath)
5648
self.assertTrue(copied_file.exists())
5749

@@ -61,7 +53,7 @@ def test_copy_files(self):
6153
def test_copy_executables(self):
6254
r = runfiles.Create()
6355
rpath = r.Rlocation(
64-
"pip_repository_annotations_example/external/{}/copied_content/executable{}".format(
56+
"{}/copied_content/executable{}".format(
6557
self.wheel_pkg_dir(),
6658
".exe" if platform.system() == "windows" else ".py",
6759
)
@@ -82,7 +74,7 @@ def test_data_exclude_glob(self):
8274
current_wheel_version = "0.38.4"
8375

8476
r = runfiles.Create()
85-
dist_info_dir = "pip_repository_annotations_example/external/{}/site-packages/wheel-{}.dist-info".format(
77+
dist_info_dir = "{}/site-packages/wheel-{}.dist-info".format(
8678
self.wheel_pkg_dir(),
8779
current_wheel_version,
8880
)
@@ -113,11 +105,8 @@ def test_extra(self):
113105
# This test verifies that annotations work correctly for pip packages with extras
114106
# specified, in this case requests[security].
115107
r = runfiles.Create()
116-
rpath = r.Rlocation(
117-
"pip_repository_annotations_example/external/{}/generated_file.txt".format(
118-
self.requests_pkg_dir()
119-
)
120-
)
108+
path = "{}/generated_file.txt".format(self.requests_pkg_dir())
109+
rpath = r.Rlocation(path)
121110
generated_file = Path(rpath)
122111
self.assertTrue(generated_file.exists())
123112

‎gazelle/WORKSPACE

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,8 @@ load("//:internal_dev_deps.bzl", "internal_dev_deps")
4242

4343
internal_dev_deps()
4444

45+
register_toolchains("@rules_python//python/runtime_env_toolchains:all")
46+
4547
load("//:deps.bzl", _py_gazelle_deps = "gazelle_deps")
4648

4749
# gazelle:repository_macro deps.bzl%go_deps

‎python/config_settings/BUILD.bazel

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ load(
1111
"PrecompileSourceRetentionFlag",
1212
"VenvsSitePackages",
1313
"VenvsUseDeclareSymlinkFlag",
14+
rp_string_flag = "string_flag",
1415
)
1516
load(
1617
"//python/private/pypi:flags.bzl",
@@ -87,14 +88,27 @@ string_flag(
8788
visibility = ["//visibility:public"],
8889
)
8990

90-
string_flag(
91+
rp_string_flag(
9192
name = "bootstrap_impl",
9293
build_setting_default = BootstrapImplFlag.SYSTEM_PYTHON,
94+
override = select({
95+
# Windows doesn't yet support bootstrap=script, so force disable it
96+
":_is_windows": BootstrapImplFlag.SYSTEM_PYTHON,
97+
"//conditions:default": "",
98+
}),
9399
values = sorted(BootstrapImplFlag.__members__.values()),
94100
# NOTE: Only public because it's an implicit dependency
95101
visibility = ["//visibility:public"],
96102
)
97103

104+
# For some reason, @platforms//os:windows can't be directly used
105+
# in the select() for the flag. But it can be used when put behind
106+
# a config_setting().
107+
config_setting(
108+
name = "_is_windows",
109+
constraint_values = ["@platforms//os:windows"],
110+
)
111+
98112
# This is used for pip and hermetic toolchain resolution.
99113
string_flag(
100114
name = "py_linux_libc",

‎python/private/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ bzl_library(
8686
name = "runtime_env_toolchain_bzl",
8787
srcs = ["runtime_env_toolchain.bzl"],
8888
deps = [
89+
":config_settings_bzl",
8990
":py_exec_tools_toolchain_bzl",
9091
":toolchain_types_bzl",
9192
"//python:py_runtime_bzl",

‎python/private/config_settings.bzl

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -209,3 +209,33 @@ _current_config = rule(
209209
"_template": attr.string(default = _DEBUG_ENV_MESSAGE_TEMPLATE),
210210
},
211211
)
212+
213+
def is_python_version_at_least(name, **kwargs):
214+
flag_name = "_{}_flag".format(name)
215+
native.config_setting(
216+
name = name,
217+
flag_values = {
218+
flag_name: "yes",
219+
},
220+
)
221+
_python_version_at_least(
222+
name = flag_name,
223+
visibility = ["//visibility:private"],
224+
**kwargs
225+
)
226+
227+
def _python_version_at_least_impl(ctx):
228+
at_least = tuple(ctx.attr.at_least.split("."))
229+
current = tuple(
230+
ctx.attr._major_minor[config_common.FeatureFlagInfo].value.split("."),
231+
)
232+
value = "yes" if current >= at_least else "no"
233+
return [config_common.FeatureFlagInfo(value = value)]
234+
235+
_python_version_at_least = rule(
236+
implementation = _python_version_at_least_impl,
237+
attrs = {
238+
"at_least": attr.string(mandatory = True),
239+
"_major_minor": attr.label(default = _PYTHON_VERSION_MAJOR_MINOR_FLAG),
240+
},
241+
)

‎python/private/flags.bzl

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,38 @@ AddSrcsToRunfilesFlag = FlagEnum(
3535
is_enabled = _AddSrcsToRunfilesFlag_is_enabled,
3636
)
3737

38+
def _string_flag_impl(ctx):
39+
if ctx.attr.override:
40+
value = ctx.attr.override
41+
else:
42+
value = ctx.build_setting_value
43+
44+
if value not in ctx.attr.values:
45+
fail((
46+
"Invalid value for {name}: got {value}, must " +
47+
"be one of {allowed}"
48+
).format(
49+
name = ctx.label,
50+
value = value,
51+
allowed = ctx.attr.values,
52+
))
53+
54+
return [
55+
BuildSettingInfo(value = value),
56+
config_common.FeatureFlagInfo(value = value),
57+
]
58+
59+
string_flag = rule(
60+
implementation = _string_flag_impl,
61+
build_setting = config.string(flag = True),
62+
attrs = {
63+
"override": attr.string(),
64+
"values": attr.string_list(),
65+
},
66+
)
67+
3868
def _bootstrap_impl_flag_get_value(ctx):
39-
return ctx.attr._bootstrap_impl_flag[BuildSettingInfo].value
69+
return ctx.attr._bootstrap_impl_flag[config_common.FeatureFlagInfo].value
4070

4171
# buildifier: disable=name-conventions
4272
BootstrapImplFlag = enum(

‎python/private/get_local_runtime_info.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
"micro": sys.version_info.micro,
2323
"include": sysconfig.get_path("include"),
2424
"implementation_name": sys.implementation.name,
25+
"base_executable": sys._base_executable,
2526
}
2627

2728
config_vars = [

‎python/private/local_runtime_repo.bzl

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,20 @@ def _local_runtime_repo_impl(rctx):
8484
info = json.decode(exec_result.stdout)
8585
logger.info(lambda: _format_get_info_result(info))
8686

87+
# We use base_executable because we want the path within a Python
88+
# installation directory ("PYTHONHOME"). The problems with sys.executable
89+
# are:
90+
# * If we're in an activated venv, then we don't want the venv's
91+
# `bin/python3` path to be used -- it isn't an actual Python installation.
92+
# * If sys.executable is a wrapper (e.g. pyenv), then (1) it may not be
93+
# located within an actual Python installation directory, and (2) it
94+
# can interfer with Python recognizing when it's within a venv.
95+
#
96+
# In some cases, it may be a symlink (usually e.g. `python3->python3.12`),
97+
# but we don't realpath() it to respect what it has decided is the
98+
# appropriate path.
99+
interpreter_path = info["base_executable"]
100+
87101
# NOTE: Keep in sync with recursive glob in define_local_runtime_toolchain_impl
88102
repo_utils.watch_tree(rctx, rctx.path(info["include"]))
89103

‎python/private/py_executable.bzl

Lines changed: 31 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -350,6 +350,7 @@ def _create_executable(
350350
main_py = main_py,
351351
imports = imports,
352352
runtime_details = runtime_details,
353+
venv = venv,
353354
)
354355
extra_runfiles = ctx.runfiles([stage2_bootstrap] + venv.files_without_interpreter)
355356
zip_main = _create_zip_main(
@@ -538,11 +539,14 @@ def _create_venv(ctx, output_prefix, imports, runtime_details):
538539
ctx.actions.write(pyvenv_cfg, "")
539540

540541
runtime = runtime_details.effective_runtime
542+
541543
venvs_use_declare_symlink_enabled = (
542544
VenvsUseDeclareSymlinkFlag.get_value(ctx) == VenvsUseDeclareSymlinkFlag.YES
543545
)
546+
recreate_venv_at_runtime = False
544547

545-
if not venvs_use_declare_symlink_enabled:
548+
if not venvs_use_declare_symlink_enabled or not runtime.supports_build_time_venv:
549+
recreate_venv_at_runtime = True
546550
if runtime.interpreter:
547551
interpreter_actual_path = runfiles_root_path(ctx, runtime.interpreter.short_path)
548552
else:
@@ -557,6 +561,8 @@ def _create_venv(ctx, output_prefix, imports, runtime_details):
557561
ctx.actions.write(interpreter, "actual:{}".format(interpreter_actual_path))
558562

559563
elif runtime.interpreter:
564+
# Some wrappers around the interpreter (e.g. pyenv) use the program
565+
# name to decide what to do, so preserve the name.
560566
py_exe_basename = paths.basename(runtime.interpreter.short_path)
561567

562568
# Even though ctx.actions.symlink() is used, using
@@ -594,7 +600,8 @@ def _create_venv(ctx, output_prefix, imports, runtime_details):
594600
if "t" in runtime.abi_flags:
595601
version += "t"
596602

597-
site_packages = "{}/lib/python{}/site-packages".format(venv, version)
603+
venv_site_packages = "lib/python{}/site-packages".format(version)
604+
site_packages = "{}/{}".format(venv, venv_site_packages)
598605
pth = ctx.actions.declare_file("{}/bazel.pth".format(site_packages))
599606
ctx.actions.write(pth, "import _bazel_site_init\n")
600607

@@ -616,10 +623,12 @@ def _create_venv(ctx, output_prefix, imports, runtime_details):
616623

617624
return struct(
618625
interpreter = interpreter,
619-
recreate_venv_at_runtime = not venvs_use_declare_symlink_enabled,
626+
recreate_venv_at_runtime = recreate_venv_at_runtime,
620627
# Runfiles root relative path or absolute path
621628
interpreter_actual_path = interpreter_actual_path,
622629
files_without_interpreter = [pyvenv_cfg, pth, site_init] + site_packages_symlinks,
630+
# string; venv-relative path to the site-packages directory.
631+
venv_site_packages = venv_site_packages,
623632
)
624633

625634
def _create_site_packages_symlinks(ctx, site_packages):
@@ -716,7 +725,8 @@ def _create_stage2_bootstrap(
716725
output_sibling,
717726
main_py,
718727
imports,
719-
runtime_details):
728+
runtime_details,
729+
venv = None):
720730
output = ctx.actions.declare_file(
721731
# Prepend with underscore to prevent pytest from trying to
722732
# process the bootstrap for files starting with `test_`
@@ -731,6 +741,14 @@ def _create_stage2_bootstrap(
731741
main_py_path = "{}/{}".format(ctx.workspace_name, main_py.short_path)
732742
else:
733743
main_py_path = ""
744+
745+
# The stage2 bootstrap uses the venv site-packages location to fix up issues
746+
# that occur when the toolchain doesn't support the build-time venv.
747+
if venv and not runtime.supports_build_time_venv:
748+
venv_rel_site_packages = venv.venv_site_packages
749+
else:
750+
venv_rel_site_packages = ""
751+
734752
ctx.actions.expand_template(
735753
template = template,
736754
output = output,
@@ -741,6 +759,7 @@ def _create_stage2_bootstrap(
741759
"%main%": main_py_path,
742760
"%main_module%": ctx.attr.main_module,
743761
"%target%": str(ctx.label),
762+
"%venv_rel_site_packages%": venv_rel_site_packages,
744763
"%workspace_name%": ctx.workspace_name,
745764
},
746765
is_executable = True,
@@ -766,6 +785,12 @@ def _create_stage1_bootstrap(
766785

767786
python_binary_actual = venv.interpreter_actual_path if venv else ""
768787

788+
# Runtime may be None on Windows due to the --python_path flag.
789+
if runtime and runtime.supports_build_time_venv:
790+
resolve_python_binary_at_runtime = "0"
791+
else:
792+
resolve_python_binary_at_runtime = "1"
793+
769794
subs = {
770795
"%interpreter_args%": "\n".join([
771796
'"{}"'.format(v)
@@ -775,7 +800,9 @@ def _create_stage1_bootstrap(
775800
"%python_binary%": python_binary_path,
776801
"%python_binary_actual%": python_binary_actual,
777802
"%recreate_venv_at_runtime%": str(int(venv.recreate_venv_at_runtime)) if venv else "0",
803+
"%resolve_python_binary_at_runtime%": resolve_python_binary_at_runtime,
778804
"%target%": str(ctx.label),
805+
"%venv_rel_site_packages%": venv.venv_site_packages if venv else "",
779806
"%workspace_name%": ctx.workspace_name,
780807
}
781808

0 commit comments

Comments
 (0)