Remove --skip-existing support for non-PyPI indices

Closes #1251

Author: sigmavirus24

changelog/1251.removal.rst

@@ -0,0 +1,20 @@
+Remove hacks that support ``--skip-existing`` for indexes other than PyPI and
+TestPyPI.
+
+To date, these hacks continue to accrue and there have been numerous issues
+with them, not the least of which being that every time we update them, the
+paid index providers change things to break the compatibility we implement for
+them. Beyond that, these hacks do not work when text is internationalized in
+the response from the index provider.
+
+For a sample of past issues, see:
+
+- https://github.com/pypa/twine/issues/1251
+
+- https://github.com/pypa/twine/issues/918
+
+- https://github.com/pypa/twine/issues/856
+
+- https://github.com/pypa/twine/issues/693
+
+- https://github.com/pypa/twine/issues/332

tests/test_settings.py

@@ -19,6 +19,7 @@
 import pytest
 
 from twine import exceptions
+from twine import repository
 from twine import settings
 
 
@@ -76,11 +77,44 @@ def test_settings_transforms_repository_config_non_pypi(write_config_file):
     assert s.identity is None
     assert s.username == "someusername"
     assert s.password == "password"
-    assert s.cacert is None
     assert s.client_cert is None
     assert s.disable_progress_bar is False
 
 
+def test_settings_verify_feature_compatibility() -> None:
+    s = settings.Settings(skip_existing=True)
+    s.repository_config = {"repository": repository.WAREHOUSE}
+    try:
+        s.verify_feature_capability()
+    except exceptions.UnsupportedConfiguration as unexpected_exc:
+        pytest.fail(
+            "Expected feature capability to work with production PyPI"
+            f" but got {unexpected_exc!r}"
+        )
+
+    s.repository_config["repository"] = repository.TEST_WAREHOUSE
+    try:
+        s.verify_feature_capability()
+    except exceptions.UnsupportedConfiguration as unexpected_exc:
+        pytest.fail(
+            "Expected feature capability to work with TestPyPI but got"
+            f" {unexpected_exc!r}"
+        )
+
+    s.repository_config["repository"] = "https://not-really-pypi.example.com/legacy"
+    with pytest.raises(exceptions.UnsupportedConfiguration):
+        s.verify_feature_capability()
+
+    s.skip_existing = False
+    try:
+        s.verify_feature_capability()
+    except exceptions.UnsupportedConfiguration as unexpected_exc:
+        pytest.fail(
+            "Expected an exception only when --skip-existing is provided"
+            f" but got {unexpected_exc!r}"
+        )
+
+
 @pytest.mark.parametrize(
     "verbose, log_level", [(True, logging.INFO), (False, logging.WARNING)]
 )

tests/test_upload.py

@@ -458,27 +458,6 @@ def test_prints_skip_message_for_response(
             ),
             id="pypi",
         ),
-        pytest.param(
-            dict(
-                status_code=400,
-                reason=(
-                    "Repository does not allow updating assets: pypi for url: "
-                    "http://www.foo.bar"
-                ),
-            ),
-            id="nexus",
-        ),
-        pytest.param(
-            dict(
-                status_code=400,
-                text=(
-                    '<div class="content-section">\n'
-                    "    Repository does not allow updating assets: pypi-local\n"
-                    "</div>\n"
-                ),
-            ),
-            id="nexus_new",
-        ),
         pytest.param(
             dict(
                 status_code=409,
@@ -489,37 +468,6 @@ def test_prints_skip_message_for_response(
             ),
             id="pypiserver",
         ),
-        pytest.param(
-            dict(
-                status_code=403,
-                text=(
-                    "Not enough permissions to overwrite artifact "
-                    "'pypi-local:twine/1.5.0/twine-1.5.0-py2.py3-none-any.whl'"
-                    "(user 'twine-deployer' needs DELETE permission)."
-                ),
-            ),
-            id="artifactory_old",
-        ),
-        pytest.param(
-            dict(
-                status_code=403,
-                text=(
-                    "Not enough permissions to delete/overwrite artifact "
-                    "'pypi-local:twine/1.5.0/twine-1.5.0-py2.py3-none-any.whl'"
-                    "(user 'twine-deployer' needs DELETE permission)."
-                ),
-            ),
-            id="artifactory_new",
-        ),
-        pytest.param(
-            dict(
-                status_code=400,
-                text=(
-                    '{"message":"validation failed: file name has already been taken"}'
-                ),
-            ),
-            id="gitlab_enterprise",
-        ),
     ],
 )
 def test_skip_existing_skips_files_on_repository(response_kwargs):

twine/commands/upload.py

