Skip to content

Commit

Permalink
Upgrade to pex 1.4.3.
Browse files Browse the repository at this point in the history
This hacks around a few issues with the 1.4.x pex API. We concoct a
minimal local `Platform` to pass to `resolve` where a local interpreter
is passed to work around pex-tool/pex#511.
We also now consolidate `PythonInterpreter` construction in production
code to helper that ensures the interpreters we create are bare
(isolated) except for the specific extras we require to work around
pex-tool/pex#510.

Upgrading pex to take advantage of the worked around issues noted above
is tracked by pantsbuild#5922.

Fixes pantsbuild#5906
  • Loading branch information
jsirois committed Jun 6, 2018
1 parent 4f5a756 commit a9c8f17
Show file tree
Hide file tree
Showing 8 changed files with 118 additions and 53 deletions.
2 changes: 1 addition & 1 deletion 3rdparty/python/requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ mock==2.0.0
packaging==16.8
parameterized==0.6.1
pathspec==0.5.0
pex==1.3.2
pex==1.4.3
psutil==4.3.0
pycodestyle==2.4.0
pyflakes==2.0.0
Expand Down
10 changes: 5 additions & 5 deletions src/python/pants/backend/python/interpreter_cache.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,12 @@
import os
import shutil

from pex.interpreter import PythonIdentity, PythonInterpreter
from pex.interpreter import PythonInterpreter
from pex.package import EggPackage, Package, SourcePackage
from pex.resolver import resolve
from pex.variables import Variables

