Skip to content

Commit

Permalink
Fix PEXEnvironment platform determination. (#568)
Browse files Browse the repository at this point in the history
Have this use pex intrinsics instead of `pkg_resources` which is known
to report bad values on Apple-shipped inerpreters.

Also fix resolver extended platform determination to support the test
for the fix.

Fixes #511
Fixes #523
  • Loading branch information
jsirois authored Oct 4, 2018
1 parent cbf6908 commit d4a3c9d
Show file tree
Hide file tree
Showing 4 changed files with 133 additions and 12 deletions.
10 changes: 7 additions & 3 deletions pex/environment.py
Original file line number Diff line number Diff line change
Expand Up @@ -120,13 +120,17 @@ def __init__(self, pex, pex_info, interpreter=None, **kw):
self._interpreter = interpreter or PythonInterpreter.get()
self._inherit_path = pex_info.inherit_path
self._supported_tags = []

platform = Platform.current()
platform_name = platform.platform
super(PEXEnvironment, self).__init__(
search_path=[] if pex_info.inherit_path == 'false' else sys.path,
# NB: Our pkg_resources.Environment base-class wants the platform name string and not the
# pex.platform.Platform object.
platform=platform_name,
**kw
)
self._supported_tags.extend(
Platform.create(self.platform).supported_tags(self._interpreter)
)
self._supported_tags.extend(platform.supported_tags(self._interpreter))
TRACER.log(
'E: tags for %r x %r -> %s' % (self.platform, self._interpreter, self._supported_tags),
V=9
Expand Down
6 changes: 4 additions & 2 deletions pex/interpreter.py
Original file line number Diff line number Diff line change
Expand Up @@ -261,8 +261,10 @@ def __str__(self):
)

def __repr__(self):
return 'PythonIdentity(%r, %s, %s, %s)' % (
self._interpreter,
return 'PythonIdentity(%r, %r, %r, %r, %r, %r)' % (
self.abbr_impl,
self.abi_tag,
self.impl_ver,
self._version[0],
self._version[1],
self._version[2]
Expand Down
50 changes: 49 additions & 1 deletion pex/resolver.py
Original file line number Diff line number Diff line change
Expand Up @@ -164,10 +164,58 @@ class Resolver(object):

class Error(Exception): pass

@staticmethod
def _maybe_expand_platform(interpreter, platform=None):
# Expands `platform` if it is 'current' and abbreviated.
#
# IE: If we're on linux and handed a platform of `None`, 'current', or 'linux_x86_64', we expand
# the platform to an extended platform matching the given interpreter's abi info, eg:
# 'linux_x86_64-cp-27-cp27mu'.

cur_plat = Platform.current()
def expand_platform():
expanded_platform = Platform(platform=cur_plat.platform,
impl=interpreter.identity.abbr_impl,
version=interpreter.identity.impl_ver,
abi=interpreter.identity.abi_tag)
TRACER.log("""
Modifying given platform of {given_platform!r}:
Using the current platform of {current_platform!r}
Under current interpreter {current_interpreter!r}
To match given interpreter {given_interpreter!r}.
Calculated platform: {calculated_platform!r}""".format(
given_platform=platform,
current_platform=cur_plat,
current_interpreter=PythonInterpreter.get(),
given_interpreter=interpreter,
calculated_platform=expanded_platform),
V=9
)
return expanded_platform

if platform in (None, 'current'):
# Always expand the default local (abbreviated) platform to the given interpreter.
return expand_platform()
else:
given_platform = Platform.create(platform)
if given_platform.is_extended:
# Always respect an explicit extended platform.
return given_platform
elif given_platform.platform != cur_plat.platform:
# IE: Say we're on OSX and platform was 'linux-x86_64'; we can't expand a non-local
# platform so we leave as-is.
return given_platform
else:
# IE: Say we're on 64 bit linux and platform was 'linux-x86_64'; ie: the abbreviated local
# platform.
return expand_platform()

def __init__(self, allow_prereleases=None, interpreter=None, platform=None,
pkg_blacklist=None, use_manylinux=None):
self._interpreter = interpreter or PythonInterpreter.get()
self._platform = Platform.create(platform) if platform else Platform.current()
self._platform = self._maybe_expand_platform(self._interpreter, platform)
self._allow_prereleases = allow_prereleases
self._blacklist = pkg_blacklist.copy() if pkg_blacklist else {}
self._supported_tags = self._platform.supported_tags(
Expand Down
79 changes: 73 additions & 6 deletions tests/test_environment.py
Original file line number Diff line number Diff line change
@@ -1,23 +1,36 @@
# Copyright 2014 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

import os
import platform
import subprocess
from contextlib import contextmanager

import pytest
from twitter.common.contextutil import temporary_dir

from pex.compatibility import nested
from pex import resolver
from pex.compatibility import nested, to_bytes
from pex.environment import PEXEnvironment
from pex.installer import EggInstaller, WheelInstaller
from pex.interpreter import PythonInterpreter
from pex.package import EggPackage, SourcePackage, WheelPackage
from pex.pex import PEX
from pex.pex_builder import PEXBuilder
from pex.pex_info import PexInfo
from pex.testing import make_bdist, temporary_filename
from pex.version import SETUPTOOLS_REQUIREMENT, WHEEL_REQUIREMENT


@contextmanager
def yield_pex_builder(zip_safe=True):
with nested(temporary_dir(), make_bdist('p1', zipped=True, zip_safe=zip_safe)) as (td, p1):
pb = PEXBuilder(path=td)
pb.add_egg(p1.location)
def yield_pex_builder(zip_safe=True, installer_impl=EggInstaller, interpreter=None):
with nested(temporary_dir(),
make_bdist('p1',
zipped=True,
zip_safe=zip_safe,
installer_impl=installer_impl,
interpreter=interpreter)) as (td, p1):
pb = PEXBuilder(path=td, interpreter=interpreter)
pb.add_dist_location(p1.location)
yield pb


Expand Down Expand Up @@ -95,3 +108,57 @@ def test_load_internal_cache_unzipped():
assert len(dists) == 1
assert normalize(dists[0].location).startswith(
normalize(os.path.join(pb.path(), pb.info.internal_cache)))


_KNOWN_BAD_APPLE_INTERPRETER = ('/System/Library/Frameworks/Python.framework/Versions/'
'2.7/Resources/Python.app/Contents/MacOS/Python')


@pytest.mark.skipif(not os.path.exists(_KNOWN_BAD_APPLE_INTERPRETER),
reason='Test requires known bad Apple interpreter {}'
.format(_KNOWN_BAD_APPLE_INTERPRETER))
def test_osx_platform_intel_issue_523():
def bad_interpreter(include_site_extras=True):
return PythonInterpreter.from_binary(_KNOWN_BAD_APPLE_INTERPRETER,
include_site_extras=include_site_extras)

interpreter = bad_interpreter(include_site_extras=False)
with temporary_dir() as cache:
# We need to run the bad interpreter with a modern, non-Apple-Extras setuptools in order to
# successfully install psutil.
for requirement in (SETUPTOOLS_REQUIREMENT, WHEEL_REQUIREMENT):
for dist in resolver.resolve([requirement],
cache=cache,
# We can't use wheels since we're bootstrapping them.
precedence=(SourcePackage, EggPackage),
interpreter=interpreter):
interpreter = interpreter.with_extra(dist.key, dist.version, dist.location)

with nested(yield_pex_builder(installer_impl=WheelInstaller, interpreter=interpreter),
temporary_filename()) as (pb, pex_file):
for dist in resolver.resolve(['psutil==5.4.3'],
cache=cache,
precedence=(SourcePackage, WheelPackage),
interpreter=interpreter):
pb.add_dist_location(dist.location)
pb.build(pex_file)

# NB: We want PEX to find the bare bad interpreter at runtime.
pex = PEX(pex_file, interpreter=bad_interpreter())
args = ['-c', 'import pkg_resources; print(pkg_resources.get_supported_platform())']
env = os.environ.copy()
env['PEX_VERBOSE'] = '1'
process = pex.run(args=args,
env=env,
blocking=False,
stdout=subprocess.PIPE,
stderr=subprocess.PIPE)
stdout, stderr = process.communicate()
assert 0 == process.returncode, (
'Process failed with exit code {} and stderr:\n{}'.format(process.returncode, stderr)
)

# Verify this all worked under the previously problematic pkg_resources-reported platform.
release, _, _ = platform.mac_ver()
major_minor = '.'.join(release.split('.')[:2])
assert to_bytes('macosx-{}-intel'.format(major_minor)) == stdout.strip()

0 comments on commit d4a3c9d

Please sign in to comment.