Skip to content

Commit

Permalink
python: Various fixes for Windows launcher building in Starlark imple…
Browse files Browse the repository at this point in the history
…mentation

* Specify the attribute as an executable, since it's executable file is
  later used when building.
* Don't try to use a bootstrap template for Windows with --build_python_zip=true
  Apparently that flag defaults to true for windows, while it's false on
  other platforms; added some comments to better call that out.
* Pass the proper arguments to the launcher maker.

PiperOrigin-RevId: 505202311
Change-Id: Iede13417c3af9af76b302864a433c27f8041ee3c
  • Loading branch information
rickeylev authored and copybara-github committed Jan 27, 2023
1 parent 81acd2a commit 3b7e8d0
Showing 1 changed file with 23 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ the `srcs` of Python targets as required.
"_launcher": attr.label(
cfg = "target",
default = "@" + TOOLS_REPO + "//tools/launcher:launcher",
executable = True,
),
"_windows_launcher_maker": attr.label(
default = "@" + TOOLS_REPO + "//tools/launcher:launcher_maker",
Expand Down Expand Up @@ -197,6 +198,8 @@ def _create_executable(
)

extra_files_to_build = []

# NOTE: --build_python_zip defauls to true on Windows
build_zip_enabled = ctx.fragments.py.build_python_zip

# When --build_python_zip is enabled, then the zip file becomes
Expand All @@ -206,11 +209,12 @@ def _create_executable(

# The logic here is a bit convoluted. Essentially, there are 3 types of
# executables produced:
# 1. A bootstrap template based program.
# 2. A self-executable zip file of a bootstrap template based program.
# 3. For Windows, a native Windows executable that finds and launches
# 1. (non-Windows) A bootstrap template based program.
# 2. (non-Windows) A self-executable zip file of a bootstrap template based program.
# 3. (Windows) A native Windows executable that finds and launches
# the actual underlying Bazel program (one of the above). Note that
# it implicitly assumes one of the above is located next to it.
# it implicitly assumes one of the above is located next to it, and
# that --build_python_zip defaults to true for Windows.

should_create_executable_zip = False
bootstrap_output = None
Expand Down Expand Up @@ -240,16 +244,25 @@ def _create_executable(
fail("Should not occur: bootstrap_output should not be used " +
"when creating an executable zip")
_create_executable_zip_file(ctx, output = executable, zip_file = zip_file)
else:
if bootstrap_output == None:
fail("Should not occur: bootstrap_output should set when " +
"build a bootstrap-template-based executable")
elif bootstrap_output:
_expand_bootstrap_template(
ctx,
output = bootstrap_output,
is_for_zip = build_zip_enabled,
**common_bootstrap_template_kwargs
)
else:
# Otherwise, this should be the Windows case of launcher + zip.
# Double check this just to make sure.
if not is_windows or not build_zip_enabled:
fail(("Should not occur: The non-executable-zip and " +
"non-boostrap-template case should have windows and zip " +
"both true, but got " +
"is_windows={is_windows} " +
"build_zip_enabled={build_zip_enabled}").format(
is_windows = is_windows,
build_zip_enabled = build_zip_enabled,
))

return create_executable_result_struct(
extra_files_to_build = depset(extra_files_to_build),
Expand Down Expand Up @@ -320,8 +333,8 @@ def _create_windows_exe_launcher(

ctx.actions.run(
executable = ctx.executable._windows_launcher_maker,
arguments = [launch_info],
inputs = [ctx.file._launcher],
arguments = [ctx.executable._launcher.path, launch_info, output.path],
inputs = [ctx.executable._launcher],
outputs = [output],
mnemonic = "PyBuildLauncher",
progress_message = "Creating launcher for %{label}",
Expand Down

0 comments on commit 3b7e8d0

Please sign in to comment.