Repository: pypa/setuptools
Pull request #1898 (2019-10-31 15:30:21)

Remove "upload" and "register" commands.
## Summary of changes
The upload and register commands were deprecated over a year ago, in July 2018 (PR #1410, discussed in issue #1381). It is time to actively remove them in favor of twine.

I think the most controversial part of this might be that I've added `setuptools.errors`. It wasn't clear what the best error class to use would be. `distutils` will use `raise SystemExit` for a missing command; that didn't seem quite like the right thing to do, but I'm willing to be convinced otherwise.

CC @pypa/setuptools-developers @di 

### Pull Request Checklist
- [X] Changes have tests
- [x] News fragment added in changelog.d. See [documentation](http://setuptools.readthedocs.io/en/latest/developer-guide.html#making-a-pull-request) for details


pganssle: Oops, turns out that if you name your classes `UploadTest` instead of `TestUpload` it doesn't get detected by `pytest`. I guess that's why they were originally named `Test(Upload|Register)Test`...

Yet another time when I am appreciative that we include the tests in the coverage, and that we enforce diff coverage...

Patch:
diff --git a/changelog.d/1898.breaking.rst b/changelog.d/1898.breaking.rst
new file mode 100644
index 000000000..844a8a42f
--- /dev/null
+++ b/changelog.d/1898.breaking.rst
@@ -0,0 +1,1 @@
+Removed the "upload" and "register" commands in favor of `twine <https://pypi.org/p/twine>`_.
diff --git a/docs/setuptools.txt b/docs/setuptools.txt
index 344ea5bc3..399a56d3c 100644
--- a/docs/setuptools.txt
+++ b/docs/setuptools.txt
@@ -2087,16 +2087,13 @@ New in 41.5.0: Deprecated the test command.
 ``upload`` - Upload source and/or egg distributions to PyPI
 ===========================================================
 
-.. warning::
-    **upload** is deprecated in favor of using `twine
-    <https://pypi.org/p/twine>`_
-
-The ``upload`` command is implemented and `documented
-<https://docs.python.org/3.1/distutils/uploading.html>`_
-in distutils.
+The ``upload`` command was deprecated in version 40.0 and removed in version
+42.0. Use `twine <https://pypi.org/p/twine>`_ instead.
 
-New in 20.1: Added keyring support.
-New in 40.0: Deprecated the upload command.
+For  more information on the current best practices in uploading your packages
+to PyPI, see the Python Packaging User Guide's "Packaging Python Projects"
+tutorial specifically the section on `uploading the distribution archives
+<https://packaging.python.org/tutorials/packaging-projects/#uploading-the-distribution-archives>`_.
 
 
 -----------------------------------------
diff --git a/setuptools/command/__init__.py b/setuptools/command/__init__.py
index fe619e2e6..743f5588f 100644
--- a/setuptools/command/__init__.py
+++ b/setuptools/command/__init__.py
@@ -2,8 +2,7 @@ __all__ = [
     'alias', 'bdist_egg', 'bdist_rpm', 'build_ext', 'build_py', 'develop',
     'easy_install', 'egg_info', 'install', 'install_lib', 'rotate', 'saveopts',
     'sdist', 'setopt', 'test', 'install_egg_info', 'install_scripts',
-    'register', 'bdist_wininst', 'upload_docs', 'upload', 'build_clib',
-    'dist_info',
+    'bdist_wininst', 'upload_docs', 'build_clib', 'dist_info',
 ]
 
 from distutils.command.bdist import bdist
diff --git a/setuptools/command/register.py b/setuptools/command/register.py
index 98bc01566..b8266b9a6 100644
--- a/setuptools/command/register.py
+++ b/setuptools/command/register.py
@@ -1,18 +1,18 @@
 from distutils import log
 import distutils.command.register as orig
 
+from setuptools.errors import RemovedCommandError
+
 
 class register(orig.register):
-    __doc__ = orig.register.__doc__
+    """Formerly used to register packages on PyPI."""
 
     def run(self):
-        try:
-            # Make sure that we are using valid current name/version info
-            self.run_command('egg_info')
-            orig.register.run(self)
-        finally:
-            self.announce(
-                "WARNING: Registering is deprecated, use twine to "
-                "upload instead (https://pypi.org/p/twine/)",
-                log.WARN
-            )
+        msg = (
+            "The register command has been removed, use twine to upload "
+            + "instead (https://pypi.org/p/twine)"
+        )
+
+        self.announce("ERROR: " + msg, log.ERROR)
+
+        raise RemovedCommandError(msg)
diff --git a/setuptools/command/upload.py b/setuptools/command/upload.py
index 6db8888bb..ec7f81e22 100644
--- a/setuptools/command/upload.py
+++ b/setuptools/command/upload.py
@@ -1,196 +1,17 @@
-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.spawn import spawn
-
-from distutils.errors import DistutilsError
 
-from setuptools.extern.six.moves.urllib.request import urlopen, Request
-from setuptools.extern.six.moves.urllib.error import HTTPError
-from setuptools.extern.six.moves.urllib.parse import urlparse
+from setuptools.errors import RemovedCommandError
 
 
 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)
-        finally:
-            self.announce(
-                "WARNING: Uploading via this command is deprecated, use twine "
-                "to upload instead (https://pypi.org/p/twine/)",
-                log.WARN
-            )
+    """Formerly used to upload packages to PyPI."""
 
-    def finalize_options(self):
-        orig.upload.finalize_options(self)
-        self.username = (
-            self.username or
-            getpass.getuser()
-        )
-        # Attempt to obtain password. Short circuit evaluation at the first
-        # sign of success.
-        self.password = (
-            self.password or
-            self._load_password_from_keyring() or
-            self._prompt_for_password()
+    def run(self):
+        msg = (
+            "The upload command has been removed, use twine to upload "
+            + "instead (https://pypi.org/p/twine)"
         )
 
-    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': str(meta.get_metadata_version()),
-            '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(),
-            }
-
-        data['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 = 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)
-            raise DistutilsError(msg)
-
-    def _load_password_from_keyring(self):
-        """
-        Attempt to load password from keyring. Suppress Exceptions.
-        """
-        try:
-            keyring = __import__('keyring')
-            return keyring.get_password(self.repository, self.username)
-        except Exception:
-            pass
-
-    def _prompt_for_password(self):
-        """
-        Prompt for a password on the tty. Suppress Exceptions.
-        """
-        try:
-            return getpass.getpass()
-        except (Exception, KeyboardInterrupt):
-            pass
+        self.announce("ERROR: " + msg, log.ERROR)
+        raise RemovedCommandError(msg)
diff --git a/setuptools/errors.py b/setuptools/errors.py
new file mode 100644
index 000000000..2701747f5
--- /dev/null
+++ b/setuptools/errors.py
@@ -0,0 +1,16 @@
+"""setuptools.errors
+
+Provides exceptions used by setuptools modules.
+"""
+
+from distutils.errors import DistutilsError
+
+
+class RemovedCommandError(DistutilsError, RuntimeError):
+    """Error used for commands that have been removed in setuptools.
+
+    Since ``setuptools`` is built on ``distutils``, simply removing a command
+    from ``setuptools`` will make the behavior fall back to ``distutils``; this
+    error is raised if a command exists in ``distutils`` but has been actively
+    removed in ``setuptools``.
+    """


Test patch:
diff --git a/setuptools/tests/test_register.py b/setuptools/tests/test_register.py
index 96114595d..986058067 100644
--- a/setuptools/tests/test_register.py
+++ b/setuptools/tests/test_register.py
@@ -1,43 +1,22 @@
-import mock
-from distutils import log
-
-import pytest
-
 from setuptools.command.register import register
 from setuptools.dist import Distribution
+from setuptools.errors import RemovedCommandError
 
+try:
+    from unittest import mock
+except ImportError:
+    import mock
 
-class TestRegisterTest:
-    def test_warns_deprecation(self):
-        dist = Distribution()
-
-        cmd = register(dist)
-        cmd.run_command = mock.Mock()
-        cmd.send_metadata = mock.Mock()
-        cmd.announce = mock.Mock()
-
-        cmd.run()
+import pytest
 
-        cmd.announce.assert_called_with(
-            "WARNING: Registering is deprecated, use twine to upload instead "
-            "(https://pypi.org/p/twine/)",
-            log.WARN
-        )
 
-    def test_warns_deprecation_when_raising(self):
+class TestRegister:
+    def test_register_exception(self):
+        """Ensure that the register command has been properly removed."""
         dist = Distribution()
+        dist.dist_files = [(mock.Mock(), mock.Mock(), mock.Mock())]
 
         cmd = register(dist)
-        cmd.run_command = mock.Mock()
-        cmd.send_metadata = mock.Mock()
-        cmd.send_metadata.side_effect = Exception
-        cmd.announce = mock.Mock()
 
-        with pytest.raises(Exception):
+        with pytest.raises(RemovedCommandError):
             cmd.run()
-
-        cmd.announce.assert_called_with(
-            "WARNING: Registering is deprecated, use twine to upload instead "
-            "(https://pypi.org/p/twine/)",
-            log.WARN
-        )
diff --git a/setuptools/tests/test_upload.py b/setuptools/tests/test_upload.py
index 320c6959d..7586cb262 100644
--- a/setuptools/tests/test_upload.py
+++ b/setuptools/tests/test_upload.py
@@ -1,213 +1,22 @@
-import mock
-import os
-import re
-
-from distutils import log
-from distutils.errors import DistutilsError
-
-import pytest
-
 from setuptools.command.upload import upload
 from setuptools.dist import Distribution
-from setuptools.extern import six
-
-
-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
-
-
-@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
+from setuptools.errors import RemovedCommandError
 
-    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)
+try:
+    from unittest import mock
+except ImportError:
+    import mock
 
-    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:
-    def test_upload_metadata(self, patched_upload):
-        cmd, patch = patched_upload
-
-        # Set the metadata version to 2.1
-        cmd.distribution.metadata.metadata_version = '2.1'
-
-        # Run the command
-        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
-        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())]
-
-        cmd = upload(dist)
-        cmd.upload_file = mock.Mock()
-        cmd.announce = mock.Mock()
-
-        cmd.run()
+import pytest
 
-        cmd.announce.assert_called_once_with(
-            "WARNING: Uploading via this command is deprecated, use twine to "
-            "upload instead (https://pypi.org/p/twine/)",
-            log.WARN
-        )
 
-    def test_warns_deprecation_when_raising(self):
+class TestUpload:
+    def test_upload_exception(self):
+        """Ensure that the register command has been properly removed."""
         dist = Distribution()
         dist.dist_files = [(mock.Mock(), mock.Mock(), mock.Mock())]
 
         cmd = upload(dist)
-        cmd.upload_file = mock.Mock()
-        cmd.upload_file.side_effect = Exception
-        cmd.announce = mock.Mock()
-
-        with pytest.raises(Exception):
-            cmd.run()
-
-        cmd.announce.assert_called_once_with(
-            "WARNING: Uploading via this command is deprecated, use twine to "
-            "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()
-
-    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):
+        with pytest.raises(RemovedCommandError):
             cmd.run()
-
-        cmd.announce.assert_any_call(
-            '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)
-
-    @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'
-
-    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()