Skip to content

Don't write to runfiles #634

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Don't write to runfiles #634

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 2 additions & 13 deletions py/private/py_binary.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
13 changes: 11 additions & 2 deletions py/private/run.tmpl.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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}}"
Expand All @@ -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}}"

Expand All @@ -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}})" "$@"
"{{EXEC_PYTHON_BIN}}" {{INTERPRETER_FLAGS}} "$(rlocation {{ENTRYPOINT}})" "$@"
8 changes: 4 additions & 4 deletions py/tests/py_image_layer/my_app_layers_listing.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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/
Expand All @@ -2484,13 +2484,13 @@ 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/
- drwxr-xr-x 0 0 0 0 Jan 1 2023 ./py/tests/py_image_layer/my_app_bin.runfiles/bazel_tools/tools/bash/
- 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
24 changes: 14 additions & 10 deletions py/tools/py/src/pth.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)?;
}
_ => {
Expand Down
Loading