diff --git a/news/4104.changed.md b/news/4104.changed.md new file mode 100644 index 0000000000..e30e6baeb5 --- /dev/null +++ b/news/4104.changed.md @@ -0,0 +1,3 @@ +(bootstrap) {obj}`sys.path` adds the Python runtime in runfiles instead +of the underlying Bazel repository cache directory +([#4104](https://github.com/bazel-contrib/rules_python/pull/4104)). diff --git a/python/private/py_executable.bzl b/python/private/py_executable.bzl index 0e9c315a73..757fb2c20e 100644 --- a/python/private/py_executable.bzl +++ b/python/private/py_executable.bzl @@ -594,6 +594,7 @@ def _create_venv(ctx, output_prefix, imports, runtime_details, add_runfiles_root "%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", + "%interpreter_actual_path%": interpreter_actual_path, "%site_init_runfiles_path%": runfiles_root_path(ctx, site_init.short_path), "%workspace_name%": ctx.workspace_name, }, diff --git a/python/private/site_init_template.py b/python/private/site_init_template.py index 12be98eb57..d7565026c7 100644 --- a/python/private/site_init_template.py +++ b/python/private/site_init_template.py @@ -29,6 +29,7 @@ _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" +_INTERPRETER_ACTUAL_PATH = "%interpreter_actual_path%" def _is_verbose(): @@ -52,6 +53,7 @@ def _print_verbose(*args, mapping=None, values=None): _print_verbose("workspace_name:", _WORKSPACE_NAME) _print_verbose("self_runfiles_path:", _SELF_RUNFILES_RELATIVE_PATH) _print_verbose("coverage_tool:", _COVERAGE_TOOL) +_print_verbose("interpreter_actual_path:", _INTERPRETER_ACTUAL_PATH) def _find_runfiles_root(): @@ -238,7 +240,94 @@ def _fixup_sys_base_executable(): sys._base_executable = exe +def _fixup_stdlib_paths(): + """Remap non-runfiles runtime paths to their runfiles locations. + + Replaces non-runfiles sys prefix roots (e.g. sys.base_prefix) with the + runtime root inside runfiles across sys.path, sys prefixes, and + site.PREFIXES. + """ + if not _INTERPRETER_ACTUAL_PATH or os.path.isabs(_INTERPRETER_ACTUAL_PATH): + return + if not _RUNFILES_ROOT: + return + + abs_interpreter = os.path.join(_RUNFILES_ROOT, _INTERPRETER_ACTUAL_PATH) + parent = os.path.dirname(abs_interpreter) + if os.path.basename(parent).lower() in ("bin", "scripts"): + runtime_root = os.path.dirname(parent) + else: + runtime_root = parent + + def _norm_path(path_str): + return os.path.normcase(path_str).replace("\\", "/").rstrip("/") + + runfiles_norm = _norm_path(_RUNFILES_ROOT) + runfiles_prefix = runfiles_norm + "/" + + def _in_runfiles(path_str): + norm = _norm_path(path_str) + return norm == runfiles_norm or norm.startswith(runfiles_prefix) + + target_root = _get_windows_path_with_unc_prefix(runtime_root) + if _is_windows(): + target_root = target_root.replace("/", os.sep) + + # When running in a virtual environment (sys.prefix != sys.base_prefix), + # sys.prefix points to the .venv directory (which on Windows may reside + # outside the runfiles tree). Never overwrite sys.prefix / sys.exec_prefix + # with the base Python stdlib root in a venv. + in_venv = sys.prefix != sys.base_prefix + if in_venv: + attrs = ("base_prefix", "base_exec_prefix") + else: + attrs = ("base_prefix", "base_exec_prefix", "prefix", "exec_prefix") + old_prefixes = set() + for attr in attrs: + old_prefix = getattr(sys, attr) + if _in_runfiles(old_prefix): + continue + + # Only remap prefixes leaked from Bazel (external repositories, repo + # cache, or execution root). This avoids remapping system or platform + # Python runtimes (e.g. /usr) when using runtime_env_toolchain. + norm_prefix = _norm_path(old_prefix) + if not any( + marker in norm_prefix for marker in ("/external/", "/cache/", "/execroot/") + ): + continue + + old_prefixes.add(old_prefix) + + _print_verbose(f"remap sys.{attr}:", old_prefix, "->", target_root) + setattr(sys, attr, target_root) + + # Fast path: if no runtime prefixes were replaced, no paths leaked outside + # the tree and no further remapping is needed. + if not old_prefixes: + return + + for i, p in enumerate(sys.path): + norm_p = _norm_path(p) + for old_prefix in old_prefixes: + norm_old = _norm_path(old_prefix) + if norm_p == norm_old or norm_p.startswith(norm_old + "/"): + new_path = target_root + p[len(old_prefix) :] + _print_verbose("remap stdlib sys.path:", p, "->", new_path) + sys.path[i] = new_path + break + + import site + + if hasattr(site, "PREFIXES"): + for i, prefix in enumerate(site.PREFIXES): + if not _in_runfiles(prefix): + _print_verbose("remap site.PREFIXES:", prefix, "->", target_root) + site.PREFIXES[i] = target_root + + _fixup_sys_base_executable() +_fixup_stdlib_paths() COVERAGE_SETUP = _setup_sys_path() _print_verbose("DONE") diff --git a/tests/bootstrap_impls/BUILD.bazel b/tests/bootstrap_impls/BUILD.bazel index 89cd682a6a..2c4eae8d21 100644 --- a/tests/bootstrap_impls/BUILD.bazel +++ b/tests/bootstrap_impls/BUILD.bazel @@ -17,6 +17,7 @@ load("//python:py_test.bzl", "py_test") load("//tests/support:py_reconfig.bzl", "py_reconfig_binary", "py_reconfig_test") load("//tests/support:sh_py_run_test.bzl", "sh_py_run_test") load("//tests/support:support.bzl", "SUPPORTS_BOOTSTRAP_SCRIPT") +load("//tests/support/pytest_test:pytest_test.bzl", "pytest_test") load(":venv_relative_path_tests.bzl", "relative_path_test_suite") py_reconfig_binary( @@ -127,6 +128,18 @@ py_reconfig_test( target_compatible_with = SUPPORTS_BOOTSTRAP_SCRIPT, ) +pytest_test( + name = "stdlib_symlink_syspath_bootstrap_script_test", + srcs = ["stdlib_symlink_syspath_test.py"], + config_settings = { + "//python/config_settings:bootstrap_impl": "script", + }, + target_compatible_with = SUPPORTS_BOOTSTRAP_SCRIPT, + deps = [ + "//python/runfiles", + ], +) + py_reconfig_test( name = "sys_path_order_bootstrap_system_python_test", srcs = ["sys_path_order_test.py"], diff --git a/tests/bootstrap_impls/stdlib_symlink_syspath_test.py b/tests/bootstrap_impls/stdlib_symlink_syspath_test.py new file mode 100644 index 0000000000..a8b1dfadd7 --- /dev/null +++ b/tests/bootstrap_impls/stdlib_symlink_syspath_test.py @@ -0,0 +1,53 @@ +"""Tests that stdlib entries in sys.path point to runfiles locations. + +Verifies stdlib is not added from the underlying repository location. +""" + +from __future__ import annotations + +import os +import pathlib +import re +import sys + +from python.runfiles import runfiles + + +def _is_stdlib_path(path_str: str) -> bool: + norm = path_str.replace("\\", "/").rstrip("/") + base = norm.split("/")[-1].lower() + if base.endswith("-packages"): + return False + if re.match(r"^python\d*\.zip$", base): + return True + if base in ("lib-dynload", "dlls", "lib"): + return True + if re.match(r"^python3\.\d+$", base): + return True + return False + + +def test_stdlib_sys_path_in_runfiles() -> None: + rf = runfiles.CreateOrRaise() + runfiles_root = rf.root() + + stdlib_paths = [p for p in sys.path if _is_stdlib_path(p)] + assert stdlib_paths, ( + "Expected to find at least one stdlib path in sys.path:\n" + "\n".join(sys.path) + ) + + norm_root = pathlib.Path(os.path.normcase(runfiles_root)) + violations = [] + for p in stdlib_paths: + norm_p = pathlib.Path(os.path.normcase(p)) + if not norm_p.is_relative_to(norm_root): + violations.append(p) + + assert not violations, ( + "Expected stdlib sys.path entries to be located within " + f"runfiles tree ({runfiles_root}), but got underlying " + "repository locations:\n" + + "\n".join(f" {v}" for v in violations) + + "\nFull sys.path:\n" + + "\n".join(f" {p}" for p in sys.path) + )