From 04faacdcb348fad657a4bb401e555739a511a726 Mon Sep 17 00:00:00 2001
From: chowder <16789070+chowder@users.noreply.github.com>
Date: Sat, 23 Nov 2024 20:05:07 +0000
Subject: [PATCH 01/12] fix: correctly count the escapes required for the venv
 created by script bootstrap implementation

---
 python/private/py_executable_bazel.bzl | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/python/private/py_executable_bazel.bzl b/python/private/py_executable_bazel.bzl
index 60c3815c99..38a0c82905 100644
--- a/python/private/py_executable_bazel.bzl
+++ b/python/private/py_executable_bazel.bzl
@@ -191,6 +191,7 @@ def _create_executable(
         hasattr(runtime_details.effective_runtime, "stage2_bootstrap_template")):
         venv = _create_venv(
             ctx,
+            executable = executable,
             output_prefix = base_executable_name,
             imports = imports,
             runtime_details = runtime_details,
@@ -350,7 +351,7 @@ def _create_zip_main(ctx, *, stage2_bootstrap, runtime_details, venv):
 # * https://snarky.ca/how-virtual-environments-work/
 # * https://github.com/python/cpython/blob/main/Modules/getpath.py
 # * https://github.com/python/cpython/blob/main/Lib/site.py
-def _create_venv(ctx, output_prefix, imports, runtime_details):
+def _create_venv(ctx, executable, output_prefix, imports, runtime_details):
     venv = "_{}.venv".format(output_prefix.lstrip("_"))
 
     # The pyvenv.cfg file must be present to trigger the venv site hooks.
@@ -368,8 +369,12 @@ 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))
+        interpreter_actual_path = runtime.interpreter.short_path  # Always relative to .runfiles/${workspace}
+
+        escapes = 2  # To escape out of ${target}.venv/bin
+        escapes += executable.short_path.count("/") # To escape into .runfiles/${workspace}
+
+        parent = "/".join([".."] * escapes)
         rel_path = parent + "/" + interpreter_actual_path
         ctx.actions.symlink(output = interpreter, target_path = rel_path)
     else:

From ca1d310ea7062774ec5e776930afe2e6e792a6e6 Mon Sep 17 00:00:00 2001
From: chowder <16789070+chowder@users.noreply.github.com>
Date: Sat, 23 Nov 2024 20:13:31 +0000
Subject: [PATCH 02/12] buildifier

---
 python/private/py_executable_bazel.bzl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/python/private/py_executable_bazel.bzl b/python/private/py_executable_bazel.bzl
index 38a0c82905..5846d63a01 100644
--- a/python/private/py_executable_bazel.bzl
+++ b/python/private/py_executable_bazel.bzl
@@ -372,7 +372,7 @@ def _create_venv(ctx, executable, output_prefix, imports, runtime_details):
         interpreter_actual_path = runtime.interpreter.short_path  # Always relative to .runfiles/${workspace}
 
         escapes = 2  # To escape out of ${target}.venv/bin
-        escapes += executable.short_path.count("/") # To escape into .runfiles/${workspace}
+        escapes += executable.short_path.count("/")  # To escape into .runfiles/${workspace}
 
         parent = "/".join([".."] * escapes)
         rel_path = parent + "/" + interpreter_actual_path

From f0b36ae866006d286ace9e529fd80b92677bc0b3 Mon Sep 17 00:00:00 2001
From: chowder <16789070+chowder@users.noreply.github.com>
Date: Sun, 24 Nov 2024 00:24:56 +0000
Subject: [PATCH 03/12] compute relative path from venv bin dir to actual
 interpreter path

---
 python/private/py_executable_bazel.bzl | 34 +++++++++++++++++++-------
 1 file changed, 25 insertions(+), 9 deletions(-)

