Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .bazelci/presubmit.yml
Original file line number Diff line number Diff line change
Expand Up @@ -256,8 +256,14 @@ tasks:
<<: *reusable_config
name: "Default: Windows"
platform: windows
build_targets:
- "--"
- "//tests/py_zipapp:system_python_zipapp_test"
test_flags:
- "--test_tag_filters=-integration-test,-fix-windows"
test_targets:
- "--"
- "//tests/py_zipapp:system_python_zipapp_test"
rbe_min:
<<: *minimum_supported_version
<<: *reusable_config
Expand Down
96 changes: 87 additions & 9 deletions python/private/zipapp/py_zipapp_rule.bzl
Original file line number Diff line number Diff line change
@@ -1,16 +1,56 @@
"""Implementation of the zipapp rules."""

load("@bazel_skylib//lib:paths.bzl", "paths")
load("@rules_python_internal//:rules_python_config.bzl", rp_config = "config")
load("//python/private:attributes.bzl", "apply_config_settings_attr")
load("//python/private:builders.bzl", "builders")
load("//python/private:common.bzl", "BUILTIN_BUILD_PYTHON_ZIP", "actions_run", "maybe_builtin_build_python_zip", "maybe_create_repo_mapping", "runfiles_root_path")
load("//python/private:common.bzl", "BUILTIN_BUILD_PYTHON_ZIP", "actions_run", "maybe_builtin_build_python_zip", "maybe_create_repo_mapping", "runfiles_root_path", "target_platform_has_any_constraint")
load("//python/private:common_labels.bzl", "labels")
load("//python/private:py_executable_info.bzl", "PyExecutableInfo")
load("//python/private:py_internal.bzl", "py_internal")
load("//python/private:py_runtime_info.bzl", "PyRuntimeInfo")
load("//python/private:toolchain_types.bzl", "EXEC_TOOLS_TOOLCHAIN_TYPE")
load("//python/private:transition_labels.bzl", "TRANSITION_LABELS")

_LAUNCHER_MAKER_TOOLCHAIN_TYPE = "@bazel_tools//tools/launcher:launcher_maker_toolchain_type"

def _find_launcher_maker(ctx):
if rp_config.bazel_9_or_later:
return (ctx.toolchains[_LAUNCHER_MAKER_TOOLCHAIN_TYPE].binary, _LAUNCHER_MAKER_TOOLCHAIN_TYPE)
return (ctx.executable._windows_launcher_maker, None)

def _create_windows_exe_launcher(
ctx,
*,
output,
python_binary_path,
use_zip_file):
launch_info = ctx.actions.args()
launch_info.use_param_file("%s", use_always = True)
launch_info.set_param_file_format("multiline")
launch_info.add("binary_type=Python")
launch_info.add(ctx.workspace_name, format = "workspace_name=%s")
launch_info.add(
"1" if py_internal.runfiles_enabled(ctx) else "0",
format = "symlink_runfiles_enabled=%s",
)
launch_info.add(python_binary_path, format = "python_bin_path=%s")
launch_info.add("1" if use_zip_file else "0", format = "use_zip_file=%s")

launcher = ctx.attr._launcher[DefaultInfo].files_to_run.executable
executable, toolchain = _find_launcher_maker(ctx)
ctx.actions.run(
executable = executable,
arguments = [launcher.path, launch_info, output.path],
inputs = [launcher],
outputs = [output],
mnemonic = "PyBuildLauncher",
progress_message = "Creating launcher for %{label}",
# Needed to inherit PATH when using non-MSVC compilers like MinGW
use_default_shell_env = True,
toolchain = toolchain,
)

def _is_symlink(f):
if hasattr(f, "is_symlink"):
return str(int(f.is_symlink))
Expand Down Expand Up @@ -184,20 +224,39 @@ def _py_zipapp_executable_impl(ctx):

zip_file = _create_zip(ctx, py_runtime, py_executable, stage2_bootstrap)
if ctx.attr.executable:
preamble = _create_shell_bootstrap(ctx, py_runtime, py_executable, stage2_bootstrap)
executable = _create_self_executable_zip(ctx, preamble, zip_file)
default_output = executable
if target_platform_has_any_constraint(ctx, ctx.attr._windows_constraints):
executable = ctx.actions.declare_file(ctx.label.name + ".exe")

python_exe = py_executable.venv_python_exe
if python_exe:
python_exe_path = runfiles_root_path(ctx, python_exe.short_path)
elif py_runtime.interpreter:
python_exe_path = runfiles_root_path(ctx, py_runtime.interpreter.short_path)
else:
python_exe_path = py_runtime.interpreter_path

