Fix environment markers evaluation - issue #3829 (#4051)

Author: xavfernandez

pip/req/req_install.py

@@ -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:
@@ -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()
@@ -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
 

pip/req/req_set.py

@@ -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
@@ -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 []
@@ -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.
@@ -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)

tests/functional/test_install.py

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

tests/functional/test_install_reqs.py

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

tests/unit/test_req.py

@@ -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
 
@@ -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'
@@ -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
@@ -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):