diff --git a/python/private/py_executable_bazel.bzl b/python/private/py_executable_bazel.bzl
index 5846d63a01..e99faf2ca1 100644
--- a/python/private/py_executable_bazel.bzl
+++ b/python/private/py_executable_bazel.bzl
@@ -191,7 +191,6 @@ def _create_executable(
         hasattr(runtime_details.effective_runtime, "stage2_bootstrap_template")):
         venv = _create_venv(
             ctx,
-            executable = executable,
             output_prefix = base_executable_name,
             imports = imports,
             runtime_details = runtime_details,
@@ -345,13 +344,34 @@ 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):
+    from_parts = from_.split("/")
+    to_parts = to.split("/")
+
+    # Strip common "../" parts from both paths
+    # (no while loops in starlark :( )
+    n = max(len(from_parts), len(to_parts))
+    for _ in range(n):
+        if from_parts[0] == ".." and to_parts[0] == "..":
+            from_parts.pop(0)
+            to_parts.pop(0)
+        else:
+            break
+
+    parent = "/".join([".."] * len(from_parts))
+    return "/".join([parent] + to_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
 # * https://snarky.ca/how-virtual-environments-work/
 # * https://github.com/python/cpython/blob/main/Modules/getpath.py
 # * https://github.com/python/cpython/blob/main/Lib/site.py
-def _create_venv(ctx, executable, output_prefix, imports, runtime_details):
+def _create_venv(ctx, output_prefix, imports, runtime_details):
     venv = "_{}.venv".format(output_prefix.lstrip("_"))
 
     # The pyvenv.cfg file must be present to trigger the venv site hooks.
@@ -369,13 +389,9 @@ def _create_venv(ctx, executable, 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  # Always relative to .runfiles/${workspace}
-
-        escapes = 2  # To escape out of ${target}.venv/bin
-        escapes += executable.short_path.count("/")  # To escape into .runfiles/${workspace}
-
-        parent = "/".join([".."] * escapes)
-        rel_path = parent + "/" + interpreter_actual_path
+        interpreter_actual_path = runtime.interpreter.short_path
+        venv_bin_dir = paths.dirname(interpreter.short_path)
+        rel_path = _relative_path(from_=venv_bin_dir, to=interpreter_actual_path)
         ctx.actions.symlink(output = interpreter, target_path = rel_path)
     else:
         py_exe_basename = paths.basename(runtime.interpreter_path)

From 60d4b91613d79367cd5de85da7604315a418dfd2 Mon Sep 17 00:00:00 2001
From: chowder <16789070+chowder@users.noreply.github.com>
Date: Sun, 24 Nov 2024 01:09:08 +0000
Subject: [PATCH 04/12] add tests for relative_path

---
 python/private/py_executable_bazel.bzl        |  18 ++-
 tests/bootstrap_impls/BUILD.bazel             |   4 +
 .../venv_relative_path_tests.bzl              | 121 ++++++++++++++++++
 3 files changed, 136 insertions(+), 7 deletions(-)
 create mode 100644 tests/bootstrap_impls/venv_relative_path_tests.bzl

diff --git a/python/private/py_executable_bazel.bzl b/python/private/py_executable_bazel.bzl
index e99faf2ca1..641cb6e08e 100644
--- a/python/private/py_executable_bazel.bzl
+++ b/python/private/py_executable_bazel.bzl
@@ -347,22 +347,26 @@ def _create_zip_main(ctx, *, stage2_bootstrap, runtime_details, venv):
 
 # 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):
+def relative_path(from_, to):
     from_parts = from_.split("/")
     to_parts = to.split("/")
 
-    # Strip common "../" parts from both paths
+    # Strip common leading parts from both paths
     # (no while loops in starlark :( )
-    n = max(len(from_parts), len(to_parts))
+    n = min(len(from_parts), len(to_parts))
     for _ in range(n):
-        if from_parts[0] == ".." and to_parts[0] == "..":
+        if from_parts[0] == to_parts[0]:
             from_parts.pop(0)
             to_parts.pop(0)
         else:
             break
 
-    parent = "/".join([".."] * len(from_parts))
-    return "/".join([parent] + to_parts)
+    # 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 "/".join(parts)
 
 
 # Create a venv the executable can use.
@@ -391,7 +395,7 @@ def _create_venv(ctx, output_prefix, imports, runtime_details):
         interpreter = ctx.actions.declare_symlink("{}/bin/{}".format(venv, py_exe_basename))
         interpreter_actual_path = runtime.interpreter.short_path
         venv_bin_dir = paths.dirname(interpreter.short_path)
-        rel_path = _relative_path(from_=venv_bin_dir, to=interpreter_actual_path)
+        rel_path = relative_path(from_=venv_bin_dir, to=interpreter_actual_path)
         ctx.actions.symlink(output = interpreter, target_path = rel_path)
     else:
         py_exe_basename = paths.basename(runtime.interpreter_path)
diff --git a/tests/bootstrap_impls/BUILD.bazel b/tests/bootstrap_impls/BUILD.bazel
index 1586821896..097898917c 100644
--- a/tests/bootstrap_impls/BUILD.bazel
+++ b/tests/bootstrap_impls/BUILD.bazel
@@ -87,3 +87,7 @@ sh_py_run_test(
     sh_src = "sys_executable_inherits_sys_path_test.sh",
     target_compatible_with = _SUPPORTS_BOOTSTRAP_SCRIPT,
 )
+
+load(":venv_relative_path_tests.bzl", "relative_path_test_suite")
+
+relative_path_test_suite("relative_path_test")
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..443621d3b8
--- /dev/null
+++ b/tests/bootstrap_impls/venv_relative_path_tests.bzl
@@ -0,0 +1,121 @@
+# 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 yaml.bzl"
+
+load("@bazel_skylib//lib:unittest.bzl", "analysistest", "asserts", "unittest")
+load("//python/private:py_executable_bazel.bzl", "relative_path")  # buildifier: disable=bzl-visibility
+
+def _relative_path_test_impl(ctx):
+    env = unittest.begin(ctx)
+
+    # Basic test cases
+
+    asserts.equals(
+        env,
+        "../../c/d",
+        relative_path(
+            from_ = "a/b",
+            to = "c/d",
+        ),
+    )
+
+    asserts.equals(
+        env,
+        "../../c/d",
+        relative_path(
+            from_ = "../a/b",
+            to = "../c/d",
+        ),
+    )
+
+    asserts.equals(
+        env,
+        "../../../c/d",
+        relative_path(
+            from_ = "../a/b",
+            to = "../../c/d",
+        ),
+    )
+
+    asserts.equals(
+        env,
+        "../../d",
+        relative_path(
+            from_ = "a/b/c",
+            to = "a/d",
+        ),
+    )
+
+    asserts.equals(
+        env,
+        "d/e",
+        relative_path(
+            from_ = "a/b/c",
+            to = "a/b/c/d/e",
+        ),
+    )
+
+    # Real examples
+
+    # external py_binary uses external python runtime
+    asserts.equals(
+        env,
+        "../../../../../rules_python~~python~python_3_9_x86_64-unknown-linux-gnu/bin/python3",
+        relative_path(
+            from_ = "../rules_python~/python/private/_py_console_script_gen_py.venv/bin",
+            to = "../rules_python~~python~python_3_9_x86_64-unknown-linux-gnu/bin/python3",
+        ),
+    )
+
+    # internal py_binary uses external python runtime
+    asserts.equals(
+        env,
+        "../../../../rules_python~~python~python_3_9_x86_64-unknown-linux-gnu/bin/python3",
+        relative_path(
+            from_ = "test/version_default.venv/bin",
+            to = "../rules_python~~python~python_3_9_x86_64-unknown-linux-gnu/bin/python3",
+        ),
+    )
+
+    # external py_binary uses internal python runtime
+    # asserts.equals(
+    #    env,
+    #    "???",
+    #    relative_path(
+    #        from_ = "../rules_python~/python/private/_py_console_script_gen_py.venv/bin",
+    #        to = "python/python_3_9_x86_64-unknown-linux-gnu/bin/python3",
+    #    ),
+    #)
+    # ^ TODO: Technically we can infer ".." to be the workspace name?
+
+    # internal py_binary uses internal python runtime
+    asserts.equals(
+        env,
+        "../../../python/python_3_9_x86_64-unknown-linux-gnu/bin/python3",
+        relative_path(
+            from_ = "scratch/main.venv/bin",
+            to = "python/python_3_9_x86_64-unknown-linux-gnu/bin/python3",
+        ),
+    )
+
+    return unittest.end(env)
+
+relative_path_test = unittest.make(
+    _relative_path_test_impl,
+    attrs = {},
+)
+
+def relative_path_test_suite(name):
+    unittest.suite(name, relative_path_test)

From 74c0b41b2d53a6f550068eb962bf9ad4ba6d32cc Mon Sep 17 00:00:00 2001
From: Richard Levasseur <rlevasseur@google.com>
Date: Sun, 24 Nov 2024 15:05:38 -0800
Subject: [PATCH 05/12] add test to demonstrate error

---
 tests/bootstrap_impls/a/b/c/BUILD.bazel       | 16 ++++++++++++++
 .../bootstrap_impls/a/b/c/nested_dir_test.py  | 22 +++++++++++++++++++
 2 files changed, 38 insertions(+)
 create mode 100644 tests/bootstrap_impls/a/b/c/BUILD.bazel
 create mode 100644 tests/bootstrap_impls/a/b/c/nested_dir_test.py

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..9bb9e0c1bb
--- /dev/null
+++ b/tests/bootstrap_impls/a/b/c/BUILD.bazel
@@ -0,0 +1,16 @@
+load("@bazel_skylib//rules:copy_file.bzl", "copy_file")
+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")
+
+_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..70ea5fe443
--- /dev/null
+++ b/tests/bootstrap_impls/a/b/c/nested_dir_test.py
@@ -0,0 +1,22 @@
+# 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()

From 638b5215858f7bb4daccb610d7b508188a166d33 Mon Sep 17 00:00:00 2001
From: Richard Levasseur <rlevasseur@google.com>
Date: Sun, 24 Nov 2024 16:09:57 -0800
Subject: [PATCH 06/12] make relative path computation between two
 runfiles-root relative paths instead of main-repo-root relative paths

---
 python/private/py_executable_bazel.bzl      | 42 ++++++++++++++-------
 python/private/stage1_bootstrap_template.sh |  1 +
 2 files changed, 29 insertions(+), 14 deletions(-)

diff --git a/python/private/py_executable_bazel.bzl b/python/private/py_executable_bazel.bzl
index 641cb6e08e..3652678604 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,7 +344,6 @@ 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):
@@ -366,8 +365,7 @@ def relative_path(from_, to):
         fail("cannot compute relative path from '%s' to '%s'", from_, to)
 
     parts = ([".."] * len(from_parts)) + to_parts
-    return "/".join(parts)
-
+    return paths.join(*parts)
 
 # Create a venv the executable can use.
 # For venv details and the venv startup process, see:
@@ -393,9 +391,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
-        venv_bin_dir = paths.dirname(interpreter.short_path)
-        rel_path = relative_path(from_=venv_bin_dir, to=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)
@@ -437,7 +441,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],
     )
