fix: make plain zipapp work with bootstrap=script (#2598)

The `__main__.py` template (zip_main_template.py) was using the wrong
path when creating
the interpreter symlinks. It as computing it correctly, just the wrong
variable was used
in the symlink() call.

To fix, pass the correct variable.

Also adds a test to check that it's runnable.

Fixes #2596

Author: rickeylev

CHANGELOG.md

@@ -62,6 +62,8 @@ Unreleased changes template.
 
 {#v0-0-0-fixed}
 ### Fixed
+* (rules) `python_zip_file` output with `--bootstrap_impl=script` works again
+  ([#2596](https://github.com/bazelbuild/rules_python/issues/2596)).
 * (docs) Using `python_version` attribute for specifying python versions introduced in `v1.1.0`
 * (gazelle) Providing multiple input requirements files to `gazelle_python_manifest` now works correctly.
 * (pypi) Handle trailing slashes in pip index URLs in environment variables,

python/private/zip_main_template.py

@@ -286,10 +286,10 @@ def main():
     # The bin/ directory may not exist if it is empty.
     os.makedirs(os.path.dirname(python_program), exist_ok=True)
     try:
-        os.symlink(_PYTHON_BINARY_ACTUAL, python_program)
+        os.symlink(symlink_to, python_program)
     except OSError as e:
         raise Exception(
-            f"Unable to create venv python interpreter symlink: {python_program} -> {PYTHON_BINARY_ACTUAL}"
+            f"Unable to create venv python interpreter symlink: {python_program} -> {symlink_to}"
         ) from e
 
     # Some older Python versions on macOS (namely Python 3.7) may unintentionally

tests/bootstrap_impls/BUILD.bazel

@@ -1,3 +1,5 @@
+load("@rules_shell//shell:sh_test.bzl", "sh_test")
+
 # Copyright 2023 The Bazel Authors. All rights reserved.
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -11,10 +13,40 @@
 # 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.
-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_binary", "py_reconfig_test", "sh_py_run_test")
 load("//tests/support:support.bzl", "SUPPORTS_BOOTSTRAP_SCRIPT")
 load(":venv_relative_path_tests.bzl", "relative_path_test_suite")
 
+py_reconfig_binary(
+    name = "bootstrap_script_zipapp_bin",
+    srcs = ["bin.py"],
+    bootstrap_impl = "script",
+    # Force it to not be self-executable
+    build_python_zip = "no",
+    main = "bin.py",
+    target_compatible_with = SUPPORTS_BOOTSTRAP_SCRIPT,
+)
+
+filegroup(
+    name = "bootstrap_script_zipapp_zip",
+    testonly = 1,
+    srcs = [":bootstrap_script_zipapp_bin"],
+    output_group = "python_zip_file",
+)
+
+sh_test(
+    name = "bootstrap_script_zipapp_test",
+    srcs = ["bootstrap_script_zipapp_test.sh"],
+    data = [":bootstrap_script_zipapp_zip"],
+    env = {
+        "ZIP_RLOCATION": "$(rlocationpaths :bootstrap_script_zipapp_zip)".format(),
+    },
+    target_compatible_with = SUPPORTS_BOOTSTRAP_SCRIPT,
+    deps = [
+        "@bazel_tools//tools/bash/runfiles",
+    ],
+)
+
 sh_py_run_test(
     name = "run_binary_zip_no_test",
     build_python_zip = "no",

tests/bootstrap_impls/bootstrap_script_zipapp_test.sh

@@ -0,0 +1,47 @@
+# 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.
+
+# --- begin runfiles.bash initialization v3 ---
+# Copy-pasted from the Bazel Bash runfiles library v3.
+set -uo pipefail; set +e; f=bazel_tools/tools/bash/runfiles/runfiles.bash
+source "${RUNFILES_DIR:-/dev/null}/$f" 2>/dev/null || \
+  source "$(grep -sm1 "^$f " "${RUNFILES_MANIFEST_FILE:-/dev/null}" | cut -f2- -d' ')" 2>/dev/null || \
+  source "$0.runfiles/$f" 2>/dev/null || \
+  source "$(grep -sm1 "^$f " "$0.runfiles_manifest" | cut -f2- -d' ')" 2>/dev/null || \
+  source "$(grep -sm1 "^$f " "$0.exe.runfiles_manifest" | cut -f2- -d' ')" 2>/dev/null || \
+  { echo>&2 "ERROR: cannot find $f"; exit 1; }; f=; set -e
+# --- end runfiles.bash initialization v3 ---
+set +e
+
+bin=$(rlocation $ZIP_RLOCATION)
+if [[ -z "$bin" ]]; then
+  echo "Unable to locate test binary: $ZIP_RLOCATION"
+  exit 1
+fi
+set -x
+actual=$(python3 $bin)
+
+# How we detect if a zip file was executed from depends on which bootstrap
+# is used.
+# bootstrap_impl=script outputs RULES_PYTHON_ZIP_DIR=<somepath>
+# bootstrap_impl=system_python outputs file:.*Bazel.runfiles
+expected_pattern="Hello"
+if ! (echo "$actual" | grep "$expected_pattern" ) >/dev/null; then
+  echo "Test case failed: $1"
+  echo "expected output to match: $expected_pattern"
+  echo "but got:\n$actual"
+  exit 1
+fi
+
+exit 0

tests/support/sh_py_run_test.bzl

@@ -86,6 +86,7 @@ def _py_reconfig_impl(ctx):
                 default_info.default_runfiles,
             ),
         ),
+        ctx.attr.target[OutputGroupInfo],
         # Inherit the expanded environment from the inner target.
         ctx.attr.target[RunEnvironmentInfo],
     ]
@@ -120,31 +121,48 @@ _py_reconfig_binary = _make_reconfig_rule(executable = True)
 
 _py_reconfig_test = _make_reconfig_rule(test = True)
 
-def py_reconfig_test(*, name, **kwargs):
-    """Create a py_test with customized build settings for testing.
-
-    Args:
-        name: str, name of teset target.
-        **kwargs: kwargs to pass along to _py_reconfig_test and py_test.
-    """
+def _py_reconfig_executable(*, name, py_reconfig_rule, py_inner_rule, **kwargs):
     reconfig_kwargs = {}
     reconfig_kwargs["bootstrap_impl"] = kwargs.pop("bootstrap_impl", None)
     reconfig_kwargs["extra_toolchains"] = kwargs.pop("extra_toolchains", None)
     reconfig_kwargs["python_version"] = kwargs.pop("python_version", None)
     reconfig_kwargs["target_compatible_with"] = kwargs.get("target_compatible_with")
+    reconfig_kwargs["build_python_zip"] = kwargs.pop("build_python_zip", None)
 
     inner_name = "_{}_inner".format(name)
-    _py_reconfig_test(
+    py_reconfig_rule(
         name = name,
         target = inner_name,
         **reconfig_kwargs
     )
-    py_test(
+    py_inner_rule(
         name = inner_name,
         tags = ["manual"],
         **kwargs
     )
 
+def py_reconfig_test(*, name, **kwargs):
+    """Create a py_test with customized build settings for testing.
+
+    Args:
+        name: str, name of teset target.
+        **kwargs: kwargs to pass along to _py_reconfig_test and py_test.
+    """
+    _py_reconfig_executable(
+        name = name,
+        py_reconfig_rule = _py_reconfig_test,
+        py_inner_rule = py_test,
+        **kwargs
+    )
+
+def py_reconfig_binary(*, name, **kwargs):
+    _py_reconfig_executable(
+        name = name,
+        py_reconfig_rule = _py_reconfig_binary,
+        py_inner_rule = py_binary,
+        **kwargs
+    )
+
 def sh_py_run_test(*, name, sh_src, py_src, **kwargs):
     """Run a py_binary within a sh_test.