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

Release more flexible pex binaries. #654

Closed
jsirois opened this issue Jan 24, 2019 · 11 comments

Comments

Projects
None yet
3 participants
@jsirois
Copy link
Member

commented Jan 24, 2019

Currently we release pex27 and pex37. Since pex is dependency free and supports both python 2.7 and python 3.4-3.7, we would ideally release one binary that could run under all these interpreters.

Typical systems will have on the PATH:

  1. python - the default python
  2. python2 - the default python 2.*
  3. python3 - the default python 3.*
  4. python3.7, python2.7, ... - the ~specific pythons

The current release process creates binaries with shebangs from the fourth category which has the upshot of not providing a binary for pythons 3.4, 3.5 or 3.6 even though both the pex27 and pex37 could otherwise run under these pythons.

We should release binaries to support 3.4, 3.5 and 3.6 either by releasing one binary that uses a shebang with python and interpreter constraints to ensure that python falls in the acceptable ranges or else by releasing two binaries as we do today but with more flexible python2 and python3 shebangs (really only needed for python3 since python2.7 is actually all we support in the 2 series). The third alternative of releasing one binary per minor supported version seems wasteful.

@jsirois jsirois self-assigned this Jan 24, 2019

@jsirois jsirois added the in progress label Jan 24, 2019

@jsirois

This comment has been minimized.

Copy link
Member Author

commented Jan 24, 2019

Urgh, --interpreter-constraint is buggy in a few ways foiling ideas one and two.

For idea one, passing multiple --interpreter-constraint does not OR, it ANDS - which the , operator already does and so is not useful. If instead multiple --interpreter-constraint OR'd then we could say --interpreter-constraint='>=2.7,<3' --interpreter-constraint='>=3.4,<4', which combined with a #!/usr/bin/env python shebang would allow for a safe universal pex. See here and note all calls pass meet_all_constraints=True:

