Handle URL requirements with constraints files. #11907
Merged
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
[ci skip-rust] [ci skip-build-wheels]
Eric-Arellano
approved these changes
Apr 13, 2021
# 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]
jsirois
reviewed
Apr 13, 2021
| canonicalize_project_name(Requirement.parse(req).project_name) for req in exact_reqs | ||
| } | ||
|
|
||
| url_reqs = set() # E.g., 'foobar@ git+https://github.com/foo/bar.git@branch' |
There was a problem hiding this comment.
This PR adds a lot of randomness from all appearances. Lots of sets and set math. It seems the original order of the constraints and or requirements should be kept.
There was a problem hiding this comment.
I had the same thought, but the PexRequirements ctor sorts its argument. So we were already not preserving the original order, but we were (and still are) eliminating randomness.
[ci skip-rust] [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]
[ci skip-rust] [ci skip-build-wheels]
Merged
[ci skip-rust] [ci skip-build-wheels]
jsirois
reviewed
May 30, 2021
| constraint_file_projects = { | ||
| canonicalize_project_name(req.project_name) for req in constraints_file_reqs | ||
| } | ||
| unconstrained_projects = exact_req_projects - constraint_file_projects | ||
| # Constraints files must only contain name reqs, not URL reqs (those are already | ||
| # constrained by their very nature). See https://github.com/pypa/pip/issues/8210. |
There was a problem hiding this comment.
Noting that Pip now has support for URL reqs in constraint files via: pypa/pip#9673
Pex won't be using that Pip for a while, but just noting.
pypa/pip#8210 spawned -> pypa/pip#8253 which was fixed by -> pypa/pip#9673
Eric-Arellano
added a commit
that referenced
this pull request
Jul 14, 2021
This allows you to use the new lockfile format, generated by pip-tools via `./pants --tag=-lockfile_ignore lock ::` and #12300. A lockfile cannot be used at the same time as a constraints file. This makes the code easier to implement and means that we don't break any prior APIs. We will likely deprecate constraints when the dust settles. There are several major deficiencies: - Only `pex_from_targets.py` consumes this lockfile. This means that tool lockfiles will now have no constraints and no lockfile, for now. - Does not handle requirements disjoint to the lockfile. - Does not support multiple user lockfiles, which, for example, is necessary to properly handle `platforms` with `pex_binary` and `python_awslambda`: we need one lockfile per platform, as demonstrated in https://github.com/Eric-Arellano/lockfile-platforms-problem/tree/main/multiple_pex__stu_proposal. ### Lockfile vs. constraints file (and `[python-setup].resolve_all_constraints`) We're currently using pip's `--constraints` file support, which allows you to specify constraints that may not actually be used. At the same time, we default to `[python-setup].resolve_all_constraints`, which _does_ first install the entire constraints file, and then uses Pex's repository PEX feature to extract the relevant subset. This is generally a performance optimization, but there are some times `--resolve-all-constraints` is not desirable: 1. It is not safe to first install the superset, and you can only install the proper subset. This especially can happen when `platforms` are used. See #12222. - We proactively disable `--resolve-all-constraints` when `platforms` are used. 2. User does not like the performance tradeoff, e.g. because they have a huge repository PEX so it's slow to access. -- In contrast, this PR stops using `--constraints` and roughly always does `[python-setup].resolve_all_constraints` (we now run `pex -r requirements.txt --no-transitive` and use repository PEXes). Multiple user lockfiles will allow us to solve the above issues: > 1. It is not safe to first install the superset, and you can only install the proper subset. We'll have a distinct lockfile for each `platform`, which avoids this situation. See https://github.com/Eric-Arellano/lockfile-platforms-problem/tree/main/multiple_pex__stu_proposal for an example. > 2. User does not like the performance tradeoff They can use multiple lockfiles to work around this. -- Always using `[python-setup].resolve_all_constraints` reduces complexity: less code to support, fewer concepts for users to learn. Likewise, if we did still want to use `--constraints`, we would also need to upgrade Pex to use Pip 21+, which gained support for URL constraints. We [hacked around URL constraints before](#11907), but that isn't robust. However, Pip 21+ drops Python 2 and 3.5 support: we'd need to release Pex 3 w/o Py2 support, and upgrade Pants to have workarounds that allow Py2 to still be used. To avoid project creep, it's better to punt on Pex 3. [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.
Requirements can be expressed as name requirements (e.g., foobar==x.y.x) or URL requirements
(e.g., VCS requirements such as foobar@ git+https://github.com/foo/bar.git@branch).
But pip constraint files cannot contain URL requirements, only name requirements (e.g., foobar==x.y.x).
Our check that the constraints file covered all the requirements did not take this into account.
This change subtracts out URL requirements before checking constraint coverage, and then adds them
back when creating the "repository pex".
Note that using this requires some care in constructing the constraints file. For example,
pip freezewillnot do the right thing.
We will handle this case properly when we have a first-class feature in Pants for creating and consuming
proper lockfiles.
[ci skip-rust]
[ci skip-build-wheels]