Merge pull request #1089 from benoit-pierre/fix_requires_handling

Fix requires handling

10 files changed, 164 insertions, 65 deletions

diff --git a/.travis.yml b/.travis.yml
index cbf49da6..b207a8d2 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -43,7 +43,4 @@ install:
 # update egg_info based on setup.py in checkout
 - python bootstrap.py
 
-# Check that setuptools can be installed in a clean environment
-- tests/clean_install.sh
-
 script: tox
diff --git a/pkg_resources/tests/test_markers.py b/pkg_resources/tests/test_markers.py
index 9306d5b3..15a3b499 100644
--- a/pkg_resources/tests/test_markers.py
+++ b/pkg_resources/tests/test_markers.py
@@ -1,4 +1,4 @@
-from unittest import mock
+import mock
 
 from pkg_resources import evaluate_marker
 
diff --git a/setuptools/dist.py b/setuptools/dist.py
index 68c8747a..9a034db5 100644
--- a/setuptools/dist.py
+++ b/setuptools/dist.py
@@ -9,7 +9,6 @@ import distutils.core
 import distutils.cmd
 import distutils.dist
 import itertools
-import operator
 from collections import defaultdict
 from distutils.errors import (
     DistutilsOptionError, DistutilsPlatformError, DistutilsSetupError,
@@ -17,7 +16,7 @@ from distutils.errors import (
 from distutils.util import rfc822_escape
 
 from setuptools.extern import six
-from setuptools.extern.six.moves import map, filter
+from setuptools.extern.six.moves import map
 from pkg_resources.extern import packaging
 
 from setuptools.depends import Require
@@ -146,17 +145,7 @@ def _check_extra(extra, reqs):
     name, sep, marker = extra.partition(':')
     if marker and pkg_resources.invalid_marker(marker):
         raise DistutilsSetupError("Invalid environment marker: " + marker)
-
-    # extras requirements cannot themselves have markers
-    parsed = pkg_resources.parse_requirements(reqs)
-    marked_reqs = filter(operator.attrgetter('marker'), parsed)
-    bad_req = next(marked_reqs, None)
-    if bad_req:
-        tmpl = (
-            "'extras_require' requirements cannot include "
-            "environment markers, in {name!r}: '{bad_req!s}'"
-        )
-        raise DistutilsSetupError(tmpl.format(**locals()))
+    list(pkg_resources.parse_requirements(reqs))
 
 
 def assert_bool(dist, attr, value):
@@ -366,23 +355,41 @@ class Distribution(Distribution_parse_config_files, _Distribution):
 
     def _finalize_requires(self):
         """
-        Move requirements in `install_requires` that
-        are using environment markers to `extras_require`.
+        Fix environment markers in `install_requires` and `extras_require`.
+
+        - move requirements in `install_requires` that are using environment
+          markers or extras to `extras_require`.
+        - convert requirements in `extras_require` of the form
+          `"extra": ["barbazquux; {marker}"]` to
+          `"extra:{marker}": ["barbazquux"]`.
         """
-        if not self.install_requires:
-            return
-        extras_require = defaultdict(list, (
-            (k, list(pkg_resources.parse_requirements(v)))
-            for k, v in (self.extras_require or {}).items()
-        ))
+        extras_require = defaultdict(list)
+        for k, v in (
+            getattr(self, 'extras_require', None) or {}
+        ).items():
+            for r in pkg_resources.parse_requirements(v):
+                marker = r.marker
+                if marker:
+                    r.marker = None
+                    extras_require[k + ':' + str(marker)].append(r)
+                else:
+                    extras_require[k].append(r)
         install_requires = []
-        for r in pkg_resources.parse_requirements(self.install_requires):
+        for r in pkg_resources.parse_requirements(
+            getattr(self, 'install_requires', None) or ()
+        ):
             marker = r.marker
-            if not marker:
+            extras = r.extras
+            if not marker and not extras:
                 install_requires.append(r)
                 continue
+            r.extras = ()
             r.marker = None
-            extras_require[':' + str(marker)].append(r)
+            for e in extras or ('',):
+                section = e
+                if marker:
+                    section += ':' + str(marker)
+                extras_require[section].append(r)
         self.extras_require = dict(
             (k, [str(r) for r in v])
             for k, v in extras_require.items()
diff --git a/setuptools/tests/test_build_clib.py b/setuptools/tests/test_build_clib.py
index 7e3d1de9..aebcc350 100644
--- a/setuptools/tests/test_build_clib.py
+++ b/setuptools/tests/test_build_clib.py
@@ -2,7 +2,7 @@ import pytest
 import os
 import shutil
 
-from unittest import mock
+import mock
 from distutils.errors import DistutilsSetupError
 from setuptools.command.build_clib import build_clib
 from setuptools.dist import Distribution
diff --git a/setuptools/tests/test_easy_install.py b/setuptools/tests/test_easy_install.py
index 1b7178a1..26cdd90d 100644
--- a/setuptools/tests/test_easy_install.py
+++ b/setuptools/tests/test_easy_install.py
@@ -14,7 +14,7 @@ import itertools
 import distutils.errors
 import io
 import zipfile
-from unittest import mock
+import mock
 
 import time
 from setuptools.extern.six.moves import urllib
diff --git a/setuptools/tests/test_egg_info.py b/setuptools/tests/test_egg_info.py
index 07bd8818..5ea55d61 100644
--- a/setuptools/tests/test_egg_info.py
+++ b/setuptools/tests/test_egg_info.py
@@ -182,8 +182,13 @@ class TestEggInfo(object):
     mismatch_marker = "python_version<'{this_ver}'".format(
         this_ver=sys.version_info[0],
     )
+    # Alternate equivalent syntax.
+    mismatch_marker_alternate = 'python_version < "{this_ver}"'.format(
+        this_ver=sys.version_info[0],
+    )
+    invalid_marker = "<=>++"
 
-    def test_install_requires_with_markers(self, tmpdir_cwd, env):
+    def test_install_requires_with_marker(self, tmpdir_cwd, env):
         tmpl = 'install_requires=["barbazquux;{marker}"],'
         req = tmpl.format(marker=self.mismatch_marker)
         self._setup_script_with_requires(req)
@@ -193,9 +198,40 @@ class TestEggInfo(object):
         with open(requires_txt) as fp:
             install_requires = fp.read()
         expected_requires = DALS('''
-             [:python_version < "{sys.version_info[0]}"]
+             [:{marker}]
+             barbazquux
+             ''').format(marker=self.mismatch_marker_alternate)
+        assert install_requires.lstrip() == expected_requires
+        assert glob.glob(os.path.join(env.paths['lib'], 'barbazquux*')) == []
+
+    def test_install_requires_with_extra(self, tmpdir_cwd, env):
+        req = 'install_requires=["barbazquux [test]"],'
+        self._setup_script_with_requires(req)
+        self._run_install_command(tmpdir_cwd, env)
+        egg_info_dir = self._find_egg_info_files(env.paths['lib']).base
+        requires_txt = os.path.join(egg_info_dir, 'requires.txt')
+        with open(requires_txt) as fp:
+            install_requires = fp.read()
+        expected_requires = DALS('''
+             [test]
+             barbazquux
+             ''')
+        assert install_requires.lstrip() == expected_requires
+        assert glob.glob(os.path.join(env.paths['lib'], 'barbazquux*')) == []
+
+    def test_install_requires_with_extra_and_marker(self, tmpdir_cwd, env):
+        tmpl = 'install_requires=["barbazquux [test]; {marker}"],'
+        req = tmpl.format(marker=self.mismatch_marker)
+        self._setup_script_with_requires(req)
+        self._run_install_command(tmpdir_cwd, env)
+        egg_info_dir = self._find_egg_info_files(env.paths['lib']).base
+        requires_txt = os.path.join(egg_info_dir, 'requires.txt')
+        with open(requires_txt) as fp:
+            install_requires = fp.read()
+        expected_requires = DALS('''
+             [test:{marker}]
              barbazquux
-             ''').format(sys=sys)
+             ''').format(marker=self.mismatch_marker_alternate)
         assert install_requires.lstrip() == expected_requires
         assert glob.glob(os.path.join(env.paths['lib'], 'barbazquux*')) == []
 
@@ -214,19 +250,53 @@ class TestEggInfo(object):
             tmpdir_cwd, env, cmd=['test'], output="Ran 0 tests in")
         assert glob.glob(os.path.join(env.paths['lib'], 'barbazquux*')) == []
 
-    def test_extras_require_with_markers(self, tmpdir_cwd, env):
+    def test_extras_require_with_marker(self, tmpdir_cwd, env):
         tmpl = 'extras_require={{":{marker}": ["barbazquux"]}},'
         req = tmpl.format(marker=self.mismatch_marker)
         self._setup_script_with_requires(req)
         self._run_install_command(tmpdir_cwd, env)
+        egg_info_dir = self._find_egg_info_files(env.paths['lib']).base
+        requires_txt = os.path.join(egg_info_dir, 'requires.txt')
+        with open(requires_txt) as fp:
+            install_requires = fp.read()
+        expected_requires = DALS('''
+             [:{marker}]
+             barbazquux
+             ''').format(marker=self.mismatch_marker)
+        assert install_requires.lstrip() == expected_requires
         assert glob.glob(os.path.join(env.paths['lib'], 'barbazquux*')) == []
 
-    def test_extras_require_with_markers_in_req(self, tmpdir_cwd, env):
+    def test_extras_require_with_marker_in_req(self, tmpdir_cwd, env):
         tmpl = 'extras_require={{"extra": ["barbazquux; {marker}"]}},'
         req = tmpl.format(marker=self.mismatch_marker)
         self._setup_script_with_requires(req)
+        self._run_install_command(tmpdir_cwd, env)
+        egg_info_dir = self._find_egg_info_files(env.paths['lib']).base
+        requires_txt = os.path.join(egg_info_dir, 'requires.txt')
+        with open(requires_txt) as fp:
+            install_requires = fp.read()
+        expected_requires = DALS('''
+             [extra:{marker}]
+             barbazquux
+             ''').format(marker=self.mismatch_marker_alternate)
+        assert install_requires.lstrip() == expected_requires
+        assert glob.glob(os.path.join(env.paths['lib'], 'barbazquux*')) == []
+
+    def test_extras_require_with_invalid_marker(self, tmpdir_cwd, env):
+        tmpl = 'extras_require={{":{marker}": ["barbazquux"]}},'
+        req = tmpl.format(marker=self.invalid_marker)
+        self._setup_script_with_requires(req)
+        with pytest.raises(AssertionError):
+            self._run_install_command(tmpdir_cwd, env)
+        assert glob.glob(os.path.join(env.paths['lib'], 'barbazquux*')) == []
+
+    def test_extras_require_with_invalid_marker_in_req(self, tmpdir_cwd, env):
+        tmpl = 'extras_require={{"extra": ["barbazquux; {marker}"]}},'
+        req = tmpl.format(marker=self.invalid_marker)
+        self._setup_script_with_requires(req)
         with pytest.raises(AssertionError):
             self._run_install_command(tmpdir_cwd, env)
+        assert glob.glob(os.path.join(env.paths['lib'], 'barbazquux*')) == []
 
     def test_python_requires_egg_info(self, tmpdir_cwd, env):
         self._setup_script_with_requires(
diff --git a/setuptools/tests/test_msvc.py b/setuptools/tests/test_msvc.py
index fbeed1d5..32d7a907 100644
--- a/setuptools/tests/test_msvc.py
+++ b/setuptools/tests/test_msvc.py
@@ -5,7 +5,7 @@ Tests for msvc support module.
 import os
 import contextlib
 import distutils.errors
-from unittest import mock
+import mock
 
 import pytest
 
diff --git a/setuptools/tests/test_virtualenv.py b/setuptools/tests/test_virtualenv.py
new file mode 100644
index 00000000..a7f485a4
--- /dev/null
+++ b/setuptools/tests/test_virtualenv.py
@@ -0,0 +1,50 @@
+import glob
+import os
+
+from pytest import yield_fixture
+from pytest_fixture_config import yield_requires_config
+
+import pytest_virtualenv
+
+
+@yield_requires_config(pytest_virtualenv.CONFIG, ['virtualenv_executable'])
+@yield_fixture(scope='function')
+def bare_virtualenv():
+    """ Bare virtualenv (no pip/setuptools/wheel).
+    """
+    with pytest_virtualenv.VirtualEnv(args=(
+        '--no-wheel',
+        '--no-pip',
+        '--no-setuptools',
+    )) as venv:
+        yield venv
+
+
+SOURCE_DIR = os.path.join(os.path.dirname(__file__), '../..')
+
+def test_clean_env_install(bare_virtualenv):
+    """
+    Check setuptools can be installed in a clean environment.
+    """
+    bare_virtualenv.run(' && '.join((
+        'cd {source}',
+        'python setup.py install',
+    )).format(source=SOURCE_DIR))
+
+def test_pip_upgrade_from_source(virtualenv):
+    """
+    Check pip can upgrade setuptools from source.
+    """
+    dist_dir = virtualenv.workspace
+    # Generate source distribution / wheel.
+    virtualenv.run(' && '.join((
+        'cd {source}',
+        'python setup.py -q sdist -d {dist}',
+        'python setup.py -q bdist_wheel -d {dist}',
+    )).format(source=SOURCE_DIR, dist=dist_dir))
+    sdist = glob.glob(os.path.join(dist_dir, '*.zip'))[0]
+    wheel = glob.glob(os.path.join(dist_dir, '*.whl'))[0]
+    # Then update from wheel.
+    virtualenv.run('pip install ' + wheel)
+    # And finally try to upgrade from source.
+    virtualenv.run('pip install --no-cache-dir --upgrade ' + sdist)
diff --git a/tests/clean_install.sh b/tests/clean_install.sh
deleted file mode 100755
index f8f80dc7..00000000
--- a/tests/clean_install.sh
+++ /dev/null
@@ -1,27 +0,0 @@
-#!/usr/bin/env bash
-
-# This test was created in
-# https://github.com/pypa/setuptools/pull/1050
-# but it really should be incorporated into the test suite
-# such that it runs on Windows and doesn't depend on
-# virtualenv. Moving to test_integration will likely address
-# those concerns.
-
-set -o errexit
-set -o xtrace
-
-# Create a temporary directory to install the virtualenv in
-VENV_DIR="$(mktemp -d)"
-function cleanup() {
-  rm -rf "$VENV_DIR"
-}
-trap cleanup EXIT
-
-# Create a virtualenv that doesn't have pip or setuptools installed
-wget https://raw.githubusercontent.com/pypa/virtualenv/master/virtualenv.py
-python virtualenv.py --no-wheel --no-pip --no-setuptools "$VENV_DIR"
-source "$VENV_DIR/bin/activate"
-
-# Now try to install setuptools
-python bootstrap.py
-python setup.py install
diff --git a/tests/requirements.txt b/tests/requirements.txt
index 6e2e78e2..0c4df8ef 100644
--- a/tests/requirements.txt
+++ b/tests/requirements.txt
@@ -1,4 +1,6 @@
+importlib; python_version<"2.7"
+mock
 pytest-flake8
+pytest-virtualenv>=1.2.7
 pytest>=3.0.2
-# pinned to 1.2 as temporary workaround for #1038
-backports.unittest_mock>=1.2,<1.3
+virtualenv