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,