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

[internal] Simplify PEX setup for ./pants run #12806

Merged
merged 1 commit into from
Sep 10, 2021

Conversation

Eric-Arellano
Copy link
Contributor

@Eric-Arellano Eric-Arellano commented Sep 9, 2021

To implement #11619, we need to set --console-script on the requirements PEX, not the runner PEX. It does not work to set on the runner PEX because it will not have any distributions available.

In trying to fix that, I realized it is useful to simplify no longer having a requirements PEX vs. runner PEX. Originally, we split this so that the requirements PEX shared the cache between goals like test and lint. That is less relevant now that we first resolve from the constraints/lockfile, then extract the subset of deps - most of the heavy lifting will already be shared.

*#12806 was necessary to land before for this to work. Otherwise, the lockfile.pex would have much less reuse because of setting additional_args on the PexFromTargetsRequest.

[ci skip-rust]
[ci skip-build-wheels]

@Eric-Arellano

This comment has been minimized.

@stuhood

This comment has been minimized.

@Eric-Arellano Eric-Arellano changed the title [wip] [internal] Simplify PEX setup for ./pants run [internal] Simplify PEX setup for ./pants run Sep 9, 2021
[ci skip-rust]

[ci skip-build-wheels]
@Eric-Arellano Eric-Arellano marked this pull request as ready for review September 9, 2021 23:39
@stuhood
Copy link
Member

stuhood commented Sep 10, 2021

In trying to fix that, I realized it is useful to simplify no longer having a requirements PEX vs. runner PEX. Originally, we split this so that the requirements PEX shared the cache between goals like test and lint. That is less relevant now that we first resolve from the constraints/lockfile, then extract the subset of deps - most of the heavy lifting will already be shared.

This isn't the default yet, but it probably can be once we have multiple user resolves?

@Eric-Arellano
Copy link
Contributor Author

This isn't the default yet, but it probably can be once we have multiple user resolves?

It's the default if you have set up a constraints file, which we strongly recommend: https://www.pantsbuild.org/v2.8/docs/python-third-party-dependencies#using-a-lockfile-strongly-recommended

@Eric-Arellano Eric-Arellano merged commit 7b78032 into pantsbuild:main Sep 10, 2021
@Eric-Arellano Eric-Arellano deleted the refactor-run branch September 10, 2021 18:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants