Repository: pypa/setuptools (python)

Issue:
setup.cfg metadata doesnt correctly support setup_requires
while experimenting with pushing all metadata into setup.cfg i noticed that when using setup_requires - the packages only seem to be considered when preinstalled or listed in `setup.py`

Pull request:
support `setup_requires` in setup.cfg
Fix #1054.

The patch is not straightforward because: parsing `setup.cfg` is not part of `Distribution.__init__` (it's done later explicitly by `distutils.setup`), but installing `setup_requires`' requirements is (and this must be kept early, so all options passed to `setup` are correctly interpreted)... Additionally, instantiating a `Distribution` object with no arguments should not raise an error if there's an invalid `setup.cfg` in the current directory.

Patch:
diff --git a/CHANGES.rst b/CHANGES.rst
index 6b84c03f5..e3d82cc2c 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -1,3 +1,8 @@
+v36.7.0
+-------
+
+* #1054: Support ``setup_requires`` in ``setup.cfg`` files.
+
 v36.6.1
 -------
 
diff --git a/setuptools/__init__.py b/setuptools/__init__.py
index 04f767408..7da47fbed 100644
--- a/setuptools/__init__.py
+++ b/setuptools/__init__.py
@@ -109,7 +109,27 @@ class PEP420PackageFinder(PackageFinder):
 
 find_packages = PackageFinder.find
 
-setup = distutils.core.setup
+
+def _install_setup_requires(attrs):
+    # Note: do not use `setuptools.Distribution` directly, as
+    # our PEP 517 backend patch `distutils.core.Distribution`.
+    dist = distutils.core.Distribution(dict(
+        (k, v) for k, v in attrs.items()
+        if k in ('dependency_links', 'setup_requires')
+    ))
+    # Honor setup.cfg's options.
+    dist.parse_config_files(ignore_option_errors=True)
+    if dist.setup_requires:
+        dist.fetch_build_eggs(dist.setup_requires)
+
+
+def setup(**attrs):
+    # Make sure we have any requirements needed to interpret 'attrs'.
+    _install_setup_requires(attrs)
+    return distutils.core.setup(**attrs)
+
+setup.__doc__ = distutils.core.setup.__doc__
+
 
 _Command = monkey.get_unpatched(distutils.core.Command)
 
diff --git a/setuptools/dist.py b/setuptools/dist.py
index a2ca87951..aa304500e 100644
--- a/setuptools/dist.py
+++ b/setuptools/dist.py
@@ -316,23 +316,19 @@ class Distribution(Distribution_parse_config_files, _Distribution):
         have_package_data = hasattr(self, "package_data")
         if not have_package_data:
             self.package_data = {}
-        _attrs_dict = attrs or {}
-        if 'features' in _attrs_dict or 'require_features' in _attrs_dict:
+        attrs = attrs or {}
+        if 'features' in attrs or 'require_features' in attrs:
             Feature.warn_deprecated()
         self.require_features = []
         self.features = {}
         self.dist_files = []
-        self.src_root = attrs and attrs.pop("src_root", None)
+        self.src_root = attrs.pop("src_root", None)
         self.patch_missing_pkg_info(attrs)
-        self.long_description_content_type = _attrs_dict.get(
+        self.long_description_content_type = attrs.get(
             'long_description_content_type'
         )
-        # Make sure we have any eggs needed to interpret 'attrs'
-        if attrs is not None:
-            self.dependency_links = attrs.pop('dependency_links', [])
-            assert_string_list(self, 'dependency_links', self.dependency_links)
-        if attrs and 'setup_requires' in attrs:
-            self.fetch_build_eggs(attrs['setup_requires'])
+        self.dependency_links = attrs.pop('dependency_links', [])
+        self.setup_requires = attrs.pop('setup_requires', [])
         for ep in pkg_resources.iter_entry_points('distutils.setup_keywords'):
             vars(self).setdefault(ep.name, None)
         _Distribution.__init__(self, attrs)
@@ -427,14 +423,15 @@ class Distribution(Distribution_parse_config_files, _Distribution):
         req.marker = None
         return req
 
-    def parse_config_files(self, filenames=None):
+    def parse_config_files(self, filenames=None, ignore_option_errors=False):
         """Parses configuration files from various levels
         and loads configuration.
 
         """
         _Distribution.parse_config_files(self, filenames=filenames)
 
-        parse_configuration(self, self.command_options)
+        parse_configuration(self, self.command_options,
+                            ignore_option_errors=ignore_option_errors)
         self._finalize_requires()
 
     def parse_command_line(self):
@@ -497,19 +494,20 @@ class Distribution(Distribution_parse_config_files, _Distribution):
         """Fetch an egg needed for building"""
         from setuptools.command.easy_install import easy_install
         dist = self.__class__({'script_args': ['easy_install']})
-        dist.parse_config_files()
         opts = dist.get_option_dict('easy_install')
-        keep = (
-            'find_links', 'site_dirs', 'index_url', 'optimize',
-            'site_dirs', 'allow_hosts'
-        )
-        for key in list(opts):
-            if key not in keep:
-                del opts[key]  # don't use any other settings
+        opts.clear()
+        opts.update(
+            (k, v)
+            for k, v in self.get_option_dict('easy_install').items()
+            if k in (
+                # don't use any other settings
+                'find_links', 'site_dirs', 'index_url',
+                'optimize', 'site_dirs', 'allow_hosts',
+        ))
         if self.dependency_links:
             links = self.dependency_links[:]
             if 'find_links' in opts:
-                links = opts['find_links'][1].split() + links
+                links = opts['find_links'][1] + links
             opts['find_links'] = ('setup', links)
         install_dir = self.get_egg_cache_dir()
         cmd = easy_install(


Test patch:
diff --git a/setuptools/tests/test_dist.py b/setuptools/tests/test_dist.py
index 435ffec0f..c4c9bd038 100644
--- a/setuptools/tests/test_dist.py
+++ b/setuptools/tests/test_dist.py
@@ -39,6 +39,7 @@ def test_dist_fetch_build_egg(tmpdir):
     '''.split()
     with tmpdir.as_cwd():
         dist = Distribution()
+        dist.parse_config_files()
         resolved_dists = [
             dist.fetch_build_egg(r)
             for r in reqs
diff --git a/setuptools/tests/test_easy_install.py b/setuptools/tests/test_easy_install.py
index c9d396f4e..1d3390c56 100644
--- a/setuptools/tests/test_easy_install.py
+++ b/setuptools/tests/test_easy_install.py
@@ -381,7 +381,15 @@ class TestSetupRequires:
                 """))])
             yield dist_path
 
-    def test_setup_requires_overrides_version_conflict(self):
+    use_setup_cfg = (
+        (),
+        ('dependency_links',),
+        ('setup_requires',),
+        ('dependency_links', 'setup_requires'),
+    )
+
+    @pytest.mark.parametrize('use_setup_cfg', use_setup_cfg)
+    def test_setup_requires_overrides_version_conflict(self, use_setup_cfg):
         """
         Regression test for distribution issue 323:
         https://bitbucket.org/tarek/distribute/issues/323
@@ -397,7 +405,7 @@ class TestSetupRequires:
 
         with contexts.save_pkg_resources_state():
             with contexts.tempdir() as temp_dir:
-                test_pkg = create_setup_requires_package(temp_dir)
+                test_pkg = create_setup_requires_package(temp_dir, use_setup_cfg=use_setup_cfg)
                 test_setup_py = os.path.join(test_pkg, 'setup.py')
                 with contexts.quiet() as (stdout, stderr):
                     # Don't even need to install the package, just
@@ -406,9 +414,10 @@ class TestSetupRequires:
 
                 lines = stdout.readlines()
                 assert len(lines) > 0
-                assert lines[-1].strip(), 'test_pkg'
+                assert lines[-1].strip() == 'test_pkg'
 
-    def test_setup_requires_override_nspkg(self):
+    @pytest.mark.parametrize('use_setup_cfg', use_setup_cfg)
+    def test_setup_requires_override_nspkg(self, use_setup_cfg):
         """
         Like ``test_setup_requires_overrides_version_conflict`` but where the
         ``setup_requires`` package is part of a namespace package that has
@@ -446,7 +455,8 @@ class TestSetupRequires:
                 """)
 
                 test_pkg = create_setup_requires_package(
-                    temp_dir, 'foo.bar', '0.2', make_nspkg_sdist, template)
+                    temp_dir, 'foo.bar', '0.2', make_nspkg_sdist, template,
+                    use_setup_cfg=use_setup_cfg)
 
                 test_setup_py = os.path.join(test_pkg, 'setup.py')
 
@@ -464,6 +474,38 @@ class TestSetupRequires:
                 assert len(lines) > 0
                 assert lines[-1].strip() == 'test_pkg'
 
+    @pytest.mark.parametrize('use_setup_cfg', use_setup_cfg)
+    def test_setup_requires_with_attr_version(self, use_setup_cfg):
+        def make_dependency_sdist(dist_path, distname, version):
+            make_sdist(dist_path, [
+                ('setup.py',
+                 DALS("""
+                      import setuptools
+                      setuptools.setup(
+                          name={name!r},
+                          version={version!r},
+                          py_modules=[{name!r}],
+                      )
+                      """.format(name=distname, version=version))),
+                (distname + '.py',
+                 DALS("""
+                      version = 42
+                      """
+                     ))])
+        with contexts.save_pkg_resources_state():
+            with contexts.tempdir() as temp_dir:
+                test_pkg = create_setup_requires_package(
+                    temp_dir, setup_attrs=dict(version='attr: foobar.version'),
+                    make_package=make_dependency_sdist,
+                    use_setup_cfg=use_setup_cfg+('version',),
+                )
+                test_setup_py = os.path.join(test_pkg, 'setup.py')
+                with contexts.quiet() as (stdout, stderr):
+                    run_setup(test_setup_py, ['--version'])
+                lines = stdout.readlines()
+                assert len(lines) > 0
+                assert lines[-1].strip() == '42'
+
 
 def make_trivial_sdist(dist_path, distname, version):
     """
@@ -532,7 +574,8 @@ def make_sdist(dist_path, files):
 
 def create_setup_requires_package(path, distname='foobar', version='0.1',
                                   make_package=make_trivial_sdist,
-                                  setup_py_template=None):
+                                  setup_py_template=None, setup_attrs={},
+                                  use_setup_cfg=()):
     """Creates a source tree under path for a trivial test package that has a
     single requirement in setup_requires--a tarball for that requirement is
     also created and added to the dependency_links argument.
@@ -547,11 +590,39 @@ def create_setup_requires_package(path, distname='foobar', version='0.1',
         'setup_requires': ['%s==%s' % (distname, version)],
         'dependency_links': [os.path.abspath(path)]
     }
+    test_setup_attrs.update(setup_attrs)
 
     test_pkg = os.path.join(path, 'test_pkg')
-    test_setup_py = os.path.join(test_pkg, 'setup.py')
     os.mkdir(test_pkg)
 
+    if use_setup_cfg:
+        test_setup_cfg = os.path.join(test_pkg, 'setup.cfg')
+        options = []
+        metadata = []
+        for name in use_setup_cfg:
+            value = test_setup_attrs.pop(name)
+            if name in 'name version'.split():
+                section = metadata
+            else:
+                section = options
+            if isinstance(value, (tuple, list)):
+                value = ';'.join(value)
+            section.append('%s: %s' % (name, value))
+        with open(test_setup_cfg, 'w') as f:
+            f.write(DALS(
+                """
+                [metadata]
+                {metadata}
+                [options]
+                {options}
+                """
+            ).format(
+                options='\n'.join(options),
+                metadata='\n'.join(metadata),
+            ))
+
+    test_setup_py = os.path.join(test_pkg, 'setup.py')
+
     if setup_py_template is None:
         setup_py_template = DALS("""\
             import setuptools