_create_windows_exe_launcher(
ctx,
output = executable,
python_binary_path = python_exe_path,
use_zip_file = True,
)
default_output = depset([executable, zip_file])
else:
preamble = _create_shell_bootstrap(ctx, py_runtime, py_executable, stage2_bootstrap)
executable = _create_self_executable_zip(ctx, preamble, zip_file)
default_output = depset([executable])
else:
# Bazel requires executable=True rules to have an executable given, so give
# a fake one to satisfy that.
default_output = zip_file
default_output = depset([zip_file])
executable = ctx.actions.declare_file(ctx.label.name + "-not-executable")
ctx.actions.write(executable, "echo 'ERROR: Non executable zip file'; exit 1")

return [
DefaultInfo(
files = depset([default_output]),
runfiles = ctx.runfiles(files = [default_output]),
files = default_output,
runfiles = ctx.runfiles(files = default_output.to_list()),
executable = executable,
),
]
Expand Down Expand Up @@ -277,6 +336,18 @@ Whether the output should be an executable zip file.
cfg = "exec",
default = "//tools/private/zipapp:exe_zip_maker",
),
"_launcher": attr.label(
cfg = "target",
# NOTE: This is an executable, but is only used for Windows. It
# can't have executable=True because the backing target is an
# empty target for other platforms.
default = "//tools/launcher:launcher",
),
"_windows_constraints": attr.label_list(
default = [
"@platforms//os:windows",
],
),
"_zip_shell_template": attr.label(
default = ":zip_shell_template",
allow_single_file = True,
Expand All @@ -285,8 +356,15 @@ Whether the output should be an executable zip file.
cfg = "exec",
default = "//tools/private/zipapp:zipper",
),
}
_TOOLCHAINS = [EXEC_TOOLS_TOOLCHAIN_TYPE]
} | ({
"_windows_launcher_maker": attr.label(
default = "@bazel_tools//tools/launcher:launcher_maker",
cfg = "exec",
executable = True,
),
} if not rp_config.bazel_9_or_later else {})

_TOOLCHAINS = [EXEC_TOOLS_TOOLCHAIN_TYPE] + ([_LAUNCHER_MAKER_TOOLCHAIN_TYPE] if rp_config.bazel_9_or_later else [])

py_zipapp_binary = rule(
doc = """
Expand Down
14 changes: 7 additions & 7 deletions tests/py_zipapp/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,14 @@ py_binary(
"//python/config_settings:venvs_site_packages": "yes",
},
main = "main.py",
target_compatible_with = NOT_WINDOWS,
##target_compatible_with = NOT_WINDOWS,
deps = ["@dev_pip//absl_py"],
)

py_zipapp_binary(
name = "venv_zipapp",
binary = ":venv_bin",
target_compatible_with = NOT_WINDOWS,
##target_compatible_with = NOT_WINDOWS,
)

py_test(
Expand All @@ -34,7 +34,7 @@ py_test(
"BZLMOD_ENABLED": str(int(BZLMOD_ENABLED)),
"TEST_ZIPAPP": "$(location :venv_zipapp)",
},
target_compatible_with = NOT_WINDOWS,
##target_compatible_with = NOT_WINDOWS,
)

py_binary(
Expand All @@ -46,14 +46,14 @@ py_binary(
},
main = "main.py",
# TODO: #2586 - Add windows support
target_compatible_with = NOT_WINDOWS,
deps = ["@dev_pip//absl_py"],
##target_compatible_with = NOT_WINDOWS,
##deps = ["@dev_pip//absl_py"],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

This dependency on @dev_pip//absl_py has been commented out. However, the main source file for this target (main.py) imports absl. Without this dependency, the test will fail at runtime with an ImportError. This dependency should be restored.

Suggested change
##deps = ["@dev_pip//absl_py"],
deps = ["@dev_pip//absl_py"],

)

py_zipapp_binary(
name = "system_python_zipapp",
binary = ":system_python_bin",
target_compatible_with = NOT_WINDOWS,
##target_compatible_with = NOT_WINDOWS,
)

py_test(
Expand All @@ -63,5 +63,5 @@ py_test(
env = {
"TEST_ZIPAPP": "$(location :system_python_zipapp)",
},
target_compatible_with = NOT_WINDOWS,
##target_compatible_with = NOT_WINDOWS,
)
8 changes: 4 additions & 4 deletions tests/py_zipapp/system_python_zipapp_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,10 @@ def test_zipapp_contents(self):
self.assertTrue(os.path.exists(zipapp_path))
self.assertTrue(os.path.isfile(zipapp_path))

# The zipapp itself is a shell script prepended to the zip file.
with open(zipapp_path, "rb") as f:
content = f.read()
self.assertTrue(content.startswith(b"#!/usr/bin/env bash"))
### The zipapp itself is a shell script prepended to the zip file.
##with open(zipapp_path, "rb") as f:
## content = f.read()
##self.assertTrue(content.startswith(b"#!/usr/bin/env bash"))

output = subprocess.check_output([zipapp_path]).decode("utf-8").strip()
self.assertIn("Hello from zipapp", output)
Expand Down