From d9f7c2c28e217be71eb1d95e8539897da23622d6 Mon Sep 17 00:00:00 2001 From: John Sirois Date: Mon, 4 Jun 2018 16:15:56 -0600 Subject: [PATCH] Upgrade to pex 1.4.3. 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 https://github.com/pantsbuild/pex/issues/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 https://github.com/pantsbuild/pex/issues/510. Upgrading pex to take advantage of the worked around issues noted above is tracked by https://github.com/pantsbuild/pants/issues/5922. Fixes #5906 --- 3rdparty/python/requirements.txt | 2 +- src/python/pants/backend/python/BUILD | 10 ++- .../pants/backend/python/interpreter_cache.py | 10 +-- src/python/pants/backend/python/pex_util.py | 35 ++++++++ src/python/pants/backend/python/tasks/BUILD | 13 +-- .../backend/python/tasks/pex_build_util.py | 5 +- .../python/tasks/python_binary_create.py | 3 +- .../python/tasks/select_interpreter.py | 25 ++++-- .../tasks/test_python_run_integration.py | 9 +- .../python/tasks/test_select_interpreter.py | 82 ++++++++++++------- 10 files changed, 134 insertions(+), 60 deletions(-) create mode 100644 src/python/pants/backend/python/pex_util.py diff --git a/3rdparty/python/requirements.txt b/3rdparty/python/requirements.txt index 1a6976fcddc..ab169c06764 100644 --- a/3rdparty/python/requirements.txt +++ b/3rdparty/python/requirements.txt @@ -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 diff --git a/src/python/pants/backend/python/BUILD b/src/python/pants/backend/python/BUILD index c90f853bb93..9ae9f54c4ba 100644 --- a/src/python/pants/backend/python/BUILD +++ b/src/python/pants/backend/python/BUILD @@ -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', @@ -58,7 +59,6 @@ python_library( ] ) - python_library( name = 'python_artifact', sources = ['python_artifact.py'], @@ -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', + ] +) diff --git a/src/python/pants/backend/python/interpreter_cache.py b/src/python/pants/backend/python/interpreter_cache.py index c670f9a9d72..aa2b6fd111f 100644 --- a/src/python/pants/backend/python/interpreter_cache.py +++ b/src/python/pants/backend/python/interpreter_cache.py @@ -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 @@ -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 @@ -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: diff --git a/src/python/pants/backend/python/pex_util.py b/src/python/pants/backend/python/pex_util.py new file mode 100644 index 00000000000..f5ba1eee250 --- /dev/null +++ b/src/python/pants/backend/python/pex_util.py @@ -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 \ No newline at end of file diff --git a/src/python/pants/backend/python/tasks/BUILD b/src/python/pants/backend/python/tasks/BUILD index 6bf3f83183c..27d64085f7d 100644 --- a/src/python/pants/backend/python/tasks/BUILD +++ b/src/python/pants/backend/python/tasks/BUILD @@ -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', diff --git a/src/python/pants/backend/python/tasks/pex_build_util.py b/src/python/pants/backend/python/tasks/pex_build_util.py index 7170826ca6f..45392661a8c 100644 --- a/src/python/pants/backend/python/tasks/pex_build_util.py +++ b/src/python/pants/backend/python/tasks/pex_build_util.py @@ -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 @@ -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): @@ -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, diff --git a/src/python/pants/backend/python/tasks/python_binary_create.py b/src/python/pants/backend/python/tasks/python_binary_create.py index 3f8af639695..85cc176bd6f 100644 --- a/src/python/pants/backend/python/tasks/python_binary_create.py +++ b/src/python/pants/backend/python/tasks/python_binary_create.py @@ -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)) diff --git a/src/python/pants/backend/python/tasks/select_interpreter.py b/src/python/pants/backend/python/tasks/select_interpreter.py index d4aea564122..95978c115d5 100644 --- a/src/python/pants/backend/python/tasks/select_interpreter.py +++ b/src/python/pants/backend/python/tasks/select_interpreter.py @@ -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 @@ -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) @@ -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: @@ -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)) @@ -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 diff --git a/tests/python/pants_test/backend/python/tasks/test_python_run_integration.py b/tests/python/pants_test/backend/python/tasks/test_python_run_integration.py index 4bf458e2d53..0f1f9448533 100644 --- a/tests/python/pants_test/backend/python/tasks/test_python_run_integration.py +++ b/tests/python/pants_test/backend/python/tasks/test_python_run_integration.py @@ -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) diff --git a/tests/python/pants_test/backend/python/tasks/test_select_interpreter.py b/tests/python/pants_test/backend/python/tasks/test_select_interpreter.py index 8d9d19a7576..1e5a13de7aa 100644 --- a/tests/python/pants_test/backend/python/tasks/test_select_interpreter.py +++ b/tests/python/pants_test/backend/python/tasks/test_select_interpreter.py @@ -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 @@ -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: + # + 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]) @@ -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])