-
-
Notifications
You must be signed in to change notification settings - Fork 292
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
The --interpreter-constraint
option should OR
#655
Comments
Discovered in #654 and pantsbuild/pants#7097 |
But why wouldn't the author user just widen the single constraint in this case? Is this for when they want an incompatible "gap" in the middle?
|
Now that we're setting interpreter constraints for the built binary, some of the tests are running into upstream Pex issues that occur when interpreter constraints are set. Specifically, we seem to be hitting pex-tool/pex#655, which is that Pex does not properly OR interpreter constraints. Because this PR is blocking pantsbuild#7235 (specifying Py2 ABI) and thus releasing Py3 wheels, we skip the impacted tests until it gets fixed upstream.
In #678, I try to resolve this by following the advice from This feels wrong to me. I would expect as a user consistent behavior regardless of those env vars being set. However, it sounds like Twitter has some use cases where they need a list of constraints to AND. What do you all think of this proposal: introduce a new setting/env var, e.g. |
We should be able to just OR them in both cases and things should be fine. It should not break Twitter usage. The only reason it makes sense to AND for PPP is because with PPP you will deterministically select the lowest compatible interpreter on PPP if we OR the constraints. This shouldn't be an issue if users expect this and set constraints on pants.ini and Python targets accordingly within Pants context. |
Per pex-tool#655 (comment), Chris is okay with always ORing. This is a much more consistent and simpler approach, so is ideal over the earlier conditional logic.
…erpreter constraints requested (#7285) ### Problem When running `./pants binary`, we do not consider the global interpreter constraints in passing them to the Pex builder. This resulted in a bug where the interpreter used to build a PEX in CI differed from the Python used to run the PEX. See pex-tool/pex#676. To resolve this issue, we need some way to specify the Pex builder must use the specific Python version 2.7.13 (UCS4). This blocks #7235. ### Solution Modify `PexBuilderWrapper` to use `compatibility_or_constrains()`. This will first try to get the compatibility constraints from the target itself, and then resort to using the PythonSetup subsystem's constraints (for the global instance, resolved from `pants.ini`, `PANTS_PYTHON_SETUP_INTERPRETER_CONSTRAINTS`, and `--interpreter-constraints`). ### Result Users of the function `add_interpreter_constraints_from()` will now add the global `--interpreter-constraints` to the Pex builder if the targets do not themselves specify a constraint. At the moment, this only impacts `./pants binary`, as `python_binary_create.py` is the only file which uses this function. Note this is still not a great solution. It would be better to kill `add_interpreter_constraint()` and `add_interpreter_constraints_from()` to instead automatically set the interpreter constraints from the targets' graph. This PR does not make that change to avoid scope creep. #### Skipped tests Due to pex-tool/pex#655, we must now skip several tests that now fail. PEX does not correctly OR interpreter constraints, so these tests complain that they cannot find an appropriate interpreter to satisfy `['CPython>=3.6,<4', 'CPython>=2.7,<3']`. Because this PR blocks #7235, which blocks #7197, we skip these tests until the upstream fix pex-tool/pex#678 is merged into PEX and Pants upgrades its PEX to the new version #7186.
## Problem Resolves #655. We would expect a list of constraints, such as `['CPython>=2.7,<3', 'CPython>=3.4']`, to OR the separate list entries, i.e. allow an interpreter that satisfies any of these distinct requirement strings. Instead, we always AND lists of constraints. ## Solution Always `OR` a list of interpreter constraints. If the user wants `AND`, they may do so within the single requirement string via `,`, e.g. `'CPython>=2.7,<3'`.
Right now, the full list of interpreter constraints is ANDed. This is boh redundant (the comma operator already can AND constraints inside a single constraint string) and restrictive. There are common important cases where disjoint interpreter constraints are desirable; eg: A "universal" pex that supports being run under python 2.7 or python 3.4-3.7. Pex itself is an example of this.
The text was updated successfully, but these errors were encountered: