Support multiple disjoint Python resolves via [python].experimental_resolves
#14299
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…`python_google_cloud_function` Also don't unnecessarily use `pex_form_targets` with flake8 [ci skip-rust] [ci skip-build-wheels]
Still just uses the default resolve # Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]
# Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]
We know we don't want compatible_resolves on a `python_source` target anymore # Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]
# Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]
|
A follow up will fix MyPy and Pylint to partition by resolve. In the meantime, users of multiple resolves will have to manually partition via CLI args. #14295 sketches a fix for |
Eric-Arellano
commented
Jan 28, 2022
| "If you need multiple resolves:\n\n" | ||
| " 1. Define via this option multiple resolve " | ||
| "names and their lockfile path. The names should be meaningful to your " | ||
| "repository, such as `data-science` or `pants-plugins`.\n" |
There was a problem hiding this comment.
Thanks @cognifloyd for the suggestion with the pants-plugins name. I think that could be a common use case for this feature.
# Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]
|
Going to automerge this because I don't know when Yi plans to do the release and I would love to ship this. I'll happily respond to any feedback in follow ups :) |
stuhood
reviewed
Jan 28, 2022
Yea, fine. Can land in followups. |
# Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]
# Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]
What about pex_binary? |
That and |
Eric-Arellano
added a commit
that referenced
this pull request
Feb 1, 2022
…14326) We believe that this feature is now stable enough to not hedge with the `--experimental` name. The key insight is that we won't have `compatible_resolves` on `python_source` targets, per #14299. We still don't have this feature on by default via `--enable-resolves`, and we likely won't until using PEX for lockfile generation. But this lowers the barrier to entry for people who want to try out this new mechanism. [ci skip-rust]
Eric-Arellano
added a commit
that referenced
this pull request
Feb 10, 2022
…ves: list[str]` (#14420) Generally, we've realized that targets like `python_source` should use `resolve: str`: #14299. This solves several problems, particularly how to choose the resolve when operating directly on `python_source` targets e.g. with MyPy. We wanted to keep `compatible_resovles` for `python_requirement` out of convenience, that it's nice for callers to be able to depend on `//:reqs#numpy` rather than `//:reqs#numpy@resolve=a`. The argument was that `python_requirement` is the "leaf" - it's not directly operated on. That was a bad assumption. `python_requirement` is directly operated on with `export` and `repl`, e.g. `./pants repl 3rdparty/python::`. We were defaulting in that case to `[python].default_resolve` -- but that doesn't make sense! If you're running `./pants repl pants-plugins/3rdparty::`, why would Pants try to use `python-default`? We don't want to get into the business of trying to disambiguate which resolve you want, either, as that's very magical and confusing. Beyond `repl` and `export`, @stuhood has wisely pointed out that this change is important for clarity. Even though `//:reqs#numpy` would have the same raw requirement string, the actual pinned version might change depending on the resolve! For example, `numpy>=2.0,<3` might result in `==2.3.1` in resolve A, but `==2.7.4` in resolve B. _They are not the same thing_. [ci skip-rust] [ci skip-build-wheels]
Eric-Arellano
added a commit
to Eric-Arellano/pants
that referenced
this pull request
Feb 10, 2022
…ves: list[str]` (pantsbuild#14420) Generally, we've realized that targets like `python_source` should use `resolve: str`: pantsbuild#14299. This solves several problems, particularly how to choose the resolve when operating directly on `python_source` targets e.g. with MyPy. We wanted to keep `compatible_resovles` for `python_requirement` out of convenience, that it's nice for callers to be able to depend on `//:reqs#numpy` rather than `//:reqs#numpy@resolve=a`. The argument was that `python_requirement` is the "leaf" - it's not directly operated on. That was a bad assumption. `python_requirement` is directly operated on with `export` and `repl`, e.g. `./pants repl 3rdparty/python::`. We were defaulting in that case to `[python].default_resolve` -- but that doesn't make sense! If you're running `./pants repl pants-plugins/3rdparty::`, why would Pants try to use `python-default`? We don't want to get into the business of trying to disambiguate which resolve you want, either, as that's very magical and confusing. Beyond `repl` and `export`, @stuhood has wisely pointed out that this change is important for clarity. Even though `//:reqs#numpy` would have the same raw requirement string, the actual pinned version might change depending on the resolve! For example, `numpy>=2.0,<3` might result in `==2.3.1` in resolve A, but `==2.7.4` in resolve B. _They are not the same thing_. [ci skip-rust] [ci skip-build-wheels]
Eric-Arellano
added a commit
that referenced
this pull request
Feb 10, 2022
…ves: list[str]` (Cherry-pick of #14420) (#14435) Generally, we've realized that targets like `python_source` should use `resolve: str`: #14299. This solves several problems, particularly how to choose the resolve when operating directly on `python_source` targets e.g. with MyPy. We wanted to keep `compatible_resovles` for `python_requirement` out of convenience, that it's nice for callers to be able to depend on `//:reqs#numpy` rather than `//:reqs#numpy@resolve=a`. The argument was that `python_requirement` is the "leaf" - it's not directly operated on. That was a bad assumption. `python_requirement` is directly operated on with `export` and `repl`, e.g. `./pants repl 3rdparty/python::`. We were defaulting in that case to `[python].default_resolve` -- but that doesn't make sense! If you're running `./pants repl pants-plugins/3rdparty::`, why would Pants try to use `python-default`? We don't want to get into the business of trying to disambiguate which resolve you want, either, as that's very magical and confusing. Beyond `repl` and `export`, @stuhood has wisely pointed out that this change is important for clarity. Even though `//:reqs#numpy` would have the same raw requirement string, the actual pinned version might change depending on the resolve! For example, `numpy>=2.0,<3` might result in `==2.3.1` in resolve A, but `==2.7.4` in resolve B. _They are not the same thing_. [ci skip-rust] [ci skip-build-wheels]
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Read the
helpmessage inpython/setup.pyfor an explanation of how this works + what we mean by "disjoint".This wires up
experimental_resolvetopython_source,python_awslambda, andpython_google_cloud_function. Thanks to the implementation inpex_from_targets.py, that means that they automatically support this new feature. The only target not yet supported ispython_distribution.Key note:
python_sourcehas the fieldresolve, rather than what we originally envisioned withcompatible_resolves. This is because we realized in #13882 and its design doc at https://docs.google.com/document/d/1mzZWnXiE6OkgMH_Dm7WeA3ck9veusQmr_c6GWPb_tsQ/edit# that the way to support apython_sourcetarget working with multiple resolves is via parametrization, which will create a distinct target for each resolve.However,
python_requirementstill usescompatible_resolves(for now at least) because we don't directly run tools like MyPy and Pytest on those. More concretely, they are never the "root" used to determine which resolve to use for a build.