8000 fix(windows): symlink bootstrap script when not building zip (#2015) · brunodea/rules_python@2cfbe73 · GitHub
[go: up one dir, main page]
Skip to content

Commit 2cfbe73

Browse files
KoltesDigitalrickeylev
KoltesDigital
and
rickeylev
authored
fix(windows): symlink bootstrap script when not building zip (bazel-contrib#2015)
This fixes bazel-contrib#1840 by supporting again `--build_python_zip=false` on Windows. When the zip file isn't build, the transition executable looks for the eponymous bootstrap script but the latter doesn't exist. I've just added a symlink, and refactored a bit because logic would have been duplicated. It seems you don't run CICD on Windows. FWIW I've manually tested it, both with `build_python_zip` as `true` and `false`. --------- Co-authored-by: Richard Levasseur <rlevasseur@google.com>
1 parent e6b9cff commit 2cfbe73

File tree

2 files changed

+115
-16
lines changed

2 files changed

+115
-16
lines changed

‎python/config_settings/transition.bzl

Lines changed: 39 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -43,24 +43,40 @@ def _transition_py_impl(ctx):
4343
output = executable,
4444
target_file = target[DefaultInfo].files_to_run.executable,
4545
)
46-
zipfile_symlink = None
46+
default_outputs = []
4747
if target_is_windows:
48-
# Under Windows, the expected "<name>.zip" does not exist, so we have to
49-
# create the symlink ourselves to achieve the same behaviour as in macOS
50-
# and Linux.
51-
zipfile = None
52-
expected_target_path = target[DefaultInfo].files_to_run.executable.short_path[:-4] + ".zip"
53-
for file in target[DefaultInfo].default_runfiles.files.to_list():
54-
if file.short_path == expected_target_path:
55-
zipfile = file
48+
# NOTE: Bazel 6 + host=linux + target=windows results in the .exe extension missing
49+
inner_bootstrap_path = _strip_suffix(target[DefaultInfo].files_to_run.executable.short_path, ".exe")
50+
inner_bootstrap = None
51+
inner_zip_file_path = inner_bootstrap_path + ".zip"
52+
inner_zip_file = None
53+
for file in target[DefaultInfo].files.to_list():
54+
if file.short_path == inner_bootstrap_path:
55+
inner_bootstrap = file
56+
elif file.short_path == inner_zip_file_path:
57+
inner_zip_file = file
5658

57-
if zipfile:
58-
zipfile_symlink = ctx.actions.declare_file(ctx.attr.name + ".zip")
59+
# TODO: Use `fragments.py.build_python_zip` once Bazel 6 support is dropped.
60+
# Which file the Windows .exe looks for depends on the --build_python_zip file.
61+
# Bazel 7+ has APIs to know the effective value of that flag, but not Bazel 6.
62+
# To work around this, we treat the existence of a .zip in the default outputs
63+
# to mean --build_python_zip=true.
64+
if inner_zip_file:
65+
suffix = ".zip"
66+
underlying_launched_file = inner_zip_file
67+
else:
68+
suffix = ""
69+
underlying_launched_file = inner_bootstrap
70+
71+
if underlying_launched_file:
72+
launched_file_symlink = ctx.actions.declare_file(ctx.attr.name + suffix)
5973
ctx.actions.symlink(
6074
is_executable = True,
61-
output = zipfile_symlink,
62-
target_file = zipfile,
75+
output = launched_file_symlink,
76+
target_file = underlying_launched_file,
6377
)
78+
default_outputs.append(launched_file_symlink)
79+
6480
env = {}
6581
for k, v in ctx.attr.env.items():
6682
env[k] = ctx.expand_location(v)
@@ -85,8 +101,8 @@ def _transition_py_impl(ctx):
85101
providers = [
86102
DefaultInfo(
87103
executable = executable,
88-
files = depset([zipfile_symlink] if zipfile_symlink else [], transitive = [target[DefaultInfo].files]),
89-
runfiles = ctx.runfiles([zipfile_symlink] if zipfile_symlink else []).merge(target[DefaultInfo].default_runfiles),
104+
files = depset(default_outputs, transitive = [target[DefaultInfo].files]),
105+
runfiles = ctx.runfiles(default_outputs).merge(target[DefaultInfo].default_runfiles),
90106
),
91107
py_info,
92108
py_runtime_info,
@@ -169,13 +185,15 @@ _transition_py_binary = rule(
169185
attrs = _COMMON_ATTRS | _PY_TEST_ATTRS,
170186
cfg = _transition_python_version,
171187
executable = True,
188+
fragments = ["py"],
172189
)
173190

174191
_transition_py_test = rule(
175192
_transition_py_impl,
176193
attrs = _COMMON_ATTRS | _PY_TEST_ATTRS,
177194
cfg = _transition_python_version,
178195
test = True,
196+
fragments = ["py"],
179197
)
180198

181199
def _py_rule(rule_impl, transition_rule, name, python_version, **kwargs):
@@ -263,3 +281,9 @@ def py_binary(name, python_version, **kwargs):
263281

264282
def py_test(name, python_version, **kwargs):
265283
return _py_rule(_py_test, _transition_py_test, name, python_version, **kwargs)
284+
285+
def _strip_suffix(s, suffix):
286+
if s.endswith(suffix):
287+
return s[:-len(suffix)]
288+
else:
289+
return s

‎tests/config_settings/transition/multi_version_tests.bzl

Lines changed: 76 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,9 @@
1515

1616
load("@rules_testing//lib:analysis_test.bzl", "analysis_test")
1717
load("@rules_testing//lib:test_suite.bzl", "test_suite")
18-
load("@rules_testing//lib:util.bzl", rt_util = "util")
18+
load("@rules_testing//lib:util.bzl", "TestingAspectInfo", rt_util = "util")
1919
load("//python/config_settings:transition.bzl", py_binary_transitioned = "py_binary", py_test_transitioned = "py_test")
20+
load("//python/private:util.bzl", "IS_BAZEL_7_OR_HIGHER") # buildifier: disable=bzl-visibility
2021

2122
# NOTE @aignas 2024-06-04: we are using here something that is registered in the MODULE.Bazel
2223
# and if you find tests failing, it could be because of the toolchain resolution issues here.
@@ -68,6 +69,80 @@ def _test_py_binary_with_transition_impl(env, target):
6869

6970
_tests.append(_test_py_binary_with_transition)
7071

72+
def _setup_py_binary_windows(name, *, impl, build_python_zip):
73+
rt_util.helper_target(
74+
py_binary_transitioned,
75+
name = name + "_subject",
76+
srcs = [name + "_subject.py"],
77+
python_version = _PYTHON_VERSION,
78+
)
79+
80+
analysis_test(
81+
name = name,
82+
target = name + "_subject",
83+
impl = impl,
84+
config_settings = {
85+
"//command_line_option:build_python_zip": build_python_zip,
86+
"//command_line_option:extra_toolchains": "//tests/cc:all",
87+
"//command_line_option:platforms": str(Label("//tests/support:windows_x86_64")),
88+
},
89+
)
90+
91+
def _test_py_binary_windows_build_python_zip_false(name):
92+
_setup_py_binary_windows(
93+
name,
94+
build_python_zip = "false",
95+
impl = _test_py_binary_windows_build_python_zip_false_impl,
96+
)
97+
98+
def _test_py_binary_windows_build_python_zip_false_impl(env, target):
99+
default_outputs = env.expect.that_target(target).default_outputs()
100+
if IS_BAZEL_7_OR_HIGHER:
101+
# TODO: These outputs aren't correct. The outputs shouldn't
102+
# have the "_" prefix on them (those are coming from the underlying
103+
# wrapped binary).
104+
env.expect.that_target(target).default_outputs().contains_exactly([
105+
"{package}/_{test_name}_subject",
106+
"{package}/_{test_name}_subject.exe",
107+
"{package}/{test_name}_subject",
108+
"{package}/{test_name}_subject.py",
109+
])
110+
else:
111+
inner_exe = target[TestingAspectInfo].attrs.target[DefaultInfo].files_to_run.executable
112+
default_outputs.contains_at_least([
113+
inner_exe.short_path,
114+
])
115+
116+
_tests.append(_test_py_binary_windows_build_python_zip_false)
117+
118+
def _test_py_binary_windows_build_python_zip_true(name):
119+
_setup_py_binary_windows(
120+
name,
121+
build_python_zip = "true",
122+
impl = _test_py_binary_windows_build_python_zip_true_impl,
123+
)
124+
125+
def _test_py_binary_windows_build_python_zip_true_impl(env, target):
126+
default_outputs = env.expect.that_target(target).default_outputs()
127+
if IS_BAZEL_7_OR_HIGHER:
128+
# TODO: These outputs aren't correct. The outputs shouldn't
129+
# have the "_" prefix on them (those are coming from the underlying
130+
# wrapped binary).
131+
default_outputs.contains_exactly([
132+
"{package}/_{test_name}_subject.exe",
133+
"{package}/_{test_name}_subject.zip",
134+
"{package}/{test_name}_subject.py",
135+
"{package}/{test_name}_subject.zip",
136+
])
137+
else:
138+
inner_exe = target[TestingAspectInfo].attrs.target[DefaultInfo].files_to_run.executable
139+
default_outputs.contains_at_least([
140+
"{package}/{test_name}_subject.zip",
141+
inner_exe.short_path,
142+
])
143+
144+
_tests.append(_test_py_binary_windows_build_python_zip_true)
145+
71146
def multi_version_test_suite(name):
72147
test_suite(
73148
name = name,

0 commit comments

Comments
 (0)
0