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 pex-tool#456

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))

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,

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

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)