diff --git a/python/private/BUILD.bazel b/python/private/BUILD.bazel index 9e4313d1f8..81baf9f615 100644 --- a/python/private/BUILD.bazel +++ b/python/private/BUILD.bazel @@ -207,6 +207,14 @@ py_library( ], ) +filegroup( + name = "exec_interpreter_template", + srcs = select({ + "@platforms//os:windows": [":interpreter_template.bat"], + "//conditions:default": [":interpreter_template.sh"], + }), +) + # The current toolchain's interpreter as an excutable, usable with # executable=True attributes. current_interpreter_executable( @@ -537,6 +545,7 @@ bzl_library( name = "py_exec_tools_toolchain", srcs = ["py_exec_tools_toolchain.bzl"], deps = [ + ":common", ":common_labels", ":py_exec_tools_info", ":sentinel_impl", diff --git a/python/private/common.bzl b/python/private/common.bzl index 5cff7f8723..cb946cb128 100644 --- a/python/private/common.bzl +++ b/python/private/common.bzl @@ -648,18 +648,14 @@ def actions_run( EXEC_TOOLS_TOOLCHAIN_TYPE, toolchain, )) - exec_runtime = ctx.toolchains[EXEC_TOOLS_TOOLCHAIN_TYPE].exec_tools.exec_runtime - if exec_runtime.interpreter: - action_exe = exec_runtime.interpreter - action_inputs.add(exec_runtime.files) - elif exec_runtime.interpreter_path: - action_exe = exec_runtime.interpreter_path - else: - fail(("Action {}: PyRuntimeInfo from exec tools toolchain is " + - "malformed: requires one of `interpreter` or " + - "`interpreter_path` set").format( + exec_tools = ctx.toolchains[EXEC_TOOLS_TOOLCHAIN_TYPE].exec_tools + if not exec_tools.exec_interpreter: + fail(("Action {}: tool {} provides PyInterpreterProgramInfo, " + + "but exec_tools.exec_interpreter is not configured").format( mnemonic, + executable, )) + action_exe = exec_tools.exec_interpreter[DefaultInfo].files_to_run program_info = executable[PyInterpreterProgramInfo] diff --git a/python/private/interpreter.bzl b/python/private/interpreter.bzl index b281be9639..aa1fd34845 100644 --- a/python/private/interpreter.bzl +++ b/python/private/interpreter.bzl @@ -46,7 +46,10 @@ def _interpreter_binary_impl(ctx): template = ctx.file._template, output = executable, substitutions = { - "%target_file%": runfiles_root_path(ctx, runtime.interpreter.short_path), + "%python_exe_runfiles_path%": runfiles_root_path( + ctx, + runtime.interpreter.short_path, + ), }, is_executable = True, ) @@ -75,7 +78,7 @@ interpreter_binary = rule( default = "@bazel_tools//tools/bash/runfiles", ), "_template": attr.label( - default = "//python/private:interpreter_tmpl.sh", + default = "//python/private:exec_interpreter_template", allow_single_file = True, ), }, diff --git a/python/private/interpreter_template.bat b/python/private/interpreter_template.bat new file mode 100644 index 0000000000..5225f79a05 --- /dev/null +++ b/python/private/interpreter_template.bat @@ -0,0 +1,83 @@ +@echo off +SETLOCAL ENABLEEXTENSIONS +SETLOCAL ENABLEDELAYEDEXPANSION + +set "PYTHON_EXE_RUNFILES_PATH=%python_exe_runfiles_path%" +set "TF_WIN=!PYTHON_EXE_RUNFILES_PATH:/=\!" +set "MAIN_BIN=" + +REM --- Resolve manifest file --- +set "MF=" +if defined RUNFILES_MANIFEST_FILE ( + set "MF=%RUNFILES_MANIFEST_FILE:/=\%" +) +if not defined MF ( + if exist "%~f0.runfiles_manifest" ( + set "MF=%~f0.runfiles_manifest" + ) else if exist "%~dpn0.runfiles_manifest" ( + set "MF=%~dpn0.runfiles_manifest" + ) else if exist "%~f0.runfiles\MANIFEST" ( + set "MF=%~f0.runfiles\MANIFEST" + ) else if exist "%~dpn0.runfiles\MANIFEST" ( + set "MF=%~dpn0.runfiles\MANIFEST" + ) +) + +if "%RUNFILES_MANIFEST_ONLY%" neq "1" ( + if defined RUNFILES_DIR ( + if exist "%RUNFILES_DIR%\!TF_WIN!" ( + set "MAIN_BIN=%RUNFILES_DIR%\!TF_WIN!" + ) + ) + if not defined MAIN_BIN ( + if exist "%~f0.runfiles\!TF_WIN!" ( + set "MAIN_BIN=%~f0.runfiles\!TF_WIN!" + ) else if exist "%~dpn0.runfiles\!TF_WIN!" ( + set "MAIN_BIN=%~dpn0.runfiles\!TF_WIN!" + ) + ) +) + +if not defined MAIN_BIN if defined MF ( + if exist "!MF!" ( + for /F "usebackq tokens=1* delims= " %%a in ("!MF!") do ( + if "%%a"=="!PYTHON_EXE_RUNFILES_PATH!" ( + set "MAIN_BIN=%%b" + goto :found_bin + ) + ) + ) +) +:found_bin + +if not defined MAIN_BIN ( + echo>&2 ERROR: interpreter executable not found: !PYTHON_EXE_RUNFILES_PATH! + exit /b 1 +) + +set "MAIN_BIN=!MAIN_BIN:/=\!" +if not exist "!MAIN_BIN!" ( + echo>&2 ERROR: interpreter executable not found: !MAIN_BIN! + echo>&2 (from !PYTHON_EXE_RUNFILES_PATH!) + exit /b 1 +) + +rem Determine PYTHONHOME (installation prefix containing Lib). +rem We must set PYTHONHOME to point to the runfiles directory because runfiles +rem provides the unified merge of source files and generated files needed by +rem the standard library and runtime. +rem In Windows layouts, Lib is typically sibling to python.exe (%%~dpdLib). +rem In hierarchical layouts, Lib may be in the parent directory (%%~dpd..\Lib). +for %%d in ("!MAIN_BIN!") do ( + if exist "%%~dpdLib" ( + set "PYTHONHOME=%%~dpd" + ) else if exist "%%~dpd..\Lib" ( + for %%p in ("%%~dpd..") do set "PYTHONHOME=%%~fp" + ) else ( + set "PYTHONHOME=%%~dpd" + ) +) +if "!PYTHONHOME:~-1!"=="\" set "PYTHONHOME=!PYTHONHOME:~0,-1!" + +"!MAIN_BIN!" %* +exit /b !ERRORLEVEL! diff --git a/python/private/interpreter_template.sh b/python/private/interpreter_template.sh new file mode 100644 index 0000000000..a69381fb58 --- /dev/null +++ b/python/private/interpreter_template.sh @@ -0,0 +1,48 @@ +#!/usr/bin/env sh +set -eu + +PYTHON_EXE_RUNFILES_PATH="%python_exe_runfiles_path%" +MAIN_BIN="" + +if [ -n "${RUNFILES_DIR:-}" ] && \ + [ -e "${RUNFILES_DIR}/${PYTHON_EXE_RUNFILES_PATH}" ]; then + MAIN_BIN="${RUNFILES_DIR}/${PYTHON_EXE_RUNFILES_PATH}" +elif [ -n "${RUNFILES_MANIFEST_FILE:-}" ] && \ + [ -f "${RUNFILES_MANIFEST_FILE}" ]; then + MAIN_BIN="$(grep -F -m1 "${PYTHON_EXE_RUNFILES_PATH} " \ + "${RUNFILES_MANIFEST_FILE}" 2>/dev/null | cut -f2- -d' ' || true)" +elif [ -e "$0.runfiles/${PYTHON_EXE_RUNFILES_PATH}" ]; then + MAIN_BIN="$0.runfiles/${PYTHON_EXE_RUNFILES_PATH}" +elif [ -f "$0.runfiles_manifest" ]; then + MAIN_BIN="$(grep -F -m1 "${PYTHON_EXE_RUNFILES_PATH} " \ + "$0.runfiles_manifest" 2>/dev/null | cut -f2- -d' ' || true)" +elif [ -f "$0.exe.runfiles_manifest" ]; then + MAIN_BIN="$(grep -F -m1 "${PYTHON_EXE_RUNFILES_PATH} " \ + "$0.exe.runfiles_manifest" 2>/dev/null | cut -f2- -d' ' || true)" +fi + +if [ -z "${MAIN_BIN:-}" ] || [ ! -e "${MAIN_BIN}" ]; then + echo "ERROR: interpreter executable not found: ${MAIN_BIN:-}" \ + "(from ${PYTHON_EXE_RUNFILES_PATH})" >&2 + exit 1 +fi + +# Determine PYTHONHOME (installation prefix containing lib/pythonX.Y). +# We must set PYTHONHOME to point to the runfiles directory because runfiles +# provides the unified merge of source files and generated files needed by +# the standard library and runtime. +# In standard POSIX layouts, MAIN_BIN is at /bin/python3, so the prefix +# is $(dirname $(dirname "$MAIN_BIN")). In flat toolchain layouts where the +# binary is a sibling to lib, the prefix is $(dirname "$MAIN_BIN"). +if [ -d "$(dirname "$MAIN_BIN")/lib" ]; then + PYTHONHOME="$(dirname "$MAIN_BIN")" +else + PYTHONHOME="$(dirname "$(dirname "$MAIN_BIN")")" +fi +if [ -d "$PYTHONHOME" ]; then + PYTHONHOME="$(cd "$PYTHONHOME" && pwd)" +fi +export PYTHONHOME + +exec "${MAIN_BIN}" "$@" + diff --git a/python/private/interpreter_tmpl.sh b/python/private/interpreter_tmpl.sh deleted file mode 100644 index c4e87fbb43..0000000000 --- a/python/private/interpreter_tmpl.sh +++ /dev/null @@ -1,23 +0,0 @@ -#!/usr/bin/env bash - -# --- begin runfiles.bash initialization v3 --- -# Copy-pasted from the Bazel Bash runfiles library v3. -set -uo pipefail; set +e; f=bazel_tools/tools/bash/runfiles/runfiles.bash -# shellcheck disable=SC1090 -source "${RUNFILES_DIR:-/dev/null}/$f" 2>/dev/null || \ - source "$(grep -sm1 "^$f " "${RUNFILES_MANIFEST_FILE:-/dev/null}" | cut -f2- -d' ')" 2>/dev/null || \ - source "$0.runfiles/$f" 2>/dev/null || \ - source "$(grep -sm1 "^$f " "$0.runfiles_manifest" | cut -f2- -d' ')" 2>/dev/null || \ - source "$(grep -sm1 "^$f " "$0.exe.runfiles_manifest" | cut -f2- -d' ')" 2>/dev/null || \ - { echo>&2 "ERROR: cannot find $f"; exit 1; }; f=; set -e -# --- end runfiles.bash initialization v3 --- - -set +e # allow us to check for errors more easily -readonly TARGET_FILE="%target_file%" -MAIN_BIN=$(rlocation "$TARGET_FILE") - -if [[ -z "$MAIN_BIN" || ! -e "$MAIN_BIN" ]]; then - echo "ERROR: interpreter executable not found: $MAIN_BIN (from $TARGET_FILE)" - exit 1 -fi -exec "${MAIN_BIN}" "$@" diff --git a/python/private/py_console_script_gen.bzl b/python/private/py_console_script_gen.bzl index de016036b2..1f310217b5 100644 --- a/python/private/py_console_script_gen.bzl +++ b/python/private/py_console_script_gen.bzl @@ -54,7 +54,7 @@ def _py_console_script_gen_impl(ctx): arguments = [args], mnemonic = "PyConsoleScriptBinaryGen", progress_message = "Generating py_console_script_binary main: %{label}", - executable = ctx.executable._tool, + executable = ctx.attr._tool[DefaultInfo].files_to_run, ) return [DefaultInfo( diff --git a/python/private/py_exec_tools_info.bzl b/python/private/py_exec_tools_info.bzl index 43213d5a13..94729244b1 100644 --- a/python/private/py_exec_tools_info.bzl +++ b/python/private/py_exec_tools_info.bzl @@ -39,10 +39,13 @@ toolchain. ::: :::{warning} -This does not work correctly with RBE. Use {obj}`exec_runtime` instead. +This may not work correctly with RBE. Use {obj}`exec_runtime` instead. -Once [bazelbuild/bazel#23620](https://github.com/bazelbuild/bazel/issues/23620) is resolved this warning -may be removed. +A launcher script is now used to set `PYTHONHOME` and runfiles, which is +expected to resolve RBE compatibility, but this still needs to be verified on +RBE via CI. Once verified or once +[bazelbuild/bazel#23620](https://github.com/bazelbuild/bazel/issues/23620) is +resolved, this warning may be removed. ::: """, "exec_runtime": """ diff --git a/python/private/py_exec_tools_toolchain.bzl b/python/private/py_exec_tools_toolchain.bzl index d126262033..2f370321d8 100644 --- a/python/private/py_exec_tools_toolchain.bzl +++ b/python/private/py_exec_tools_toolchain.bzl @@ -16,6 +16,7 @@ load("@bazel_skylib//lib:paths.bzl", "paths") load("@bazel_skylib//rules:common_settings.bzl", "BuildSettingInfo") +load(":common.bzl", "is_windows_platform", "runfiles_root_path") load(":common_labels.bzl", "labels") load(":py_exec_tools_info.bzl", "PyExecToolsInfo") load(":sentinel_impl.bzl", "SentinelInfo") @@ -109,22 +110,42 @@ def _current_interpreter_executable_impl(ctx): # because of things like pyenv: they use $0 to determine what to # re-exec. If it's not a recognized name, then they fail. if runtime.interpreter: - executable = ctx.actions.declare_file(runtime.interpreter.basename) - - # NOTE: Using ctx.actions.symlink() here doesn't always work with RBE - # because it's not guaranteed that it will materialize as a symlink, but - # we rely on it being a symlink so that Python can find its actual - # PYTHONHOME. - # See https://github.com/bazelbuild/bazel/issues/23620 - ctx.actions.symlink(output = executable, target_file = runtime.interpreter, is_executable = True) + is_windows = is_windows_platform(ctx) + basename = runtime.interpreter.basename + if is_windows: + if basename.lower().endswith(".exe"): + basename = basename[:-4] + basename = basename + ".bat" + + executable = ctx.actions.declare_file(basename) + + ctx.actions.expand_template( + template = ctx.file._template, + output = executable, + substitutions = { + "%python_exe_runfiles_path%": runfiles_root_path( + ctx, + runtime.interpreter.short_path, + ), + }, + is_executable = True, + ) + runfiles = ctx.runfiles([executable], transitive_files = runtime.files) else: - executable = ctx.actions.declare_symlink(paths.basename(runtime.interpreter_path)) - ctx.actions.symlink(output = executable, target_path = runtime.interpreter_path) + executable = ctx.actions.declare_symlink( + paths.basename(runtime.interpreter_path), + ) + ctx.actions.symlink( + output = executable, + target_path = runtime.interpreter_path, + ) + runfiles = ctx.runfiles([executable], transitive_files = runtime.files) + return [ toolchain, DefaultInfo( executable = executable, - runfiles = ctx.runfiles([executable], transitive_files = runtime.files), + runfiles = runfiles, ), ] @@ -132,4 +153,15 @@ current_interpreter_executable = rule( implementation = _current_interpreter_executable_impl, toolchains = [TARGET_TOOLCHAIN_TYPE], executable = True, + attrs = { + "_template": attr.label( + default = "//python/private:exec_interpreter_template", + allow_single_file = True, + ), + "_windows_constraints": attr.label_list( + default = [ + "@platforms//os:windows", + ], + ), + }, ) diff --git a/tests/py_exec_tools_toolchain/BUILD.bazel b/tests/py_exec_tools_toolchain/BUILD.bazel index 092e790939..ddaaeaf3de 100644 --- a/tests/py_exec_tools_toolchain/BUILD.bazel +++ b/tests/py_exec_tools_toolchain/BUILD.bazel @@ -12,8 +12,16 @@ # See the License for the specific language governing permissions and # limitations under the License. -load(":py_exec_tools_toolchain_tests.bzl", "py_exec_tools_toolchain_test_suite") +load( + ":py_exec_tools_toolchain_tests.bzl", + "interpreter_run_in_action_test", + "py_exec_tools_toolchain_test_suite", +) py_exec_tools_toolchain_test_suite( name = "py_exec_tools_toolchain_tests", ) + +interpreter_run_in_action_test( + name = "test_run_interpreter_as_action", +) diff --git a/tests/py_exec_tools_toolchain/expected_action_output.json b/tests/py_exec_tools_toolchain/expected_action_output.json new file mode 100644 index 0000000000..6199cc984f --- /dev/null +++ b/tests/py_exec_tools_toolchain/expected_action_output.json @@ -0,0 +1,11 @@ +{ + "has_encodings": true, + "status": "ok", + "stdlib_is_in_runfiles": { + "encodings": true, + "json": true, + "os": true, + "sysconfig": true + }, + "sys_executable_is_in_runfiles": true +} diff --git a/tests/py_exec_tools_toolchain/py_exec_tools_toolchain_tests.bzl b/tests/py_exec_tools_toolchain/py_exec_tools_toolchain_tests.bzl index 3be2bc3f30..f6473a5a1d 100644 --- a/tests/py_exec_tools_toolchain/py_exec_tools_toolchain_tests.bzl +++ b/tests/py_exec_tools_toolchain/py_exec_tools_toolchain_tests.bzl @@ -13,9 +13,18 @@ # limitations under the License. """Starlark tests for py_exec_tools_toolchain rule.""" +load("@bazel_skylib//rules:diff_test.bzl", "diff_test") load("@rules_testing//lib:analysis_test.bzl", "analysis_test") load("@rules_testing//lib:test_suite.bzl", "test_suite") -load("//python/private:py_exec_tools_toolchain.bzl", "py_exec_tools_toolchain") # buildifier: disable=bzl-visibility +load( + "//python/private:py_exec_tools_toolchain.bzl", + "current_interpreter_executable", + "py_exec_tools_toolchain", +) # buildifier: disable=bzl-visibility +load( + "//python/private:toolchain_types.bzl", + "EXEC_TOOLS_TOOLCHAIN_TYPE", +) # buildifier: disable=bzl-visibility _tests = [] @@ -36,5 +45,102 @@ def _test_disable_exec_interpreter_impl(env, target): _tests.append(_test_disable_exec_interpreter) +def _test_default_exec_interpreter(name): + py_exec_tools_toolchain( + name = name + "_subject", + ) + analysis_test( + name = name, + target = name + "_subject", + impl = _test_default_exec_interpreter_impl, + ) + +def _test_default_exec_interpreter_impl(env, target): + exec_tools = target[platform_common.ToolchainInfo].exec_tools + env.expect.that_bool(exec_tools.exec_interpreter != None).equals(True) + files_to_run = exec_tools.exec_interpreter[DefaultInfo].files_to_run + env.expect.that_bool(files_to_run != None).equals(True) + env.expect.that_bool(files_to_run.executable != None).equals(True) + +_tests.append(_test_default_exec_interpreter) + +def _test_current_interpreter_executable(name): + current_interpreter_executable( + name = name + "_subject", + ) + analysis_test( + name = name, + target = name + "_subject", + impl = _test_current_interpreter_executable_impl, + ) + +def _test_current_interpreter_executable_impl(env, target): + default_info = target[DefaultInfo] + env.expect.that_bool(default_info.files_to_run != None).equals(True) + env.expect.that_bool( + default_info.files_to_run.executable != None, + ).equals(True) + env.expect.that_bool(default_info.default_runfiles != None).equals(True) + +_tests.append(_test_current_interpreter_executable) + def py_exec_tools_toolchain_test_suite(name): test_suite(name = name, tests = _tests) + +def _run_interpreter_action_impl(ctx): + out = ctx.actions.declare_file(ctx.label.name + ".json") + exec_tools = ctx.toolchains[EXEC_TOOLS_TOOLCHAIN_TYPE].exec_tools + if not exec_tools.exec_interpreter: + fail("exec_tools.exec_interpreter is not configured") + executable = exec_tools.exec_interpreter[DefaultInfo].files_to_run + ctx.actions.run( + outputs = [out], + inputs = [ctx.file.src], + executable = executable, + arguments = [ctx.file.src.path, out.path], + mnemonic = "RunInterpreterAction", + progress_message = "Running interpreter action %{label}", + toolchain = EXEC_TOOLS_TOOLCHAIN_TYPE, + ) + return [DefaultInfo(files = depset([out]))] + +_run_interpreter_action = rule( + implementation = _run_interpreter_action_impl, + attrs = { + "src": attr.label( + allow_single_file = True, + mandatory = True, + ), + }, + toolchains = [EXEC_TOOLS_TOOLCHAIN_TYPE], +) + +def interpreter_run_in_action_test( + name, + src = None, + expected = None, + **kwargs): + """Runs a Python script in a build action using the exec interpreter. + + Args: + name: Name of the test target. + src: The Python source file to execute. + expected: The expected output file to compare against. + **kwargs: Passed to diff_test. + """ + src = src or str(Label("//tests/py_exec_tools_toolchain:test_action.py")) + expected = expected or str( + Label("//tests/py_exec_tools_toolchain:expected_action_output.json"), + ) + action_target = name + "_action" + _run_interpreter_action( + name = action_target, + src = src, + tags = ["manual"], + ) + diff_test( + name = name, + file1 = ":" + action_target, + file2 = expected, + **kwargs + ) diff --git a/tests/py_exec_tools_toolchain/test_action.py b/tests/py_exec_tools_toolchain/test_action.py new file mode 100644 index 0000000000..f4ad1b17cd --- /dev/null +++ b/tests/py_exec_tools_toolchain/test_action.py @@ -0,0 +1,53 @@ +import encodings +import json +import os +import sys +import sysconfig +from pathlib import Path + +# Verify that sys.executable points to a path in runfiles. +executable = Path(sys.executable) +is_in_runfiles = ".runfiles" in executable.parts or ".runfiles" in sys.executable + +if not is_in_runfiles: + sys.exit(f"Expected sys.executable to be in runfiles, got: {sys.executable}") + +if not executable.is_file(): + sys.exit(f"Expected sys.executable to be an existing file, got: {sys.executable}") + +# Verify that stdlib modules came from a runfiles location. +stdlib_modules = [encodings, json, os, sysconfig] +for mod in stdlib_modules: + mod_file = getattr(mod, "__file__", None) + if not mod_file: + sys.exit(f"Expected {mod.__name__} to have __file__") + if ".runfiles" not in mod_file: + sys.exit(f"Expected {mod.__name__} to be in runfiles, got: {mod_file}") + if not Path(mod_file).exists(): + sys.exit(f"Expected {mod.__name__} file to exist, got: {mod_file}") + +stdlib_is_in_runfiles = { + mod.__name__: ( + getattr(mod, "__file__", None) is not None + and ".runfiles" in getattr(mod, "__file__", "") + and Path(getattr(mod, "__file__", "")).exists() + ) + for mod in stdlib_modules +} + +stdlib_dir = sysconfig.get_path("stdlib") +if not stdlib_dir or ".runfiles" not in stdlib_dir: + sys.exit(f"Expected stdlib directory to be in runfiles, got: {stdlib_dir}") +if not Path(stdlib_dir).is_dir(): + sys.exit(f"Expected stdlib directory to exist on disk, got: {stdlib_dir}") + +data = { + "has_encodings": bool(encodings), + "status": "ok", + "stdlib_is_in_runfiles": stdlib_is_in_runfiles, + "sys_executable_is_in_runfiles": is_in_runfiles, +} +Path(sys.argv[1]).write_text( + json.dumps(data, indent=2, sort_keys=True) + "\n", + encoding="utf-8", +)