From cbef2949856f5e0957be1cbdfdc92325f9ae9f1e Mon Sep 17 00:00:00 2001
From: Paul Ganssle <paul@ganssle.io>
Date: Sat, 27 Oct 2018 17:21:11 -0400
Subject: [PATCH 01/19] Add upload_file to setuptools.command.upload

---
 setuptools/command/upload.py | 147 ++++++++++++++++++++++++++++++++++-
 1 file changed, 146 insertions(+), 1 deletion(-)

diff --git a/setuptools/command/upload.py b/setuptools/command/upload.py
index 72f24d8f48..dae7d74dfb 100644
--- a/setuptools/command/upload.py
+++ b/setuptools/command/upload.py
@@ -1,14 +1,24 @@
+import io
+import os
+import hashlib
 import getpass
+
+from base64 import standard_b64encode
+
 from distutils import log
 from distutils.command import upload as orig
 
+from distutils.errors import DistutilsError
+
+from six.moves.urllib.request import urlopen, Request
+from six.moves.urllib.error import HTTPError
+from six.moves.urllib.parse import urlparse
 
 class upload(orig.upload):
     """
     Override default upload behavior to obtain password
     in a variety of different ways.
     """
-
     def run(self):
         try:
             orig.upload.run(self)
@@ -33,6 +43,141 @@ def finalize_options(self):
             self._prompt_for_password()
         )
 
+    def upload_file(self, command, pyversion, filename):
+        # Makes sure the repository URL is compliant
+        schema, netloc, url, params, query, fragments = \
+            urlparse(self.repository)
+        if params or query or fragments:
+            raise AssertionError("Incompatible url %s" % self.repository)
+
+        if schema not in ('http', 'https'):
+            raise AssertionError("unsupported schema " + schema)
+
+        # Sign if requested
+        if self.sign:
+            gpg_args = ["gpg", "--detach-sign", "-a", filename]
+            if self.identity:
+                gpg_args[2:2] = ["--local-user", self.identity]
+            spawn(gpg_args,
+                  dry_run=self.dry_run)
+
+        # Fill in the data - send all the meta-data in case we need to
+        # register a new release
+        with open(filename, 'rb') as f:
+            content = f.read()
+
+        meta = self.distribution.metadata
+
+        data = {
+            # action
+            ':action': 'file_upload',
+            'protocol_version': '1',
+
+            # identify release
+            'name': meta.get_name(),
+            'version': meta.get_version(),
+
+            # file content
+            'content': (os.path.basename(filename),content),
+            'filetype': command,
+            'pyversion': pyversion,
+            'md5_digest': hashlib.md5(content).hexdigest(),
+
+            # additional meta-data
+            'metadata_version': '1.0',
+            'summary': meta.get_description(),
+            'home_page': meta.get_url(),
+            'author': meta.get_contact(),
+            'author_email': meta.get_contact_email(),
+            'license': meta.get_licence(),
+            'description': meta.get_long_description(),
+            'keywords': meta.get_keywords(),
+            'platform': meta.get_platforms(),
+            'classifiers': meta.get_classifiers(),
+            'download_url': meta.get_download_url(),
+            # PEP 314
+            'provides': meta.get_provides(),
+            'requires': meta.get_requires(),
+            'obsoletes': meta.get_obsoletes(),
+            }
+        comment = ''
+        if command == 'bdist_rpm':
+            dist, version, id = platform.dist()
+            if dist:
+                comment = 'built for %s %s' % (dist, version)
+        elif command == 'bdist_dumb':
+            comment = 'built for %s' % platform.platform(terse=1)
+        data['comment'] = comment
+
+        if self.sign:
+            data['gpg_signature'] = (os.path.basename(filename) + ".asc",
+                                     open(filename+".asc", "rb").read())
+
+        # set up the authentication
+        user_pass = (self.username + ":" + self.password).encode('ascii')
+        # The exact encoding of the authentication string is debated.
+        # Anyway PyPI only accepts ascii for both username or password.
+        auth = "Basic " + standard_b64encode(user_pass).decode('ascii')
+
+        # Build up the MIME payload for the POST data
+        boundary = '--------------GHSKFJDLGDS7543FJKLFHRE75642756743254'
+        sep_boundary = b'\r\n--' + boundary.encode('ascii')
+        end_boundary = sep_boundary + b'--\r\n'
+        body = io.BytesIO()
+        for key, value in data.items():
+            title = '\r\nContent-Disposition: form-data; name="%s"' % key
+            # handle multiple entries for the same name
+            if not isinstance(value, list):
+                value = [value]
+            for value in value:
+                if type(value) is tuple:
+                    title += '; filename="%s"' % value[0]
+                    value = value[1]
+                else:
+                    value = str(value).encode('utf-8')
+                body.write(sep_boundary)
+                body.write(title.encode('utf-8'))
+                body.write(b"\r\n\r\n")
+                body.write(value)
+        body.write(end_boundary)
+        body = body.getvalue()
+
+        msg = "Submitting %s to %s" % (filename, self.repository)
+        self.announce(msg, log.INFO)
+
+        # build the Request
+        headers = {
+            'Content-type': 'multipart/form-data; boundary=%s' % boundary,
+            'Content-length': str(len(body)),
+            'Authorization': auth,
+        }
+
+        request = Request(self.repository, data=body,
+                          headers=headers)
+        # send the data
+        try:
+            result = urlopen(request)
+            status = result.getcode()
+            reason = result.msg
+        except HTTPError as e:
+            status = e.code
+            reason = e.msg
+        except OSError as e:
+            self.announce(str(e), log.ERROR)
+            raise
+
+        if status == 200:
+            self.announce('Server response (%s): %s' % (status, reason),
+                          log.INFO)
+            if self.show_response:
+                text = self._read_pypi_response(result)
+                msg = '\n'.join(('-' * 75, text, '-' * 75))
+                self.announce(msg, log.INFO)
+        else:
+            msg = 'Upload failed (%s): %s' % (status, reason)
+            self.announce(msg, log.ERROR)
+            raise DistutilsError(msg)
+
     def _load_password_from_keyring(self):
         """
         Attempt to load password from keyring. Suppress Exceptions.

From 83fb2385518f7cf7885144f12db80b9364e67c7f Mon Sep 17 00:00:00 2001
From: Paul Ganssle <paul@ganssle.io>
Date: Tue, 6 Nov 2018 09:18:05 -0500
Subject: [PATCH 02/19] Add DistributionMetadata.read_pkg_file

This is the baseline, unchanged from the version in distutils.dist, to
be modified before patching.
---
 setuptools/dist.py | 53 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 53 insertions(+)

diff --git a/setuptools/dist.py b/setuptools/dist.py
index f6078dbe0a..44e9aa89b6 100644
--- a/setuptools/dist.py
+++ b/setuptools/dist.py
@@ -53,6 +53,59 @@ def get_metadata_version(dist_md):
     return StrictVersion('1.0')
 
 
+def read_pkg_file(self, file):
+    """Reads the metadata values from a file object."""
+    msg = message_from_file(file)
+
+    def _read_field(name):
+        value = msg[name]
+        if value == 'UNKNOWN':
+            return None
+        return value
+
+    def _read_list(name):
+        values = msg.get_all(name, None)
+        if values == []:
+            return None
+        return values
+
+    metadata_version = msg['metadata-version']
+    self.name = _read_field('name')
+    self.version = _read_field('version')
+    self.description = _read_field('summary')
+    # we are filling author only.
+    self.author = _read_field('author')
+    self.maintainer = None
+    self.author_email = _read_field('author-email')
+    self.maintainer_email = None
+    self.url = _read_field('home-page')
+    self.license = _read_field('license')
+
+    if 'download-url' in msg:
+        self.download_url = _read_field('download-url')
+    else:
+        self.download_url = None
+
+    self.long_description = _read_field('description')
+    self.description = _read_field('summary')
+
+    if 'keywords' in msg:
+        self.keywords = _read_field('keywords').split(',')
+
+    self.platforms = _read_list('platform')
+    self.classifiers = _read_list('classifier')
+
+    # PEP 314 - these fields only exist in 1.1
+    if metadata_version == '1.1':
+        self.requires = _read_list('requires')
+        self.provides = _read_list('provides')
+        self.obsoletes = _read_list('obsoletes')
+    else:
+        self.requires = None
+        self.provides = None
+        self.obsoletes = None
+
+
 # Based on Python 3.5 version
 def write_pkg_file(self, file):
     """Write the PKG-INFO format data to a file object.

