fix(py_test): fix Windows crash in `py_test` main validation (#4079)
`py_test` main validation (`--validate_test_main=enabled`) crashes on
Windows, because it invokes the interpreter via
`exec_tools.exec_interpreter`, which resolves through
`current_interpreter_executable()`'s relocated copy of the interpreter.
The crash may be reproduced on existing tests in the repo, for instance:
```
bazel build \
--@rules_python//python/config_settings:validate_test_main=enabled \
//tests/validate_test_main:validate_test_main_test
...
ERROR: .../tests/validate_test_main/BUILD.bazel:3:8: Validating py_test
main //tests/validate_test_main:validate_test_main_test failed:
(Exit -1073741515): python.exe failed: error executing
PyValidateTestMain command
```
```
cd tests/integration/validate_test_main
bazel build \
--@rules_python//python/config_settings:validate_test_main=enabled \
//:good_test
...
ERROR: .../tests/integration/validate_test_main/BUILD.bazel:11:8:
Validating py_test main //:good_test failed: (Exit -1073741515):
python.exe failed: error executing PyValidateTestMain command
```
`-1073741515` is `STATUS_DLL_NOT_FOUND`: the relocated copy can't find
its DLLs beside itself.
This is also the Windows-local manifestation of #2703
(`exec_interpreter` broken on RBE): the same relocation severs the
interpreter from files resolved relative to itself, just triggered
differently: RBE's copy materialization there, a lack of symlink
privilege here.
Colocating the DLLs alone (a first attempt) traded this for a second,
still fatal error, `ModuleNotFoundError: No module named 'encodings'`,
because the copy still can't find its stdlib.
Patching each missing file individually doesn't scale: DLLs today,
stdlib tomorrow, whatever else a future toolchain needs beside itself
after that.
`_maybe_add_test_main_validation` now uses `actions_run()` with
`exec_runtime` instead of `exec_tools.exec_interpreter`'s relocated
`DefaultInfo.files_to_run`, matching
`PyExecToolsInfo.exec_interpreter`'s own documented recommendation and
the pattern `common.bzl`'s `actions_run()` and `py_zipapp_rule.bzl`
already use.
`exec_runtime.interpreter` is the real file, used directly, with its
real files as plain action inputs, so nothing is relocated and nothing
loses its siblings.
With the proposed fix[^1], above examples now build cleanly, with no
relocated runfiles tree for the interpreter at all, and
`tests/integration/validate_test_main`'s `inert_test` still fails with
its intended "will not run any tests" message rather than a crash.
`tests/integration/validate_test_main_test` is the corresponding
integration test, but it was not exercised on Windows, where it was
failing on `OSError: [WinError 193] %1 is not a valid Win32 application`
in `tests/integration/runner.py`'s Bazel-in-Bazel invocation, itself
unable to run `bazel_from_env`'s `#!` shebang line the way POSIX's
`exec` does.
The present change therefore fixes this, by resolving the shebang's
interpreter itself, and enables the test on Windows.
[^1]: This does not fix `exec_interpreter`/#2703 itself:
`precompile.bzl` still resolves the interpreter via the relocated path
and would need the same migration.
---------
Co-authored-by: Richard Levasseur <richardlev@gmail.com>
diff --git a/news/4079.fixed.md b/news/4079.fixed.md
new file mode 100644
index 0000000..7e1c323
--- /dev/null
+++ b/news/4079.fixed.md
@@ -0,0 +1,4 @@
+(toolchain) Fixed a crash in {obj}`py_test` main validation
+({obj}`validate_test_main`) on Windows: the interpreter used for the check
+couldn't find its DLLs or its stdlib once relocated
+([#4079](https://github.com/bazel-contrib/rules_python/issues/4079)).
diff --git a/python/private/py_executable.bzl b/python/private/py_executable.bzl
index 0e7e292..0e9c315 100644
--- a/python/private/py_executable.bzl
+++ b/python/private/py_executable.bzl
@@ -64,7 +64,6 @@
load(":py_executable_info.bzl", "PyExecutableInfo")
load(":py_info.bzl", "PyInfo", "VenvSymlinkKind")
load(":py_internal.bzl", "py_internal")
-load(":py_interpreter_program.bzl", "PyInterpreterProgramInfo")
load(":py_runtime_info.bzl", "DEFAULT_STUB_SHEBANG")
load(":reexports.bzl", "BuiltinPyInfo", "BuiltinPyRuntimeInfo")
load(":rule_builders.bzl", "ruleb")
@@ -1348,47 +1347,33 @@
return
exec_tools_toolchain = ctx.toolchains[EXEC_TOOLS_TOOLCHAIN_TYPE]
- if exec_tools_toolchain == None or exec_tools_toolchain.exec_tools.exec_interpreter == None:
+ if (
+ exec_tools_toolchain == None or
+ exec_tools_toolchain.exec_tools.exec_runtime == None
+ ):
fail(
"Validating py_test main modules requires the exec tools toolchain " +
- "with an exec interpreter, but none was found. Either register one " +
+ "with an exec runtime, but none was found. Either register one " +
"or set --@rules_python//python/config_settings:validate_test_main=disabled.",
)
- exec_tools = exec_tools_toolchain.exec_tools
validator = ctx.attr._validate_test_main
- program_info = validator[PyInterpreterProgramInfo]
- interpreter = exec_tools.exec_interpreter[DefaultInfo].files_to_run
- validator_files_to_run = validator[DefaultInfo].files_to_run
-
validation_output = ctx.actions.declare_file(ctx.label.name + "_validate_test_main.txt")
args = ctx.actions.args()
- args.add_all(program_info.interpreter_args)
- args.add(validator_files_to_run.executable)
args.add("--src", main_py)
args.add("--src_name", main_py.short_path)
args.add("--label", str(ctx.label))
args.add("--output", validation_output)
- execution_requirements = {}
- if testing.ExecutionInfo in validator:
- execution_requirements = validator[testing.ExecutionInfo].requirements
-
- ctx.actions.run(
- executable = interpreter,
+ actions_run(
+ ctx,
+ executable = validator,
arguments = [args],
inputs = [main_py],
outputs = [validation_output],
- tools = [validator_files_to_run],
mnemonic = "PyValidateTestMain",
progress_message = "Validating py_test main %{label}",
- env = program_info.env | {
- "PYTHONNOUSERSITE": "1",
- "PYTHONSAFEPATH": "1",
- },
- execution_requirements = execution_requirements,
- toolchain = EXEC_TOOLS_TOOLCHAIN_TYPE,
)
if "_validation" in output_groups:
output_groups["_validation"] = depset([validation_output], transitive = [output_groups["_validation"]])
diff --git a/tests/integration/BUILD.bazel b/tests/integration/BUILD.bazel
index 13b9c2e..2eae8df 100644
--- a/tests/integration/BUILD.bazel
+++ b/tests/integration/BUILD.bazel
@@ -52,6 +52,7 @@
"bzlmod_lockfile_test_bazel_9.1.0",
"local_toolchains_test_bazel_self",
"uv_lock_test_bazel_self",
+ "validate_test_main_test_bazel_self",
],
)
diff --git a/tests/integration/runner.py b/tests/integration/runner.py
index 9efcbeb..23e41f6 100644
--- a/tests/integration/runner.py
+++ b/tests/integration/runner.py
@@ -63,10 +63,10 @@
{env} \\
{args}
RESULT: exit_code: {self.exit_code}
-===== STDOUT START =====
-{self.stdout}{maybe_stdout_nl}===== STDOUT END =====
-===== STDERR START =====
-{self.stderr}{maybe_stderr_nl}===== STDERR END =====
+==================== STDOUT BEGIN ====================
+{self.stdout}{maybe_stdout_nl}==================== STDOUT END ====================
+==================== STDERR BEGIN ====================
+{self.stderr}{maybe_stderr_nl}==================== STDERR END ====================
"""
@@ -74,7 +74,17 @@
def setUp(self):
super().setUp()
self.repo_root = pathlib.Path(os.environ["BIT_WORKSPACE_DIR"])
- self.bazel = pathlib.Path(os.environ["BIT_BAZEL_BINARY"])
+ bazel = pathlib.Path(os.environ["BIT_BAZEL_BINARY"])
+ # Windows doesn't interpret shebangs, so prepend any script interpreter.
+ interpreter = []
+ if os.name == "nt":
+ with bazel.open("rb") as f:
+ first_line = f.readline()
+ if first_line.startswith(b"#!"):
+ interpreter = first_line[2:].decode().split()
+ if interpreter and interpreter[0].endswith("/env"):
+ interpreter = interpreter[1:]
+ self.bazel_cmd = (*interpreter, str(bazel))
outer_test_tmpdir = pathlib.Path(os.environ["TEST_TMPDIR"])
self.test_tmp_dir = outer_test_tmpdir / "bit_test_tmp"
# Put the global tmp not under the test tmp to better match how a real
@@ -103,7 +113,7 @@
Returns:
An `ExecuteResult` from running Bazel
"""
- cmd_args = [str(self.bazel), *args]
+ cmd_args = [*self.bazel_cmd, *args]
env = self.bazel_env
_logger.info("executing: %s", shlex.join(cmd_args))
cwd = self.repo_root
