fix(windows): symlink bootstrap script when not building zip (#2015)
This fixes #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>
diff --git a/python/config_settings/transition.bzl b/python/config_settings/transition.bzl
index 48b0447..da48a1f 100644
--- a/python/config_settings/transition.bzl
+++ b/python/config_settings/transition.bzl
@@ -43,24 +43,40 @@
output = executable,
target_file = target[DefaultInfo].files_to_run.executable,
)
- zipfile_symlink = None
+ default_outputs = []
if target_is_windows:
- # Under Windows, the expected "<name>.zip" does not exist, so we have to
- # create the symlink ourselves to achieve the same behaviour as in macOS
- # and Linux.
- zipfile = None
- expected_target_path = target[DefaultInfo].files_to_run.executable.short_path[:-4] + ".zip"
- for file in target[DefaultInfo].default_runfiles.files.to_list():
- if file.short_path == expected_target_path:
- zipfile = file
+ # NOTE: Bazel 6 + host=linux + target=windows results in the .exe extension missing
+ inner_bootstrap_path = _strip_suffix(target[DefaultInfo].files_to_run.executable.short_path, ".exe")
+ inner_bootstrap = None
+ inner_zip_file_path = inner_bootstrap_path + ".zip"
+ inner_zip_file = None
+ for file in target[DefaultInfo].files.to_list():
+ if file.short_path == inner_bootstrap_path:
+ inner_bootstrap = file
+ elif file.short_path == inner_zip_file_path:
+ inner_zip_file = file
- if zipfile:
- zipfile_symlink = ctx.actions.declare_file(ctx.attr.name + ".zip")
+ # TODO: Use `fragments.py.build_python_zip` once Bazel 6 support is dropped.
+ # Which file the Windows .exe looks for depends on the --build_python_zip file.
+ # Bazel 7+ has APIs to know the effective value of that flag, but not Bazel 6.
+ # To work around this, we treat the existence of a .zip in the default outputs
+ # to mean --build_python_zip=true.
+ if inner_zip_file:
+ suffix = ".zip"
+ underlying_launched_file = inner_zip_file
+ else:
+ suffix = ""
+ underlying_launched_file = inner_bootstrap
+
+ if underlying_launched_file:
+ launched_file_symlink = ctx.actions.declare_file(ctx.attr.name + suffix)
ctx.actions.symlink(
is_executable = True,
- output = zipfile_symlink,
- target_file = zipfile,
+ output = launched_file_symlink,
+ target_file = underlying_launched_file,
)
+ default_outputs.append(launched_file_symlink)
+
env = {}
for k, v in ctx.attr.env.items():
env[k] = ctx.expand_location(v)
@@ -85,8 +101,8 @@
providers = [
DefaultInfo(
executable = executable,
- files = depset([zipfile_symlink] if zipfile_symlink else [], transitive = [target[DefaultInfo].files]),
- runfiles = ctx.runfiles([zipfile_symlink] if zipfile_symlink else []).merge(target[DefaultInfo].default_runfiles),
+ files = depset(default_outputs, transitive = [target[DefaultInfo].files]),
+ runfiles = ctx.runfiles(default_outputs).merge(target[DefaultInfo].default_runfiles),
),
py_info,
py_runtime_info,
@@ -169,6 +185,7 @@
attrs = _COMMON_ATTRS | _PY_TEST_ATTRS,
cfg = _transition_python_version,
executable = True,
+ fragments = ["py"],
)
_transition_py_test = rule(
@@ -176,6 +193,7 @@
attrs = _COMMON_ATTRS | _PY_TEST_ATTRS,
cfg = _transition_python_version,
test = True,
+ fragments = ["py"],
)
def _py_rule(rule_impl, transition_rule, name, python_version, **kwargs):
@@ -263,3 +281,9 @@
def py_test(name, python_version, **kwargs):
return _py_rule(_py_test, _transition_py_test, name, python_version, **kwargs)
+
+def _strip_suffix(s, suffix):
+ if s.endswith(suffix):
+ return s[:-len(suffix)]
+ else:
+ return s
diff --git a/tests/config_settings/transition/multi_version_tests.bzl b/tests/config_settings/transition/multi_version_tests.bzl
index f3707db..e35590b 100644
--- a/tests/config_settings/transition/multi_version_tests.bzl
+++ b/tests/config_settings/transition/multi_version_tests.bzl
@@ -15,8 +15,9 @@
load("@rules_testing//lib:analysis_test.bzl", "analysis_test")
load("@rules_testing//lib:test_suite.bzl", "test_suite")
-load("@rules_testing//lib:util.bzl", rt_util = "util")
+load("@rules_testing//lib:util.bzl", "TestingAspectInfo", rt_util = "util")
load("//python/config_settings:transition.bzl", py_binary_transitioned = "py_binary", py_test_transitioned = "py_test")
+load("//python/private:util.bzl", "IS_BAZEL_7_OR_HIGHER") # buildifier: disable=bzl-visibility
# NOTE @aignas 2024-06-04: we are using here something that is registered in the MODULE.Bazel
# and if you find tests failing, it could be because of the toolchain resolution issues here.
@@ -68,6 +69,80 @@
_tests.append(_test_py_binary_with_transition)
+def _setup_py_binary_windows(name, *, impl, build_python_zip):
+ rt_util.helper_target(
+ py_binary_transitioned,
+ name = name + "_subject",
+ srcs = [name + "_subject.py"],
+ python_version = _PYTHON_VERSION,
+ )
+
+ analysis_test(
+ name = name,
+ target = name + "_subject",
+ impl = impl,
+ config_settings = {
+ "//command_line_option:build_python_zip": build_python_zip,
+ "//command_line_option:extra_toolchains": "//tests/cc:all",
+ "//command_line_option:platforms": str(Label("//tests/support:windows_x86_64")),
+ },
+ )
+
+def _test_py_binary_windows_build_python_zip_false(name):
+ _setup_py_binary_windows(
+ name,
+ build_python_zip = "false",
+ impl = _test_py_binary_windows_build_python_zip_false_impl,
+ )
+
+def _test_py_binary_windows_build_python_zip_false_impl(env, target):
+ default_outputs = env.expect.that_target(target).default_outputs()
+ if IS_BAZEL_7_OR_HIGHER:
+ # TODO: These outputs aren't correct. The outputs shouldn't
+ # have the "_" prefix on them (those are coming from the underlying
+ # wrapped binary).
+ env.expect.that_target(target).default_outputs().contains_exactly([
+ "{package}/_{test_name}_subject",
+ "{package}/_{test_name}_subject.exe",
+ "{package}/{test_name}_subject",
+ "{package}/{test_name}_subject.py",
+ ])
+ else:
+ inner_exe = target[TestingAspectInfo].attrs.target[DefaultInfo].files_to_run.executable
+ default_outputs.contains_at_least([
+ inner_exe.short_path,
+ ])
+
+_tests.append(_test_py_binary_windows_build_python_zip_false)
+
+def _test_py_binary_windows_build_python_zip_true(name):
+ _setup_py_binary_windows(
+ name,
+ build_python_zip = "true",
+ impl = _test_py_binary_windows_build_python_zip_true_impl,
+ )
+
+def _test_py_binary_windows_build_python_zip_true_impl(env, target):
+ default_outputs = env.expect.that_target(target).default_outputs()
+ if IS_BAZEL_7_OR_HIGHER:
+ # TODO: These outputs aren't correct. The outputs shouldn't
+ # have the "_" prefix on them (those are coming from the underlying
+ # wrapped binary).
+ default_outputs.contains_exactly([
+ "{package}/_{test_name}_subject.exe",
+ "{package}/_{test_name}_subject.zip",
+ "{package}/{test_name}_subject.py",
+ "{package}/{test_name}_subject.zip",
+ ])
+ else:
+ inner_exe = target[TestingAspectInfo].attrs.target[DefaultInfo].files_to_run.executable
+ default_outputs.contains_at_least([
+ "{package}/{test_name}_subject.zip",
+ inner_exe.short_path,
+ ])
+
+_tests.append(_test_py_binary_windows_build_python_zip_true)
+
def multi_version_test_suite(name):
test_suite(
name = name,
