Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix environment markers evaluation - issue #3829 #4051

Merged
merged 6 commits into from
Nov 2, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 14 additions & 3 deletions pip/req/req_install.py
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,10 @@ def __init__(self, req, comes_from, source_dir=None, editable=False,
self._wheel_cache = wheel_cache
self.link = self.original_link = link
self.as_egg = as_egg
self.markers = markers
if markers is not None:
self.markers = markers
else:
self.markers = req and req.marker
self._egg_info_path = None
# This holds the pkg_resources.Distribution object if this requirement
# is already available:
Expand Down Expand Up @@ -175,6 +178,8 @@ def from_line(
markers = markers.strip()
if not markers:
markers = None
else:
markers = Marker(markers)
else:
markers = None
name = name.strip()
Expand Down Expand Up @@ -819,9 +824,15 @@ def _clean_zip_name(self, name, prefix):
name = name.replace(os.path.sep, '/')
return name

def match_markers(self):
def match_markers(self, extras_requested=None):
if not extras_requested:
# Provide an extra to safely evaluate the markers
# without matching any extra
extras_requested = ('',)
if self.markers is not None:
return Marker(self.markers).evaluate()
return any(
self.markers.evaluate({'extra': extra})
for extra in extras_requested)
else:
return True

Expand Down
16 changes: 10 additions & 6 deletions pip/req/req_set.py
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,8 @@ def __repr__(self):
return ('<%s object; %d requirement(s): %s>'
% (self.__class__.__name__, len(reqs), reqs_str))

def add_requirement(self, install_req, parent_req_name=None):
def add_requirement(self, install_req, parent_req_name=None,
extras_requested=None):
"""Add install_req as a requirement to install.

:param parent_req_name: The name of the requirement that needed this
Expand All @@ -221,13 +222,15 @@ def add_requirement(self, install_req, parent_req_name=None):
links that point outside the Requirements set. parent_req must
already be added. Note that None implies that this is a user
supplied requirement, vs an inferred one.
:param extras_requested: an iterable of extras used to evaluate the
environement markers.
:return: Additional requirements to scan. That is either [] if
the requirement is not applicable, or [install_req] if the
requirement is applicable and has just been added.
"""
name = install_req.name
if not install_req.match_markers():
logger.warning("Ignoring %s: markers %r don't match your "
if not install_req.match_markers(extras_requested):
logger.warning("Ignoring %s: markers '%s' don't match your "
"environment", install_req.name,
install_req.markers)
return []
Expand Down Expand Up @@ -669,15 +672,16 @@ def _prepare_file(self,
raise
more_reqs = []

def add_req(subreq):
def add_req(subreq, extras_requested):
sub_install_req = InstallRequirement(
str(subreq),
req_to_install,
isolated=self.isolated,
wheel_cache=self._wheel_cache,
)
more_reqs.extend(self.add_requirement(
sub_install_req, req_to_install.name))
sub_install_req, req_to_install.name,
extras_requested=extras_requested))

# We add req_to_install before its dependencies, so that we
# can refer to it when adding dependencies.
Expand All @@ -704,7 +708,7 @@ def add_req(subreq):
set(dist.extras) & set(req_to_install.extras)
)
for subreq in dist.requires(available_requested):
add_req(subreq)
add_req(subreq, extras_requested=available_requested)

# cleanup tmp src
self.reqs_to_cleanup.append(req_to_install)
Expand Down
21 changes: 21 additions & 0 deletions tests/functional/test_install.py
Original file line number Diff line number Diff line change
Expand Up @@ -1140,3 +1140,24 @@ def test_install_compatible_python_requires(script):
script.pip('install', 'setuptools>24.2') # This should not be needed
res = script.pip('install', pkga_path, expect_error=True)
assert "Successfully installed pkga-0.1" in res.stdout, res


def test_install_environment_markers(script, data):
# make a dummy project
pkga_path = script.scratch_path / 'pkga'
pkga_path.mkdir()
pkga_path.join("setup.py").write(textwrap.dedent("""
from setuptools import setup
setup(name='pkga',
version='0.1',
install_requires=[
'missing_pkg; python_version=="1.0"',
],
)
"""))

res = script.pip('install', '--no-index', pkga_path, expect_stderr=True)
# missing_pkg should be ignored
assert ("Ignoring missing-pkg: markers 'python_version == \"1.0\"' don't "
"match your environment") in res.stderr, str(res)
assert "Successfully installed pkga-0.1" in res.stdout, str(res)
5 changes: 2 additions & 3 deletions tests/functional/test_install_reqs.py
Original file line number Diff line number Diff line change
Expand Up @@ -498,9 +498,8 @@ def test_install_unsupported_wheel_link_with_marker(script, data):
expect_stderr=True,
)