From b7a6d8ad1b20e94de0b0bdebd1a6e9c8fd51695a Mon Sep 17 00:00:00 2001
From: Paul Ganssle <paul@ganssle.io>
Date: Sun, 28 Oct 2018 14:19:04 -0400
Subject: [PATCH 03/19] Add failing test for issue #1381

---
 setuptools/tests/test_upload.py | 59 +++++++++++++++++++++++++++++++++
 1 file changed, 59 insertions(+)

diff --git a/setuptools/tests/test_upload.py b/setuptools/tests/test_upload.py
index 95a8d16b11..5b8e267c76 100644
--- a/setuptools/tests/test_upload.py
+++ b/setuptools/tests/test_upload.py
@@ -1,13 +1,72 @@
 import mock
+import os
+import re
+
 from distutils import log
+from distutils.version import StrictVersion
 
 import pytest
+import six
 
 from setuptools.command.upload import upload
 from setuptools.dist import Distribution
 
 
+def _parse_upload_body(body):
+    boundary = u'\r\n----------------GHSKFJDLGDS7543FJKLFHRE75642756743254'
+    entries = []
+    name_re = re.compile(u'^Content-Disposition: form-data; name="([^\"]+)"')
+
+    for entry in body.split(boundary):
+        pair = entry.split(u'\r\n\r\n')
+        if not len(pair) == 2:
+            continue
+
+        key, value = map(six.text_type.strip, pair)
+        m = name_re.match(key)
+        if m is not None:
+            key = m.group(1)
+
+        entries.append((key, value))
+
+    return entries
+
+
 class TestUploadTest:
+    @pytest.mark.xfail(reason='Issue #1381')
+    @mock.patch('setuptools.command.upload.urlopen')
+    def test_upload_metadata(self, patch, tmpdir):
+        dist = Distribution()
+        dist.metadata.metadata_version = StrictVersion('2.1')
+
+        content = os.path.join(str(tmpdir), "test_upload_metadata_content")
+
+        with open(content, 'w') as f:
+            f.write("Some content")
+
+        dist.dist_files = [('xxx', '3.7', content)]
+
+        patch.return_value = mock.Mock()
+        patch.return_value.getcode.return_value = 200
+
+        cmd = upload(dist)
+        cmd.announce = mock.Mock()
+        cmd.username = 'user'
+        cmd.password = 'hunter2'
+        cmd.ensure_finalized()
+        cmd.run()
+
+        # Make sure we did the upload
+        patch.assert_called_once()
+
+        # Make sure the metadata version is correct in the headers
+        request = patch.call_args_list[0][0][0]
+        body = request.data.decode('utf-8')
+
+        entries = dict(_parse_upload_body(body))
+        assert entries['metadata_version'] == '2.1'
+
+
     def test_warns_deprecation(self):
         dist = Distribution()
         dist.dist_files = [(mock.Mock(), mock.Mock(), mock.Mock())]

From e5d362d3736bc5972835a55bcb165d8c55913547 Mon Sep 17 00:00:00 2001
From: Paul Ganssle <paul@ganssle.io>
Date: Tue, 6 Nov 2018 09:30:24 -0500
Subject: [PATCH 04/19] Store metadata version on metadata object

---
 setuptools/dist.py | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/setuptools/dist.py b/setuptools/dist.py
index 44e9aa89b6..5b750b624b 100644
--- a/setuptools/dist.py
+++ b/setuptools/dist.py
@@ -10,7 +10,11 @@
 import distutils.cmd
 import distutils.dist
 import itertools
+
+
 from collections import defaultdict
+from email import message_from_file
+
 from distutils.errors import (
     DistutilsOptionError, DistutilsPlatformError, DistutilsSetupError,
 )
@@ -69,7 +73,7 @@ def _read_list(name):
             return None
         return values
 
