From d6931e0b8c98d313f6a4990ed6045d7b16151e07 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Fri, 27 Jan 2023 13:56:07 -0800 Subject: [PATCH] python: Various fixes for Windows launcher building in Starlark implementation * 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 --- .../common/python/py_executable_bazel.bzl | 33 +++++++++++++------ 1 file changed, 23 insertions(+), 10 deletions(-) diff --git a/src/main/starlark/builtins_bzl/common/python/py_executable_bazel.bzl b/src/main/starlark/builtins_bzl/common/python/py_executable_bazel.bzl index 8cb8cf7950109a..4f8db34b028169 100644 --- a/src/main/starlark/builtins_bzl/common/python/py_executable_bazel.bzl +++ b/src/main/starlark/builtins_bzl/common/python/py_executable_bazel.bzl @@ -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", @@ -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 @@ -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 @@ -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), @@ -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}",