UnicodeDecodeError when setup.cfg contains non-ASCII and LC_ALL=C

[benoit-pierre]

> cat >setup.py <<\EOF
# -*- coding: utf-8 -*-
from setuptools import setup
setup(
name='test',
version='1.0',
description='éàïñ',
)
EOF
> env LC_ALL=C python3 ./setup.py --description
éàïñ
> cat >setup.cfg <<\EOF
[metadata]
name = test
version = 1.0
description = éàïñ
EOF
> cat >setup.py <<\EOF
from setuptools import setup
setup()
EOF
> env LC_ALL=C python3 ./setup.py --description
Traceback (most recent call last):
  File "./setup.py", line 2, in <module>
    setup()
  File "/usr/lib/python3.6/distutils/core.py", line 121, in setup
    dist.parse_config_files()
  File "/usr/lib/python3.6/site-packages/setuptools/dist.py", line 355, in parse_config_files
    _Distribution.parse_config_files(self, filenames=filenames)
  File "/usr/lib/python3.6/distutils/dist.py", line 395, in parse_config_files
    parser.read(filename)
  File "/usr/lib/python3.6/configparser.py", line 697, in read
    self._read(fp, filename)
  File "/usr/lib/python3.6/configparser.py", line 1015, in _read
    for lineno, line in enumerate(fp, start=1):
  File "/usr/lib/python3.6/encodings/ascii.py", line 26, in decode
    return codecs.ascii_decode(input, self.errors)[0]
UnicodeDecodeError: 'ascii' codec can't decode byte 0xc3 in position 75: ordinal not in range(128)

With PKG-INFO always written as UTF-8, I think it would make sense to load setup.cfg as UTF-8 too.

I see in the code that there's a setuptools/py36compat.py providing a patched parse_config_files but as seen in the traceback, it's not called. Is that normal?

According to the log it was added to fix #889, but adding a test for that fails:

diff --git i/setuptools/tests/test_config.py w/setuptools/tests/test_config.py
index 8bd2a494..2b118b70 100644
--- i/setuptools/tests/test_config.py
+++ w/setuptools/tests/test_config.py
@@ -288,6 +288,15 @@ class TestMetadata:
         with get_dist(tmpdir) as dist:
             assert set(dist.metadata.classifiers) == expected

+    def test_no_interpolation(self, tmpdir):
+        fake_env(
+            tmpdir,
+            '[metadata]\n'
+            'description = %(message)s\n'
+        )
+        with get_dist(tmpdir) as dist:
+            assert dist.metadata.description == '%(message)s'
+

 class TestOptions:

Anyway, here is my attempt at fixing: master...benoit-pierre:fix_889_and_non-ascii_in_setup.cfg

I can make proper PR(s) for those changes that are OK.

[jaraco]

I started diving into this. I first found I could ignore most of the commits and focus on 9f81546. But even that single commit attempts to do too much, fixing two issues at the same time.

So I started to focus just on the first issue, that #889 seems not to be passing. I incorporated the test as dd404fd2e (thanks for that). But then when I went to find out why the test wasn't passing, all looked well except that, as you mentioned, the compatibility code isn't being called.

It seems that in 6913024 referencing #394, the compatibility code was overridden. And shame on me for not having a unit test to capture the regression.

[benoit-pierre]

Check da92ba4 for an alternative implementation for supporting this. Unfortunately, this can't be done for Python 2, as returning Unicode strings will break various things, e.g.: the sdist command when setup.cfg contains:

[sdist]
formats = zip

Because the distutils Python 2 code will check for str instances...

[jaraco]

Okay, the fix for #889 is now in the master. There were two flaws. One was that the compatibility behavior was not being invoked. The other was that the compatibility behavior was not working properly because the interpolation=None had to be called in two places.

[jaraco]

I should think that Setuptools should be able to support non-ascii characters in a setup.cfg file even if the target system is LANG=C. I believe LANG=C only affects default encoding behavior. Where setuptools can specify encoding behavior, the tests should run despite LANG=C.

But one thing I'm noticing is that these patches are affecting the behavior in py36compat Distribution_parse_config_files, where the docstring specifically cautions against editing that code. That code was copied from cpython and should match what's being released. If we want to tweak that behavior, we should probably contribute that behavior upstream first and then provide forward compatibility to Setuptools users by copying that code to the compat module.

[benoit-pierre]

The interpolation test still fails on Python 2 because Distribution_parse_config_files is not used. And if it is used, then interpolation=None cannot be used with Python 2, and parser.get(...,raw=True) must be used instead.

diff --git i/setuptools/py36compat.py w/setuptools/py36compat.py
--- i/setuptools/py36compat.py
+++ w/setuptools/py36compat.py
@@ -2,6 +2,8 @@ import sys
 from distutils.errors import DistutilsOptionError
 from distutils.util import strtobool
 from distutils.debug import DEBUG
+from setuptools.extern import six
+from setuptools.extern.six.moves.configparser import ConfigParser
 
 
 class Distribution_parse_config_files:
@@ -13,10 +15,9 @@ class Distribution_parse_config_files:
     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:
+        if six.PY3 and sys.prefix != sys.base_prefix:
             ignore_options = [
                 'install-base', 'install-platbase', 'install-lib',
                 'install-platlib', 'install-purelib', 'install-headers',
@@ -33,7 +34,7 @@ class Distribution_parse_config_files:
         if DEBUG:
             self.announce("Distribution.parse_config_files():")
 
-        parser = ConfigParser(interpolation=None)
+        parser = ConfigParser()
         for filename in filenames:
             if DEBUG:
                 self.announce("  reading %s" % filename)
@@ -44,13 +45,13 @@ class Distribution_parse_config_files:
 
                 for opt in options:
                     if opt != '__name__' and opt not in ignore_options:
-                        val = parser.get(section,opt)
+                        val = parser.get(section,opt,raw=True)
                         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__(interpolation=None)
+            parser.__init__()
 
         # If there was a "global" section in the config file, use it
         # to set Distribution options.
@@ -69,12 +70,6 @@ class Distribution_parse_config_files:
                     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.

[jaraco]

@benoit-pierre I've published a call for help in distutils-sig. Given your passion and involvement, I want to acknowledge that you may be interested in assisting in this effort. If so, please comment in one of the tickets (#889 or the sister distutils ticket).

[uchuugaka]

Hi there, does this issue still persist today for 2.7.x ?