-    metadata_version = msg['metadata-version']
+    self.metadata_version = StrictVersion(msg['metadata-version'])
     self.name = _read_field('name')
     self.version = _read_field('version')
     self.description = _read_field('summary')
@@ -96,7 +100,7 @@ def _read_list(name):
     self.classifiers = _read_list('classifier')
 
     # PEP 314 - these fields only exist in 1.1
-    if metadata_version == '1.1':
+    if metadata_version == StrictVersion('1.1'):
         self.requires = _read_list('requires')
         self.provides = _read_list('provides')
         self.obsoletes = _read_list('obsoletes')

From 386fcdbe02c4170a560d67742f2ac1319430ff43 Mon Sep 17 00:00:00 2001
From: Paul Ganssle <paul@ganssle.io>
Date: Mon, 5 Nov 2018 10:23:15 -0500
Subject: [PATCH 05/19] Start patching DistributionMetadata.read_pkg_file

This turns get_metadata_version into a method on DistributionMetadata,
populated either by inferrence (in the case of package metadata
specified in `setup`) or from the data in a specified PKG-INFO file.

To populate metadata_version from PKG-INFO, we need to monkey patch
read_pkg_file in addition to write_pkg_file.
---
 setuptools/dist.py   | 33 ++++++++++++++++++++-------------
 setuptools/monkey.py | 12 ++++++------
 2 files changed, 26 insertions(+), 19 deletions(-)

diff --git a/setuptools/dist.py b/setuptools/dist.py
index 5b750b624b..ae9816fd47 100644
--- a/setuptools/dist.py
+++ b/setuptools/dist.py
@@ -43,18 +43,25 @@ def _get_unpatched(cls):
     return get_unpatched(cls)
 
 
-def get_metadata_version(dist_md):
-    if dist_md.long_description_content_type or dist_md.provides_extras:
-        return StrictVersion('2.1')
-    elif (dist_md.maintainer is not None or
-          dist_md.maintainer_email is not None or
-          getattr(dist_md, 'python_requires', None) is not None):
-        return StrictVersion('1.2')
-    elif (dist_md.provides or dist_md.requires or dist_md.obsoletes or
-            dist_md.classifiers or dist_md.download_url):
-        return StrictVersion('1.1')
+def get_metadata_version(self):
+    mv = getattr(self, 'metadata_version', None)
+
+    if mv is None:
+        if self.long_description_content_type or self.provides_extras:
+            mv = StrictVersion('2.1')
+        elif (self.maintainer is not None or
+              self.maintainer_email is not None or
+              getattr(self, 'python_requires', None) is not None):
+            mv = StrictVersion('1.2')
+        elif (self.provides or self.requires or self.obsoletes or
+                self.classifiers or self.download_url):
+            mv = StrictVersion('1.1')
+        else:
+            mv = StrictVersion('1.0')
+
+        self.metadata_version = mv
 
-    return StrictVersion('1.0')
+    return mv
 
 
 def read_pkg_file(self, file):
@@ -100,7 +107,7 @@ def _read_list(name):
     self.classifiers = _read_list('classifier')
 
     # PEP 314 - these fields only exist in 1.1
-    if metadata_version == StrictVersion('1.1'):
+    if self.metadata_version == StrictVersion('1.1'):
         self.requires = _read_list('requires')
         self.provides = _read_list('provides')
         self.obsoletes = _read_list('obsoletes')
@@ -114,7 +121,7 @@ def _read_list(name):
 def write_pkg_file(self, file):
     """Write the PKG-INFO format data to a file object.
     """
-    version = get_metadata_version(self)
+    version = self.get_metadata_version()
 
     file.write('Metadata-Version: %s\n' % version)
     file.write('Name: %s\n' % self.get_name())
diff --git a/setuptools/monkey.py b/setuptools/monkey.py
index 05a738b055..3c77f8cf27 100644
--- a/setuptools/monkey.py
+++ b/setuptools/monkey.py
@@ -84,7 +84,7 @@ def patch_all():
         warehouse = 'https://upload.pypi.org/legacy/'
         distutils.config.PyPIRCCommand.DEFAULT_REPOSITORY = warehouse
 
-    _patch_distribution_metadata_write_pkg_file()
+    _patch_distribution_metadata()
 
     # Install Distribution throughout the distutils
     for module in distutils.dist, distutils.core, distutils.cmd:
@@ -101,11 +101,11 @@ def patch_all():
     patch_for_msvc_specialized_compiler()
 
 
-def _patch_distribution_metadata_write_pkg_file():
-    """Patch write_pkg_file to also write Requires-Python/Requires-External"""
-    distutils.dist.DistributionMetadata.write_pkg_file = (
-        setuptools.dist.write_pkg_file
-    )
+def _patch_distribution_metadata():
+    """Patch write_pkg_file and read_pkg_file for higher metadata standards"""
+    for attr in ('write_pkg_file', 'read_pkg_file', 'get_metadata_version'):
+        new_val = getattr(setuptools.dist, attr)
+        setattr(distutils.dist.DistributionMetadata, attr, new_val)
 
 
 def patch_func(replacement, target_mod, func_name):

From 33185837dbc1f75f7894b9cbc3e56c1c6a868c4c Mon Sep 17 00:00:00 2001
From: Paul Ganssle <paul@ganssle.io>
Date: Mon, 5 Nov 2018 10:26:50 -0500
Subject: [PATCH 06/19] Use get_metadata_version in upload_file

Previously this value was hard-coded to '1.0', which was inaccurate for
many packages.

Fixes #1381
---
 setuptools/command/upload.py    | 2 +-
 setuptools/tests/test_upload.py | 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/setuptools/command/upload.py b/setuptools/command/upload.py
index dae7d74dfb..01fa026c2e 100644
--- a/setuptools/command/upload.py
+++ b/setuptools/command/upload.py
@@ -84,7 +84,7 @@ def upload_file(self, command, pyversion, filename):
             'md5_digest': hashlib.md5(content).hexdigest(),
 
             # additional meta-data
-            'metadata_version': '1.0',
+            'metadata_version': str(meta.get_metadata_version()),
             'summary': meta.get_description(),
             'home_page': meta.get_url(),
             'author': meta.get_contact(),