s = "Ignoring asdf: markers %r don't match your environment" %\
u'sys_platform == "xyz"'
assert s in result.stderr
assert ("Ignoring asdf: markers 'sys_platform == \"xyz\"' don't match "
"your environment") in result.stderr
assert len(result.files_created) == 0


Expand Down
34 changes: 28 additions & 6 deletions tests/unit/test_req.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
from pip.req.req_install import parse_editable
from pip.utils import read_text_file
from pip._vendor import pkg_resources
from pip._vendor.packaging.markers import Marker
from pip._vendor.packaging.requirements import Requirement
from tests.lib import assert_raises_regexp, requirements_file

Expand Down Expand Up @@ -415,22 +416,22 @@ def test_markers(self):
req = InstallRequirement.from_line(line)
assert req.req.name == 'mock3'
assert str(req.req.specifier) == ''
assert req.markers == 'python_version >= "3"'
assert str(req.markers) == 'python_version >= "3"'

def test_markers_semicolon(self):
# check that the markers can contain a semicolon
req = InstallRequirement.from_line('semicolon; os_name == "a; b"')
assert req.req.name == 'semicolon'
assert str(req.req.specifier) == ''
assert req.markers == 'os_name == "a; b"'
assert str(req.markers) == 'os_name == "a; b"'

def test_markers_url(self):
# test "URL; markers" syntax
url = 'http://foo.com/?p=bar.git;a=snapshot;h=v0.1;sf=tgz'
line = '%s; python_version >= "3"' % url
req = InstallRequirement.from_line(line)
assert req.link.url == url, req.url
assert req.markers == 'python_version >= "3"'
assert str(req.markers) == 'python_version >= "3"'

# without space, markers are part of the URL
url = 'http://foo.com/?p=bar.git;a=snapshot;h=v0.1;sf=tgz'
Expand All @@ -439,15 +440,15 @@ def test_markers_url(self):
assert req.link.url == line, req.url
assert req.markers is None

def test_markers_match(self):
def test_markers_match_from_line(self):
# match
for markers in (
'python_version >= "1.0"',
'sys_platform == %r' % sys.platform,
):
line = 'name; ' + markers
req = InstallRequirement.from_line(line)
assert req.markers == markers
assert str(req.markers) == str(Marker(markers))
assert req.match_markers()

# don't match
Expand All @@ -457,7 +458,28 @@ def test_markers_match(self):
):
line = 'name; ' + markers
req = InstallRequirement.from_line(line)
assert req.markers == markers
assert str(req.markers) == str(Marker(markers))
assert not req.match_markers()

def test_markers_match(self):
# match
for markers in (
'python_version >= "1.0"',
'sys_platform == %r' % sys.platform,
):
line = 'name; ' + markers
req = InstallRequirement(line, comes_from='')
assert str(req.markers) == str(Marker(markers))
assert req.match_markers()

# don't match
for markers in (
'python_version >= "5.0"',
'sys_platform != %r' % sys.platform,
):
line = 'name; ' + markers
req = InstallRequirement(line, comes_from='')
assert str(req.markers) == str(Marker(markers))
assert not req.match_markers()

def test_extras_for_line_path_requirement(self):
Expand Down