@@ -61,12 +61,6 @@ def skip_upload(
         status == 409
         # PyPI / TestPyPI / GCP Artifact Registry
         or (status == 400 and any("already exist" in x for x in [reason, text]))
-        # Nexus Repository OSS (https://www.sonatype.com/nexus-repository-oss)
-        or (status == 400 and any("updating asset" in x for x in [reason, text]))
-        # Artifactory (https://jfrog.com/artifactory/)
-        or (status == 403 and "overwrite artifact" in text)
-        # Gitlab Enterprise Edition (https://about.gitlab.com)
-        or (status == 400 and "already been taken" in text)
     )
 
 
@@ -131,6 +125,7 @@ def upload(upload_settings: settings.Settings, dists: List[str]) -> None:
         The repository responded with an error.
     """
     upload_settings.check_repository_url()
+    upload_settings.verify_feature_capability()
     repository_url = cast(str, upload_settings.repository_config["repository"])
 
     # Attestations are only supported on PyPI and TestPyPI at the moment.

twine/exceptions.py

@@ -13,6 +13,7 @@
 # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 # See the License for the specific language governing permissions and
 # limitations under the License.
+import typing as t
 
 
 class TwineException(Exception):
@@ -77,6 +78,49 @@ def from_args(
         )
 
 
+class UnsupportedConfiguration(TwineException):
+    """An upload attempt was detected using features not supported by a repository.
+
+    The features specified either in configuration or on the command-line.
+    """
+
+    class Builder:
+        """Build the parameters for an UnsupportedConfiguration exception.
+
+        In the event we add additional features we are not allowing with
+        something other than PyPI or TestPyPI, we can use a builder to
+        accumulate them all instead of requiring someone to run multiple times
+        to discover all unsupported configuration options.
+        """
+
+        repository_url: str
+        features: t.List[str]
+
+        def __init__(self) -> None:
+            self.repository_url = ""
+            self.features = []
+
+        def with_repository_url(
+            self, repository_url: str
+        ) -> "UnsupportedConfiguration.Builder":
+            self.repository_url = repository_url
+            return self
+
+        def with_feature(self, feature: str) -> "UnsupportedConfiguration.Builder":
+            self.features.append(feature)
+            return self
+
+        def finalize(self) -> "UnsupportedConfiguration":
+            return UnsupportedConfiguration(
+                f"The configured repository {self.repository_url!r} does not "
+                "have support for the following features: "
+                f"{', '.join(self.features)} and is an unsupported "
+                "configuration",
+                self.repository_url,
+                *self.features,
+            )
+
+
 class UnreachableRepositoryURLDetected(TwineException):
     """An upload attempt was detected to a URL without a protocol prefix.
 

twine/repository.py

@@ -181,6 +181,23 @@ def upload(
     def package_is_uploaded(
         self, package: package_file.PackageFile, bypass_cache: bool = False
     ) -> bool:
+        """Determine if a package has been uploaded to PyPI already.
+
+        .. warning:: This does not support indexes other than PyPI or TestPyPI
+
+        :param package:
+            The package file that will otherwise be uploaded.
+        :type package:
+            :class:`~twine.package.PackageFile`
+        :param bypass_cache:
+            Force a request to PyPI.
+        :type bypass_cache:
+            bool
+        :returns:
+            True if package has already been uploaded, False otherwise
+        :rtype:
+            bool
+        """
         # NOTE(sigmavirus24): Not all indices are PyPI and pypi.io doesn't
         # have a similar interface for finding the package versions.
         if not self.url.startswith((LEGACY_PYPI, WAREHOUSE, OLD_WAREHOUSE)):

twine/settings.py

@@ -313,6 +313,25 @@ def _handle_certificates(
         self.cacert = utils.get_cacert(cacert, self.repository_config)
         self.client_cert = utils.get_clientcert(client_cert, self.repository_config)
 
+    def verify_feature_capability(self) -> None:
+        """Verify configured settings are supported for the configured repository.
+
+        This presently checks:
+        - ``--skip-existing`` was only provided for PyPI and TestPyPI
+
+        :raises twine.exceptions.UnsupportedConfiguration:
+            The configured features are not available with the configured
+            repository.
+        """
+        repository_url = cast(str, self.repository_config["repository"])
+
+        if self.skip_existing and not repository_url.startswith(
+            (repository.WAREHOUSE, repository.TEST_WAREHOUSE)
+        ):
+            raise exceptions.UnsupportedConfiguration.Builder().with_feature(
+                "--skip-existing"
+            ).with_repository_url(repository_url).finalize()
+
     def check_repository_url(self) -> None:
         """Verify we are not using legacy PyPI.