diff --git a/setuptools/tests/test_upload.py b/setuptools/tests/test_upload.py
index 5b8e267c76..b18f83c8d0 100644
--- a/setuptools/tests/test_upload.py
+++ b/setuptools/tests/test_upload.py
@@ -33,7 +33,6 @@ def _parse_upload_body(body):
 
 
 class TestUploadTest:
-    @pytest.mark.xfail(reason='Issue #1381')
     @mock.patch('setuptools.command.upload.urlopen')
     def test_upload_metadata(self, patch, tmpdir):
         dist = Distribution()

From 54466884a9458bc77ecfe3a30ac0f1b21126006b Mon Sep 17 00:00:00 2001
From: Paul Ganssle <paul@ganssle.io>
Date: Mon, 5 Nov 2018 10:35:45 -0500
Subject: [PATCH 07/19] Add changelog for PR #1576

---
 changelog.d/1576.change.rst | 1 +
 1 file changed, 1 insertion(+)
 create mode 100644 changelog.d/1576.change.rst

diff --git a/changelog.d/1576.change.rst b/changelog.d/1576.change.rst
new file mode 100644
index 0000000000..85c578ed4f
--- /dev/null
+++ b/changelog.d/1576.change.rst
@@ -0,0 +1 @@
+Started monkey-patching ``get_metadata_version`` and ``read_pkg_file`` onto ``distutils.DistributionMetadata`` to retain the correct version on the ``PKG-INFO`` file in the (deprecated) ``upload`` command.

From 9ffb099b1154662bf1fa8da2a9e0cbef22a2a809 Mon Sep 17 00:00:00 2001
From: Paul Ganssle <paul@ganssle.io>
Date: Tue, 6 Nov 2018 09:10:00 -0500
Subject: [PATCH 08/19] Add test for read_pkg_file

---
 setuptools/tests/test_dist.py | 96 +++++++++++++++++++++++++++++++++++
 1 file changed, 96 insertions(+)

diff --git a/setuptools/tests/test_dist.py b/setuptools/tests/test_dist.py
index 223ad90c95..36605ceabd 100644
--- a/setuptools/tests/test_dist.py
+++ b/setuptools/tests/test_dist.py
@@ -59,6 +59,102 @@ def sdist_with_index(distname, version):
 def test_dist__get_unpatched_deprecated():
     pytest.warns(DistDeprecationWarning, _get_unpatched, [""])
 
