--constraints in pex #335
--constraints in pex #335
Conversation
| @@ -477,6 +487,13 @@ def build_pex(args, options, resolver_option_builder): | |||
| for requirements_txt in options.requirement_files: | |||
| resolvables.extend(requirements_from_file(requirements_txt, resolver_option_builder)) | |||
|
|
|||
| for constraints_txt in options.constraint_files: | |||
| constraints = [] | |||
| for r in requirements_from_file(constraints_txt, resolver_option_builder): | |||
There was a problem hiding this comment.
pip states the constraints format is identical to requirements:
https://pip.pypa.io/en/stable/user_guide/#constraints-files
so I figured the way to guarantee that in pex was to share the requirements code.
There was a problem hiding this comment.
seems like a good tidbit to put into a comment
| default=[], | ||
| type=str, | ||
| action='append', | ||
| help='Add requirements from the given requirements file. This option can be used multiple ' |
There was a problem hiding this comment.
should this read Add constraints from the given constraints file instead?
|
@kwlzn @sixninetynine thanks for reviewing! added unit tests. |
| builder = ResolverOptionsBuilder() | ||
| rs = _ResolvableSet() | ||
| constraint = ResolvableRequirement.from_string('foo', builder) | ||
| constraint.is_constraint = True |
There was a problem hiding this comment.
maybe I'm missing something, but I'd expect to see a test here that:
- adds a constraint for
foo==$SOME_VERSION. - adds a non-constraint requirement for
foo. - given multiple visible
fooversions (some higher, some lower than $SOME_VERSION), selects the appropriate one based on the constraint.
and then maybe another variant of that which doesn't apply the constraint and would end up with the very latest version of foo available under the same conditions (sans the constraint)?
seem reasonable?
There was a problem hiding this comment.
yes, that makes sense. the latter sounds like a unit test that should already exist (since that's just testing standard requirement behavior), but I can add both.
toumorokoshi
force-pushed
the
constraints
branch
from
7225af8 to
29337d2
Compare
|
@kwlzn how does this look? |
|
@toumorokoshi thanks for the PR! and sorry for the delay on reviewing this - I've been out sick for a couple of days. |
this is an implementation of adding constraint support to pex:
https://pip.pypa.io/en/stable/user_guide/#constraints-files
As the "-c" keyword is already used, this implementation opts for "--constraint" by itself.
It works pretty well:
with the constraints file
resulted in a pex that had requests versioned 2.12.1, but not aiohttp.
I really wanted to start the conversation with this PR: I'm new to the Pex code and didn't know the best place to implement this, so I wanted to get feedback on an approach before doing a bunch of work to add tests.
One issue along the way: conflicts arose when attempting to reference the same package twice:
This was caused by a lack of semantic equality in the WheelPackage, so I added that too. Happy to pick that up in another PR/Issue if that's more appropriate.