@@ -487,12 +491,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,
@@ -512,7 +526,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"

From d09ad2c2a1ecbd9d826d915c82971e4c99b523a9 Mon Sep 17 00:00:00 2001
From: Richard Levasseur <rlevasseur@google.com>
Date: Sun, 24 Nov 2024 16:24:38 -0800
Subject: [PATCH 07/12] linter cleanup

---
 python/private/py_executable_bazel.bzl             | 11 +++++++++++
 tests/bootstrap_impls/BUILD.bazel                  |  5 ++---
 tests/bootstrap_impls/a/b/c/BUILD.bazel            |  3 +--
 tests/bootstrap_impls/a/b/c/nested_dir_test.py     |  2 ++
 tests/bootstrap_impls/venv_relative_path_tests.bzl |  4 ++--
 5 files changed, 18 insertions(+), 7 deletions(-)

diff --git a/python/private/py_executable_bazel.bzl b/python/private/py_executable_bazel.bzl
index 3652678604..df752e12f8 100644
--- a/python/private/py_executable_bazel.bzl
+++ b/python/private/py_executable_bazel.bzl
@@ -347,6 +347,17 @@ def _create_zip_main(ctx, *, stage2_bootstrap, runtime_details, venv):
 # 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("/")
 
diff --git a/tests/bootstrap_impls/BUILD.bazel b/tests/bootstrap_impls/BUILD.bazel
index 097898917c..5de29b1fa1 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"],
@@ -88,6 +89,4 @@ sh_py_run_test(
     target_compatible_with = _SUPPORTS_BOOTSTRAP_SCRIPT,
 )
 
