Skip to content

Commit 2036571

Browse files
meringrickeylev
andauthored
fix: Normalize main script path in Python bootstrap (#2925)
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 within runfiles and in turn the later assertion that the path to main exists fails (~L542). This happens, for example, when packaging a `py_binary` from a foreign repo into a tar/container. --------- Co-authored-by: Richard Levasseur <[email protected]> Co-authored-by: Richard Levasseur <[email protected]>
1 parent 46f08de commit 2036571

File tree

7 files changed

+56
-7
lines changed

7 files changed

+56
-7
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,7 @@ END_UNRELEASED_TEMPLATE
9191
also retrieved from the URL as opposed to only the `--hash` parameter. Fixes
9292
[#2363](https://github.com/bazel-contrib/rules_python/issues/2363).
9393
* (pypi) `whl_library` now infers file names from its `urls` attribute correctly.
94+
* (py_test, py_binary) Allow external files to be used for main
9495

9596
{#v0-0-0-added}
9697
### Added

internal_dev_deps.bzl

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -68,10 +68,10 @@ def rules_python_internal_deps():
6868
http_archive(
6969
name = "rules_pkg",
7070
urls = [
71-
"https://mirror.bazel.build/github.com/bazelbuild/rules_pkg/releases/download/0.7.0/rules_pkg-0.7.0.tar.gz",
72-
"https://github.com/bazelbuild/rules_pkg/releases/download/0.7.0/rules_pkg-0.7.0.tar.gz",
71+
"https://mirror.bazel.build/github.com/bazelbuild/rules_pkg/releases/download/1.0.1/rules_pkg-1.0.1.tar.gz",
72+
"https://github.com/bazelbuild/rules_pkg/releases/download/1.0.1/rules_pkg-1.0.1.tar.gz",
7373
],
74-
sha256 = "8a298e832762eda1830597d64fe7db58178aa84cd5926d76d5b744d6558941c2",
74+
sha256 = "d20c951960ed77cb7b341c2a59488534e494d5ad1d30c4818c736d57772a9fef",
7575
)
7676

7777
http_archive(

python/private/python_bootstrap_template.txt

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -499,8 +499,12 @@ def Main():
499499
# The magic string percent-main-percent is replaced with the runfiles-relative
500500
# filename of the main file of the Python binary in BazelPythonSemantics.java.
501501
main_rel_path = '%main%'
502-
if IsWindows():
503-
main_rel_path = main_rel_path.replace('/', os.sep)
502+
# NOTE: We call normpath for two reasons:
503+
# 1. Transform Bazel `foo/bar` to Windows `foo\bar`
504+
# 2. Transform `_main/../foo/main.py` to simply `foo/main.py`, which
505+
# matters if `_main` doesn't exist (which can occur if a binary
506+
# is packaged and needs no artifacts from the main repo)
507+
main_rel_path = os.path.normpath(main_rel_path)
504508

505509
if IsRunningFromZip():
506510
module_space = CreateModuleSpace()

tests/bootstrap_impls/BUILD.bazel

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
1-
load("@rules_shell//shell:sh_test.bzl", "sh_test")
2-
31
# Copyright 2023 The Bazel Authors. All rights reserved.
42
#
53
# Licensed under the Apache License, Version 2.0 (the "License");
@@ -13,6 +11,8 @@ load("@rules_shell//shell:sh_test.bzl", "sh_test")
1311
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
1412
# See the License for the specific language governing permissions and
1513
# limitations under the License.
14+
load("@rules_pkg//pkg:tar.bzl", "pkg_tar")
15+
load("@rules_shell//shell:sh_test.bzl", "sh_test")
1616
load("//tests/support:py_reconfig.bzl", "py_reconfig_binary", "py_reconfig_test")
1717
load("//tests/support:sh_py_run_test.bzl", "sh_py_run_test")
1818
load("//tests/support:support.bzl", "SUPPORTS_BOOTSTRAP_SCRIPT")
@@ -158,4 +158,24 @@ py_reconfig_test(
158158
target_compatible_with = SUPPORTS_BOOTSTRAP_SCRIPT,
159159
)
160160

161+
pkg_tar(
162+
name = "external_binary",
163+
testonly = True,
164+
srcs = ["@other//:external_main"],
165+
include_runfiles = True,
166+
tags = ["manual"], # Don't build as part of wildcards
167+
)
168+
169+
sh_test(
170+
name = "external_binary_test",
171+
srcs = ["external_binary_test.sh"],
172+
data = [":external_binary"],
173+
# For now, skip this test on Windows because it fails for reasons
174+
# other than the code path being tested.
175+
target_compatible_with = select({
176+
"@platforms//os:windows": ["@platforms//:incompatible"],
177+
"//conditions:default": [],
178+
}),
179+
)
180+
161181
relative_path_test_suite(name = "relative_path_tests")
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
#!/bin/bash
2+
set -euxo pipefail
3+
4+
tmpdir="${TEST_TMPDIR}/external_binary"
5+
mkdir -p "${tmpdir}"
6+
tar xf "tests/bootstrap_impls/external_binary.tar" -C "${tmpdir}"
7+
test -x "${tmpdir}/external_main"
8+
output="$("${tmpdir}/external_main")"
9+
test "$output" = "token"

tests/modules/other/BUILD.bazel

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
load("@rules_python//tests/support:py_reconfig.bzl", "py_reconfig_binary")
2+
3+
package(
4+
default_visibility = ["//visibility:public"],
5+
)
6+
7+
py_reconfig_binary(
8+
name = "external_main",
9+
srcs = [":external_main.py"],
10+
# We're testing a system_python specific code path,
11+
# so force using that bootstrap
12+
bootstrap_impl = "system_python",
13+
main = "external_main.py",
14+
)

tests/modules/other/external_main.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
print("token")

0 commit comments

Comments
 (0)