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

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,40 @@ def _create_zip_main(ctx, *, stage2_bootstrap, runtime_details, venv):
     )
     return output
 
+# Return a relative path from one path to another, where both paths are each
+# relative paths from a common root.
+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):
+        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 +402,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 +452,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 +502,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 +537,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 = ""
 

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"

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")

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,
+)

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()

tests/bootstrap_impls/venv_relative_path_tests.bzl

@@ -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")
+
+    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)