Skip to content
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.

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

fix: correctly obtain relative path required for the venv created by --bootstrap_impl=script #2439

Merged
merged 12 commits into from
Nov 25, 2024
Merged
70 changes: 59 additions & 11 deletions python/private/py_executable_bazel.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,7 @@ def _create_executable(

def _create_zip_main(ctx, *, stage2_bootstrap, runtime_details, venv):
python_binary = _runfiles_root_path(ctx, venv.interpreter.short_path)
python_binary_actual = _runfiles_root_path(ctx, venv.interpreter_actual_path)
python_binary_actual = venv.interpreter_actual_path

# The location of this file doesn't really matter. It's added to
# the zip file as the top-level __main__.py file and not included
Expand All @@ -344,6 +344,38 @@ def _create_zip_main(ctx, *, stage2_bootstrap, runtime_details, venv):
)
return output

def relative_path(from_, to):
"""Compute a relative path from one path to another.

Args:
from_: {type}`str` the starting directory. Note that it should be
a directory because relative-symlinks are relative to the
directory the symlink resides in.
to: {type}`str` the path that `from_` wants to point to

Returns:
{type}`str` a relative path
"""
from_parts = from_.split("/")
to_parts = to.split("/")

# Strip common leading parts from both paths
# (no while loops in starlark :( )
n = min(len(from_parts), len(to_parts))
for _ in range(n):
rickeylev marked this conversation as resolved.
Show resolved Hide resolved
if from_parts[0] == to_parts[0]:
from_parts.pop(0)
to_parts.pop(0)
else:
break

# Impossible to compute a relative path without knowing what ".." is
if from_parts and from_parts[0] == "..":
fail("cannot compute relative path from '%s' to '%s'", from_, to)

parts = ([".."] * len(from_parts)) + to_parts
return paths.join(*parts)

# Create a venv the executable can use.
# For venv details and the venv startup process, see:
# * https://docs.python.org/3/library/venv.html
Expand All @@ -368,9 +400,15 @@ def _create_venv(ctx, output_prefix, imports, runtime_details):
# in runfiles is always a symlink. An RBE implementation, for example,
# may choose to write what symlink() points to instead.
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

interpreter_actual_path = _runfiles_root_path(ctx, runtime.interpreter.short_path)
rel_path = relative_path(
# dirname is necessary because a relative symlink is relative to
# the directory the symlink resides within.
from_ = paths.dirname(_runfiles_root_path(ctx, interpreter.short_path)),
to = interpreter_actual_path,
)

ctx.actions.symlink(output = interpreter, target_path = rel_path)
else:
py_exe_basename = paths.basename(runtime.interpreter_path)
Expand Down Expand Up @@ -412,7 +450,7 @@ def _create_venv(ctx, output_prefix, imports, runtime_details):

return struct(
interpreter = interpreter,
# Runfiles-relative path or absolute path
# Runfiles root relative path or absolute path
interpreter_actual_path = interpreter_actual_path,
files_without_interpreter = [pyvenv_cfg, pth, site_init],
)
Expand Down Expand Up @@ -462,12 +500,22 @@ def _create_stage2_bootstrap(
)
return output

def _runfiles_root_path(ctx, path):
# The ../ comes from short_path for files in other repos.
if path.startswith("../"):
return path[3:]
def _runfiles_root_path(ctx, short_path):
"""Compute a runfiles-root relative path from `File.short_path`

Args:
ctx: current target ctx
short_path: str, a main-repo relative path from `File.short_path`

Returns:
{type}`str`, a runflies-root relative path
"""

# The ../ comes from short_path is for files in other repos.
if short_path.startswith("../"):
return short_path[3:]
else:
return "{}/{}".format(ctx.workspace_name, path)
return "{}/{}".format(ctx.workspace_name, short_path)

