From 3064d9dbd106587b2d2f50dd07cb21b79dc2e16a Mon Sep 17 00:00:00 2001 From: John Sirois Date: Tue, 9 Oct 2018 19:01:47 -0400 Subject: [PATCH] Filter top-level requirements against env markers. Previously, only transitive requirements were filtered. Additionally, the monkey-patching in `patched_packing_env` is removed in favor of directly evaluating markers. Fixes #456 --- pex/environment.py | 6 ++--- pex/interpreter.py | 13 ++++++++--- pex/resolver.py | 40 +++++++++++++------------------- tests/test_integration.py | 49 ++++++++++++++++++++++++++++++++++++++- 4 files changed, 76 insertions(+), 32 deletions(-) diff --git a/pex/environment.py b/pex/environment.py index c1afbb4aa..7365d3167 100644 --- a/pex/environment.py +++ b/pex/environment.py @@ -130,6 +130,7 @@ def __init__(self, pex, pex_info, interpreter=None, **kw): platform=platform_name, **kw ) + self._env_marker_filter = self._interpreter.identity.env_marker_filter(platform_name) 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), @@ -160,10 +161,7 @@ def _resolve(self, working_set, reqs): # Resolve them one at a time so that we can figure out which ones we need to elide should # there be an interpreter incompatibility. - for req in reqs: - if req.marker and not req.marker.evaluate(): - TRACER.log('Skipping activation of `%s` due to environment marker de-selection' % req) - continue + for req in filter(self._env_marker_filter, reqs): with TRACER.timed('Resolving %s' % req, V=2): try: resolveds.update(working_set.resolve([req], env=self)) diff --git a/pex/interpreter.py b/pex/interpreter.py index 0bfa8b22a..23b94cfcd 100644 --- a/pex/interpreter.py +++ b/pex/interpreter.py @@ -209,8 +209,13 @@ def python(self): # specifically, '2.7', '3.2', etc. return '%d.%d' % (self.version[0:2]) - def pkg_resources_env(self, platform_str): - """Returns a dict that can be used in place of packaging.default_environment.""" + def env_marker_filter(self, platform_str): + """Returns a requirement filter that filters by environment markers when present. + + :param str platform_str: The target platform to filter against. + :returns: a filter function that returns `True` if the given requirement applies + :rtype: function from :class:`pkg_resources.Requirement` to bool + """ os_name = '' platform_machine = '' platform_release = '' @@ -238,7 +243,7 @@ def pkg_resources_env(self, platform_str): platform_version = 'Darwin Kernel Version {}'.format(platform_release) sys_platform = 'darwin' - return { + environment = { 'implementation_name': self.interpreter.lower(), 'implementation_version': self.version_str, 'os_name': os_name, @@ -252,6 +257,8 @@ def pkg_resources_env(self, platform_str): 'sys_platform': sys_platform, } + return lambda req: req.marker is None or req.marker.evaluate(environment=environment) + def __str__(self): return '%s-%s.%s.%s' % ( self._interpreter, diff --git a/pex/resolver.py b/pex/resolver.py index c74c09a0d..07eb3c2d5 100644 --- a/pex/resolver.py +++ b/pex/resolver.py @@ -8,9 +8,7 @@ import shutil import time from collections import namedtuple -from contextlib import contextmanager -import pkg_resources from pkg_resources import safe_name from .common import safe_mkdir @@ -26,18 +24,6 @@ from .util import DistributionHelper -@contextmanager -def patched_packing_env(env): - """Monkey patch packaging.markers.default_environment""" - old_env = pkg_resources.packaging.markers.default_environment - new_env = lambda: env - pkg_resources._vendor.packaging.markers.default_environment = new_env - try: - yield - finally: - pkg_resources._vendor.packaging.markers.default_environment = old_env - - class Untranslateable(Exception): pass @@ -220,6 +206,8 @@ def __init__(self, allow_prereleases=None, interpreter=None, platform=None, use_ self._interpreter = interpreter or PythonInterpreter.get() self._platform = self._maybe_expand_platform(self._interpreter, platform) self._allow_prereleases = allow_prereleases + platform_name = self._platform.platform + self._env_marker_filter = self._interpreter.identity.env_marker_filter(platform_name) self._supported_tags = self._platform.supported_tags( self._interpreter, use_manylinux @@ -259,8 +247,16 @@ def build(self, package, options): 'Could not get distribution for %s on platform %s.' % (package, self._platform)) return dist + def filter_resolvables_by_markers(self, resolvables): + for resolvable in resolvables: + if not isinstance(resolvable, ResolvableRequirement): + yield resolvable + elif self._env_marker_filter(resolvable.requirement): + yield resolvable + def resolve(self, resolvables, resolvable_set=None): - resolvables = [(resolvable, None) for resolvable in resolvables] + resolvables = [(resolvable, None) + for resolvable in self.filter_resolvables_by_markers(resolvables)] resolvable_set = resolvable_set or _ResolvableSet() processed_resolvables = set() processed_packages = {} @@ -295,15 +291,11 @@ def resolve(self, resolvables, resolvable_set=None): distribution = distributions[package] processed_packages[resolvable.name] = package new_parent = '%s->%s' % (parent, resolvable) if parent else str(resolvable) - # We patch packaging.markers.default_environment here so we find optional reqs for the - # platform we're building the PEX for, rather than the one we're on. - with patched_packing_env( - self._interpreter.identity.pkg_resources_env(self._platform.platform) - ): - resolvables.extend( - (ResolvableRequirement(req, resolvable.options), new_parent) for req in - distribution.requires(extras=resolvable_set.extras(resolvable.name)) - ) + required_resolvables = self.filter_resolvables_by_markers( + ResolvableRequirement(req, resolvable.options) + for req in distribution.requires(extras=resolvable_set.extras(resolvable.name)) + ) + resolvables.extend((res, new_parent) for res in required_resolvables) resolvable_set = resolvable_set.replace_built(built_packages) # We may have built multiple distributions depending upon if we found transitive dependencies diff --git a/tests/test_integration.py b/tests/test_integration.py index 78116aee4..36bee7ac5 100644 --- a/tests/test_integration.py +++ b/tests/test_integration.py @@ -1112,7 +1112,7 @@ def test_setup_interpreter_constraint(): @pytest.mark.skipif(IS_PYPY, reason='Our pyenv interpreter setup fails under pypy: ' 'https://github.com/pantsbuild/pex/issues/477') -def test_setup_python_multiple(): +def test_setup_python_multiple_transitive_markers(): py27_interpreter = ensure_python_interpreter(PY27) py36_interpreter = ensure_python_interpreter(PY36) with temporary_dir() as out: @@ -1143,3 +1143,50 @@ def test_setup_python_multiple(): stdout = subprocess.check_output(both_program) assert to_bytes(os.path.realpath(py36_interpreter)) == stdout.strip() + + +@pytest.mark.skipif(IS_PYPY, + reason='Our pyenv interpreter setup fails under pypy: ' + 'https://github.com/pantsbuild/pex/issues/477') +def test_setup_python_direct_markers(): + py36_interpreter = ensure_python_interpreter(PY36) + with temporary_dir() as out: + pex = os.path.join(out, 'pex.pex') + results = run_pex_command(['subprocess32==3.2.7; python_version<"3"', + '--disable-cache', + '--python-shebang=#!/usr/bin/env python', + '--python={}'.format(py36_interpreter), + '-o', pex]) + results.assert_success() + + py2_only_program = [pex, '-c', 'import subprocess32'] + + with environment_as(PATH=os.path.dirname(py36_interpreter)): + with pytest.raises(subprocess.CalledProcessError): + subprocess.check_call(py2_only_program) + + +@pytest.mark.skipif(IS_PYPY, + reason='Our pyenv interpreter setup fails under pypy: ' + 'https://github.com/pantsbuild/pex/issues/477') +def test_setup_python_multiple_direct_markers(): + py36_interpreter = ensure_python_interpreter(PY36) + py27_interpreter = ensure_python_interpreter(PY27) + with temporary_dir() as out: + pex = os.path.join(out, 'pex.pex') + results = run_pex_command(['subprocess32==3.2.7; python_version<"3"', + '--disable-cache', + '--python-shebang=#!/usr/bin/env python', + '--python={}'.format(py36_interpreter), + '--python={}'.format(py27_interpreter), + '-o', pex]) + results.assert_success() + + py2_only_program = [pex, '-c', 'import subprocess32'] + + with environment_as(PATH=os.path.dirname(py36_interpreter)): + with pytest.raises(subprocess.CalledProcessError): + subprocess.check_call(py2_only_program) + + with environment_as(PATH=os.path.dirname(py27_interpreter)): + subprocess.check_call(py2_only_program)