Skip to content

Commit

Permalink
fix(rules): drop the unused argument (#1953)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
aignas authored Jun 12, 2024
1 parent 2a19374 commit b07525c
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 2 deletions.
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion python/private/common/py_executable_bazel.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
Expand Down
46 changes: 45 additions & 1 deletion tests/base_rules/py_executable_base_tests.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit b07525c

Please sign in to comment.