def _create_stage1_bootstrap(
ctx,
Expand All @@ -487,7 +535,7 @@ def _create_stage1_bootstrap(
python_binary_path = runtime_details.executable_interpreter_path

if is_for_zip and venv:
python_binary_actual = _runfiles_root_path(ctx, venv.interpreter_actual_path)
python_binary_actual = venv.interpreter_actual_path
else:
python_binary_actual = ""

Expand Down
1 change: 1 addition & 0 deletions python/private/stage1_bootstrap_template.sh
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ fi
if [[ ! -x "$python_exe" ]]; then
if [[ ! -e "$python_exe" ]]; then
echo >&2 "ERROR: Python interpreter not found: $python_exe"
ls -l $python_exe >&2
exit 1
elif [[ ! -x "$python_exe" ]]; then
echo >&2 "ERROR: Python interpreter not executable: $python_exe"
Expand Down
3 changes: 3 additions & 0 deletions tests/bootstrap_impls/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

load("//python/private:util.bzl", "IS_BAZEL_7_OR_HIGHER") # buildifier: disable=bzl-visibility
load("//tests/support:sh_py_run_test.bzl", "py_reconfig_test", "sh_py_run_test")
load(":venv_relative_path_tests.bzl", "relative_path_test_suite")

_SUPPORTS_BOOTSTRAP_SCRIPT = select({
"@platforms//os:windows": ["@platforms//:incompatible"],
Expand Down Expand Up @@ -87,3 +88,5 @@ sh_py_run_test(
sh_src = "sys_executable_inherits_sys_path_test.sh",
target_compatible_with = _SUPPORTS_BOOTSTRAP_SCRIPT,
)

relative_path_test_suite(name = "relative_path_tests")
15 changes: 15 additions & 0 deletions tests/bootstrap_impls/a/b/c/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
load("//python/private:util.bzl", "IS_BAZEL_7_OR_HIGHER") # buildifier: disable=bzl-visibility
load("//tests/support:sh_py_run_test.bzl", "py_reconfig_test")

_SUPPORTS_BOOTSTRAP_SCRIPT = select({
"@platforms//os:windows": ["@platforms//:incompatible"],
"//conditions:default": [],
}) if IS_BAZEL_7_OR_HIGHER else ["@platforms//:incompatible"]

py_reconfig_test(
name = "nested_dir_test",
srcs = ["nested_dir_test.py"],
bootstrap_impl = "script",
main = "nested_dir_test.py",
target_compatible_with = _SUPPORTS_BOOTSTRAP_SCRIPT,
)
24 changes: 24 additions & 0 deletions tests/bootstrap_impls/a/b/c/nested_dir_test.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
# Copyright 2024 The Bazel Authors. All rights reserved.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
"""Test that the binary being a different directory depth than the underlying interpreter works."""

import unittest


class RunsTest(unittest.TestCase):
def test_runs(self):
pass


unittest.main()
104 changes: 104 additions & 0 deletions tests/bootstrap_impls/venv_relative_path_tests.bzl
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
# Copyright 2023 The Bazel Authors. All rights reserved.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

"Unit tests for relative_path computation"

load("@rules_testing//lib:test_suite.bzl", "test_suite")
load("//python/private:py_executable_bazel.bzl", "relative_path") # buildifier: disable=bzl-visibility

_tests = []

def _relative_path_test(env):
# Basic test cases

env.expect.that_str(
relative_path(
from_ = "a/b",
to = "c/d",
),
).equals("../../c/d")

env.expect.that_str(
relative_path(
from_ = "../a/b",
to = "../c/d",
),
).equals("../../c/d")

env.expect.that_str(
relative_path(
from_ = "../a/b",
to = "../../c/d",
),
).equals("../../../c/d")
rickeylev marked this conversation as resolved.
Show resolved Hide resolved

env.expect.that_str(
relative_path(
from_ = "a/b/c",
to = "a/d",
),
).equals("../../d")
env.expect.that_str(
relative_path(
from_ = "a/b/c",
to = "a/b/c/d/e",
),
).equals("d/e")

# Real examples

# external py_binary uses external python runtime
env.expect.that_str(
relative_path(
from_ = "other_repo~/python/private/_py_console_script_gen_py.venv/bin",
to = "rules_python~~python~python_3_9_x86_64-unknown-linux-gnu/bin/python3",
),
).equals(
"../../../../../rules_python~~python~python_3_9_x86_64-unknown-linux-gnu/bin/python3",
)

# internal py_binary uses external python runtime
env.expect.that_str(
relative_path(
from_ = "_main/test/version_default.venv/bin",
to = "rules_python~~python~python_3_9_x86_64-unknown-linux-gnu/bin/python3",
),
).equals(
"../../../../rules_python~~python~python_3_9_x86_64-unknown-linux-gnu/bin/python3",
)

# external py_binary uses internal python runtime
env.expect.that_str(
relative_path(
from_ = "other_repo~/python/private/_py_console_script_gen_py.venv/bin",
to = "_main/python/python_3_9_x86_64-unknown-linux-gnu/bin/python3",
),
).equals(
"../../../../../_main/python/python_3_9_x86_64-unknown-linux-gnu/bin/python3",
)

# internal py_binary uses internal python runtime
env.expect.that_str(
relative_path(
from_ = "_main/scratch/main.venv/bin",
to = "_main/python/python_3_9_x86_64-unknown-linux-gnu/bin/python3",
),
).equals(
"../../../python/python_3_9_x86_64-unknown-linux-gnu/bin/python3",
)

_tests.append(_relative_path_test)

def relative_path_test_suite(*, name):
test_suite(name = name, basic_tests = _tests)