from pants.backend.python.pex_util import create_bare_interpreter, get_local_platform
from pants.backend.python.targets.python_target import PythonTarget
from pants.base.exceptions import TaskError
from pants.process.lock import OwnerPrintingInterProcessFileLock
Expand Down Expand Up @@ -94,19 +95,17 @@ def select_interpreter_for_targets(self, targets):
tgts_with_compatibilities_strs = [t.address.spec for t in tgts_with_compatibilities]
raise self.UnsatisfiableInterpreterConstraintsError(
'Unable to detect a suitable interpreter for compatibilities: {} '
'(Conflicting targets: {})'.format(' && '.join(unique_compatibilities_strs),
'(Conflicting targets: {})'.format(' && '.join(sorted(unique_compatibilities_strs)),
', '.join(tgts_with_compatibilities_strs)))
# Return the lowest compatible interpreter.
return min(allowed_interpreters)

def _interpreter_from_path(self, path, filters):
interpreter_dir = os.path.basename(path)
identity = PythonIdentity.from_path(interpreter_dir)
try:
executable = os.readlink(os.path.join(path, 'python'))
except OSError:
return None
interpreter = PythonInterpreter(executable, identity)
interpreter = create_bare_interpreter(executable)
if self._matches(interpreter, filters):
return self._resolve(interpreter)
return None
Expand Down Expand Up @@ -218,6 +217,7 @@ def _resolve_and_link(self, interpreter, requirement, target_link):
distributions = resolve(requirements=[requirement],
fetchers=self._python_repos.get_fetchers(),
interpreter=interpreter,
platform=get_local_platform(),
context=self._python_repos.get_network_context(),
precedence=precedence)
if not distributions:
Expand Down
35 changes: 35 additions & 0 deletions src/python/pants/backend/python/pex_util.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
# coding=utf-8
# Copyright 2016 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

from __future__ import (absolute_import, division, generators, nested_scopes, print_function,
unicode_literals, with_statement)

from pex.interpreter import PythonInterpreter
from pex.platforms import Platform


def create_bare_interpreter(binary_path):
"""Creates an interpreter for python binary at the given path.
The interpreter is bare in that it has no extras associated with it.
:returns: A bare python interpreter with no extras.
:rtype: :class:`pex.interpreter.PythonInterpreter`
"""
# TODO(John Sirois): Replace with a more direct PythonInterpreter construction API call when
# https://github.com/pantsbuild/pex/issues/510 is fixed.
interpreter_with_extras = PythonInterpreter.from_binary(binary_path)
return PythonInterpreter(binary_path, interpreter_with_extras.identity, extras=None)


def get_local_platform():
"""Returns the name of the local platform; eg: 'linux_x86_64' or 'macosx_10_8_x86_64'.
:returns: The local platform name.
:rtype: str
"""
# TODO(John Sirois): Kill some or all usages when https://github.com/pantsbuild/pex/issues/511
# is fixed.
current_platform = Platform.current()
return current_platform.platform
5 changes: 3 additions & 2 deletions src/python/pants/backend/python/tasks/pex_build_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
from pex.resolver import resolve
from twitter.common.collections import OrderedSet

from pants.backend.python.pex_util import get_local_platform
from pants.backend.python.subsystems.python_setup import PythonSetup
from pants.backend.python.targets.python_binary import PythonBinary
from pants.backend.python.targets.python_distribution import PythonDistribution
Expand Down Expand Up @@ -150,7 +151,7 @@ def dump_requirement_libs(builder, interpreter, req_libs, log, platforms=None):
Defaults to the platforms specified by PythonSetup.
"""
reqs = [req for req_lib in req_libs for req in req_lib.requirements]
dump_requirements(builder, interpreter, reqs, log, platforms)
dump_requirements(builder, interpreter, reqs, log, platforms=platforms)


def dump_requirements(builder, interpreter, reqs, log, platforms=None):
Expand Down Expand Up @@ -212,7 +213,7 @@ def _resolve_multi(interpreter, requirements, platforms, find_links):
requirements=[req.requirement for req in requirements],
interpreter=interpreter,
fetchers=fetchers,
platform=None if platform == 'current' else platform,
platform=get_local_platform() if platform == 'current' else platform,
context=python_repos.get_network_context(),
cache=requirements_cache_dir,
cache_ttl=python_setup.resolver_cache_ttl,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,8 @@ def _create_binary(self, binary_tgt, results_dir):
# We need to ensure that we are resolving for only the current platform if we are
# including local python dist targets that have native extensions.
build_for_current_platform_only_check(self.context.targets())
dump_requirement_libs(builder, interpreter, req_tgts, self.context.log, binary_tgt.platforms)
dump_requirement_libs(builder, interpreter, req_tgts, self.context.log,
platforms=binary_tgt.platforms)

# Build the .pex file.
pex_path = os.path.join(results_dir, '{}.pex'.format(binary_tgt.name))
Expand Down
25 changes: 16 additions & 9 deletions src/python/pants/backend/python/tasks/select_interpreter.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,10 @@
import hashlib
import os

from pex.interpreter import PythonIdentity, PythonInterpreter
from pex.interpreter import PythonInterpreter

from pants.backend.python.interpreter_cache import PythonInterpreterCache
from pants.backend.python.pex_util import create_bare_interpreter
from pants.backend.python.subsystems.python_setup import PythonSetup
from pants.backend.python.targets.python_target import PythonTarget
from pants.base.fingerprint_strategy import DefaultFingerprintHashingMixin, FingerprintStrategy
Expand Down Expand Up @@ -39,6 +40,10 @@ def compute_fingerprint(self, python_target):
class SelectInterpreter(Task):
"""Select an Python interpreter that matches the constraints of all targets in the working set."""

@classmethod
def implementation_version(cls):
return super(SelectInterpreter, cls).implementation_version() + [('SelectInterpreter', 2)]

@classmethod
def subsystem_dependencies(cls):
return super(SelectInterpreter, cls).subsystem_dependencies() + (PythonSetup, PythonRepos)
Expand All @@ -51,9 +56,11 @@ def execute(self):
python_tgts = self.context.targets(lambda tgt: isinstance(tgt, PythonTarget))
fs = PythonInterpreterFingerprintStrategy()
with self.invalidated(python_tgts, fingerprint_strategy=fs) as invalidation_check:
if PythonSetup.global_instance().interpreter_search_paths and PythonInterpreterCache.pex_python_paths():
self.context.log.warn("Detected both PEX_PYTHON_PATH and --python-setup-interpreter-search-paths. "
"Ignoring --python-setup-interpreter-search-paths.")
if (PythonSetup.global_instance().interpreter_search_paths
and PythonInterpreterCache.pex_python_paths()):
self.context.log.warn("Detected both PEX_PYTHON_PATH and "
"--python-setup-interpreter-search-paths. Ignoring "
"--python-setup-interpreter-search-paths.")
# If there are no relevant targets, we still go through the motions of selecting
# an interpreter, to prevent downstream tasks from having to check for this special case.
if invalidation_check.all_vts:
Expand All @@ -75,7 +82,7 @@ def _create_interpreter_path_file(self, interpreter_path_file, targets):
interpreter = interpreter_cache.select_interpreter_for_targets(targets)
safe_mkdir_for(interpreter_path_file)
with open(interpreter_path_file, 'w') as outfile:
outfile.write(b'{}\t{}\n'.format(interpreter.binary, str(interpreter.identity)))
outfile.write(b'{}\n'.format(interpreter.binary))
for dist, location in interpreter.extras.items():
dist_name, dist_version = dist
outfile.write(b'{}\t{}\t{}\n'.format(dist_name, dist_version, location))
Expand All @@ -87,9 +94,9 @@ def _interpreter_path_file(self, target_set_id):
def _get_interpreter(interpreter_path_file):
with open(interpreter_path_file, 'r') as infile:
lines = infile.readlines()
binary, identity = lines[0].strip().split('\t')
extras = {}
binary = lines[0].strip()
interpreter = create_bare_interpreter(binary)
for line in lines[1:]:
dist_name, dist_version, location = line.strip().split('\t')
extras[(dist_name, dist_version)] = location
return PythonInterpreter(binary, PythonIdentity.from_path(identity), extras)
interpreter = interpreter.with_extra(dist_name, dist_version, location)
return interpreter
Original file line number Diff line number Diff line change
Expand Up @@ -191,20 +191,21 @@ def test_pex_resolver_blacklist_integration(self):
return
pex = os.path.join(os.getcwd(), 'dist', 'test_bin.pex')
try:
pants_ini_config = {'python-setup': {'resolver_blacklist': {"functools32": "CPython>3"}}}
pants_ini_config = {'python-setup': {'resolver_blacklist': {'functools32': 'CPython>3'}}}
target_address_base = os.path.join(self.testproject, 'resolver_blacklist_testing')
# clean-all to ensure that Pants resolves requirements for each run.
pants_binary_36 = self.run_pants(
command=['clean-all', 'binary', '{}:test_bin'.format(os.path.join(self.testproject,'resolver_blacklist_testing'))],
command=['clean-all', 'binary', '{}:test_bin'.format(target_address_base)],
config=pants_ini_config
)
self.assert_success(pants_binary_36)
pants_run_36 = self.run_pants(
command=['clean-all', 'run', '{}:test_bin'.format(os.path.join(self.testproject,'resolver_blacklist_testing'))],
command=['clean-all', 'run', '{}:test_bin'.format(target_address_base)],
config=pants_ini_config
)
self.assert_success(pants_run_36)
pants_run_27 = self.run_pants(
command=['clean-all', 'run', '{}:test_py2'.format(os.path.join(self.testproject,'resolver_blacklist_testing'))],
command=['clean-all', 'run', '{}:test_py2'.format(target_address_base)],
config=pants_ini_config
)
self.assert_success(pants_run_27)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,18 @@
from __future__ import (absolute_import, division, generators, nested_scopes, print_function,
unicode_literals, with_statement)

import os
from textwrap import dedent

import mock
from pex.interpreter import PythonIdentity, PythonInterpreter
from pex.interpreter import PythonInterpreter

from pants.backend.python.interpreter_cache import PythonInterpreterCache
from pants.backend.python.subsystems.python_setup import PythonSetup
from pants.backend.python.targets.python_library import PythonLibrary
from pants.backend.python.tasks.select_interpreter import SelectInterpreter
from pants.base.exceptions import TaskError
from pants.util.dirutil import chmod_plus_x, safe_mkdtemp
from pants_test.task_test_base import TaskTestBase


Expand All @@ -24,22 +28,38 @@ def task_type(cls):
def setUp(self):
super(SelectInterpreterTest, self).setUp()

self.set_options(interpreter=['FakePython>=2.55'])
self.set_options(interpreter=['IronPython>=2.55'])
self.set_options_for_scope(PythonSetup.options_scope)

def fake_interpreter(id_str):
return PythonInterpreter('/fake/binary', PythonIdentity.from_id_string(id_str))
# We're tied tightly to pex implementation details here faking out a python binary that outputs
# only one value no matter what arguments, environment or input stream it has attached. That
# value is the interpreter identity which is - minimally, one line containing:
# <impl> <abi> <impl_version> <major> <minor> <patch>

def fake_interpreter(id_str):
interpreter_dir = safe_mkdtemp()
binary = os.path.join(interpreter_dir, 'binary')
with open(binary, 'w') as fp:
fp.write(dedent("""
#!{}
from __future__ import print_function
print({!r})
""".format(PythonInterpreter.get().binary, id_str)).strip())
chmod_plus_x(binary)
return PythonInterpreter.from_binary(binary)

# impl, abi, impl_version, major, minor, patch
self.fake_interpreters = [
fake_interpreter('FakePython 2 77 777'),
fake_interpreter('FakePython 2 88 888'),
fake_interpreter('FakePython 2 99 999')
fake_interpreter('ip ip2 2 2 77 777'),
fake_interpreter('ip ip2 2 2 88 888'),
fake_interpreter('ip ip2 2 2 99 999')
]

self.tgt1 = self._fake_target('tgt1')
self.tgt2 = self._fake_target('tgt2', compatibility=['FakePython>2.77.777'])
self.tgt3 = self._fake_target('tgt3', compatibility=['FakePython>2.88.888'])
self.tgt4 = self._fake_target('tgt4', compatibility=['FakePython<2.99.999'])
self.tgt2 = self._fake_target('tgt2', compatibility=['IronPython>2.77.777'])
self.tgt3 = self._fake_target('tgt3', compatibility=['IronPython>2.88.888'])
self.tgt4 = self._fake_target('tgt4', compatibility=['IronPython<2.99.999'])
self.tgt20 = self._fake_target('tgt20', dependencies=[self.tgt2])
self.tgt30 = self._fake_target('tgt30', dependencies=[self.tgt3])
self.tgt40 = self._fake_target('tgt40', dependencies=[self.tgt4])
Expand Down Expand Up @@ -75,51 +95,51 @@ def se(me, *args, **kwargs):
return interpreter.version_string

def test_interpreter_selection(self):
self.assertEquals('FakePython-2.77.777', self._select_interpreter([]))
self.assertEquals('FakePython-2.77.777', self._select_interpreter([self.tgt1]))
self.assertEquals('FakePython-2.88.888', self._select_interpreter([self.tgt2]))
self.assertEquals('FakePython-2.99.999', self._select_interpreter([self.tgt3]))
self.assertEquals('FakePython-2.77.777', self._select_interpreter([self.tgt4]))
self.assertEquals('FakePython-2.88.888', self._select_interpreter([self.tgt20]))
self.assertEquals('FakePython-2.99.999', self._select_interpreter([self.tgt30]))
self.assertEquals('FakePython-2.77.777', self._select_interpreter([self.tgt40]))
self.assertEquals('FakePython-2.99.999', self._select_interpreter([self.tgt2, self.tgt3]))
self.assertEquals('FakePython-2.88.888', self._select_interpreter([self.tgt2, self.tgt4]))
self.assertEquals('IronPython-2.77.777', self._select_interpreter([]))
self.assertEquals('IronPython-2.77.777', self._select_interpreter([self.tgt1]))
self.assertEquals('IronPython-2.88.888', self._select_interpreter([self.tgt2]))
self.assertEquals('IronPython-2.99.999', self._select_interpreter([self.tgt3]))
self.assertEquals('IronPython-2.77.777', self._select_interpreter([self.tgt4]))
self.assertEquals('IronPython-2.88.888', self._select_interpreter([self.tgt20]))
self.assertEquals('IronPython-2.99.999', self._select_interpreter([self.tgt30]))
self.assertEquals('IronPython-2.77.777', self._select_interpreter([self.tgt40]))
self.assertEquals('IronPython-2.99.999', self._select_interpreter([self.tgt2, self.tgt3]))
self.assertEquals('IronPython-2.88.888', self._select_interpreter([self.tgt2, self.tgt4]))

with self.assertRaises(TaskError) as cm:
self._select_interpreter([self.tgt3, self.tgt4])
self.assertIn('Unable to detect a suitable interpreter for compatibilities: '
'FakePython<2.99.999 && FakePython>2.88.888', str(cm.exception))
'IronPython<2.99.999 && IronPython>2.88.888', str(cm.exception))

def test_interpreter_selection_invalidation(self):
tgta = self._fake_target('tgta', compatibility=['FakePython>2.77.777'],
tgta = self._fake_target('tgta', compatibility=['IronPython>2.77.777'],
dependencies=[self.tgt3])
self.assertEquals('FakePython-2.99.999',
self.assertEquals('IronPython-2.99.999',
self._select_interpreter([tgta], should_invalidate=True))

# A new target with different sources, but identical compatibility, shouldn't invalidate.
self.create_file('tgtb/foo/bar/baz.py', 'fake content')
tgtb = self._fake_target('tgtb', compatibility=['FakePython>2.77.777'],
tgtb = self._fake_target('tgtb', compatibility=['IronPython>2.77.777'],
dependencies=[self.tgt3], sources=['foo/bar/baz.py'])
self.assertEquals('FakePython-2.99.999',
self.assertEquals('IronPython-2.99.999',
self._select_interpreter([tgtb], should_invalidate=False))

def test_compatibility_AND(self):
tgt = self._fake_target('tgt5', compatibility=['FakePython>2.77.777,<2.99.999'])
self.assertEquals('FakePython-2.88.888', self._select_interpreter([tgt]))
tgt = self._fake_target('tgt5', compatibility=['IronPython>2.77.777,<2.99.999'])
self.assertEquals('IronPython-2.88.888', self._select_interpreter([tgt]))

def test_compatibility_AND_impossible(self):
tgt = self._fake_target('tgt5', compatibility=['FakePython>2.77.777,<2.88.888'])
tgt = self._fake_target('tgt5', compatibility=['IronPython>2.77.777,<2.88.888'])

with self.assertRaises(PythonInterpreterCache.UnsatisfiableInterpreterConstraintsError):
self._select_interpreter([tgt])

def test_compatibility_OR(self):
tgt = self._fake_target('tgt6', compatibility=['FakePython>2.88.888', 'FakePython<2.7'])
self.assertEquals('FakePython-2.99.999', self._select_interpreter([tgt]))
tgt = self._fake_target('tgt6', compatibility=['IronPython>2.88.888', 'IronPython<2.7'])
self.assertEquals('IronPython-2.99.999', self._select_interpreter([tgt]))

def test_compatibility_OR_impossible(self):
tgt = self._fake_target('tgt6', compatibility=['FakePython>2.99.999', 'FakePython<2.77.777'])
tgt = self._fake_target('tgt6', compatibility=['IronPython>2.99.999', 'IronPython<2.77.777'])

with self.assertRaises(PythonInterpreterCache.UnsatisfiableInterpreterConstraintsError):
self._select_interpreter([tgt])

0 comments on commit a9c8f17

Please sign in to comment.