bpo-19610: Warn if distutils is provided something other than a list to some fields (#4685)

* Rather than raise TypeError, warn and call list() on the value.

* Fix tests, revise NEWS and whatsnew text.

* Revise documentation, a string is okay as well.

* Ensure 'requires' and 'obsoletes' are real lists.

* Test that requires and obsoletes are turned to lists.

Doc/distutils/apiref.rst

@@ -286,9 +286,9 @@ the full reference.
    Distribution constructor. :func:`setup` creates a Distribution instance.
 
    .. versionchanged:: 3.7
-      :class:`~distutils.core.Distribution` now raises a :exc:`TypeError` if
+      :class:`~distutils.core.Distribution` now warns if ``classifiers``,
-      ``classifiers``, ``keywords`` and ``platforms`` fields are not specified
+      ``keywords`` and ``platforms`` fields are not specified as a list or
-      as a list.
+      a string.
 
 .. class:: Command
 

Doc/whatsnew/3.7.rst

@@ -298,10 +298,8 @@ README.rst is now included in the list of distutils standard READMEs and
 therefore included in source distributions.
 (Contributed by Ryan Gonzalez in :issue:`11913`.)
 
-:class:`distutils.core.setup` now raises a :exc:`TypeError` if
+:class:`distutils.core.setup` now warns if the ``classifiers``, ``keywords``
-``classifiers``, ``keywords`` and ``platforms`` fields are not specified
+and ``platforms`` fields are not specified as a list or a string.
-as a list.  However, to minimize backwards incompatibility concerns,
-``keywords`` and ``platforms`` fields still accept a comma separated string.
 (Contributed by Berker Peksag in :issue:`19610`.)
 
 http.client

Lib/distutils/dist.py

@@ -27,6 +27,20 @@ from distutils.debug import DEBUG
 command_re = re.compile(r'^[a-zA-Z]([a-zA-Z0-9_]*)$')
 
 
+def _ensure_list(value, fieldname):
+    if isinstance(value, str):
+        # a string containing comma separated values is okay.  It will
+        # be converted to a list by Distribution.finalize_options().
+        pass
+    elif not isinstance(value, list):
+        # passing a tuple or an iterator perhaps, warn and convert
+        typename = type(value).__name__
+        msg = f"Warning: '{fieldname}' should be a list, got type '{typename}'"
+        log.log(log.WARN, msg)
+        value = list(value)
+    return value
+
+
 class Distribution:
     """The core of the Distutils.  Most of the work hiding behind 'setup'
     is really done within a Distribution instance, which farms the work out
@@ -257,10 +271,7 @@ Common commands: (see '--help-commands' for more)
                     setattr(self, key, val)
                 else:
                     msg = "Unknown distribution option: %s" % repr(key)
-                    if warnings is not None:
+                    warnings.warn(msg)
-                        warnings.warn(msg)
-                    else:
-                        sys.stderr.write(msg + "\n")
 
         # no-user-cfg is handled before other command line args
         # because other args override the config files, and this
@@ -1189,36 +1200,19 @@ class DistributionMetadata:
         return self.keywords or []
 
     def set_keywords(self, value):
-        # If 'keywords' is a string, it will be converted to a list
+        self.keywords = _ensure_list(value, 'keywords')
-        # by Distribution.finalize_options(). To maintain backwards
-        # compatibility, do not raise an exception if 'keywords' is
-        # a string.
-        if not isinstance(value, (list, str)):
-            msg = "'keywords' should be a 'list', not %r"
-            raise TypeError(msg % type(value).__name__)
-        self.keywords = value
 
     def get_platforms(self):
         return self.platforms or ["UNKNOWN"]
 
     def set_platforms(self, value):
-        # If 'platforms' is a string, it will be converted to a list
+        self.platforms = _ensure_list(value, 'platforms')
-        # by Distribution.finalize_options(). To maintain backwards
-        # compatibility, do not raise an exception if 'platforms' is
-        # a string.
-        if not isinstance(value, (list, str)):
-            msg = "'platforms' should be a 'list', not %r"
-            raise TypeError(msg % type(value).__name__)
-        self.platforms = value
 
     def get_classifiers(self):
         return self.classifiers or []
 
     def set_classifiers(self, value):
-        if not isinstance(value, list):
+        self.classifiers = _ensure_list(value, 'classifiers')
-            msg = "'classifiers' should be a 'list', not %r"
-            raise TypeError(msg % type(value).__name__)
-        self.classifiers = value
 
     def get_download_url(self):
         return self.download_url or "UNKNOWN"
@@ -1231,7 +1225,7 @@ class DistributionMetadata:
         import distutils.versionpredicate
         for v in value:
             distutils.versionpredicate.VersionPredicate(v)
-        self.requires = value
+        self.requires = list(value)
 
     def get_provides(self):
         return self.provides or []
@@ -1250,7 +1244,7 @@ class DistributionMetadata:
         import distutils.versionpredicate
         for v in value:
             distutils.versionpredicate.VersionPredicate(v)
-        self.obsoletes = value
+        self.obsoletes = list(value)
 
 def fix_help_options(options):
     """Convert a 4-tuple 'help_options' list as found in various command

Lib/distutils/tests/test_dist.py

@@ -11,7 +11,9 @@ from unittest import mock
 from distutils.dist import Distribution, fix_help_options, DistributionMetadata
 from distutils.cmd import Command
 
-from test.support import TESTFN, captured_stdout, run_unittest
+from test.support import (
+     TESTFN, captured_stdout, captured_stderr, run_unittest
+)
 from distutils.tests import support
 from distutils import log
 
@@ -319,6 +321,13 @@ class MetadataTestCase(support.TempdirManager, support.EnvironGuard,
                            "version": "1.0",
                            "requires": ["my.pkg (splat)"]})
 
