-
Notifications
You must be signed in to change notification settings - Fork 68
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Cross-platform native launchers for Python #275
base: main
Are you sure you want to change the base?
Changes from 14 commits
7aea8d8
a5fe5f7
7e3c166
ef27d65
b870229
a6fc56e
a54cb0d
b3d7357
c50aca9
91c779a
0d7c635
6c584e2
d3cdbe8
2368a55
72b21b5
f3ddda3
2c2156f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,37 +12,37 @@ authors: | |
|
||
# Abstract | ||
|
||
This document describes an approach for launching `py_binary` artifacts hermetically using the resolved python toolchain. | ||
This document describes an approach for launching `py_binary` artifacts hermetically using the resolved Python toolchain. | ||
|
||
|
||
# Background | ||
|
||
Currently, `py_binary` is non-hermetic and launches inconsistently between platforms. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think something worth mentioning is the "Python Launcher for Windows". Basically, a python.org Windows installs have a See |
||
|
||
On macos and Linux, there is a [python_stub](https://github.com/bazelbuild/bazel/blob/master/src/main/java/com/google/devtools/build/lib/bazel/rules/python/python_stub_template.txt) | ||
that is non-hermetic and requires a "bootstrap" python interpreter on the host. The "shebang" can be overridden, but | ||
that is non-hermetic and requires a bootstrap Python interpreter on the host. The "shebang" can be overridden, but | ||
a "shebang" is always dependent on the runtime host. | ||
|
||
On Windows, there is a [native launcher](https://github.com/meteorcloudy/bazel/blob/master/src/tools/launcher/python_launcher.cc) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think you meant to link to bazelbuild here, not meteorcloudy? https://github.com/bazelbuild/bazel/blob/master/src/tools/launcher/python_launcher.cc |
||
that launches `python.exe` on the host which then launches the `py_binary` with the same `python_stub` as macos and linux. | ||
that launches `python.exe` on the host which then launches the `py_binary` with the same `python_stub` as macos and Linux. | ||
|
||
Related issues: | ||
* [py_binary with hermetic toolchain requires a system interpreter](https://github.com/bazelbuild/rules_python/issues/691) | ||
* [Neither python_top nor python toolchain works with starlark actions on windows](https://github.com/bazelbuild/bazel/issues/7947#issuecomment-495265016) | ||
|
||
This situation is undesirable because it assumes that the target platform has a bootstrapping python interpreter | ||
available and makes the hermetic python interpreters available with `rules_python` less useful. It is also surprising to | ||
users who expect bazel to output self-contained binary artifacts for a target platform. | ||
available and makes the hermetic Python interpreters available with `rules_python` less useful. It is also surprising to | ||
users who expect Bazel to output self-contained binary artifacts for a target platform. | ||
|
||
The reason this situation exists is because of "bootstrapping". Ultimately, *something* needs to find the python | ||
interpreter in the runfiles and use that to launch the program. Currently, bazel assumes the target platform will | ||
be able to provide the "bootstrapping" functionality. | ||
The reason this situation exists is because of bootstrapping. Ultimately, *something* needs to find the Python | ||
interpreter in the runfiles and use that to launch the program. Currently, Bazel assumes the target platform will | ||
be able to provide the bootstrapping functionality. | ||
|
||
|
||
# Proposal | ||
|
||
Extend the native launcher functionality to all platforms and use it to locate the relevant python interpreter and | ||
python program in the `runfiles` tree to launch the `py_binary`. No assumptions should be made about the target platform. | ||
Extend the native launcher functionality to all platforms and use it to locate the relevant Python interpreter and | ||
Python program in the `runfiles` tree to launch the `py_binary`. No assumptions should be made about the target platform. | ||
|
||
In pseudo-code, the proposal is as follows: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is pretty high-level psuedo-code :). Something a little more concrete would be better. e.g., it has to find the runfiles directory to resolve the relative path names. |
||
|
||
|
@@ -67,9 +67,9 @@ Some related work has been done that fixes Linux to Windows cross-builds of the | |
This proposal would aim to go further and have these launchers available on all platforms, including cross_builds where appropriate toolchains are in place. | ||
|
||
Once this proposal is implemented, it would enable cross-builds of hermetic `py_binary` for all major platforms. It | ||
would also remove the complexity introduced by having so many chains of nested execution to launch a python program. | ||
would also remove the complexity introduced by having so many chains of nested execution to launch a Python program. | ||
|
||
Finally, while this proposal is specific to python, this solution could perhaps be reused for `java_binary`, `sh_binary` | ||
Finally, while this proposal is specific to Python, this solution could perhaps be reused for `java_binary`, `sh_binary` | ||
and perhaps be made available for any custom rules that require an interpreter to launch. | ||
|
||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall, I'm +1 on the core idea.