fix(bootstrap): preserve custom executables across platforms

Author: jrandolf

python/private/_rules_python_bootstrap/process.py

@@ -45,19 +45,23 @@ def check(self):
     def restore(self):
         for signum, previous in self.handlers.items():
             signal.signal(signum, previous)
-        if self.mask is not None:
+        if os.name == "posix" and self.mask is not None:
             signal.pthread_sigmask(signal.SIG_SETMASK, self.mask)
 
     def prepare_exec(self):
-        # Stop recording cancellation before restoring the caller's mask.
-        # Pending kernel signals then see the caller's dispositions; requests
-        # already recorded by Python are checked after every handler is restored.
-        signal.pthread_sigmask(signal.SIG_BLOCK, self.handlers)
-        self.check()
-        for signum, previous in self.handlers.items():
-            signal.signal(signum, previous)
-        self.check()
-        signal.pthread_sigmask(signal.SIG_SETMASK, self.mask)
+        if os.name == "posix":
+            assert self.mask is not None, "POSIX cancellation context was not entered"
+            # Stop recording cancellation before restoring the caller's mask.
+            # Pending kernel signals then see the caller's dispositions; requests
+            # already recorded by Python are checked after every handler is restored.
+            signal.pthread_sigmask(signal.SIG_BLOCK, self.handlers)
+            self.check()
+            for signum, previous in self.handlers.items():
+                signal.signal(signum, previous)
+            self.check()
+            signal.pthread_sigmask(signal.SIG_SETMASK, self.mask)
+        else:
+            raise RuntimeError("Native exec requires a POSIX cancellation context")
 
     def __exit__(self, _kind, error, _traceback):
         self.restore()

python/private/zipapp/py_zipapp_rule.bzl

@@ -156,17 +156,9 @@ def _create_zip(ctx, py_runtime, py_executable, stage2_bootstrap):
 
     runfiles = runfiles.build(ctx)
 
