Skip to content
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

Always provide Python-for-Pants-scripts #18433

Merged
merged 27 commits into from
Mar 13, 2023
Merged

Conversation

thejcannon
Copy link
Member

This PR decouples the Python Pants uses for its own nefarious purposes (like running PEX or gunzip) from the user search paths by either using sys.executable locally or downloading and using Python Build Standalone in a Docker environment.

Additionally when making this change, the Python/pex code was refactored so that we always use this Python to run pex, with Python either being chosen by pex, or by using PEX_PYTHON env var at runtime. I think this is a nice cleanup of the handshake between CompletePexEnvironment.create_argv and CompletePexEnvironment.environment_dict used to have.

On a side note, this (with scie-pants and #18352) this marks a complete divorce from any Python on the user system (or lack thereof) and running Python code.

),
)

result = await Get(
Copy link
Member Author

Choose a reason for hiding this comment

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

NB: I can't use ExternalTool due to a circular reference: ExternalTool -> ExtractArchive -> Gunzip -> (This).

@thejcannon
Copy link
Member Author

If someone wants to educate me on the easiest way to test the rule-in-docker code, please do. Otherwise I'll start searching other tests.

Copy link
Member

@kaos kaos left a comment

Choose a reason for hiding this comment

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

cool stuff ⭐

@thejcannon thejcannon changed the title Always provide Python-for-Pants Always provide Python-for-Pants-scripts Mar 7, 2023
Copy link
Member

@stuhood stuhood left a comment

Choose a reason for hiding this comment

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

Thanks!

@thejcannon thejcannon merged commit 61d57d8 into pantsbuild:main Mar 13, 2023
@thejcannon thejcannon deleted the pbs-binary branch March 14, 2023 00:39
thejcannon added a commit to thejcannon/pants that referenced this pull request Mar 14, 2023
This PR decouples the Python Pants uses for its own nefarious purposes
(like running PEX or `gunzip`) from the user search paths by either
using `sys.executable` locally or downloading and using Python Build
Standalone in a Docker environment.

Additionally when making this change, the Python/pex code was refactored
so that we always use this Python to run pex, with Python either being
chosen by pex, or by using `PEX_PYTHON` env var at runtime. I think this
is a nice cleanup of the handshake between
`CompletePexEnvironment.create_argv` and
`CompletePexEnvironment.environment_dict` used to have.

On a side note, this (with `scie-pants` and
pantsbuild#18352) this marks a complete
divorce from any Python on the user system (or lack thereof) and running
Python code.
thejcannon added a commit that referenced this pull request Mar 14, 2023
This PR decouples the Python Pants uses for its own nefarious purposes
(like running PEX or `gunzip`) from the user search paths by either
using `sys.executable` locally or downloading and using Python Build
Standalone in a Docker environment.

Additionally when making this change, the Python/pex code was refactored
so that we always use this Python to run pex, with Python either being
chosen by pex, or by using `PEX_PYTHON` env var at runtime. I think this
is a nice cleanup of the handshake between
`CompletePexEnvironment.create_argv` and
`CompletePexEnvironment.environment_dict` used to have.

On a side note, this (with `scie-pants` and
#18352) this marks a complete
divorce from any Python on the user system (or lack thereof) and running
Python code.
stuhood added a commit that referenced this pull request Jun 1, 2023
This should've been done in
#18433 but wasn't.

Same for the removal of the Gunzip cruft. It was supposed to be moved in
that PR, but was only added.

---------

Co-authored-by: Stu Hood <stuhood@gmail.com>
github-actions bot pushed a commit that referenced this pull request Jun 1, 2023
This should've been done in
#18433 but wasn't.

Same for the removal of the Gunzip cruft. It was supposed to be moved in
that PR, but was only added.

---------

Co-authored-by: Stu Hood <stuhood@gmail.com>
thejcannon added a commit that referenced this pull request Jun 7, 2023
This should've been done in
#18433 but wasn't.

Same for the removal of the Gunzip cruft. It was supposed to be moved in
that PR, but was only added.

---------

Co-authored-by: Joshua Cannon <joshdcannon@gmail.com>
Co-authored-by: Stu Hood <stuhood@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants