diff --git a/python/private/py_executable_bazel.bzl b/python/private/py_executable_bazel.bzl index 60c3815c99..3778c192b4 100644 --- a/python/private/py_executable_bazel.bzl +++ b/python/private/py_executable_bazel.bzl @@ -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 @@ -344,6 +344,37 @@ 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 + n = min(len(from_parts), len(to_parts)) + for _ in range(n): + 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 @@ -368,9 +399,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) @@ -412,7 +449,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], ) @@ -462,12 +499,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, @@ -487,7 +534,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 = "" diff --git a/python/private/stage1_bootstrap_template.sh b/python/private/stage1_bootstrap_template.sh index afa1ee84bd..b05b4a54cd 100644 --- a/python/private/stage1_bootstrap_template.sh +++ b/python/private/stage1_bootstrap_template.sh @@ -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" diff --git a/tests/bootstrap_impls/BUILD.bazel b/tests/bootstrap_impls/BUILD.bazel index 1586821896..2fb1f38ff0 100644 --- a/tests/bootstrap_impls/BUILD.bazel +++ b/tests/bootstrap_impls/BUILD.bazel @@ -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"], @@ -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") diff --git a/tests/bootstrap_impls/a/b/c/BUILD.bazel b/tests/bootstrap_impls/a/b/c/BUILD.bazel new file mode 100644 index 0000000000..8ffcbcd479 --- /dev/null +++ b/tests/bootstrap_impls/a/b/c/BUILD.bazel @@ -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, +) diff --git a/tests/bootstrap_impls/a/b/c/nested_dir_test.py b/tests/bootstrap_impls/a/b/c/nested_dir_test.py new file mode 100644 index 0000000000..2db0e6c771 --- /dev/null +++ b/tests/bootstrap_impls/a/b/c/nested_dir_test.py @@ -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() diff --git a/tests/bootstrap_impls/venv_relative_path_tests.bzl b/tests/bootstrap_impls/venv_relative_path_tests.bzl new file mode 100644 index 0000000000..b21f220205 --- /dev/null +++ b/tests/bootstrap_impls/venv_relative_path_tests.bzl @@ -0,0 +1,90 @@ +# 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/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)