From 447eb1587dafa02f61bd506d04f4e342aff99404 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Fri, 21 Nov 2025 17:39:23 -0800 Subject: [PATCH 1/3] fix: add runfiles root for system_python bootstrap --- CHANGELOG.md | 3 ++- python/private/py_executable.bzl | 6 +++++- python/private/site_init_template.py | 5 +++++ tests/bootstrap_impls/BUILD.bazel | 15 ++++++++++++++ .../bazel_tools_importable_test.py | 20 +++++++++++++++++++ 5 files changed, 47 insertions(+), 2 deletions(-) create mode 100644 tests/bootstrap_impls/bazel_tools_importable_test.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 7d3723c7af..d16a6d6fb7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -77,7 +77,8 @@ END_UNRELEASED_TEMPLATE * (performance) 90% reduction in py_binary/py_test analysis phase cost. ([#3381](https://github.com/bazel-contrib/rules_python/pull/3381)). * (gazelle) Fix `gazelle_python_manifest.test` so that it accesses manifest files via `runfile` path handling rather than directly ([#3397](https://github.com/bazel-contrib/rules_python/issues/3397)). - +* (core rules) For the system_python bootstrap, the runfiles root is added to + sys.path. {#v0-0-0-added} ### Added diff --git a/python/private/py_executable.bzl b/python/private/py_executable.bzl index 669951e172..121c45be22 100644 --- a/python/private/py_executable.bzl +++ b/python/private/py_executable.bzl @@ -341,6 +341,9 @@ def _create_executable( output_prefix = base_executable_name, imports = imports, runtime_details = runtime_details, + add_runfiles_root_to_sys_path = ( + "1" if BootstrapImplFlag.get_value(ctx) == BootstrapImplFlag.SYSTEM_PYTHON else "0" + ), ) stage2_bootstrap = _create_stage2_bootstrap( @@ -504,7 +507,7 @@ def _create_zip_main(ctx, *, stage2_bootstrap, runtime_details, venv): # * https://snarky.ca/how-virtual-environments-work/ # * https://github.com/python/cpython/blob/main/Modules/getpath.py # * https://github.com/python/cpython/blob/main/Lib/site.py -def _create_venv(ctx, output_prefix, imports, runtime_details): +def _create_venv(ctx, output_prefix, imports, runtime_details, add_runfiles_root_to_sys_path): create_full_venv = BootstrapImplFlag.get_value(ctx) == BootstrapImplFlag.SCRIPT venv = "_{}.venv".format(output_prefix.lstrip("_")) @@ -592,6 +595,7 @@ def _create_venv(ctx, output_prefix, imports, runtime_details): template = runtime.site_init_template, output = site_init, substitutions = { + "%add_runfiles_root_to_sys_path%": add_runfiles_root_to_sys_path, "%coverage_tool%": _get_coverage_tool_runfiles_path(ctx, runtime), "%import_all%": "True" if read_possibly_native_flag(ctx, "python_import_all_repositories") else "False", "%site_init_runfiles_path%": "{}/{}".format(ctx.workspace_name, site_init.short_path), diff --git a/python/private/site_init_template.py b/python/private/site_init_template.py index a87a0d2a8f..fb71f1bf9f 100644 --- a/python/private/site_init_template.py +++ b/python/private/site_init_template.py @@ -26,6 +26,8 @@ _SELF_RUNFILES_RELATIVE_PATH = "%site_init_runfiles_path%" # Runfiles-relative path to the coverage tool entry point, if any. _COVERAGE_TOOL = "%coverage_tool%" +# True if the runfiles root should be added to sys.path +_ADD_RUNFILES_ROOT_TO_SYS_PATH = "%add_runfiles_root_to_sys_path%" == "1" def _is_verbose(): @@ -147,6 +149,9 @@ def _maybe_add_path(path): sys.path.append(path) seen.add(path) + if _ADD_RUNFILES_ROOT_TO_SYS_PATH: + _maybe_add_path(_RUNFILES_ROOT) + for rel_path in _IMPORTS_STR.split(":"): abs_path = os.path.join(_RUNFILES_ROOT, rel_path) _maybe_add_path(abs_path) diff --git a/tests/bootstrap_impls/BUILD.bazel b/tests/bootstrap_impls/BUILD.bazel index dcc27514f7..5f7e5afd95 100644 --- a/tests/bootstrap_impls/BUILD.bazel +++ b/tests/bootstrap_impls/BUILD.bazel @@ -104,6 +104,18 @@ sh_py_run_test( target_compatible_with = SUPPORTS_BOOTSTRAP_SCRIPT, ) +py_reconfig_test( + name = "bazel_tools_importable_system_python_test", + srcs = ["bazel_tools_importable_test.py"], + bootstrap_impl = "system_python", + # Necessary because bazel_tools doesn't have __init__.py files. + legacy_create_init = True, + main = "bazel_tools_importable_test.py", + deps = [ + "@bazel_tools//tools/python/runfiles", + ], +) + py_reconfig_test( name = "sys_path_order_bootstrap_script_test", srcs = ["sys_path_order_test.py"], @@ -121,6 +133,9 @@ py_reconfig_test( env = {"BOOTSTRAP": "system_python"}, imports = ["./site-packages"], main = "sys_path_order_test.py", + deps = [ + "@bazel_tools//tools/python/runfiles", + ], ) py_reconfig_test( diff --git a/tests/bootstrap_impls/bazel_tools_importable_test.py b/tests/bootstrap_impls/bazel_tools_importable_test.py new file mode 100644 index 0000000000..1c7ebb64c5 --- /dev/null +++ b/tests/bootstrap_impls/bazel_tools_importable_test.py @@ -0,0 +1,20 @@ +import sys +import unittest + + +class SysPathOrderTest(unittest.TestCase): + def test_bazel_tools_importable(self): + try: + import bazel_tools + import bazel_tools.tools.python + import bazel_tools.tools.python.runfiles + except ImportError as exc: + raise AssertionError( + "Failed to import bazel_tools.python.runfiles\n" + + "sys.path:\n" + + "\n".join(f"{i}: {v}" for i, v in enumerate(sys.path)) + ) from exc + + +if __name__ == "__main__": + unittest.main() From 4e50366bbd8aaea314843f9c61bb1954a4d02a98 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Fri, 21 Nov 2025 18:50:31 -0800 Subject: [PATCH 2/3] update doc --- python/private/site_init_template.py | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/python/private/site_init_template.py b/python/private/site_init_template.py index fb71f1bf9f..97d16b71c2 100644 --- a/python/private/site_init_template.py +++ b/python/private/site_init_template.py @@ -129,11 +129,6 @@ def _search_path(name): def _setup_sys_path(): """Perform Bazel/binary specific sys.path setup. - NOTE: We do not add _RUNFILES_ROOT to sys.path for two reasons: - 1. Under workspace, it makes every external repository importable. If a Bazel - repository matches a Python import name, they conflict. - 2. Under bzlmod, the repo names in the runfiles directory aren't importable - Python names, so there's no point in adding the runfiles root to sys.path. """ seen = set(sys.path) python_path_entries = [] @@ -149,6 +144,14 @@ def _maybe_add_path(path): sys.path.append(path) seen.add(path) + # Adding the runfiles root to sys.path is a legacy behavior that will be + # removed. We don't want to add it to sys.path for two reasons: + # 1. Under workspace, it makes every external repository importable. If a Bazel + # repository matches a Python import name, they conflict. + # 2. Under bzlmod, the repo names in the runfiles directory aren't importable + # Python names, so there's no point in adding the runfiles root to sys.path. + # For temporary compatibility with the original system_python bootstrap + # behavior, it is conditionally added for that boostrap mode. if _ADD_RUNFILES_ROOT_TO_SYS_PATH: _maybe_add_path(_RUNFILES_ROOT) From ddfa3221a6367c9c91fbdea9aedf626831458b38 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Fri, 21 Nov 2025 19:57:52 -0800 Subject: [PATCH 3/3] Update tests/bootstrap_impls/bazel_tools_importable_test.py Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> --- tests/bootstrap_impls/bazel_tools_importable_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/bootstrap_impls/bazel_tools_importable_test.py b/tests/bootstrap_impls/bazel_tools_importable_test.py index 1c7ebb64c5..ad753bc03d 100644 --- a/tests/bootstrap_impls/bazel_tools_importable_test.py +++ b/tests/bootstrap_impls/bazel_tools_importable_test.py @@ -2,7 +2,7 @@ import unittest -class SysPathOrderTest(unittest.TestCase): +class BazelToolsImportableTest(unittest.TestCase): def test_bazel_tools_importable(self): try: import bazel_tools