From d68262ce467fb102c17e1968648b15abb4fd84be Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Thu, 14 Nov 2024 15:54:33 -0800 Subject: [PATCH] always use declare_symlink --- python/private/py_executable_bazel.bzl | 9 +- python/private/py_runtime_info.bzl | 4 +- python/private/py_runtime_rule.bzl | 2 +- python/private/stage1_bootstrap_template.sh | 9 +- python/private/stage2_bootstrap_template.py | 95 +------------------- tests/bootstrap_impls/sys_path_order_test.py | 2 +- 6 files changed, 17 insertions(+), 104 deletions(-) diff --git a/python/private/py_executable_bazel.bzl b/python/private/py_executable_bazel.bzl index b5f519efb..ac58b9b15 100644 --- a/python/private/py_executable_bazel.bzl +++ b/python/private/py_executable_bazel.bzl @@ -362,9 +362,14 @@ def _create_venv(ctx, output_prefix, imports, runtime_details): runtime = runtime_details.effective_runtime if runtime.interpreter: py_exe_basename = paths.basename(runtime.interpreter.short_path) - interpreter = ctx.actions.declare_file("{}/bin/{}".format(venv, py_exe_basename)) - ctx.actions.symlink(output = interpreter, target_file = runtime.interpreter) + + # A real symlink is required to support RBE. RBE may materialize what + # ctx.actions.symlink() points to. + interpreter = ctx.actions.declare_symlink("{}/bin/{}".format(venv, py_exe_basename)) interpreter_actual_path = runtime.interpreter.short_path + parent = "/".join([".."] * (interpreter_actual_path.count("/") + 1)) + rel_path = parent + "/" + interpreter_actual_path + ctx.actions.symlink(output = interpreter, target_path = rel_path) else: py_exe_basename = paths.basename(runtime.interpreter_path) interpreter = ctx.actions.declare_symlink("{}/bin/{}".format(venv, py_exe_basename)) diff --git a/python/private/py_runtime_info.bzl b/python/private/py_runtime_info.bzl index 038b4ef72..34be0db69 100644 --- a/python/private/py_runtime_info.bzl +++ b/python/private/py_runtime_info.bzl @@ -147,7 +147,7 @@ the same conventions as the standard CPython interpreter. The runtime's ABI flags, i.e. `sys.abiflags`. -:::{versionadded} 0.39.0 +:::{versionadded} 0.41.0 ::: """, "bootstrap_template": """ @@ -281,7 +281,7 @@ are (only) `"PY2"` and `"PY3"`. The template to use for the binary-specific site-init hook run by the interpreter at startup. -:::{versionadded} VERSION_NEXT_FEATURE +:::{versionadded} 0.41.0 ::: """, "stage2_bootstrap_template": """ diff --git a/python/private/py_runtime_rule.bzl b/python/private/py_runtime_rule.bzl index bccf05c2d..5ce8161cf 100644 --- a/python/private/py_runtime_rule.bzl +++ b/python/private/py_runtime_rule.bzl @@ -324,7 +324,7 @@ value. The template to use for the binary-specific site-init hook run by the interpreter at startup. -:::{versionadded} VERSION_NEXT_FEATURE +:::{versionadded} 0.41.0 ::: """, ), diff --git a/python/private/stage1_bootstrap_template.sh b/python/private/stage1_bootstrap_template.sh index dc5d1b0b0..1a79476e3 100644 --- a/python/private/stage1_bootstrap_template.sh +++ b/python/private/stage1_bootstrap_template.sh @@ -129,10 +129,11 @@ if [[ "$IS_ZIPFILE" == "1" ]]; then mkdir -p "$(dirname $python_exe)" ln -s "$symlink_to" "$python_exe" else - set -x - stat "$python_exe" - ls -l $(dirname $python_exe) - set +x + : + #set -x + #stat "$python_exe" + #ls -l $(dirname $python_exe) + #set +x fi # At this point, we should have a valid reference to the interpreter. diff --git a/python/private/stage2_bootstrap_template.py b/python/private/stage2_bootstrap_template.py index b505d3649..30f0b7749 100644 --- a/python/private/stage2_bootstrap_template.py +++ b/python/private/stage2_bootstrap_template.py @@ -16,7 +16,6 @@ import os import re import runpy -import subprocess import uuid # ===== Template substitutions start ===== @@ -24,8 +23,6 @@ # Runfiles-relative path to the main Python source file. MAIN = "%main%" -# Runfiles-relative path to the coverage tool entry point, if any. -COVERAGE_TOOL = "%coverage_tool%" # ===== Template substitutions end ===== @@ -114,45 +111,6 @@ def is_verbose_coverage(): return os.environ.get("VERBOSE_COVERAGE") or is_verbose() -##def find_coverage_entry_point(module_space): -## cov_tool = COVERAGE_TOOL -## if cov_tool: -## print_verbose_coverage("Using toolchain coverage_tool %r" % cov_tool) -## else: -## cov_tool = os.environ.get("PYTHON_COVERAGE") -## if cov_tool: -## print_verbose_coverage("PYTHON_COVERAGE: %r" % cov_tool) -## if cov_tool: -## return find_binary(module_space, cov_tool) -## return None - - -def find_binary(module_space, bin_name): - """Finds the real binary if it's not a normal absolute path.""" - if not bin_name: - return None - if bin_name.startswith("//"): - # Case 1: Path is a label. Not supported yet. - raise AssertionError( - "Bazel does not support execution of Python interpreters via labels yet" - ) - elif os.path.isabs(bin_name): - # Case 2: Absolute path. - return bin_name - # Use normpath() to convert slashes to os.sep on Windows. - elif os.sep in os.path.normpath(bin_name): - # Case 3: Path is relative to the repo root. - return os.path.join(module_space, bin_name) - else: - # Case 4: Path has to be looked up in the search path. - return search_path(bin_name) - - -##def create_python_path_entries(python_imports, module_space): -## parts = python_imports.split(":") -## return [module_space] + ["%s/%s" % (module_space, path) for path in parts] - - def find_runfiles_root(main_rel_path): """Finds the runfiles tree.""" # When the calling process used the runfiles manifest to resolve the @@ -197,15 +155,6 @@ def find_runfiles_root(main_rel_path): raise AssertionError("Cannot find .runfiles directory for %s" % sys.argv[0]) -# Returns repository roots to add to the import path. -##def get_repositories_imports(module_space, import_all): -## if import_all: -## repo_dirs = [os.path.join(module_space, d) for d in os.listdir(module_space)] -## repo_dirs.sort() -## return [d for d in repo_dirs if os.path.isdir(d)] -## return [os.path.join(module_space, WORKSPACE_NAME)] - - def runfiles_envvar(module_space): """Finds the runfiles manifest or the runfiles directory. @@ -245,15 +194,6 @@ def runfiles_envvar(module_space): return (None, None) -def deduplicate(items): - """Efficiently filter out duplicates, keeping the first element only.""" - seen = set() - for it in items: - if it not in seen: - seen.add(it) - yield it - - def instrumented_file_paths(): """Yields tuples of realpath of each instrumented file with the relative path.""" manifest_filename = os.environ.get("COVERAGE_MANIFEST") @@ -424,29 +364,12 @@ def main(): # # To replicate this behavior, we add main's directory within the runfiles # when safe path isn't enabled. - python_path_entries = [] - prepend_path_entries = [] if not getattr(sys.flags, "safe_path", False): prepend_path_entries = [ os.path.join(module_space, os.path.dirname(main_rel_path)) ] else: prepend_path_entries = [] - ##python_path_entries = create_python_path_entries(IMPORTS_STR, module_space) - ##python_path_entries += get_repositories_imports(module_space, IMPORT_ALL) - python_path_entries = [ - get_windows_path_with_unc_prefix(d) for d in python_path_entries - ] - - # Remove duplicates to avoid overly long PYTHONPATH (#10977). Preserve order, - # keep first occurrence only. - python_path_entries = list(deduplicate(python_path_entries)) - - ##if is_windows(): - ## python_path_entries = [p.replace("/", os.sep) for p in python_path_entries] - ##else: - ## # deduplicate returns a generator, but we need a list after this. - ## python_path_entries = list(python_path_entries) runfiles_envkey, runfiles_envvalue = runfiles_envvar(module_space) if runfiles_envkey: @@ -463,28 +386,12 @@ def main(): sys.stdout.flush() - # Add the user imports after the stdlib, but before the runtime's - # site-packages directory. This gives the stdlib precedence, while allowing - # users to override non-stdlib packages that may have been bundled with - # the runtime (usually pip). - # NOTE: There isn't a good way to identify the stdlib paths, so we just - # expect site-packages comes after it, per - # https://docs.python.org/3/library/sys_path_init.html#sys-path-init - for i, path in enumerate(sys.path): - # dist-packages is a debian convention, see - # https://wiki.debian.org/Python#Deviations_from_upstream - if os.path.basename(path) in ("site-packages", "dist-packages"): - sys.path[i:i] = python_path_entries - break - else: - # Otherwise, no site-packages directory was found, which is odd but ok. - sys.path.extend(python_path_entries) - # NOTE: The sys.path must be modified before coverage is imported/activated # NOTE: Perform this after the user imports are appended. This avoids a # user import accidentally triggering the site-packages logic above. sys.path[0:0] = prepend_path_entries + # todo: also check COVERAGE_DIR env var import _bazel_site_init with _maybe_collect_coverage(enable=_bazel_site_init.COVERAGE_SETUP): diff --git a/tests/bootstrap_impls/sys_path_order_test.py b/tests/bootstrap_impls/sys_path_order_test.py index ae8c163db..97c62a6be 100644 --- a/tests/bootstrap_impls/sys_path_order_test.py +++ b/tests/bootstrap_impls/sys_path_order_test.py @@ -35,7 +35,7 @@ def test_sys_path_order(self): for i, value in enumerate(sys.path): # The runtime's root repo may be added to sys.path, but it # counts as a user directory, not stdlib directory. - if value == sys.prefix: + if value in (sys.prefix, sys.base_prefix): category = "user" elif value.startswith(sys.base_prefix): # The runtime's site-package directory might be called