From 14d536b4d50b03d3ad2eab2b494c04c47693de64 Mon Sep 17 00:00:00 2001 From: Jaden Peterson Date: Thu, 21 Aug 2025 10:22:45 -0400 Subject: [PATCH] fix(runfiles): Don't write to runfiles In remote execution contexts, runfiles aren't always writable. --- py/private/py_binary.bzl | 15 ++---------- py/private/run.tmpl.sh | 13 ++++++++-- .../py_image_layer/my_app_layers_listing.yaml | 8 +++---- py/tools/py/src/pth.rs | 24 +++++++++++-------- 4 files changed, 31 insertions(+), 29 deletions(-) diff --git a/py/private/py_binary.bzl b/py/private/py_binary.bzl index a4727f47..70f721fe 100644 --- a/py/private/py_binary.bzl +++ b/py/private/py_binary.bzl @@ -26,25 +26,14 @@ def _py_binary_rule_impl(ctx): pth_lines.use_param_file("%s", use_always = True) pth_lines.set_param_file_format("multiline") - # The venv is created at the root in the runfiles tree, in 'VENV_NAME', the full path is "${RUNFILES_DIR}/${VENV_NAME}", - # but depending on if we are running as the top level binary or a tool, then $RUNFILES_DIR may be absolute or relative. - # Paths in the .pth are relative to the site-packages folder where they reside. - # All "import" paths from `py_library` start with the workspace name, so we need to go back up the tree for - # each segment from site-packages in the venv to the root of the runfiles tree. - # Five .. will get us back to the root of the venv: - # {name}.runfiles/.{name}.venv/lib/python{version}/site-packages/first_party.pth - # If the target is defined with a slash, it adds to the level of nesting - target_depth = len(ctx.label.name.split("/")) - 1 - escape = "/".join(([".."] * (4 + target_depth))) - # A few imports rely on being able to reference the root of the runfiles tree as a Python module, # the common case here being the @rules_python//python/runfiles target that adds the runfiles helper, # which ends up in bazel_tools/tools/python/runfiles/runfiles.py, but there are no imports attrs that hint we # should be adding the root to the PYTHONPATH # Maybe in the future we can opt out of this? - pth_lines.add(escape) + pth_lines.add(".") - pth_lines.add_all(imports_depset, format_each = "{}/%s".format(escape)) + pth_lines.add_all(imports_depset) site_packages_pth_file = ctx.actions.declare_file("{}.venv.pth".format(ctx.attr.name)) ctx.actions.write( diff --git a/py/private/run.tmpl.sh b/py/private/run.tmpl.sh index 2f009584..9a75c0d0 100644 --- a/py/private/run.tmpl.sh +++ b/py/private/run.tmpl.sh @@ -11,6 +11,7 @@ runfiles_export_envvars set -o errexit -o nounset -o pipefail PWD="$(pwd)" +TEMPORARY_DIRECTORY="$(mktemp -d)" # Returns an absolute path to the given location if the path is relative, otherwise return # the path unchanged. @@ -23,6 +24,10 @@ function alocation { fi } +function cleanup { + rm -rf "${TEMPORARY_DIRECTORY}" +} + function python_location { local PYTHON="{{ARG_PYTHON}}" local RUNFILES_INTERPRETER="{{RUNFILES_INTERPRETER}}" @@ -34,14 +39,18 @@ function python_location { fi } +trap cleanup EXIT + VENV_TOOL="$(rlocation {{VENV_TOOL}})" -VIRTUAL_ENV="$(alocation "${RUNFILES_DIR}/{{ARG_VENV_NAME}}")" +VIRTUAL_ENV="$(alocation "${TEMPORARY_DIRECTORY}/{{ARG_VENV_NAME}}")" + export VIRTUAL_ENV "${VENV_TOOL}" \ --location "${VIRTUAL_ENV}" \ --python "$(python_location)" \ --pth-file "$(rlocation {{ARG_PTH_FILE}})" \ + --pth-entry-prefix "$(alocation ${RUNFILES_DIR})" \ --collision-strategy "{{ARG_COLLISION_STRATEGY}}" \ --venv-name "{{ARG_VENV_NAME}}" @@ -58,4 +67,4 @@ if [ -n "${BASH:-}" -o -n "${ZSH_VERSION:-}" ] ; then hash -r 2> /dev/null fi -exec "{{EXEC_PYTHON_BIN}}" {{INTERPRETER_FLAGS}} "$(rlocation {{ENTRYPOINT}})" "$@" \ No newline at end of file +"{{EXEC_PYTHON_BIN}}" {{INTERPRETER_FLAGS}} "$(rlocation {{ENTRYPOINT}})" "$@" diff --git a/py/tests/py_image_layer/my_app_layers_listing.yaml b/py/tests/py_image_layer/my_app_layers_listing.yaml index 257458ed..69abc198 100644 --- a/py/tests/py_image_layer/my_app_layers_listing.yaml +++ b/py/tests/py_image_layer/my_app_layers_listing.yaml @@ -2471,7 +2471,7 @@ files: - drwxr-xr-x 0 0 0 0 Jan 1 2023 ./py/tests/ - drwxr-xr-x 0 0 0 0 Jan 1 2023 ./py/tests/py_image_layer/ - -rwxr-xr-x 0 0 0 204 Jan 1 2023 ./py/tests/py_image_layer/__main__.py - - -rwxr-xr-x 0 0 0 2895 Jan 1 2023 ./py/tests/py_image_layer/my_app_bin + - -rwxr-xr-x 0 0 0 3067 Jan 1 2023 ./py/tests/py_image_layer/my_app_bin - drwxr-xr-x 0 0 0 0 Jan 1 2023 ./py/tests/py_image_layer/my_app_bin.runfiles/ - drwxr-xr-x 0 0 0 0 Jan 1 2023 ./py/tests/py_image_layer/my_app_bin.runfiles/aspect_rules_py/ - drwxr-xr-x 0 0 0 0 Jan 1 2023 ./py/tests/py_image_layer/my_app_bin.runfiles/aspect_rules_py/py/ @@ -2484,8 +2484,8 @@ files: - -rwxr-xr-x 0 0 0 204 Jan 1 2023 ./py/tests/py_image_layer/my_app_bin.runfiles/aspect_rules_py/py/tests/py_image_layer/__main__.py - drwxr-xr-x 0 0 0 0 Jan 1 2023 ./py/tests/py_image_layer/my_app_bin.runfiles/aspect_rules_py/py/tests/py_image_layer/branding/ - -rwxr-xr-x 0 0 0 42 Jan 1 2023 ./py/tests/py_image_layer/my_app_bin.runfiles/aspect_rules_py/py/tests/py_image_layer/branding/__init__.py - - -rwxr-xr-x 0 0 0 2895 Jan 1 2023 ./py/tests/py_image_layer/my_app_bin.runfiles/aspect_rules_py/py/tests/py_image_layer/my_app_bin - - -rwxr-xr-x 0 0 0 183 Jan 1 2023 ./py/tests/py_image_layer/my_app_bin.runfiles/aspect_rules_py/py/tests/py_image_layer/my_app_bin.venv.pth + - -rwxr-xr-x 0 0 0 3067 Jan 1 2023 ./py/tests/py_image_layer/my_app_bin.runfiles/aspect_rules_py/py/tests/py_image_layer/my_app_bin + - -rwxr-xr-x 0 0 0 125 Jan 1 2023 ./py/tests/py_image_layer/my_app_bin.runfiles/aspect_rules_py/py/tests/py_image_layer/my_app_bin.venv.pth - drwxr-xr-x 0 0 0 0 Jan 1 2023 ./py/tests/py_image_layer/my_app_bin.runfiles/aspect_rules_py/py/tools/ - drwxr-xr-x 0 0 0 0 Jan 1 2023 ./py/tests/py_image_layer/my_app_bin.runfiles/bazel_tools/ - drwxr-xr-x 0 0 0 0 Jan 1 2023 ./py/tests/py_image_layer/my_app_bin.runfiles/bazel_tools/tools/ @@ -2493,4 +2493,4 @@ files: - drwxr-xr-x 0 0 0 0 Jan 1 2023 ./py/tests/py_image_layer/my_app_bin.runfiles/bazel_tools/tools/bash/runfiles/ - -rwxr-xr-x 0 0 0 21622 Jan 1 2023 ./py/tests/py_image_layer/my_app_bin.runfiles/bazel_tools/tools/bash/runfiles/runfiles.bash - drwxr-xr-x 0 0 0 0 Jan 1 2023 ./py/tests/py_image_layer/my_app_bin.runfiles/pypi_colorama/ - - -rwxr-xr-x 0 0 0 183 Jan 1 2023 ./py/tests/py_image_layer/my_app_bin.venv.pth + - -rwxr-xr-x 0 0 0 125 Jan 1 2023 ./py/tests/py_image_layer/my_app_bin.venv.pth diff --git a/py/tools/py/src/pth.rs b/py/tools/py/src/pth.rs index ba00a3fd..f9320da5 100644 --- a/py/tools/py/src/pth.rs +++ b/py/tools/py/src/pth.rs @@ -19,7 +19,7 @@ def _FindPythonRunfilesRoot(): # bazel-bin/py/tests/external-deps/foo.runfiles/.foo.venv/lib/python3.9/site-packages/runfiles # ╰─────────────────────┬─────────────────────╯╰───┬───╯╰─────────────┬─────────────╯╰───┬───╯ # bazel runfiles root venv root Python packages root Python package - + for _ in range("rules_python/python/runfiles/runfiles.py".count("/") + 3): root = os.path.dirname(root) return root @@ -94,15 +94,19 @@ impl PthFile { match entry.file_name() { Some(name) if name == "site-packages" => { - let src_dir = dest - .join(entry.clone()) - .canonicalize() - .into_diagnostic() - .wrap_err(format!( - "Unable to get full source dir path for {} relative to {}", - entry.display(), - dest.display(), - ))?; + let src_dir = if entry.is_absolute() { + entry.clone() + } else { + dest.join(&entry) + } + .canonicalize() + .into_diagnostic() + .wrap_err(format!( + "Unable to get full source dir path for {} relative to {}", + entry.display(), + dest.display(), + ))?; + create_symlinks(&src_dir, &src_dir, &dest, &opts.collision_strategy)?; } _ => {