+
+def __read_test_cases():
+    # Metadata version 1.0
+    base_attrs = {
+        "name": "package",
+        "version": "0.0.1",
+        "author": "Foo Bar",
+        "author_email": "foo@bar.net",
+        "long_description": "Long\ndescription",
+        "description": "Short description",
+        "keywords": ["one", "two"]
+    }
+
+    def merge_dicts(d1, d2):
+        d1 = d1.copy()
+        d1.update(d2)
+
+        return d1
+
+    test_cases = [
+        ('Metadata version 1.0', base_attrs.copy()),
+        ('Metadata version 1.1: Provides', merge_dicts(base_attrs, {
+            'provides': ['package']
+        })),
+        ('Metadata version 1.1: Obsoletes', merge_dicts(base_attrs, {
+            'obsoletes': ['foo']
+        })),
+        ('Metadata version 1.1: Classifiers', merge_dicts(base_attrs, {
+            'classifiers': [
+                'Programming Language :: Python :: 3',
+                'Programming Language :: Python :: 3.7',
+                'License :: OSI Approved :: MIT License'
+        ]})),
+        ('Metadata version 1.1: Download URL', merge_dicts(base_attrs, {
+            'download_url': 'https://example.com'
+        })),
+        ('Metadata Version 1.2: Requires-Python', merge_dicts(base_attrs, {
+            'python_requires': '>=3.7'
+        })),
+        pytest.param('Metadata Version 1.2: Project-Url',
+            merge_dicts(base_attrs, {
+                'project_urls': {
+                    'Foo': 'https://example.bar'
+                }
+            }), marks=pytest.mark.xfail(
+                reason="Issue #1578: project_urls not read"
+        )),
+        ('Metadata Version 2.1: Long Description Content Type',
+         merge_dicts(base_attrs, {
+             'long_description_content_type': 'text/x-rst; charset=UTF-8'
+         })),
+        pytest.param('Metadata Version 2.1: Provides Extra',
+            merge_dicts(base_attrs, {
+                'provides_extras': ['foo', 'bar']
+        }), marks=pytest.mark.xfail(reason="provides_extras not read")),
+    ]
+
+    return test_cases
+
+
+@pytest.mark.parametrize('name,attrs', __read_test_cases())
+def test_read_metadata(name, attrs, tmpdir):
+    dist = Distribution(attrs)
+    metadata_out = dist.metadata
+    dist_class = metadata_out.__class__
+
+    # Write to PKG_INFO and then load into a new metadata object
+    fn = tmpdir.mkdir('pkg_info')
+    fn_s = str(fn)
+
+    metadata_out.write_pkg_info(fn_s)
+
+    metadata_in = dist_class()
+    with io.open(str(fn.join('PKG-INFO')), 'r', encoding='utf-8') as f:
+        metadata_in.read_pkg_file(f)
+
+    tested_attrs = [
+        ('name', dist_class.get_name),
+        ('version', dist_class.get_version),
+        ('metadata_version', dist_class.get_metadata_version),
+        ('provides', dist_class.get_provides),
+        ('description', dist_class.get_description),
+        ('download_url', dist_class.get_download_url),
+        ('keywords', dist_class.get_keywords),
+        ('platforms', dist_class.get_platforms),
+        ('obsoletes', dist_class.get_obsoletes),
+        ('requires', dist_class.get_requires),
+        ('classifiers', dist_class.get_classifiers),
+        ('project_urls', lambda s: getattr(s, 'project_urls', {})),
+        ('provides_extras', lambda s: getattr(s, 'provides_extras', set())),
+    ]
+
+    for attr, getter in tested_attrs:
+        assert getter(metadata_in) == getter(metadata_out)
+
+
 def __maintainer_test_cases():
     attrs = {"name": "package",
              "version": "1.0",

From c34962d08168e0149a71485f1f71ddfd22146140 Mon Sep 17 00:00:00 2001
From: Paul Ganssle <paul@ganssle.io>
Date: Tue, 6 Nov 2018 11:48:47 -0500
Subject: [PATCH 09/19] Use write_field in write_pkg_file

This creates a wrapper function for writing fields in the PKG-INFO file,
both to simplify the syntax and to add a point where we can inject an
encoding function in order to support Python 2.7 compatibility.
---
 setuptools/dist.py | 46 ++++++++++++++++++++++++++--------------------
 1 file changed, 26 insertions(+), 20 deletions(-)

diff --git a/setuptools/dist.py b/setuptools/dist.py
index ae9816fd47..b741c64880 100644
--- a/setuptools/dist.py
+++ b/setuptools/dist.py
@@ -123,15 +123,23 @@ def write_pkg_file(self, file):
     """
     version = self.get_metadata_version()
 
-    file.write('Metadata-Version: %s\n' % version)
-    file.write('Name: %s\n' % self.get_name())
-    file.write('Version: %s\n' % self.get_version())
-    file.write('Summary: %s\n' % self.get_description())
-    file.write('Home-page: %s\n' % self.get_url())
+    if six.PY2:
+        def write_field(key, value):
+            file.write("%s: %s\n" % (key, self._encode_field(value)))
+    else:
+        def write_field(key, value):
+            file.write("%s: %s\n" % (key, value))
+
+
+    write_field('Metadata-Version', str(version))
+    write_field('Name', self.get_name())
+    write_field('Version', self.get_version())
+    write_field('Summary', self.get_description())
+    write_field('Home-page', self.get_url())
 
     if version < StrictVersion('1.2'):
-        file.write('Author: %s\n' % self.get_contact())
-        file.write('Author-email: %s\n' % self.get_contact_email())
+        write_field('Author:', self.get_contact())
+        write_field('Author-email:', self.get_contact_email())
     else:
         optional_fields = (
             ('Author', 'author'),
@@ -142,28 +150,26 @@ def write_pkg_file(self, file):
 
         for field, attr in optional_fields:
             attr_val = getattr(self, attr)
-            if six.PY2:
-                attr_val = self._encode_field(attr_val)
 
             if attr_val is not None:
-                file.write('%s: %s\n' % (field, attr_val))
+                write_field(field, attr_val)
 
-    file.write('License: %s\n' % self.get_license())
+    write_field('License', self.get_license())
     if self.download_url:
-        file.write('Download-URL: %s\n' % self.download_url)
+        write_field('Download-URL', self.download_url)
     for project_url in self.project_urls.items():
-        file.write('Project-URL: %s, %s\n' % project_url)
+        write_field('Project-URL',  '%s, %s' % project_url)
 
     long_desc = rfc822_escape(self.get_long_description())
-    file.write('Description: %s\n' % long_desc)
+    write_field('Description', long_desc)
 
     keywords = ','.join(self.get_keywords())
     if keywords:
-        file.write('Keywords: %s\n' % keywords)
+        write_field('Keywords', keywords)
 
     if version >= StrictVersion('1.2'):
         for platform in self.get_platforms():
-            file.write('Platform: %s\n' % platform)
+            write_field('Platform', platform)
     else:
         self._write_list(file, 'Platform', self.get_platforms())
 
@@ -176,17 +182,17 @@ def write_pkg_file(self, file):
 
     # Setuptools specific for PEP 345
     if hasattr(self, 'python_requires'):
-        file.write('Requires-Python: %s\n' % self.python_requires)
+        write_field('Requires-Python', self.python_requires)
 
     # PEP 566
     if self.long_description_content_type:
-        file.write(
-            'Description-Content-Type: %s\n' %
+        write_field(
+            'Description-Content-Type',
             self.long_description_content_type
         )
     if self.provides_extras:
         for extra in self.provides_extras:
-            file.write('Provides-Extra: %s\n' % extra)
+            write_field('Provides-Extra', extra)
 
 
 sequence = tuple, list

From bef22678626f4e201ef1bb6c5bc753dd01e6bf5a Mon Sep 17 00:00:00 2001
From: Paul Ganssle <paul@ganssle.io>
Date: Tue, 6 Nov 2018 11:51:42 -0500
Subject: [PATCH 10/19] Use an in-memory IO object instead of a temp file

Rather than writing to a file in a temporary directory, we can write to
and read from an in-memory buffer, now that the encoding functionality
in write_pkg_file is fixed.
---
 setuptools/tests/test_dist.py | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/setuptools/tests/test_dist.py b/setuptools/tests/test_dist.py
index 36605ceabd..a7f4452b1a 100644
--- a/setuptools/tests/test_dist.py
+++ b/setuptools/tests/test_dist.py
@@ -12,6 +12,7 @@
 from .test_easy_install import make_nspkg_sdist
 
 import pytest
+import six
 
 
 def test_dist_fetch_build_egg(tmpdir):
@@ -120,20 +121,22 @@ def merge_dicts(d1, d2):
 
 
 @pytest.mark.parametrize('name,attrs', __read_test_cases())
-def test_read_metadata(name, attrs, tmpdir):
+def test_read_metadata(name, attrs):
     dist = Distribution(attrs)
     metadata_out = dist.metadata
     dist_class = metadata_out.__class__
 
     # Write to PKG_INFO and then load into a new metadata object
-    fn = tmpdir.mkdir('pkg_info')
-    fn_s = str(fn)
+    if six.PY2:
+        PKG_INFO = io.BytesIO()
+    else:
+        PKG_INFO = io.StringIO()
 
-    metadata_out.write_pkg_info(fn_s)
+    metadata_out.write_pkg_file(PKG_INFO)
 
+    PKG_INFO.seek(0)
     metadata_in = dist_class()
-    with io.open(str(fn.join('PKG-INFO')), 'r', encoding='utf-8') as f:
-        metadata_in.read_pkg_file(f)
+    metadata_in.read_pkg_file(PKG_INFO)
 
     tested_attrs = [
         ('name', dist_class.get_name),

From dfe1b3afb433c7e1d0679c005407ac12104e6141 Mon Sep 17 00:00:00 2001
From: Paul Ganssle <paul@ganssle.io>
Date: Wed, 7 Nov 2018 17:15:22 -0500
Subject: [PATCH 11/19] Add upload fixture

This is a fixture to create an upload command with a patched version of
urlopen so that no HTTP queries are sent.
---
 setuptools/tests/test_upload.py | 45 +++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/setuptools/tests/test_upload.py b/setuptools/tests/test_upload.py
index b18f83c8d0..1b70301b5a 100644
--- a/setuptools/tests/test_upload.py
+++ b/setuptools/tests/test_upload.py
@@ -32,6 +32,51 @@ def _parse_upload_body(body):
     return entries
 
 
+@pytest.fixture
+def patched_upload(tmpdir):
+    class Fix:
+        def __init__(self, cmd, urlopen):
+            self.cmd = cmd
+            self.urlopen = urlopen
+
+        def __iter__(self):
+            return iter((self.cmd, self.urlopen))
+
+        def get_uploaded_metadata(self):
+            request = self.urlopen.call_args_list[0][0][0]
+            body = request.data.decode('utf-8')
+            entries = dict(_parse_upload_body(body))
+
+            return entries
+
+    class ResponseMock(mock.Mock):
+        def getheader(self, name, default=None):
+            """Mocked getheader method for response object"""
+            return {
+                'content-type': 'text/plain; charset=utf-8',
+            }.get(name.lower(), default)
+
+    with mock.patch('setuptools.command.upload.urlopen') as urlopen:
+        urlopen.return_value = ResponseMock()
+        urlopen.return_value.getcode.return_value = 200
+        urlopen.return_value.read.return_value = b''
+
+        content = os.path.join(str(tmpdir), "content_data")
+
+        with open(content, 'w') as f:
+            f.write("Some content")
+
+        dist = Distribution()
+        dist.dist_files = [('sdist', '3.7.0', content)]
+
+        cmd = upload(dist)
+        cmd.announce = mock.Mock()
+        cmd.username = 'user'
+        cmd.password = 'hunter2'
+
+        yield Fix(cmd, urlopen)
+
+
 class TestUploadTest:
     @mock.patch('setuptools.command.upload.urlopen')
     def test_upload_metadata(self, patch, tmpdir):

From f4458afd8c83a233ae637af1d77e77404d2da1e5 Mon Sep 17 00:00:00 2001
From: Paul Ganssle <paul@ganssle.io>
Date: Wed, 7 Nov 2018 17:21:05 -0500
Subject: [PATCH 12/19] Use the patched_upload fixture in upload_metadata

`test_upload_metadata` was written before the fixture, so this updates
the test to use the fixture.
---
 setuptools/tests/test_upload.py | 27 ++++++---------------------
 1 file changed, 6 insertions(+), 21 deletions(-)

diff --git a/setuptools/tests/test_upload.py b/setuptools/tests/test_upload.py
index 1b70301b5a..caabb8866c 100644
--- a/setuptools/tests/test_upload.py
+++ b/setuptools/tests/test_upload.py
@@ -78,25 +78,13 @@ def getheader(self, name, default=None):
 
 
 class TestUploadTest:
-    @mock.patch('setuptools.command.upload.urlopen')
-    def test_upload_metadata(self, patch, tmpdir):
-        dist = Distribution()
-        dist.metadata.metadata_version = StrictVersion('2.1')
-
-        content = os.path.join(str(tmpdir), "test_upload_metadata_content")
-
-        with open(content, 'w') as f:
-            f.write("Some content")
-
-        dist.dist_files = [('xxx', '3.7', content)]
+    def test_upload_metadata(self, patched_upload):
+        cmd, patch = patched_upload
 
-        patch.return_value = mock.Mock()
-        patch.return_value.getcode.return_value = 200
+        # Set the metadata version to 2.1
+        cmd.distribution.metadata.metadata_version = '2.1'
 
-        cmd = upload(dist)
-        cmd.announce = mock.Mock()
-        cmd.username = 'user'
-        cmd.password = 'hunter2'
+        # Run the command
         cmd.ensure_finalized()
         cmd.run()
 
@@ -104,10 +92,7 @@ def test_upload_metadata(self, patch, tmpdir):
         patch.assert_called_once()
 
         # Make sure the metadata version is correct in the headers
-        request = patch.call_args_list[0][0][0]
-        body = request.data.decode('utf-8')
-
-        entries = dict(_parse_upload_body(body))
+        entries = patched_upload.get_uploaded_metadata()
         assert entries['metadata_version'] == '2.1'
 
 

From 7417740076af72f49705088009d0b21afea7dd98 Mon Sep 17 00:00:00 2001
From: Paul Ganssle <paul@ganssle.io>
Date: Wed, 7 Nov 2018 11:43:21 -0500
Subject: [PATCH 13/19] Add test for invalid URLs in upload_file

---
 setuptools/tests/test_upload.py | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/setuptools/tests/test_upload.py b/setuptools/tests/test_upload.py
index caabb8866c..6f497f086c 100644
--- a/setuptools/tests/test_upload.py
+++ b/setuptools/tests/test_upload.py
@@ -95,7 +95,6 @@ def test_upload_metadata(self, patched_upload):
         entries = patched_upload.get_uploaded_metadata()
         assert entries['metadata_version'] == '2.1'
 
-
     def test_warns_deprecation(self):
         dist = Distribution()
         dist.dist_files = [(mock.Mock(), mock.Mock(), mock.Mock())]
@@ -129,3 +128,21 @@ def test_warns_deprecation_when_raising(self):
             "upload instead (https://pypi.org/p/twine/)",
             log.WARN
         )
+
+    @pytest.mark.parametrize('url', [
+        'https://example.com/a;parameter',    # Has parameters
+        'https://example.com/a?query',        # Has query
+        'https://example.com/a#fragment',     # Has fragment
+        'ftp://example.com',                  # Invalid scheme
+
+    ])
+    def test_upload_file_invalid_url(self, url, patched_upload):
+        patched_upload.urlopen.side_effect = Exception("Should not be reached")
+
+        cmd = patched_upload.cmd
+        cmd.repository = url
+
+        cmd.ensure_finalized()
+        with pytest.raises(AssertionError):
+            cmd.run()
+

From 77b661a9599225721ac416cc342d56d1afb105a1 Mon Sep 17 00:00:00 2001
From: Paul Ganssle <paul@ganssle.io>
Date: Wed, 7 Nov 2018 15:43:08 -0500
Subject: [PATCH 14/19] Add test for HTTPError in upload_file

---
 setuptools/tests/test_upload.py | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/setuptools/tests/test_upload.py b/setuptools/tests/test_upload.py
index 6f497f086c..129159a7b8 100644
--- a/setuptools/tests/test_upload.py
+++ b/setuptools/tests/test_upload.py
@@ -3,6 +3,7 @@
 import re
 
 from distutils import log
+from distutils.errors import DistutilsError
 from distutils.version import StrictVersion
 
 import pytest
@@ -146,3 +147,22 @@ def test_upload_file_invalid_url(self, url, patched_upload):
         with pytest.raises(AssertionError):
             cmd.run()
 
+    def test_upload_file_http_error(self, patched_upload):
+        patched_upload.urlopen.side_effect = six.moves.urllib.error.HTTPError(
+            'https://example.com',
+            404,
+            'File not found',
+            None,
+            None
+        )
+
+        cmd = patched_upload.cmd
+        cmd.ensure_finalized()
+
+        with pytest.raises(DistutilsError):
+            cmd.run()
+
+        cmd.announce.assert_any_call(
+            'Upload failed (404): File not found',
+            log.ERROR)
+

From 1bca7ffdea25ee7ae7d335d676b0804a2f467d52 Mon Sep 17 00:00:00 2001
From: Paul Ganssle <paul@ganssle.io>
Date: Wed, 7 Nov 2018 16:22:41 -0500
Subject: [PATCH 15/19] Add test for OSError in upload_file

---
 setuptools/tests/test_upload.py | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/setuptools/tests/test_upload.py b/setuptools/tests/test_upload.py
index 129159a7b8..6aaac0753b 100644
--- a/setuptools/tests/test_upload.py
+++ b/setuptools/tests/test_upload.py
@@ -166,3 +166,13 @@ def test_upload_file_http_error(self, patched_upload):
             'Upload failed (404): File not found',
             log.ERROR)
 
+    def test_upload_file_os_error(self, patched_upload):
+        patched_upload.urlopen.side_effect = OSError("Invalid")
+
+        cmd = patched_upload.cmd
+        cmd.ensure_finalized()
+
+        with pytest.raises(OSError):
+            cmd.run()
+
+        cmd.announce.assert_any_call('Invalid', log.ERROR)

From b5c9c5f42db36a07dc27d39c1be2a311cc567d99 Mon Sep 17 00:00:00 2001
From: Paul Ganssle <paul@ganssle.io>
Date: Wed, 7 Nov 2018 16:23:13 -0500
Subject: [PATCH 16/19] Fix gpg signature code in upload_file

This fixes an issue where `distutils.spawn.spawn` was not available in
the ported upload_file, which is only used when signing the data.

This also adds a test that the gpg signature command is invoked and
included in the uploaded data.
---
 setuptools/command/upload.py    |  1 +
 setuptools/tests/test_upload.py | 26 ++++++++++++++++++++++++++
 2 files changed, 27 insertions(+)

diff --git a/setuptools/command/upload.py b/setuptools/command/upload.py
index 01fa026c2e..1851ed28b3 100644
--- a/setuptools/command/upload.py
+++ b/setuptools/command/upload.py
@@ -7,6 +7,7 @@
 
 from distutils import log
 from distutils.command import upload as orig
+from distutils.spawn import spawn
 
 from distutils.errors import DistutilsError
 
diff --git a/setuptools/tests/test_upload.py b/setuptools/tests/test_upload.py
index 6aaac0753b..3a1bbba9b6 100644
--- a/setuptools/tests/test_upload.py
+++ b/setuptools/tests/test_upload.py
@@ -176,3 +176,29 @@ def test_upload_file_os_error(self, patched_upload):
             cmd.run()
 
         cmd.announce.assert_any_call('Invalid', log.ERROR)
+
+    @mock.patch('setuptools.command.upload.spawn')
+    def test_upload_file_gpg(self, spawn, patched_upload):
+        cmd, urlopen = patched_upload
+
+        cmd.sign = True
+        cmd.identity = "Alice"
+        cmd.dry_run = True
+        content_fname = cmd.distribution.dist_files[0][2]
+        signed_file = content_fname + '.asc'
+
+        with open(signed_file, 'wb') as f:
+            f.write("signed-data".encode('utf-8'))
+
+        cmd.ensure_finalized()
+        cmd.run()
+
+        # Make sure that GPG was called
+        spawn.assert_called_once_with([
+            "gpg", "--detach-sign", "--local-user", "Alice", "-a",
+            content_fname
+        ], dry_run=True)
+
+        # Read the 'signed' data that was transmitted
+        entries = patched_upload.get_uploaded_metadata()
+        assert entries['gpg_signature'] == 'signed-data'

From 727dd60f6a11f38d165250c543ba135687fa2e61 Mon Sep 17 00:00:00 2001
From: Paul Ganssle <paul@ganssle.io>
Date: Wed, 7 Nov 2018 16:41:19 -0500
Subject: [PATCH 17/19] Fix bdist_rpm and bdist_dumb in upload_file

This fixes uploads when bdist_rpm or bdist_dumb are the command, both of
which insert a comment about what platform they are built for.
---
 setuptools/command/upload.py    |  1 +
 setuptools/tests/test_upload.py | 21 +++++++++++++++++++++
 2 files changed, 22 insertions(+)

diff --git a/setuptools/command/upload.py b/setuptools/command/upload.py
index 1851ed28b3..99d86011b7 100644
--- a/setuptools/command/upload.py
+++ b/setuptools/command/upload.py
@@ -2,6 +2,7 @@
 import os
 import hashlib
 import getpass
+import platform
 
 from base64 import standard_b64encode
 
diff --git a/setuptools/tests/test_upload.py b/setuptools/tests/test_upload.py
index 3a1bbba9b6..7bf8e31268 100644
--- a/setuptools/tests/test_upload.py
+++ b/setuptools/tests/test_upload.py
@@ -202,3 +202,24 @@ def test_upload_file_gpg(self, spawn, patched_upload):
         # Read the 'signed' data that was transmitted
         entries = patched_upload.get_uploaded_metadata()
         assert entries['gpg_signature'] == 'signed-data'
+
+    @pytest.mark.parametrize('bdist', ['bdist_rpm', 'bdist_dumb'])
+    @mock.patch('setuptools.command.upload.platform')
+    def test_bdist_rpm_upload(self, platform, bdist, patched_upload):
+        # Set the upload command to include bdist_rpm
+        cmd = patched_upload.cmd
+        dist_files = cmd.distribution.dist_files
+        dist_files = [(bdist,) + dist_files[0][1:]]
+        cmd.distribution.dist_files = dist_files
+
+        # Mock out the platform commands to make this platform-independent
+        platform.dist.return_value = ('redhat', '', '')
+
+        cmd.ensure_finalized()
+        cmd.run()
+
+        entries = patched_upload.get_uploaded_metadata()
+
+        assert entries['comment'].startswith(u'built for')
+        assert len(entries['comment']) > len(u'built for')
+

From fe2c9e4292699635c91174bc049aefe81bf6116c Mon Sep 17 00:00:00 2001
From: Paul Ganssle <paul@ganssle.io>
Date: Wed, 7 Nov 2018 17:07:58 -0500
Subject: [PATCH 18/19] Fix show_response behavior on Python 2

The `upload.show_response` feature was not added until Python 3. Rather
than backport it, it is now enabled only if supported.

This also adds a "smoke test" for the feature.
---
 setuptools/command/upload.py    | 8 +++++---
 setuptools/tests/test_upload.py | 9 +++++++++
 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/setuptools/command/upload.py b/setuptools/command/upload.py
index 99d86011b7..f57fe796be 100644
--- a/setuptools/command/upload.py
+++ b/setuptools/command/upload.py
@@ -172,9 +172,11 @@ def upload_file(self, command, pyversion, filename):
             self.announce('Server response (%s): %s' % (status, reason),
                           log.INFO)
             if self.show_response:
-                text = self._read_pypi_response(result)
-                msg = '\n'.join(('-' * 75, text, '-' * 75))
-                self.announce(msg, log.INFO)
+                text = getattr(self, '_read_pypi_response',
+                               lambda x: None)(result)
+                if text is not None:
+                    msg = '\n'.join(('-' * 75, text, '-' * 75))
+                    self.announce(msg, log.INFO)
         else:
             msg = 'Upload failed (%s): %s' % (status, reason)
             self.announce(msg, log.ERROR)
diff --git a/setuptools/tests/test_upload.py b/setuptools/tests/test_upload.py
index 7bf8e31268..319ed7a220 100644
--- a/setuptools/tests/test_upload.py
+++ b/setuptools/tests/test_upload.py
@@ -223,3 +223,12 @@ def test_bdist_rpm_upload(self, platform, bdist, patched_upload):
         assert entries['comment'].startswith(u'built for')
         assert len(entries['comment']) > len(u'built for')
 
+    def test_show_response_no_error(self, patched_upload):
+        # This test is just that show_response doesn't throw an error
+        # It is not really important what the printed response looks like
+        # in a deprecated command, but we don't want to introduce new
+        # errors when importing this function from distutils
+
+        patched_upload.cmd.show_response = True
+        patched_upload.cmd.ensure_finalized()
+        patched_upload.cmd.run()

From 2b5b91332a01c665cab77ad7962e87525850d7f5 Mon Sep 17 00:00:00 2001
From: Paul Ganssle <paul@ganssle.io>
Date: Mon, 12 Nov 2018 10:08:55 -0500
Subject: [PATCH 19/19] Remove bdist_rpm and bdist_dumb comment

This comment is not used anywhere and `platform.dist()` is deprecated.

See CPython PR #10414: https://github.com/python/cpython/pull/10414
and bpo-35186: https://bugs.python.org/issue35186
---
 setuptools/command/upload.py    | 10 ++--------
 setuptools/tests/test_upload.py | 20 --------------------
 2 files changed, 2 insertions(+), 28 deletions(-)

diff --git a/setuptools/command/upload.py b/setuptools/command/upload.py
index f57fe796be..3b8cab5e06 100644
--- a/setuptools/command/upload.py
+++ b/setuptools/command/upload.py
@@ -102,14 +102,8 @@ def upload_file(self, command, pyversion, filename):
             'requires': meta.get_requires(),
             'obsoletes': meta.get_obsoletes(),
             }
-        comment = ''
-        if command == 'bdist_rpm':
-            dist, version, id = platform.dist()
-            if dist:
-                comment = 'built for %s %s' % (dist, version)
-        elif command == 'bdist_dumb':
-            comment = 'built for %s' % platform.platform(terse=1)
-        data['comment'] = comment
+
+        data['comment'] = ''
 
         if self.sign:
             data['gpg_signature'] = (os.path.basename(filename) + ".asc",
diff --git a/setuptools/tests/test_upload.py b/setuptools/tests/test_upload.py
index 319ed7a220..9229bba172 100644
--- a/setuptools/tests/test_upload.py
+++ b/setuptools/tests/test_upload.py
@@ -203,26 +203,6 @@ def test_upload_file_gpg(self, spawn, patched_upload):
         entries = patched_upload.get_uploaded_metadata()
         assert entries['gpg_signature'] == 'signed-data'
 
-    @pytest.mark.parametrize('bdist', ['bdist_rpm', 'bdist_dumb'])
-    @mock.patch('setuptools.command.upload.platform')
-    def test_bdist_rpm_upload(self, platform, bdist, patched_upload):
-        # Set the upload command to include bdist_rpm
-        cmd = patched_upload.cmd
-        dist_files = cmd.distribution.dist_files
-        dist_files = [(bdist,) + dist_files[0][1:]]
-        cmd.distribution.dist_files = dist_files
-
-        # Mock out the platform commands to make this platform-independent
-        platform.dist.return_value = ('redhat', '', '')
-
-        cmd.ensure_finalized()
-        cmd.run()
-
-        entries = patched_upload.get_uploaded_metadata()
-
-        assert entries['comment'].startswith(u'built for')
-        assert len(entries['comment']) > len(u'built for')
-
     def test_show_response_no_error(self, patched_upload):
         # This test is just that show_response doesn't throw an error
         # It is not really important what the printed response looks like