From b07525cbb15352caefbe2f23697250cecd984430 Mon Sep 17 00:00:00 2001
From: Ignas Anikevicius <240938+aignas@users.noreply.github.com>
Date: Thu, 13 Jun 2024 00:40:58 +0900
Subject: [PATCH] fix(rules): drop the unused argument (#1953)

It seems that there is an extra argument that is there but I am not sure
about
the reason, it is most likely a leftover. As part of this change we add
an
analysis test for non-windows zip building. I could reproduce the
failure and
now the newly added test passes.

Fixes #1954
---
 CHANGELOG.md                                  |  8 ++++
 python/private/common/py_executable_bazel.bzl |  1 -
 tests/base_rules/py_executable_base_tests.bzl | 46 ++++++++++++++++++-
 3 files changed, 53 insertions(+), 2 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index fd1763391..9abbd44de 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -33,6 +33,14 @@ A brief description of the categories of changes:
 ### Removed
 * Nothing yet
 
+## [0.33.1] - 2024-06-13
+
+[0.33.1]: https://github.com/bazelbuild/rules_python/releases/tag/0.33.1
+
+### Fixed
+* (py_binary) Fix building of zip file when using `--build_python_zip`
+  argument. Fixes [#1954](https://github.com/bazelbuild/rules_python/issues/1954).
+
 ## [0.33.0] - 2024-06-12
 
 [0.33.0]: https://github.com/bazelbuild/rules_python/releases/tag/0.33.0
diff --git a/python/private/common/py_executable_bazel.bzl b/python/private/common/py_executable_bazel.bzl
index bc7f66b2d..361e075a3 100644
--- a/python/private/common/py_executable_bazel.bzl
+++ b/python/private/common/py_executable_bazel.bzl
@@ -270,7 +270,6 @@ def _create_executable(
             ctx,
             output = executable,
             zip_file = zip_file,
-            python_binary_path = runtime_details.executable_interpreter_path,
             stage2_bootstrap = stage2_bootstrap,
             runtime_details = runtime_details,
         )
diff --git a/tests/base_rules/py_executable_base_tests.bzl b/tests/base_rules/py_executable_base_tests.bzl
index 43e800a99..c96ec4e10 100644
--- a/tests/base_rules/py_executable_base_tests.bzl
+++ b/tests/base_rules/py_executable_base_tests.bzl
@@ -20,7 +20,7 @@ load("@rules_testing//lib:truth.bzl", "matching")
 load("@rules_testing//lib:util.bzl", rt_util = "util")
 load("//tests/base_rules:base_tests.bzl", "create_base_tests")
 load("//tests/base_rules:util.bzl", "WINDOWS_ATTR", pt_util = "util")
-load("//tests/support:support.bzl", "WINDOWS_X86_64")
+load("//tests/support:support.bzl", "LINUX_X86_64", "WINDOWS_X86_64")
 
 _BuiltinPyRuntimeInfo = PyRuntimeInfo
 
@@ -67,6 +67,50 @@ def _test_basic_windows_impl(env, target):
 
 _tests.append(_test_basic_windows)
 
+def _test_basic_zip(name, config):
+    if rp_config.enable_pystar:
+        target_compatible_with = select({
+            # Disable the new test on windows because we have _test_basic_windows.
+            "@platforms//os:windows": ["@platforms//:incompatible"],
+            "//conditions:default": [],
+        })
+    else:
+        target_compatible_with = ["@platforms//:incompatible"]
+    rt_util.helper_target(
+        config.rule,
+        name = name + "_subject",
+        srcs = ["main.py"],
+        main = "main.py",
+    )
+    analysis_test(
+        name = name,
+        impl = _test_basic_zip_impl,
+        target = name + "_subject",
+        config_settings = {
+            # NOTE: The default for this flag is based on the Bazel host OS, not
+            # the target platform. For windows, it defaults to true, so force
+            # it to that to match behavior when this test runs on other
+            # platforms.
+            "//command_line_option:build_python_zip": "true",
+            "//command_line_option:cpu": "linux_x86_64",
+            "//command_line_option:crosstool_top": Label("//tests/cc:cc_toolchain_suite"),
+            "//command_line_option:extra_toolchains": [str(Label("//tests/cc:all"))],
+            "//command_line_option:platforms": [LINUX_X86_64],
+        },
+        attr_values = {"target_compatible_with": target_compatible_with},
+    )
+
+def _test_basic_zip_impl(env, target):
+    target = env.expect.that_target(target)
+    target.runfiles().contains_predicate(matching.str_endswith(
+        target.meta.format_str("/{name}.zip"),
+    ))
+    target.runfiles().contains_predicate(matching.str_endswith(
+        target.meta.format_str("/{name}"),
+    ))
+
+_tests.append(_test_basic_zip)
+
 def _test_executable_in_runfiles(name, config):
     rt_util.helper_target(
         config.rule,