From 784e2815fafb0b9c0423cbeca4dd8b7096ab4009 Mon Sep 17 00:00:00 2001 From: Ian Stapleton Cordasco Date: Tue, 17 Jun 2025 21:17:10 -0500 Subject: [PATCH] Remove --skip-existing support for non-PyPI indices Closes #1251 --- changelog/1251.removal.rst | 20 +++++++++++++++ tests/test_settings.py | 36 +++++++++++++++++++++++++- tests/test_upload.py | 52 -------------------------------------- twine/commands/upload.py | 7 +---- twine/exceptions.py | 44 ++++++++++++++++++++++++++++++++ twine/repository.py | 17 +++++++++++++ twine/settings.py | 19 ++++++++++++++ 7 files changed, 136 insertions(+), 59 deletions(-) create mode 100644 changelog/1251.removal.rst diff --git a/changelog/1251.removal.rst b/changelog/1251.removal.rst new file mode 100644 index 00000000..1413ce7e --- /dev/null +++ b/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 diff --git a/tests/test_settings.py b/tests/test_settings.py index 39944d2b..af720555 100644 --- a/tests/test_settings.py +++ b/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)] ) diff --git a/tests/test_upload.py b/tests/test_upload.py index 24916399..c631f9b4 100644 --- a/tests/test_upload.py +++ b/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=( - '
\n' - " Repository does not allow updating assets: pypi-local\n" - "
\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): diff --git a/twine/commands/upload.py b/twine/commands/upload.py index 939e1dfa..ffc05b74 100644 --- a/twine/commands/upload.py +++ b/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. diff --git a/twine/exceptions.py b/twine/exceptions.py index 29b7d8a1..1b9b02b6 100644 --- a/twine/exceptions.py +++ b/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. diff --git a/twine/repository.py b/twine/repository.py index cc8b01de..28ecca35 100644 --- a/twine/repository.py +++ b/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)): diff --git a/twine/settings.py b/twine/settings.py index 5aa4d964..ba473524 100644 --- a/twine/settings.py +++ b/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.