-load(":venv_relative_path_tests.bzl", "relative_path_test_suite")
-
-relative_path_test_suite("relative_path_test")
+relative_path_test_suite(name = "relative_path_test")
diff --git a/tests/bootstrap_impls/a/b/c/BUILD.bazel b/tests/bootstrap_impls/a/b/c/BUILD.bazel
index 9bb9e0c1bb..8ffcbcd479 100644
--- a/tests/bootstrap_impls/a/b/c/BUILD.bazel
+++ b/tests/bootstrap_impls/a/b/c/BUILD.bazel
@@ -1,6 +1,5 @@
-load("@bazel_skylib//rules:copy_file.bzl", "copy_file")
 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("//tests/support:sh_py_run_test.bzl", "py_reconfig_test")
 
 _SUPPORTS_BOOTSTRAP_SCRIPT = select({
     "@platforms//os:windows": ["@platforms//:incompatible"],
diff --git a/tests/bootstrap_impls/a/b/c/nested_dir_test.py b/tests/bootstrap_impls/a/b/c/nested_dir_test.py
index 70ea5fe443..2db0e6c771 100644
--- a/tests/bootstrap_impls/a/b/c/nested_dir_test.py
+++ b/tests/bootstrap_impls/a/b/c/nested_dir_test.py
@@ -15,8 +15,10 @@
 
 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
index 443621d3b8..7c9bb62fd9 100644
--- a/tests/bootstrap_impls/venv_relative_path_tests.bzl
+++ b/tests/bootstrap_impls/venv_relative_path_tests.bzl
@@ -14,7 +14,7 @@
 
 "Unit tests for yaml.bzl"
 
-load("@bazel_skylib//lib:unittest.bzl", "analysistest", "asserts", "unittest")
+load("@bazel_skylib//lib:unittest.bzl", "asserts", "unittest")
 load("//python/private:py_executable_bazel.bzl", "relative_path")  # buildifier: disable=bzl-visibility
 
 def _relative_path_test_impl(ctx):
@@ -117,5 +117,5 @@ relative_path_test = unittest.make(
     attrs = {},
 )
 
-def relative_path_test_suite(name):
+def relative_path_test_suite(*, name):
     unittest.suite(name, relative_path_test)

From 39e7e5ac89cc0414f15065859874de7ee6c69e2b Mon Sep 17 00:00:00 2001
From: Richard Levasseur <rlevasseur@google.com>
Date: Sun, 24 Nov 2024 16:38:23 -0800
Subject: [PATCH 08/12] update tests to use rules_tests; implement commented
 out case; update real examples to use runfiles-root relative paths

---
 tests/bootstrap_impls/BUILD.bazel             |  2 +-
 .../venv_relative_path_tests.bzl              | 95 ++++++++-----------
 2 files changed, 40 insertions(+), 57 deletions(-)

diff --git a/tests/bootstrap_impls/BUILD.bazel b/tests/bootstrap_impls/BUILD.bazel
index 5de29b1fa1..2fb1f38ff0 100644
--- a/tests/bootstrap_impls/BUILD.bazel
+++ b/tests/bootstrap_impls/BUILD.bazel
@@ -89,4 +89,4 @@ sh_py_run_test(
     target_compatible_with = _SUPPORTS_BOOTSTRAP_SCRIPT,
 )
 
-relative_path_test_suite(name = "relative_path_test")
+relative_path_test_suite(name = "relative_path_tests")
diff --git a/tests/bootstrap_impls/venv_relative_path_tests.bzl b/tests/bootstrap_impls/venv_relative_path_tests.bzl
index 7c9bb62fd9..7c8fa6d42f 100644
--- a/tests/bootstrap_impls/venv_relative_path_tests.bzl
+++ b/tests/bootstrap_impls/venv_relative_path_tests.bzl
@@ -12,110 +12,93 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-"Unit tests for yaml.bzl"
+"Unit tests for relative_path computation"
 
-load("@bazel_skylib//lib:unittest.bzl", "asserts", "unittest")
+load("@rules_testing//lib:test_suite.bzl", "test_suite")
 load("//python/private:py_executable_bazel.bzl", "relative_path")  # buildifier: disable=bzl-visibility
 
-def _relative_path_test_impl(ctx):
-    env = unittest.begin(ctx)
+_tests = []
 
+def _relative_path_test(env):
     # Basic test cases
 
-    asserts.equals(
-        env,
-        "../../c/d",
+    env.expect.that_str(
         relative_path(
             from_ = "a/b",
             to = "c/d",
         ),
-    )
+    ).equals("../../c/d")
 
-    asserts.equals(
-        env,
-        "../../c/d",
+    env.expect.that_str(
         relative_path(
             from_ = "../a/b",
             to = "../c/d",
         ),
-    )
+    ).equals("../../c/d")
 
-    asserts.equals(
-        env,
-        "../../../c/d",
+    env.expect.that_str(
         relative_path(
             from_ = "../a/b",
             to = "../../c/d",
         ),
-    )
+    ).equals("../../../c/d")
 
-    asserts.equals(
-        env,
-        "../../d",
+    env.expect.that_str(
         relative_path(
             from_ = "a/b/c",
             to = "a/d",
         ),
-    )
-
-    asserts.equals(
-        env,
-        "d/e",
+    ).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
-    asserts.equals(
-        env,
-        "../../../../../rules_python~~python~python_3_9_x86_64-unknown-linux-gnu/bin/python3",
+    env.expect.that_str(
         relative_path(
-            from_ = "../rules_python~/python/private/_py_console_script_gen_py.venv/bin",
-            to = "../rules_python~~python~python_3_9_x86_64-unknown-linux-gnu/bin/python3",
+            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
-    asserts.equals(
-        env,
-        "../../../../rules_python~~python~python_3_9_x86_64-unknown-linux-gnu/bin/python3",
+    env.expect.that_str(
         relative_path(
-            from_ = "test/version_default.venv/bin",
-            to = "../rules_python~~python~python_3_9_x86_64-unknown-linux-gnu/bin/python3",
+            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
-    # asserts.equals(
-    #    env,
-    #    "???",
-    #    relative_path(
-    #        from_ = "../rules_python~/python/private/_py_console_script_gen_py.venv/bin",
-    #        to = "python/python_3_9_x86_64-unknown-linux-gnu/bin/python3",
-    #    ),
-    #)
-    # ^ TODO: Technically we can infer ".." to be the workspace name?
+    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
-    asserts.equals(
-        env,
-        "../../../python/python_3_9_x86_64-unknown-linux-gnu/bin/python3",
+    env.expect.that_str(
         relative_path(
-            from_ = "scratch/main.venv/bin",
-            to = "python/python_3_9_x86_64-unknown-linux-gnu/bin/python3",
+            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",
     )
 
-    return unittest.end(env)
-
-relative_path_test = unittest.make(
-    _relative_path_test_impl,
-    attrs = {},
-)
+_tests.append(_relative_path_test)
 
 def relative_path_test_suite(*, name):
-    unittest.suite(name, relative_path_test)
+    test_suite(name = name, basic_tests = _tests)

From 6aa8a42aeaed4a58273912af1346f75265de5eae Mon Sep 17 00:00:00 2001
From: Richard Levasseur <rlevasseur@google.com>
Date: Sun, 24 Nov 2024 16:57:48 -0800
Subject: [PATCH 09/12] fixup! update tests to use rules_tests; implement
 commented out case; update real examples to use runfiles-root relative paths

---
 python/private/py_executable_bazel.bzl | 2 --
 1 file changed, 2 deletions(-)

diff --git a/python/private/py_executable_bazel.bzl b/python/private/py_executable_bazel.bzl
index df752e12f8..81c8a468bd 100644
--- a/python/private/py_executable_bazel.bzl
+++ b/python/private/py_executable_bazel.bzl
@@ -344,8 +344,6 @@ 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.
 

From 67ec856264ba7bcce9873be7a8cf809d9630a519 Mon Sep 17 00:00:00 2001
From: Richard Levasseur <richardlev@gmail.com>
Date: Sun, 24 Nov 2024 20:34:40 -0800
Subject: [PATCH 10/12] loop over len of to

Co-authored-by: Ignas Anikevicius <240938+aignas@users.noreply.github.com>
---
 python/private/py_executable_bazel.bzl | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/python/private/py_executable_bazel.bzl b/python/private/py_executable_bazel.bzl
index 81c8a468bd..8db867e64e 100644
--- a/python/private/py_executable_bazel.bzl
+++ b/python/private/py_executable_bazel.bzl
@@ -360,9 +360,7 @@ def relative_path(from_, to):
     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):
+    for _ in range(len(to)):
         if from_parts[0] == to_parts[0]:
             from_parts.pop(0)
             to_parts.pop(0)

From 2b362aa7db6440c47d218ebe97c86234f4e46bc7 Mon Sep 17 00:00:00 2001
From: Richard Levasseur <rlevasseur@google.com>
Date: Sun, 24 Nov 2024 20:50:44 -0800
Subject: [PATCH 11/12] remove parent-dir input tests for relative paths

---
 tests/bootstrap_impls/venv_relative_path_tests.bzl | 14 --------------
 1 file changed, 14 deletions(-)

diff --git a/tests/bootstrap_impls/venv_relative_path_tests.bzl b/tests/bootstrap_impls/venv_relative_path_tests.bzl
index 7c8fa6d42f..b21f220205 100644
--- a/tests/bootstrap_impls/venv_relative_path_tests.bzl
+++ b/tests/bootstrap_impls/venv_relative_path_tests.bzl
@@ -29,20 +29,6 @@ def _relative_path_test(env):
         ),
     ).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",

From 4720a07b1c626f025bf9a3e20c861ffdd9cbc11a Mon Sep 17 00:00:00 2001
From: Richard Levasseur <rlevasseur@google.com>
Date: Sun, 24 Nov 2024 20:59:07 -0800
Subject: [PATCH 12/12] switch back to len(min of parts); avoids OOB array
 access

---
 python/private/py_executable_bazel.bzl | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/python/private/py_executable_bazel.bzl b/python/private/py_executable_bazel.bzl
index 8db867e64e..3778c192b4 100644
--- a/python/private/py_executable_bazel.bzl
+++ b/python/private/py_executable_bazel.bzl
@@ -360,7 +360,8 @@ def relative_path(from_, to):
     to_parts = to.split("/")
 
     # Strip common leading parts from both paths
-    for _ in range(len(to)):
+    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)