From 2cf99571c31770350c8ebf51fba037151fdac64d Mon Sep 17 00:00:00 2001 From: maleo Date: Thu, 22 May 2025 11:47:38 +0000 Subject: [PATCH 1/8] Fix Python bootstrap main_rel_path Use os.path.normpath() to resolve "_main/../repo" to "repo" and convert forward slashes to backward slashes on Windows. This fixes an issue where "_main" doesn't exist and in turn "assert os.path.exists("_main/../repo")" fails. This happens for example when packaging a py_binary from a foreign repo into a tar/container. --- CHANGELOG.md | 1 + python/private/python_bootstrap_template.txt | 3 +-- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a76241018d..9655b90487 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -91,6 +91,7 @@ END_UNRELEASED_TEMPLATE also retrieved from the URL as opposed to only the `--hash` parameter. Fixes [#2363](https://github.com/bazel-contrib/rules_python/issues/2363). * (pypi) `whl_library` now infers file names from its `urls` attribute correctly. +* (py_test, py_binary) Allow external files to be used for main {#v0-0-0-added} ### Added diff --git a/python/private/python_bootstrap_template.txt b/python/private/python_bootstrap_template.txt index 210987abf9..aa67611d2a 100644 --- a/python/private/python_bootstrap_template.txt +++ b/python/private/python_bootstrap_template.txt @@ -499,8 +499,7 @@ def Main(): # The magic string percent-main-percent is replaced with the runfiles-relative # filename of the main file of the Python binary in BazelPythonSemantics.java. main_rel_path = '%main%' - if IsWindows(): - main_rel_path = main_rel_path.replace('/', os.sep) + main_rel_path = os.path.normpath(main_rel_path) if IsRunningFromZip(): module_space = CreateModuleSpace() From 2daa79ca09b56881e11736afc37549f71ebc5bad Mon Sep 17 00:00:00 2001 From: maleo Date: Fri, 23 May 2025 17:12:54 +0000 Subject: [PATCH 2/8] Add test --- tests/bootstrap_impls/BUILD.bazel | 17 +++++++++++++++-- tests/bootstrap_impls/external_binary_test.sh | 9 +++++++++ tests/modules/other/BUILD.bazel | 12 ++++++++++++ tests/modules/other/external_main.py | 1 + 4 files changed, 37 insertions(+), 2 deletions(-) create mode 100755 tests/bootstrap_impls/external_binary_test.sh create mode 100644 tests/modules/other/external_main.py diff --git a/tests/bootstrap_impls/BUILD.bazel b/tests/bootstrap_impls/BUILD.bazel index b669da5669..9668c639a4 100644 --- a/tests/bootstrap_impls/BUILD.bazel +++ b/tests/bootstrap_impls/BUILD.bazel @@ -1,5 +1,3 @@ -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"); @@ -13,6 +11,8 @@ load("@rules_shell//shell:sh_test.bzl", "sh_test") # 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("@rules_pkg//pkg:tar.bzl", "pkg_tar") +load("@rules_shell//shell:sh_test.bzl", "sh_test") load("//tests/support:py_reconfig.bzl", "py_reconfig_binary", "py_reconfig_test") load("//tests/support:sh_py_run_test.bzl", "sh_py_run_test") load("//tests/support:support.bzl", "SUPPORTS_BOOTSTRAP_SCRIPT") @@ -158,4 +158,17 @@ py_reconfig_test( target_compatible_with = SUPPORTS_BOOTSTRAP_SCRIPT, ) +pkg_tar( + name = "external_binary", + testonly = True, + srcs = ["@other//:external_main"], + include_runfiles = True, +) + +sh_test( + name = "external_binary_test", + srcs = ["external_binary_test.sh"], + data = [":external_binary"], +) + relative_path_test_suite(name = "relative_path_tests") diff --git a/tests/bootstrap_impls/external_binary_test.sh b/tests/bootstrap_impls/external_binary_test.sh new file mode 100755 index 0000000000..e3516af18e --- /dev/null +++ b/tests/bootstrap_impls/external_binary_test.sh @@ -0,0 +1,9 @@ +#!/bin/bash +set -euxo pipefail + +tmpdir="${TEST_TMPDIR}/external_binary" +mkdir -p "${tmpdir}" +tar xf "tests/bootstrap_impls/external_binary.tar" -C "${tmpdir}" +test -x "${tmpdir}/external_main" +output="$("${tmpdir}/external_main")" +test "$output" = "token" diff --git a/tests/modules/other/BUILD.bazel b/tests/modules/other/BUILD.bazel index e69de29bb2..8dc01d2270 100644 --- a/tests/modules/other/BUILD.bazel +++ b/tests/modules/other/BUILD.bazel @@ -0,0 +1,12 @@ +load("@rules_python//tests/support:py_reconfig.bzl", "py_reconfig_binary") + +package( + default_visibility = ["//visibility:public"], +) + +py_reconfig_binary( + name = "external_main", + srcs = [":external_main.py"], + bootstrap_impl = "system_python", + main = "external_main.py", +) diff --git a/tests/modules/other/external_main.py b/tests/modules/other/external_main.py new file mode 100644 index 0000000000..f742ebab60 --- /dev/null +++ b/tests/modules/other/external_main.py @@ -0,0 +1 @@ +print("token") From cb7994afafb12648b8acc44c8ec33a38a7bbf7f5 Mon Sep 17 00:00:00 2001 From: maleo Date: Fri, 23 May 2025 18:17:47 +0000 Subject: [PATCH 3/8] Update rules_pkg in WORKSPACE to align with MODULE.bazel This is required to correctly populate runfiles in pkg_tar with include_runfiles=True. --- internal_dev_deps.bzl | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal_dev_deps.bzl b/internal_dev_deps.bzl index 87690be1ad..f2b33e279e 100644 --- a/internal_dev_deps.bzl +++ b/internal_dev_deps.bzl @@ -68,10 +68,10 @@ def rules_python_internal_deps(): http_archive( name = "rules_pkg", urls = [ - "https://mirror.bazel.build/github.com/bazelbuild/rules_pkg/releases/download/0.7.0/rules_pkg-0.7.0.tar.gz", - "https://github.com/bazelbuild/rules_pkg/releases/download/0.7.0/rules_pkg-0.7.0.tar.gz", + "https://mirror.bazel.build/github.com/bazelbuild/rules_pkg/releases/download/1.0.1/rules_pkg-1.0.1.tar.gz", + "https://github.com/bazelbuild/rules_pkg/releases/download/1.0.1/rules_pkg-1.0.1.tar.gz", ], - sha256 = "8a298e832762eda1830597d64fe7db58178aa84cd5926d76d5b744d6558941c2", + sha256 = "d20c951960ed77cb7b341c2a59488534e494d5ad1d30c4818c736d57772a9fef", ) http_archive( From ac1c7dbc49ed69983c526aa4603dfaff0b8ea02c Mon Sep 17 00:00:00 2001 From: maleo Date: Fri, 23 May 2025 18:56:52 +0000 Subject: [PATCH 4/8] Disable test on Windows --- tests/bootstrap_impls/BUILD.bazel | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/bootstrap_impls/BUILD.bazel b/tests/bootstrap_impls/BUILD.bazel index 9668c639a4..d0a80c689e 100644 --- a/tests/bootstrap_impls/BUILD.bazel +++ b/tests/bootstrap_impls/BUILD.bazel @@ -169,6 +169,10 @@ sh_test( name = "external_binary_test", srcs = ["external_binary_test.sh"], data = [":external_binary"], + target_compatible_with = select({ + "@platforms//os:windows": ["@platforms//:incompatible"], + "//conditions:default": [], + }), ) relative_path_test_suite(name = "relative_path_tests") From 4d224b60791b628d856ff62f3a57db9c3511689c Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Fri, 23 May 2025 12:18:42 -0700 Subject: [PATCH 5/8] add comment about why normpath is called --- python/private/python_bootstrap_template.txt | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/python/private/python_bootstrap_template.txt b/python/private/python_bootstrap_template.txt index aa67611d2a..a979fd4422 100644 --- a/python/private/python_bootstrap_template.txt +++ b/python/private/python_bootstrap_template.txt @@ -499,6 +499,11 @@ def Main(): # The magic string percent-main-percent is replaced with the runfiles-relative # filename of the main file of the Python binary in BazelPythonSemantics.java. main_rel_path = '%main%' + # NOTE: We call normpath for two reasons: + # 1. Transform Bazel `foo/bar` to Windows `foo\bar` + # 2. Transform `_main/../foo/main.py` to simply `foo/main.py`, which + # matters if `_main` doesn't exist (which can occur if a binary + # is packaged and needs no artifacts from the main repo) main_rel_path = os.path.normpath(main_rel_path) if IsRunningFromZip(): From f4464181a21d91334a98aa39c82a2a70e615259c Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Fri, 23 May 2025 12:35:24 -0700 Subject: [PATCH 6/8] add manual tag, comment why skip on windows --- tests/bootstrap_impls/BUILD.bazel | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/bootstrap_impls/BUILD.bazel b/tests/bootstrap_impls/BUILD.bazel index d0a80c689e..393979c2a2 100644 --- a/tests/bootstrap_impls/BUILD.bazel +++ b/tests/bootstrap_impls/BUILD.bazel @@ -163,12 +163,15 @@ pkg_tar( testonly = True, srcs = ["@other//:external_main"], include_runfiles = True, + tags = ["manual"], # Don't build as part of wildcards ) sh_test( name = "external_binary_test", srcs = ["external_binary_test.sh"], data = [":external_binary"], + # For now, skip this test on Windows because it fails for reasons + # other than the code path being tested. target_compatible_with = select({ "@platforms//os:windows": ["@platforms//:incompatible"], "//conditions:default": [], From 5d140f8e14d9bdab759d091c076adee54cbeb648 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Fri, 23 May 2025 12:36:26 -0700 Subject: [PATCH 7/8] comment why bootstrap is specified --- tests/modules/other/BUILD.bazel | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/modules/other/BUILD.bazel b/tests/modules/other/BUILD.bazel index 8dc01d2270..46f1b96faa 100644 --- a/tests/modules/other/BUILD.bazel +++ b/tests/modules/other/BUILD.bazel @@ -7,6 +7,8 @@ package( py_reconfig_binary( name = "external_main", srcs = [":external_main.py"], + # We're testing a system_python specific code path, + # so force using that bootstrap bootstrap_impl = "system_python", main = "external_main.py", ) From f2ff66e181755dffa5a3e990dd2e67fcc5ae03e3 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Fri, 23 May 2025 12:39:47 -0700 Subject: [PATCH 8/8] run buildifier --- tests/bootstrap_impls/BUILD.bazel | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/bootstrap_impls/BUILD.bazel b/tests/bootstrap_impls/BUILD.bazel index 393979c2a2..c3d44df240 100644 --- a/tests/bootstrap_impls/BUILD.bazel +++ b/tests/bootstrap_impls/BUILD.bazel @@ -163,7 +163,7 @@ pkg_tar( testonly = True, srcs = ["@other//:external_main"], include_runfiles = True, - tags = ["manual"], # Don't build as part of wildcards + tags = ["manual"], # Don't build as part of wildcards ) sh_test(