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

When a pex is built and a platform-specific wheel is required, the interpreter constraints are not narrowed to the platform in question. #676

Open
jsirois opened this issue Feb 23, 2019 · 3 comments

Comments

Projects
None yet
2 participants
@jsirois
Copy link
Member

commented Feb 23, 2019

If you build a pex as such:

./pex27 thing -o thing.pex

You may end up creating a pex that contains platform-specific wheels if thing itself or its transitive dependencies are platform-specific. In a case like this, where a ~random minimum python2.7 interpreter on the PATH is picked, that interpreter will either be ucs2 (cp27m) or ucs4 (cp27mu) and likewise the platform specific wheel(s) embedded in the pex. As such, the pex can then only execute properly at runtime with a python 2.7 interpreter that matches the built platform-specific wheels. PEX should probably detect this situation at build time and inject interpreter constraints that narrow the field to python 2.7 interpreters of correct ucs level.

At the time of writing I'm not sure ucs level can even be currently picked out via interpreter constraints, so some work may need to be done there, or else the runtime picking of interpreter will simply need to take this into account and fail with a meaningful error message if a matching ucs level interpreter cannot be found.

@jsirois jsirois added the bug label Feb 23, 2019

@Eric-Arellano

This comment has been minimized.

Copy link
Contributor

commented Feb 23, 2019

For the record, this was discovered while working on pantsbuild/pants#7235 with the CI run https://travis-ci.org/pantsbuild/pants/jobs/497208431#L891 (also see line 1931 in this run).

Eric-Arellano added a commit to pantsbuild/pants that referenced this issue Mar 7, 2019

Propagate global interpreter constraints when building PEXes with int…
…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 pantsbuild/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 pantsbuild/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 pantsbuild/pex#678 is merged into PEX and Pants upgrades its PEX to the new version #7186.
@Eric-Arellano

This comment has been minimized.

Copy link
Contributor

commented Mar 19, 2019

I'm not sure ucs level can even be currently picked out via interpreter constraints, so some work may need to be done there.

It's possible!

>>> import sys
>>> print(sys.maxunicode)
1114111 # UCS4

# or....

65535 # UCS2

See https://stackoverflow.com/a/1446456.

@jsirois

This comment has been minimized.

Copy link
Member Author

commented Mar 19, 2019

By interpreter constraints I mean --interpreter-constraints; eg: CPython>=3.6. But ~ack, we'll need to use something like the code snippet of yours at runtime in combination with some new metadata instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.