+    def test_requires_to_list(self):
+        attrs = {"name": "package",
+                 "requires": iter(["other"])}
+        dist = Distribution(attrs)
+        self.assertIsInstance(dist.metadata.requires, list)
+
+
     def test_obsoletes(self):
         attrs = {"name": "package",
                  "version": "1.0",
@@ -341,6 +350,12 @@ class MetadataTestCase(support.TempdirManager, support.EnvironGuard,
                            "version": "1.0",
                            "obsoletes": ["my.pkg (splat)"]})
 
+    def test_obsoletes_to_list(self):
+        attrs = {"name": "package",
+                 "obsoletes": iter(["other"])}
+        dist = Distribution(attrs)
+        self.assertIsInstance(dist.metadata.obsoletes, list)
+
     def test_classifier(self):
         attrs = {'name': 'Boa', 'version': '3.0',
                  'classifiers': ['Programming Language :: Python :: 3']}
@@ -353,9 +368,14 @@ class MetadataTestCase(support.TempdirManager, support.EnvironGuard,
     def test_classifier_invalid_type(self):
         attrs = {'name': 'Boa', 'version': '3.0',
                  'classifiers': ('Programming Language :: Python :: 3',)}
-        msg = "'classifiers' should be a 'list', not 'tuple'"
+        with captured_stderr() as error:
-        with self.assertRaises(TypeError, msg=msg):
+            d = Distribution(attrs)
-            Distribution(attrs)
+        # should have warning about passing a non-list
+        self.assertIn('should be a list', error.getvalue())
+        # should be converted to a list
+        self.assertIsInstance(d.metadata.classifiers, list)
+        self.assertEqual(d.metadata.classifiers,
+                         list(attrs['classifiers']))
 
     def test_keywords(self):
         attrs = {'name': 'Monty', 'version': '1.0',
@@ -367,9 +387,13 @@ class MetadataTestCase(support.TempdirManager, support.EnvironGuard,
     def test_keywords_invalid_type(self):
         attrs = {'name': 'Monty', 'version': '1.0',
                  'keywords': ('spam', 'eggs', 'life of brian')}
-        msg = "'keywords' should be a 'list', not 'tuple'"
+        with captured_stderr() as error:
-        with self.assertRaises(TypeError, msg=msg):
+            d = Distribution(attrs)
-            Distribution(attrs)
+        # should have warning about passing a non-list
+        self.assertIn('should be a list', error.getvalue())
+        # should be converted to a list
+        self.assertIsInstance(d.metadata.keywords, list)
+        self.assertEqual(d.metadata.keywords, list(attrs['keywords']))
 
     def test_platforms(self):
         attrs = {'name': 'Monty', 'version': '1.0',
@@ -381,9 +405,13 @@ class MetadataTestCase(support.TempdirManager, support.EnvironGuard,
     def test_platforms_invalid_types(self):
         attrs = {'name': 'Monty', 'version': '1.0',
                  'platforms': ('GNU/Linux', 'Some Evil Platform')}
-        msg = "'platforms' should be a 'list', not 'tuple'"
+        with captured_stderr() as error:
-        with self.assertRaises(TypeError, msg=msg):
+            d = Distribution(attrs)
-            Distribution(attrs)
+        # should have warning about passing a non-list
+        self.assertIn('should be a list', error.getvalue())
+        # should be converted to a list
+        self.assertIsInstance(d.metadata.platforms, list)
+        self.assertEqual(d.metadata.platforms, list(attrs['platforms']))
 
     def test_download_url(self):
         attrs = {'name': 'Boa', 'version': '3.0',

Misc/NEWS.d/next/Library/2017-11-23-16-15-55.bpo-19610.Dlca2P.rst

@@ -1,5 +1,4 @@
-``setup()`` now raises :exc:`TypeError` for invalid types.
+``setup()`` now warns about invalid types for some fields.
 
-The ``distutils.dist.Distribution`` class now explicitly raises an exception
+The ``distutils.dist.Distribution`` class now warns when ``classifiers``,
-when ``classifiers``, ``keywords`` and ``platforms`` fields are not
+``keywords`` and ``platforms`` fields are not specified as a list or a string.
-specified as a list.