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

Complete manylinux support in pex. #480

Merged
merged 2 commits into from
May 15, 2018
Merged

Conversation

kwlzn
Copy link
Contributor

@kwlzn kwlzn commented May 15, 2018

This PR includes two parts:

  1. a squash merge of Add manylinux wheel support and fix a few bugs along the way #316 that implements partial manylinux support and ABI targeting.
  2. a squash merge of a fast-follow retrofit of Add support for manylinux resolves. #399 that completes manylinux support and ABI targeting, fixes all broken tests, adds tests and corrects a few breaking issues.

N.B. #316 has already been reviewed, so feel free to review only the second commit in the "Commits" view.

The second part of this change provides the following:

  • Puts manylinux support behind a feature flag for operational purposes (with implicit support for manylinux in any linux platform targeting).
  • Carries forward the Platform abstraction w/ the notion of the new extended form and adds test coverage.
  • Removes manual Resolver._identity construction in favor of PythonInterpreter.identity usage.
  • Realigns the extended Platform form to (platform, impl, version, abi) and updates docs.
  • Fixes breakage of --platform + --interpreter CLI modes and adds test coverage.
  • Fixes breakage of platform incompatibility warnings and adds test coverage.
  • Fixes interpreter version identification breakage in PlatformIdentity and adds test coverage.
  • Fixes breakage of runtime resolution for mixed linux + manylinux inner dists and adds test coverage.
  • Pushes getsource() calls into a function call.
  • Adds (forked from upstream) test coverage for the fork of pep425tags.py.
  • Adds integration test coverage for ABI targeting and other cases.
  • Adds better debug logging for diagnosing resolve failures.
  • Cleans up some terminology and misc style.

Once this change is approved, the plan is to:

  1. land Add manylinux wheel support and fix a few bugs along the way #316 independently.
  2. rebase this PR to only include the second squash merged commit and land that immediately after.
  3. cut pex 1.4.0

Copy link

@stuhood stuhood left a comment

Choose a reason for hiding this comment

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

Thanks!

pex/finders.py Outdated
@@ -101,7 +101,8 @@ class WheelMetadata(pkg_resources.EggMetadata):

@classmethod
def _split_wheelname(cls, wheelname):
Copy link

Choose a reason for hiding this comment

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

It sortof looks like this method is assuming it will get 4 entries (because it later takes the last 3, which might not make sense if there weren't 4). Is it worth asserting that len(split_wheelname) == 4?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure, fixed.

pex/glibc.py Outdated
# random junk that might come after the minor version -- this might happen
# in patched/forked versions of glibc (e.g. Linaro's version of glibc
# uses version strings like "2.20-2014.11"). See gh-3588.
m = re.match(r"(?P<major>[0-9]+)\.(?P<minor>[0-9]+)", version_str)
Copy link

Choose a reason for hiding this comment

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

Does this run frequently enough to be worth pre-compiling in a static?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

don't believe it's necessary - this runs roughly once per platform x interpreter intersection, so e.g. 4x for an osx+linux x 2.x+3.x pex build.

pex/package.py Outdated
elif self.py_version == '3.2':
self._supported_tags.add(('pp321', abi_tag, tag_platform))
elif self.py_version == '3.3':
self._supported_tags.add(('pp352', abi_tag, tag_platform))
Copy link

Choose a reason for hiding this comment

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

3.3 vs 352... intentional? Maybe comment worthy?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

these are PyPy ABI tags (pp -> PyPy), so they don't map exactly to CPython/cp. PyPy 3.3 was implicitly pp352 in PyPy versioning - see: http://doc.pypy.org/en/latest/release-pypy3.3-v5.2-alpha1.html

also commented.

Copy link
Contributor

@CMLivingston CMLivingston left a comment

Choose a reason for hiding this comment

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

Looks good to me. One comment.

pex/bin/pex.py Outdated
'cp36m, cp27mu, abi3, none). For example: manylinux1_x86_64-36-cp-cp36m. '
'Default: current platform.')
'to create a multi-platform pex. To use wheels for specific interpreter/platform tags'
'platform tags, you can append them to the platform with hyphens like: '
Copy link
Contributor

Choose a reason for hiding this comment

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

dupe?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@kwlzn
Copy link
Contributor Author

kwlzn commented May 15, 2018

merged #316 and rebasing this one.

@kwlzn kwlzn merged commit 547b382 into pex-tool:master May 15, 2018
@kwlzn kwlzn mentioned this pull request May 15, 2018
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