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

Support arbitrary interpreter constraint expressions. #918

Conversation

benjyw
Copy link
Collaborator

@benjyw benjyw commented Mar 8, 2020

Previously we only supported atomic constraint strings, e.g., CPython>=2.7,<3,
or a single conjunction of such strings, by specifying the option multiple times.

Now we support arbitrary boolean expressions, using the grammar &|~(). E.g.,

(CPython>=2.7,<3 | CPython>=3.5) & CPython>=3.6 & ~CPython==3.7.0

This allows Pants to merge constraints from different targets (by ANDing them).

For backwards compatibility we continue to allow specifying the option multiple times, which we now implement by joining the option values with the | operator. A future change can deprecate this ability, as it's now redundant.

The implementation uses the boolean.py dist to process the boolean algebra, and therefore vendors it appropriately.

@benjyw benjyw requested review from Eric-Arellano and jsirois March 8, 2020 02:56
@benjyw
Copy link
Collaborator Author

benjyw commented Mar 8, 2020

The commits are independently reviewable. One is just vendoring boolean.py, the other is the actual functionality changes.

Copy link
Member

@jsirois jsirois left a comment

Choose a reason for hiding this comment

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

This allows Pants to merge constraints from different targets (by ANDing them).

Why can't Pants use the comma operator for this?

@Eric-Arellano
Copy link
Contributor

Why can't Pants use the comma operator for this?

PEX (currently) does not allow using a comma operator on full requirement strings.

▶ pex --interpreter-constraint='CPython==3.7.*,CPython==3.8.*'
Compatibility requirements are not formatted properly: Unknown requirement string: CPython==3.7.*,CPython==3.8.*

In this example, target A requires Python 3.7 but its dependency B requires Python 3.8. As humans, we could easily see that these interpreter constraints will have no resolution and avoid using them, but Pants has no programmatic way to determine this.

Another example:

▶ pex --interpreter-constraint='CPython>=3.5,<3.8,CPython==3.7'
Compatibility requirements are not formatted properly: Unknown requirement string: CPython>=3.5,<3.8,CPython==3.7

Target A works with Python 3.5, 3.6, 3.7, and its dependency requires Python 3.7. Again, as humans, we could figure out we can simplify this to CPython==3.7 and pass that single constraint to Pex, but there is no programmatic way to do this.

--

(The behavior we found for Pants interpreter constraints it that we OR within --python-setup-interpreter-constraints and the individual target's compatibility list, but then we AND across the distinct targets.

Currently, we are ORing every single constraint into a flattened list and it's the wrong behavior.)

--

Benjy and I were thinking the best solution is for Pants to make no attempt to interpret the constraints it has and to simply pass them to Pex, which will do all resolution.

So, Pex needs a way to AND full requirement strings. Even though it can end strings like CPython>=2.7,<3, that functionality is too limited for Pants having to AND entire requirement strings, e.g. PyPy & (CPython==3.7.* | CPython==2.7.*).

@jsirois
Copy link
Member

jsirois commented Mar 9, 2020

A parsed Requirement gives you the project name (CPython) and the SpecifierSet (>=2.7,!=3.0.*,!=3.1.*,!=3.2.*,!=3.3.*,!=3.4.*,<4). Pants can definitely parse all these into project name and SpecifierSet and, for a given project name, combine all SpecifierSets with AND (,). For a given set of targets this will leave, for example:

  1. CPython<huge ANDed SpecifierSet>
  2. <huge ANDed SpecifierSet> (The no project name case; ie <any> python interpreter type).
  3. PyPy<huge ANDed SpecifierSet>

Pants should fail fast if all 3 are combined, you can't have an interpreter that satisfies <any> and more than one other specific interpreter type. This leaves combinations of 1 & 2 or 2 & 3 to present to Pex. For those cases the <any> SpecifierSet should just be ANDed onto the other specific interpreter type project name SpecifierSet.

In short, Pants definitely has all the info and types it already needs (in pkg_resources from setuptools) to calculate the interpreter constraints to pass to Pex. Further, Pex doesn't need to support this complexity. I don't think it ever actually had a need to support the OR case it currently does (misfeature added for Pants in the past, see #427).

@Eric-Arellano
Copy link
Contributor

Great proposal.

We had originally wanted to avoid Pants parsing requirement strings because the specifiers can have multiple entries and I was thinking it would be too difficult to work with in a programmatic way. It flew right past me that it's as simple as calling specifiers.split(",") followed by ",".join(final_specifiers).

Thank you for the recommendation @jsirois!

@benjyw benjyw closed this Apr 15, 2020
@benjyw benjyw deleted the interpreter_constraint_boolean_expressions branch April 15, 2020 22:12
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.

3 participants