Skip to content

Commit

Permalink
Upgrade to pex 1.4.3. (#5910)
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 #5922.

Fixes #5906
  • Loading branch information
jsirois committed Jun 7, 2018
1 parent 8c2e484 commit adffe37
Show file tree
Hide file tree
Showing 10 changed files with 134 additions and 60 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: 9 additions & 1 deletion src/python/pants/backend/python/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ python_library(
sources = ['interpreter_cache.py'],
dependencies = [
'3rdparty/python:pex',
':pex_util',
'src/python/pants/backend/python/targets',
'src/python/pants/base:exceptions',
'src/python/pants/process',
Expand All @@ -58,7 +59,6 @@ python_library(
]
)


python_library(
name = 'python_artifact',
sources = ['python_artifact.py'],
Expand All @@ -76,3 +76,11 @@ python_library(
name = 'python_requirements',
sources = ['python_requirements.py'],
)

python_library(
name = 'pex_util',
sources = ['pex_util.py'],
dependencies = [
'3rdparty/python:pex',
]
)
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
13 changes: 7 additions & 6 deletions src/python/pants/backend/python/tasks/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -3,32 +3,33 @@

python_library(
dependencies=[
'3rdparty/python:pex',
'3rdparty/python/twitter/commons:twitter.common.collections',
'3rdparty/python/twitter/commons:twitter.common.dirutil',
'3rdparty/python:pex',
'src/python/pants/backend/native/subsystems',
'src/python/pants/backend/python:python_requirement',
'src/python/pants/backend/python:python_requirements',
'src/python/pants/backend/python:interpreter_cache',
'src/python/pants/backend/python/subsystems',
'src/python/pants/backend/python/targets',
'src/python/pants/backend/python/tasks/coverage:plugin',
'src/python/pants/backend/python:interpreter_cache',
'src/python/pants/backend/python:pex_util',
'src/python/pants/backend/python:python_requirement',
'src/python/pants/backend/python:python_requirements',
'src/python/pants/base:build_environment',
'src/python/pants/base:exceptions',
'src/python/pants/base:fingerprint_strategy',
'src/python/pants/base:hash_utils',
'src/python/pants/base:specs',
'src/python/pants/build_graph',
'src/python/pants/engine:rules',
'src/python/pants/engine:selectors',
'src/python/pants/build_graph',
'src/python/pants/invalidation',
'src/python/pants/python',
'src/python/pants/task',
'src/python/pants/util:contextutil',
'src/python/pants/util:dirutil',
'src/python/pants/util:fileutil',
'src/python/pants/util:meta',
'src/python/pants/util:memo',
'src/python/pants/util:meta',
'src/python/pants/util:objects',
'src/python/pants/util:process_handler',
'src/python/pants/util:xml_parser',
Expand Down
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
Loading

0 comments on commit adffe37

Please sign in to comment.