-    interpreter_symlinks = []
-    if py_executable.venv_python_exe and py_runtime.interpreter:
-        # Runtime-created venvs represent this file with an `actual:` marker.
-        # A standalone archive can create the real relative link at build time,
-        # regardless of the original executable's venv strategy.
-        interpreter_symlinks.append(struct(
-            runfiles_path = runfiles_root_path(ctx, py_executable.venv_python_exe.short_path),
-            link_to_path = runfiles_root_path(ctx, py_runtime.interpreter.short_path),
-            files = depset([py_runtime.interpreter]),
-        ))
-    explicit_symlinks = depset(direct = interpreter_symlinks, transitive = [
+    # A custom rule may supply its own executable. Only replace archive files
+    # with links that the provider explicitly declares.
+    explicit_symlinks = depset(transitive = [
         py_executable.venv_interpreter_symlinks,
         py_executable.venv_app_symlinks,
     ])

tests/bootstrap_impls/BUILD.bazel

@@ -12,6 +12,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 load("@rules_pkg//pkg:tar.bzl", "pkg_tar")
+load("@rules_python_internal//:rules_python_config.bzl", "config")
 load("@rules_shell//shell:sh_test.bzl", "sh_test")
 load("//python:py_runtime.bzl", "py_runtime")
 load("//python:py_runtime_pair.bzl", "py_runtime_pair")
@@ -21,7 +22,7 @@ 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")
 load("//tests/support/pytest_test:pytest_test.bzl", "pytest_test")
-load(":application_tests.bzl", "application_test_suite", "public_executable", "template_fixture")
+load(":application_tests.bzl", "application_test_suite", "public_executable", "template_fixture", "template_output")
 load(":venv_relative_path_tests.bzl", "relative_path_test_suite")
 
 py_reconfig_binary(
@@ -256,11 +257,25 @@ template_fixture(
     contents = "custom zip\r\nworkspace=%workspace_name%\r\nmain=%stage2_bootstrap%",
 )
 
+template_fixture(
+    name = "custom_application_interpreter",
+    contents = "This interpreter is packaged for template inspection, never executed.",
+)
+
 py_runtime(
     name = "custom_application_runtime",
     # Also supplies the non-test current_interpreter_executable toolchain alias.
     bootstrap_template = ":custom_directory_template",
-    interpreter_path = "/custom/python",
+    # Windows venvs declare an interpreter symlink even for custom templates.
+    # Use a declared file so remote packaging does not need a worker-local path.
+    interpreter = select({
+        "@platforms//os:windows": ":custom_application_interpreter",
+        "//conditions:default": None,
+    }),
+    interpreter_path = select({
+        "@platforms//os:windows": "",
+        "//conditions:default": "/custom/python",
+    }),
     supports_build_time_venv = False,
     zip_main_template = ":custom_zip_template",
 )
@@ -296,6 +311,12 @@ py_zipapp_binary(
     executable = False,
 )
 
+template_output(
+    name = "custom_application_bootstrap",
+    testonly = True,
+    binary = ":custom_application",
+)
+
 filegroup(
     name = "custom_application_archive",
     testonly = True,
@@ -334,18 +355,17 @@ pytest_test(
     name = "application_templates_test",
     srcs = ["application_templates_test.py"],
     data = [
-        ":custom_application",
         ":custom_application_archive",
+        ":custom_application_bootstrap",
         ":public_external_archive",
         "//python/private:stage1_bootstrap_template",
     ],
     env = {
-        "CUSTOM_BINARY": "$(rlocationpath :custom_application)",
+        "CUSTOM_BOOTSTRAP": "$(rlocationpath :custom_application_bootstrap)",
         "CUSTOM_ZIP": "$(rlocationpath :custom_application_archive)",
         "PUBLIC_ZIP": "$(rlocationpath :public_external_archive)",
         "RAW_SHELL_TEMPLATE": "$(rlocationpath //python/private:stage1_bootstrap_template)",
     },
-    target_compatible_with = SUPPORTS_BOOTSTRAP_SCRIPT,
     deps = ["//python/runfiles"],
 )
 
@@ -472,6 +492,8 @@ pytest_test(
         "PEER_RLOCATION": "$(rlocationpath cleanup_terminal_peer.py)",
         "PROBE_RLOCATION": "$(rlocationpath cleanup_probe.py)",
         "PYVENV_RLOCATION": "$(rlocationpath :cleanup_pyvenv)",
+        # Bazel 7's system_python bootstrap uses its interpreter directly.
+        "PYVENV_TEMPORARY": "1" if config.bazel_8_or_later else "0",
         "PYZIP_RLOCATION": "$(rlocationpath :cleanup_pyzip)",
         "STARTUP_RLOCATION": "$(rlocationpath cleanup_sitecustomize.py)",
         "VENV_RLOCATION": "$(rlocationpath :cleanup_venv)",

tests/bootstrap_impls/application_templates_test.py

@@ -13,10 +13,13 @@
 
 
 def test_custom_directory_template_preserves_bytes():
-    path = runfiles.CreateOrRaise().Rlocation(os.environ["CUSTOM_BINARY"])
+    path = runfiles.CreateOrRaise().Rlocation(os.environ["CUSTOM_BOOTSTRAP"])
     assert path is not None
     contents = Path(path).read_bytes()
-    assert contents.startswith(b"custom\r\nworkspace=_main\r\nmain=_main/")
+    workspace = os.environ["TEST_WORKSPACE"]
+    assert contents.startswith(
+        f"custom\r\nworkspace={workspace}\r\nmain={workspace}/".encode()
+    )
     assert contents.endswith(b"_custom_application_stage2_bootstrap.py")
     assert b"_rules_python_bootstrap" not in contents
 
@@ -26,7 +29,8 @@ def test_public_provider_keeps_independently_supplied_executable():
     assert path is not None
     with zipfile.ZipFile(path) as archive:
         contents = archive.read(
-            "runfiles/_main/tests/bootstrap_impls/supplied_venv.txt"
+            f"runfiles/{os.environ['TEST_WORKSPACE']}/"
+            "tests/bootstrap_impls/supplied_venv.txt"
         )
     assert contents == b"custom executable supplied without interpreter runfiles"
 
@@ -36,12 +40,16 @@ def test_custom_zip_template_preserves_public_substitutions():
     assert path is not None
     with zipfile.ZipFile(path) as archive:
         contents = archive.read("__main__.py")
-    assert contents.startswith(b"custom zip\r\nworkspace=_main\r\nmain=_main/")
+    workspace = os.environ["TEST_WORKSPACE"]
+    assert contents.startswith(
+        f"custom zip\r\nworkspace={workspace}\r\nmain={workspace}/".encode()
+    )
     assert contents.endswith(b"_custom_application_stage2_bootstrap.py")
     assert b"_rules_python_bootstrap" not in contents
 
 
 @pytest.mark.parametrize("status", [0, 23])
+@pytest.mark.skipif(os.name == "nt", reason="The raw template uses Bash")
 def test_raw_shell_template_accepts_historical_substitutions(tmp_path, status):
     template = runfiles.CreateOrRaise().Rlocation(os.environ["RAW_SHELL_TEMPLATE"])
     assert template is not None

tests/bootstrap_impls/application_tests.bzl

@@ -62,6 +62,21 @@ template_fixture = rule(
     attrs = {"contents": attr.string()},
 )
 
+def _template_output_impl(ctx):
+    binary = ctx.attr.binary[DefaultInfo]
+    path = binary.files_to_run.executable.path
+    if path.endswith(".exe"):
+        path = path[:-4]
+    outputs = [file for file in binary.files.to_list() if file.path == path]
+    if len(outputs) != 1:
+        fail("Expected one text bootstrap output for {}".format(ctx.attr.binary.label))
+    return [DefaultInfo(files = depset(outputs))]
+
+template_output = rule(
+    implementation = _template_output_impl,
+    attrs = {"binary": attr.label(mandatory = True)},
+)
+
 def _public_executable_impl(ctx):
     binary = ctx.attr.binary
     public = binary[PyExecutableInfo]

tests/bootstrap_impls/bootstrap_cleanup_test.py

@@ -42,6 +42,9 @@ def __init__(self, root, mode):
             "wrapper_zip",
         )
         self.archive = mode not in ("venv", "pyvenv", "wrapper")
+        self.temporary_runtime = (
+            mode != "pyvenv" or os.environ["PYVENV_TEMPORARY"] == "1"
+        )
         self.files = runfiles.CreateOrRaise()
         self.scratch = root / "temporary runtime with spaces"
         self.scratch.mkdir()
@@ -155,7 +158,10 @@ def test_spaces_tmpdir_stdin_arguments_and_status(launcher, status):
     actual = json.loads(result.stdout)
     assert actual["argv"] == ["argument with spaces"]
     assert actual["stdin"] == "input with spaces"
-    assert Path(actual["executable"]).is_relative_to(launcher.scratch)
+    assert (
+        Path(actual["executable"]).is_relative_to(launcher.scratch)
+        == launcher.temporary_runtime
+    )
     assert actual["ignored"] == [False, False]
     assert (
         actual["xoptions"]["bootstrap_target"]
@@ -212,7 +218,9 @@ def test_shell_handoff_exports_selected_runfiles_with_manifest(venv):
     root = venv.root / "provided.runfiles"
     root.symlink_to(os.fspath(venv.files.root()), target_is_directory=True)
     manifest = venv.root / "provided.runfiles_manifest"
-    manifest.write_text("_main/fixture " + str(venv.binary) + "\n")
+    manifest.write_text(
+        os.environ["TEST_WORKSPACE"] + "/fixture " + str(venv.binary) + "\n"
+    )
     result = venv.run(
         "echo",
         "0",
@@ -285,7 +293,7 @@ def test_signal_waits_for_application_cleanup(launcher, value, group):
             lambda: (launcher.root / "stopping").exists(), "cancellation did not arrive"
         )
         assert process.poll() is None
-        assert list(launcher.scratch.iterdir())
+        assert bool(list(launcher.scratch.iterdir())) == launcher.temporary_runtime
         process.send_signal(signal.SIGTERM)
         process.send_signal(signal.SIGHUP)
         assert process.stdin is not None
@@ -422,10 +430,13 @@ def test_relative_tmpdir_and_zip_working_directory(launcher):
     result = launcher.run("echo", "0", environment=environment)
     assert result.returncode == 0, result.stderr
     actual = json.loads(result.stdout)
-    assert Path(actual["executable"]).is_relative_to(launcher.scratch)
+    assert (
+        Path(actual["executable"]).is_relative_to(launcher.scratch)
+        == launcher.temporary_runtime
+    )
     if launcher.archive:
         assert actual["cwd"] == os.path.normpath(
-            str(Path(actual["runfiles"]) / "_main")
+            str(Path(actual["runfiles"]) / os.environ["TEST_WORKSPACE"])
         )
     launcher.cleaned()
 
@@ -519,7 +530,7 @@ def test_warm_shell_zip_executes_without_preparation(tmp_path):
     assert actual["argv"] == ["warm argument"]
     assert actual["runfiles"] == prepared["runfiles"]
     assert actual["executable"] == prepared["executable"]
-    assert Path(actual["cwd"]).samefile(runfiles_root / "_main")
+    assert Path(actual["cwd"]).samefile(runfiles_root / os.environ["TEST_WORKSPACE"])
     assert actual["xoptions"]["warm_cache"] == "once"
     assert "RULES_PYTHON_ZIP_DIR" not in actual["xoptions"]
     assert sentinel.read_text() == "keep"
@@ -757,7 +768,10 @@ def test_inherited_path_variables_cannot_select_cleanup(launcher, allocation_fai
     if allocation_fails:
         environment["TMPDIR"] = str(launcher.root / "missing temporary root")
     result = launcher.run("echo", "0", environment=environment)
-    if allocation_fails and not launcher.shell:
+    if not launcher.temporary_runtime:
+        assert result.returncode == 0, result.stderr
+        assert Path(json.loads(result.stdout)["executable"]).is_file()
+    elif allocation_fails and not launcher.shell:
         # tempfile falls back from an invalid TMPDIR to the platform temp root.
         assert result.returncode == 0, result.stderr
         wait_until(
@@ -843,7 +857,9 @@ def test_verbose_mode_retains_runtime(launcher):
     assert Path(json.loads(result.stdout)["executable"]).is_file()
     if not launcher.shell:
         assert "rules_python bootstrap: launching interpreter" in result.stderr
-        assert "rules_python bootstrap: retaining workspace" in result.stderr
+        assert (
+            "rules_python bootstrap: retaining workspace" in result.stderr
+        ) == launcher.temporary_runtime
         if launcher.archive:
             assert "rules_python bootstrap: extracting archive" in result.stderr
 
@@ -949,7 +965,7 @@ def test_foreground_input_and_job_control(launcher, ignored_continue):
             lambda: os.tcgetpgrp(terminal) == shell_pid,
             "shell did not recover foreground",
         )
-        assert list(launcher.scratch.iterdir()), "stopped process lost its runtime"
+        assert bool(list(launcher.scratch.iterdir())) == launcher.temporary_runtime
         stopped = launcher.root / "background-stop"
         # Wait in the owning shell for the new SIGTTIN stop. Sampling `ps`
         # here can observe the old Ctrl-Z stop before `bg` has run.