def matched_interpreters(interpreters, constraints, meet_all_constraints=False):
"""Given some filters, yield any interpreter that matches at least one of them, or all of them
if meet_all_constraints is set to True.

For idea one and two, --interpreter-constraint only takes effect if PEX_PYTHON or PEX_PYTHON_PATH is set:

pex/pex/pex_bootstrapper.py

Lines 141 to 150 in a16f0ec

target = None
with TRACER.timed('Selecting runtime interpreter based on pexrc', V=3):
if ENV.PEX_PYTHON and not ENV.PEX_PYTHON_PATH:
# preserve PEX_PYTHON re-exec for backwards compatibility
# TODO: Kill this off completely in favor of PEX_PYTHON_PATH
# https://github.com/pantsbuild/pex/issues/431
target = _select_pex_python_interpreter(ENV.PEX_PYTHON,
compatibility_constraints)
elif ENV.PEX_PYTHON_PATH:
target = _select_interpreter(ENV.PEX_PYTHON_PATH, compatibility_constraints)

@jsirois

This comment has been minimized.

Copy link
Member Author

commented Jan 24, 2019

I've gone ahead and brute-forced for the 1.6.1 release by including pex{27,3{4,5,6,7}}: https://github.com/pantsbuild/pex/releases/tag/v1.6.1

I'll leave this issue open until I break out --interpreter-constraint issues to track the prerequisites for doing all this the right way.

@jsirois

This comment has been minimized.

Copy link
Member Author

commented Jan 25, 2019

@CMLivingston if you can comment on this in the form of either - ack, sounds unintended or, to the contrary, that there was intent behind these behaviors, I'd be grateful.

@CMLivingston

This comment has been minimized.

Copy link
Contributor

commented Jan 25, 2019

This was intended. The confusion and issues we are seeing that stem from OR vs AND are largely due to my ignorance of pex use cases (I was relatively new to the codebase when I made these changes), and I was having difficulty trying to understand why anyone would OR at all within the context of the change I was making at the time, which was to move from a single PEX_PYTHON (use this interpreter) -> PEX_PYTHON_PATH (use one of these based on constraints of the binary). That was the use case I was targeting with meet_all_constraints=true - to target a specific interpreter on PPP, assuming that the use case there is to have control over the exact interpreter you need (iirc there was some issues with the , AND once you started getting complicated and went beyond simple ones like CPython>=2.7,<3, but I am not completely sure).

Long story short, it was intended for the use case and I was confused at the time / thought that ORing constraints was a bug, so I added that option in to be more explicit and give finer grain control. Ultimately, I think the custom nature of PPP and PP is different from what you are describing here because it is much more pointed, custom setting regarding which interpreter to use vs. giving the freedom and flexibility to encompass a wide range (this being largely because it grew out of the singular PEX_PYTHON). By design, PPP-based selection will not fall back to $PATH, and will error out if the AND is not satisfied. Admittedly, this was heavily influenced by monorepo philosophy / pants, where we were trying to go from one canonical interpreter to two, feature-incompatible canonical interpreters.

I think one possible solution here would be to fragment the call to matched_interpreters based on this condition

and use meet_all_constraints=False, reserving the AND for PPP/PP only. I need to ramp up on other places this bites us (so there may be a case for never ANDing), but given my current knowledge level I think it would make sense to fragment.

@jsirois

This comment has been minimized.

Copy link
Member Author

commented Jan 27, 2019

Thanks @CMLivingston - that helps me move forward without breaking Twitter things.

@jsirois

This comment has been minimized.

Copy link
Member Author

commented Jan 30, 2019

Noting that full support for multi-python pex binaries will also require fixing #415.

@stuhood

This comment has been minimized.

Copy link
Member

commented Feb 6, 2019

Hey @jsirois ! Is there any separable work that could be broken out of this that we (Twitter) could prioritize in order to help get this fixed?

@jsirois

This comment has been minimized.

Copy link
Member Author

commented Feb 6, 2019

Yes - all the linked issues here that are not labelled in-progress, so:

If anyone does pitch in, please assign the issue to yourself and label in-progress.

@stuhood

This comment has been minimized.

Copy link
Member

commented Feb 6, 2019

@jsirois : Ok, thank you! Is there a best choice issue for someone to start with? If you add a few more details to "the most separable" issue, I can commit one of us to finishing it before the end of next week.

@jsirois

This comment has been minimized.

Copy link
Member Author

commented Feb 6, 2019

#656 seems teed up already to me. I'm happy to answer questions if that somehow is not explicit enough.

@jsirois

This comment has been minimized.

Copy link
Member Author

commented Feb 6, 2019

And fwiw - @CMLivingston should be intimately familiar with #655 and #656 both.

@jsirois jsirois referenced this issue Feb 11, 2019

Closed

Release 1.6.3 #665

2 of 2 tasks complete

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

Keep default AND for PP and PPP, else OR
It looks like we need to keep in some places the behavior of ANDing a list of constraints per pantsbuild#654 (comment). But in general we want to OR.

If PEX_PYTHON or PEX_PYTHON_PATH are not set, OR.

@jsirois jsirois referenced this issue Mar 10, 2019

Closed

Release 1.6.4 #681

2 of 3 tasks complete

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

Add support for releasing Python 3 wheels (#7197)
### Problem
The final step in #6062 is to add support for releasing Pants and its contrib packages as compatible with Python 2.7 _and_  3.6 and 3.7. Specifically, universal wheels should be marked as `py27.py36.py37` and `pantsbuild.pants` should be marked as `cp27-cp27{m,mu}` or as `cp36-abi3` because it uses native code (see [comment in packages.py](https://github.com/pantsbuild/pants/pull/7197/files#diff-cccea5c0944f25138aa83f5815236ea2R95) for an explanation on `abi3`).

`packages.py` and `release.sh` had several issues preventing building Python 3 wheels. Further, we needed to add shards to build Python 3 wheels in CI.

This resolves the wheels portion of #6450.

### Solution
* Add a `-3` CLI flag to `release.sh` to indicate it should build wheels compatible with Py3. Also add `--py3` to `packages.py`.
* Update the default `Package` config to setup support for `py27.py36.py37`, meaning any Python implementation that is one of those 3 versions.
* Update the `pantsbuild.pants` flags to indicate either `cp27` or `cp36-abi3`. The former is only compatible with CPython2.7, and the latter is compatible with any CPython implementation >= 3.6 due to the ABI constraint (see https://docs.python.org/3/c-api/stable.html).
* Add Python 3 wheels shards to CI.
* Pull down either a pex27 or pex36 release, rather than alway using pex27.
* Modernize Python code, such as fixing bad `subprocess32` import.
* Remove the compatibility tags from `PantsRequirement`. John explained it was wrong to put here, I think because it should be marked in` packages.py` when we actually build the wheels? This results in also tweaking the corresponding unit tests.

### Result
`./build-support/bin/release.sh -3n` builds Python 3 compatible wheels ready for release.

Note we still cannot release a Python 3 compatible PEX, which is blocked by pantsbuild/pex#654.

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

Release pants as both a Python 2.7 PEX and a Python 3.6 PEX (#7401)
### Problem
While wheels are now released with Python 3 support thanks to #7197, this is only one of two ways people consume Pants. The other way is through a Pex released to Github. 

We decided to release both a Py27 and Py36 Pex. 

We will not be releasing a Py37 Pex, as it is would require Py37 wheel building shards which would require a Centos7 docker image. We decided this is not worth the work for how few people we anticipate would use Py37 with Pex combined.

We will also not be releasing a more flexible Pex that works with either Py27 or Py36, as it is blocked by pantsbuild/pex#654. We also want organizations pinning their Python version, so a flexible release is not particularly useful in this instance.

This implements #6450 (comment).

### Solution
Release both `pants.${VERSION}.py{27,36}.pex`.

This requires changing how we run `release.sh -p` and adding new CI shards to deploy on both Py2 and Py3.

### Result
The next release, we will release both `pants.1.15.0.rc1.py27.pex` and  `pants.1.15.0.rc1.py36.pex`.

stuhood added a commit to pantsbuild/pants that referenced this issue Mar 21, 2019

Release pants as both a Python 2.7 PEX and a Python 3.6 PEX (#7401)
### Problem
While wheels are now released with Python 3 support thanks to #7197, this is only one of two ways people consume Pants. The other way is through a Pex released to Github. 

We decided to release both a Py27 and Py36 Pex. 

We will not be releasing a Py37 Pex, as it is would require Py37 wheel building shards which would require a Centos7 docker image. We decided this is not worth the work for how few people we anticipate would use Py37 with Pex combined.

We will also not be releasing a more flexible Pex that works with either Py27 or Py36, as it is blocked by pantsbuild/pex#654. We also want organizations pinning their Python version, so a flexible release is not particularly useful in this instance.

This implements #6450 (comment).

### Solution
Release both `pants.${VERSION}.py{27,36}.pex`.

This requires changing how we run `release.sh -p` and adding new CI shards to deploy on both Py2 and Py3.

### Result
The next release, we will release both `pants.1.15.0.rc1.py27.pex` and  `pants.1.15.0.rc1.py36.pex`.

jsirois added a commit to jsirois/pex that referenced this issue Apr 3, 2019

@jsirois jsirois added the in progress label Apr 3, 2019

This was referenced Apr 3, 2019

@jsirois jsirois closed this in #698 Apr 3, 2019

jsirois added a commit that referenced this issue Apr 3, 2019

@jsirois jsirois removed the in progress label Apr 3, 2019

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.