Merge pull request #1180 from benoit-pierre/fix_889_and_non-ascii_in_setup.cfg_take_2

improve encoding handling for `setup.cfg`

7 files changed, 289 insertions, 89 deletions

diff --git a/changelog.d/1180.change.rst b/changelog.d/1180.change.rst
new file mode 100644
index 00000000..2e0f78bf
--- /dev/null
+++ b/changelog.d/1180.change.rst
@@ -0,0 +1 @@
+Add support for non-ASCII in setup.cfg (#1062). Add support for native strings on some parameters (#1136).
diff --git a/setuptools/__init__.py b/setuptools/__init__.py
index e438036a..a71b2bbd 100644
--- a/setuptools/__init__.py
+++ b/setuptools/__init__.py
@@ -5,12 +5,14 @@ import sys
 import functools
 import distutils.core
 import distutils.filelist
+import re
+from distutils.errors import DistutilsOptionError
 from distutils.util import convert_path
 from fnmatch import fnmatchcase
 
 from ._deprecation_warning import SetuptoolsDeprecationWarning
 
-from setuptools.extern.six import PY3
+from setuptools.extern.six import PY3, string_types
 from setuptools.extern.six.moves import filter, map
 
 import setuptools.version
@@ -161,6 +163,37 @@ class Command(_Command):
         _Command.__init__(self, dist)
         vars(self).update(kw)
 
+    def _ensure_stringlike(self, option, what, default=None):
+        val = getattr(self, option)
+        if val is None:
+            setattr(self, option, default)
+            return default
+        elif not isinstance(val, string_types):
+            raise DistutilsOptionError("'%s' must be a %s (got `%s`)"
+                                       % (option, what, val))
+        return val
+
+    def ensure_string_list(self, option):
+        r"""Ensure that 'option' is a list of strings.  If 'option' is
+        currently a string, we split it either on /,\s*/ or /\s+/, so
+        "foo bar baz", "foo,bar,baz", and "foo,   bar baz" all become
+        ["foo", "bar", "baz"].
+        """
+        val = getattr(self, option)
+        if val is None:
+            return
+        elif isinstance(val, string_types):
+            setattr(self, option, re.split(r',\s*|\s+', val))
+        else:
+            if isinstance(val, list):
+                ok = all(isinstance(v, string_types) for v in val)
+            else:
+                ok = False
+            if not ok:
+                raise DistutilsOptionError(
+                      "'%s' must be a list of strings (got %r)"
+                      % (option, val))
+
     def reinitialize_command(self, command, reinit_subcommands=0, **kw):
         cmd = _Command.reinitialize_command(self, command, reinit_subcommands)
         vars(cmd).update(kw)
diff --git a/setuptools/dist.py b/setuptools/dist.py
index 7062ae8d..b8551228 100644
--- a/setuptools/dist.py
+++ b/setuptools/dist.py
@@ -1,6 +1,8 @@
 # -*- coding: utf-8 -*-
 __all__ = ['Distribution']
 
+import io
+import sys
 import re
 import os
 import warnings
@@ -9,9 +11,12 @@ import distutils.log
 import distutils.core
 import distutils.cmd
 import distutils.dist
+from distutils.errors import DistutilsOptionError
+from distutils.util import strtobool
+from distutils.debug import DEBUG
+from distutils.fancy_getopt import translate_longopt
 import itertools
 
-
 from collections import defaultdict
 from email import message_from_file
 
@@ -31,8 +36,8 @@ from setuptools.depends import Require
 from setuptools import windows_support
 from setuptools.monkey import get_unpatched
 from setuptools.config import parse_configuration
+from .unicode_utils import detect_encoding
 import pkg_resources
-from .py36compat import Distribution_parse_config_files
 
 __import__('setuptools.extern.packaging.specifiers')
 __import__('setuptools.extern.packaging.version')
@@ -332,7 +337,7 @@ def check_packages(dist, attr, value):
 _Distribution = get_unpatched(distutils.core.Distribution)
 
 
-class Distribution(Distribution_parse_config_files, _Distribution):
+class Distribution(_Distribution):
     """Distribution with support for features, tests, and package data
 
     This is an enhanced version of 'distutils.dist.Distribution' that
@@ -556,12 +561,125 @@ class Distribution(Distribution_parse_config_files, _Distribution):
         req.marker = None
         return req
 
+    def _parse_config_files(self, filenames=None):
+        """
+        Adapted from distutils.dist.Distribution.parse_config_files,
+        this method provides the same functionality in subtly-improved
+        ways.
+        """
+        from setuptools.extern.six.moves.configparser import ConfigParser
+
+        # Ignore install directory options if we have a venv
+        if six.PY3 and sys.prefix != sys.base_prefix:
+            ignore_options = [
+                'install-base', 'install-platbase', 'install-lib',
+                'install-platlib', 'install-purelib', 'install-headers',
+                'install-scripts', 'install-data', 'prefix', 'exec-prefix',
+                'home', 'user', 'root']
+        else:
+            ignore_options = []
+
+        ignore_options = frozenset(ignore_options)
+
+        if filenames is None:
+            filenames = self.find_config_files()
+
+        if DEBUG:
+            self.announce("Distribution.parse_config_files():")
+
+        parser = ConfigParser()
+        for filename in filenames:
+            with io.open(filename, 'rb') as fp:
+                encoding = detect_encoding(fp)
+                if DEBUG:
+                    self.announce("  reading %s [%s]" % (
+                        filename, encoding or 'locale')
+                    )
+                reader = io.TextIOWrapper(fp, encoding=encoding)
+                (parser.read_file if six.PY3 else parser.readfp)(reader)
+            for section in parser.sections():
+                options = parser.options(section)
+                opt_dict = self.get_option_dict(section)
+
+                for opt in options:
+                    if opt != '__name__' and opt not in ignore_options:
+                        val = parser.get(section, opt)
+                        opt = opt.replace('-', '_')
+                        opt_dict[opt] = (filename, val)
+
+            # Make the ConfigParser forget everything (so we retain
+            # the original filenames that options come from)
+            parser.__init__()
+
+        # If there was a "global" section in the config file, use it
+        # to set Distribution options.
+
+        if 'global' in self.command_options:
+            for (opt, (src, val)) in self.command_options['global'].items():
+                alias = self.negative_opt.get(opt)
+                try:
+                    if alias:
+                        setattr(self, alias, not strtobool(val))
+                    elif opt in ('verbose', 'dry_run'):  # ugh!
+                        setattr(self, opt, strtobool(val))
+                    else:
+                        setattr(self, opt, val)
+                except ValueError as msg:
+                    raise DistutilsOptionError(msg)
+
+    def _set_command_options(self, command_obj, option_dict=None):
+        """
+        Set the options for 'command_obj' from 'option_dict'.  Basically
+        this means copying elements of a dictionary ('option_dict') to
+        attributes of an instance ('command').
+
+        'command_obj' must be a Command instance.  If 'option_dict' is not
+        supplied, uses the standard option dictionary for this command
+        (from 'self.command_options').
+
+        (Adopted from distutils.dist.Distribution._set_command_options)
+        """
+        command_name = command_obj.get_command_name()
+        if option_dict is None:
+            option_dict = self.get_option_dict(command_name)
+
+        if DEBUG:
+            self.announce("  setting options for '%s' command:" % command_name)
+        for (option, (source, value)) in option_dict.items():
+            if DEBUG:
+                self.announce("    %s = %s (from %s)" % (option, value,
+                                                         source))
+            try:
+                bool_opts = [translate_longopt(o)
+                             for o in command_obj.boolean_options]
+            except AttributeError:
+                bool_opts = []
+            try:
+                neg_opt = command_obj.negative_opt
+            except AttributeError:
+                neg_opt = {}
+
+            try:
+                is_string = isinstance(value, six.string_types)
+                if option in neg_opt and is_string:
+                    setattr(command_obj, neg_opt[option], not strtobool(value))
+                elif option in bool_opts and is_string:
+                    setattr(command_obj, option, strtobool(value))
+                elif hasattr(command_obj, option):
+                    setattr(command_obj, option, value)
+                else:
+                    raise DistutilsOptionError(
+                        "error in %s: command '%s' has no such option '%s'"
+                        % (source, command_name, option))
+            except ValueError as msg:
+                raise DistutilsOptionError(msg)
+
     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)
+        self._parse_config_files(filenames=filenames)
 
         parse_configuration(self, self.command_options,
                             ignore_option_errors=ignore_option_errors)
diff --git a/setuptools/py36compat.py b/setuptools/py36compat.py
deleted file mode 100644
index f5279696..00000000
--- a/setuptools/py36compat.py
+++ /dev/null
@@ -1,82 +0,0 @@
-import sys
-from distutils.errors import DistutilsOptionError
-from distutils.util import strtobool
-from distutils.debug import DEBUG
-
-
-class Distribution_parse_config_files:
-    """
-    Mix-in providing forward-compatibility for functionality to be
-    included by default on Python 3.7.
-
-    Do not edit the code in this class except to update functionality
-    as implemented in distutils.
-    """
-    def parse_config_files(self, filenames=None):
-        from configparser import ConfigParser
-
-        # Ignore install directory options if we have a venv
-        if sys.prefix != sys.base_prefix:
-            ignore_options = [
-                'install-base', 'install-platbase', 'install-lib',
-                'install-platlib', 'install-purelib', 'install-headers',
-                'install-scripts', 'install-data', 'prefix', 'exec-prefix',
-                'home', 'user', 'root']
-        else:
-            ignore_options = []
-
-        ignore_options = frozenset(ignore_options)
-
-        if filenames is None:
-            filenames = self.find_config_files()
-
-        if DEBUG:
-            self.announce("Distribution.parse_config_files():")
-
-        parser = ConfigParser(interpolation=None)
-        for filename in filenames:
-            if DEBUG:
-                self.announce("  reading %s" % filename)
-            parser.read(filename)
-            for section in parser.sections():
-                options = parser.options(section)
-                opt_dict = self.get_option_dict(section)
-
-                for opt in options:
-                    if opt != '__name__' and opt not in ignore_options:
-                        val = parser.get(section,opt)
-                        opt = opt.replace('-', '_')
-                        opt_dict[opt] = (filename, val)
-
-            # Make the ConfigParser forget everything (so we retain
-            # the original filenames that options come from)
-            parser.__init__()
-
-        # If there was a "global" section in the config file, use it
-        # to set Distribution options.
-
-        if 'global' in self.command_options:
-            for (opt, (src, val)) in self.command_options['global'].items():
-                alias = self.negative_opt.get(opt)
-                try:
-                    if alias:
-                        setattr(self, alias, not strtobool(val))
-                    elif opt in ('verbose', 'dry_run'): # ugh!
-                        setattr(self, opt, strtobool(val))
-                    else:
-                        setattr(self, opt, val)
-                except ValueError as msg:
-                    raise DistutilsOptionError(msg)
-
-
-if sys.version_info < (3,):
-    # Python 2 behavior is sufficient
-    class Distribution_parse_config_files:
-        pass
-
-
-if False:
-    # When updated behavior is available upstream,
-    # disable override here.
-    class Distribution_parse_config_files:
-        pass
diff --git a/setuptools/tests/test_config.py b/setuptools/tests/test_config.py
index a6b44b9f..6b177709 100644
--- a/setuptools/tests/test_config.py
+++ b/setuptools/tests/test_config.py
@@ -1,3 +1,6 @@
+# -*- coding: UTF-8 -*-
+from __future__ import unicode_literals
+
 import contextlib
 import pytest
 
@@ -5,6 +8,8 @@ from distutils.errors import DistutilsOptionError, DistutilsFileError
 from mock import patch
 from setuptools.dist import Distribution, _Distribution
 from setuptools.config import ConfigHandler, read_configuration
+from setuptools.extern.six.moves.configparser import InterpolationMissingOptionError
+from setuptools.tests import is_ascii
 from . import py2_only, py3_only
 from .textwrap import DALS
 
@@ -24,7 +29,7 @@ def make_package_dir(name, base_dir, ns=False):
     return dir_package, init_file
 
 
-def fake_env(tmpdir, setup_cfg, setup_py=None, package_path='fake_package'):
+def fake_env(tmpdir, setup_cfg, setup_py=None, encoding='ascii', package_path='fake_package'):
 
     if setup_py is None:
         setup_py = (
@@ -34,7 +39,7 @@ def fake_env(tmpdir, setup_cfg, setup_py=None, package_path='fake_package'):
 
     tmpdir.join('setup.py').write(setup_py)
     config = tmpdir.join('setup.cfg')
-    config.write(setup_cfg)
+    config.write(setup_cfg.encode(encoding), mode='wb')
 
     package_dir, init_file = make_package_dir(package_path, tmpdir)
 
@@ -429,6 +434,72 @@ class TestMetadata:
                 assert metadata.description == 'Some description'
                 assert metadata.requires == ['some', 'requirement']
 
+    def test_interpolation(self, tmpdir):
+        fake_env(
+            tmpdir,
+            '[metadata]\n'
+            'description = %(message)s\n'
+        )
+        with pytest.raises(InterpolationMissingOptionError):
+            with get_dist(tmpdir):
+                pass
+
+    skip_if_not_ascii = pytest.mark.skipif(not is_ascii, reason='Test not supported with this locale')
+
+    @skip_if_not_ascii
+    def test_non_ascii_1(self, tmpdir):
+        fake_env(
+            tmpdir,
+            '[metadata]\n'
+            'description = éàïôñ\n',
+            encoding='utf-8'
+        )
+        with pytest.raises(UnicodeDecodeError):
+            with get_dist(tmpdir):
+                pass
+
+    def test_non_ascii_2(self, tmpdir):
+        fake_env(
+            tmpdir,
+            '# -*- coding: invalid\n'
+        )
+        with pytest.raises(LookupError):
+            with get_dist(tmpdir):
+                pass
+
+    def test_non_ascii_3(self, tmpdir):
+        fake_env(
+            tmpdir,
+            '\n'
+            '# -*- coding: invalid\n'
+        )
+        with get_dist(tmpdir):
+            pass
+
+    @skip_if_not_ascii
+    def test_non_ascii_4(self, tmpdir):
+        fake_env(
+            tmpdir,
+            '# -*- coding: utf-8\n'
+            '[metadata]\n'
+            'description = éàïôñ\n',
+            encoding='utf-8'
+        )
+        with get_dist(tmpdir) as dist:
+            assert dist.metadata.description == 'éàïôñ'
+
+    @skip_if_not_ascii
+    def test_non_ascii_5(self, tmpdir):
+        fake_env(
+            tmpdir,
+            '# vim: set fileencoding=iso-8859-15 :\n'
+            '[metadata]\n'
+            'description = éàïôñ\n',
+            encoding='iso-8859-15'
+        )
+        with get_dist(tmpdir) as dist:
+            assert dist.metadata.description == 'éàïôñ'
+
 
 class TestOptions:
 
diff --git a/setuptools/tests/test_egg_info.py b/setuptools/tests/test_egg_info.py
index 571e6054..d5fa2558 100644
--- a/setuptools/tests/test_egg_info.py
+++ b/setuptools/tests/test_egg_info.py
@@ -652,3 +652,49 @@ class TestEggInfo:
 
     def test_get_pkg_info_revision_deprecated(self):
         pytest.warns(EggInfoDeprecationWarning, get_pkg_info_revision)
+
+    EGG_INFO_TESTS = (
+        # Check for issue #1136: invalid string type when
+        # reading declarative `setup.cfg` under Python 2.
+        {
+            'setup.py': DALS(
+                """
+                from setuptools import setup
+                setup(
+                    name="foo",
+                )
+                """),
+            'setup.cfg': DALS(
+                """
+                [options]
+                package_dir =
+                    = src
+                """),
+            'src': {},
+        },
+        # Check Unicode can be used in `setup.py` under Python 2.
+        {
+            'setup.py': DALS(
+                """
+                # -*- coding: utf-8 -*-
+                from __future__ import unicode_literals
+                from setuptools import setup, find_packages
+                setup(
+                    name="foo",
+                    package_dir={'': 'src'},
+                )
+                """),
+            'src': {},
+        }
+    )
+
+    @pytest.mark.parametrize('package_files', EGG_INFO_TESTS)
+    def test_egg_info(self, tmpdir_cwd, env, package_files):
+        """
+        """
+        build_files(package_files)
+        code, data = environment.run_setup_py(
+            cmd=['egg_info'],
+            data_stream=1,
+        )
+        assert not code, data
diff --git a/setuptools/unicode_utils.py b/setuptools/unicode_utils.py
index 7c63efd2..3b8179a8 100644
--- a/setuptools/unicode_utils.py
+++ b/setuptools/unicode_utils.py
@@ -1,5 +1,6 @@
 import unicodedata
 import sys
+import re
 
 from setuptools.extern import six
 
@@ -42,3 +43,15 @@ def try_encode(string, enc):
         return string.encode(enc)
     except UnicodeEncodeError:
         return None
+
+
+CODING_RE = re.compile(br'^[ \t\f]*#.*?coding[:=][ \t]*([-\w.]+)')
+
+
+def detect_encoding(fp):
+    first_line = fp.readline()
+    fp.seek(0)
+    m = CODING_RE.match(first_line)
+    if m is None:
+        return None
+    return m.group(1).decode('ascii')