Skip to content

Commit

Permalink
Filter top-level requirements against env markers. (pex-tool#592)
Browse files Browse the repository at this point in the history
Previously, only transitive requirements were filtered.

Fixes pex-tool#456
Fixes pex-tool#122
  • Loading branch information
jsirois authored Oct 10, 2018
1 parent a4eac2f commit 884b459
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 6 deletions.
3 changes: 2 additions & 1 deletion pex/environment.py
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ def __init__(self, pex, pex_info, interpreter=None, **kw):
platform=platform_name,
**kw
)
self._target_interpreter_env = self._interpreter.identity.pkg_resources_env(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),
Expand Down Expand Up @@ -161,7 +162,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():
if req.marker and not req.marker.evaluate(environment=self._target_interpreter_env):
TRACER.log('Skipping activation of `%s` due to environment marker de-selection' % req)
continue
with TRACER.timed('Resolving %s' % req, V=2):
Expand Down
17 changes: 13 additions & 4 deletions pex/resolver.py
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,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._target_interpreter_env = self._interpreter.identity.pkg_resources_env(platform_name)
self._supported_tags = self._platform.supported_tags(
self._interpreter,
use_manylinux
Expand Down Expand Up @@ -259,8 +261,17 @@ def build(self, package, options):
'Could not get distribution for %s on platform %s.' % (package, self._platform))
return dist

def is_resolvable_in_target_interpreter_env(self, resolvable):
if not isinstance(resolvable, ResolvableRequirement):
return True
elif resolvable.requirement.marker is None:
return True
else:
return resolvable.requirement.marker.evaluate(environment=self._target_interpreter_env)

def resolve(self, resolvables, resolvable_set=None):
resolvables = [(resolvable, None) for resolvable in resolvables]
resolvables = [(resolvable, None) for resolvable in resolvables
if self.is_resolvable_in_target_interpreter_env(resolvable)]
resolvable_set = resolvable_set or _ResolvableSet()
processed_resolvables = set()
processed_packages = {}
Expand Down Expand Up @@ -297,9 +308,7 @@ def resolve(self, resolvables, resolvable_set=None):
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)
):
with patched_packing_env(self._target_interpreter_env):
resolvables.extend(
(ResolvableRequirement(req, resolvable.options), new_parent) for req in
distribution.requires(extras=resolvable_set.extras(resolvable.name))
Expand Down
49 changes: 48 additions & 1 deletion tests/test_integration.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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)

0 comments on commit 884b459

Please sign in to comment.