check: ignore attestations, like signatures

This fixes a bug that I accidentally introduced with attestations support: `twine upload` learned the difference between distributions and attestations, but `twine check` didn't. As a result, `twine check dist/*` would fail with an `InvalidDistribution` error whenever attestations are present in the dist directory, like so: ``` Checking dist/svgcheck-0.9.0.tar.gz: PASSED Checking dist/svgcheck-0.9.0.tar.gz.publish.attestation: ERROR InvalidDistribution: Unknown distribution format: 'svgcheck-0.9.0.tar.gz.publish.attestation' ``` This fixes the behavior of `twine check` by having it skip attestations in the input list, like it does with `.asc` signatures. To do this, I reused the `_split_inputs` helper that was added with #1095, meaning that `twine upload` and `twine check` now have the same input splitting/filtering logic. See pypa/gh-action-pypi-publish#283 for some additional breakage context. Signed-off-by: William Woodruff <[email protected]>
pypa · Oct 31, 2024 · dd61356 · dd61356
1 parent 2b33343
commit dd61356
Show file tree

Hide file tree

Showing 5 changed files with 84 additions and 84 deletions.
diff --git a/tests/test_commands.py b/tests/test_commands.py
@@ -2,6 +2,7 @@
 
 import pytest
 
+from tests import helpers
 from twine import commands
 from twine import exceptions
 
@@ -49,3 +50,43 @@ def test_find_dists_handles_real_files():
     ]
     files = commands._find_dists(expected)
     assert expected == files
+
+
+def test_split_inputs():
+    """Split inputs into dists, signatures, and attestations."""
+    inputs = [
+        helpers.WHEEL_FIXTURE,
+        helpers.WHEEL_FIXTURE + ".asc",
+        helpers.WHEEL_FIXTURE + ".build.attestation",
+        helpers.WHEEL_FIXTURE + ".publish.attestation",
+        helpers.SDIST_FIXTURE,
+        helpers.SDIST_FIXTURE + ".asc",
+        helpers.NEW_WHEEL_FIXTURE,
+        helpers.NEW_WHEEL_FIXTURE + ".frob.attestation",
+        helpers.NEW_SDIST_FIXTURE,
+    ]
+
+    inputs = commands._split_inputs(inputs)
+
+    assert inputs.dists == [
+        helpers.WHEEL_FIXTURE,
+        helpers.SDIST_FIXTURE,
+        helpers.NEW_WHEEL_FIXTURE,
+        helpers.NEW_SDIST_FIXTURE,
+    ]
+
+    expected_signatures = {
+        os.path.basename(dist) + ".asc": dist + ".asc"
+        for dist in [helpers.WHEEL_FIXTURE, helpers.SDIST_FIXTURE]
+    }
+    assert inputs.signatures == expected_signatures
+
+    assert inputs.attestations_by_dist == {
+        helpers.WHEEL_FIXTURE: [
+            helpers.WHEEL_FIXTURE + ".build.attestation",
+            helpers.WHEEL_FIXTURE + ".publish.attestation",
+        ],
+        helpers.SDIST_FIXTURE: [],
+        helpers.NEW_WHEEL_FIXTURE: [helpers.NEW_WHEEL_FIXTURE + ".frob.attestation"],
+        helpers.NEW_SDIST_FIXTURE: [],
+    }
diff --git a/tests/test_upload.py b/tests/test_upload.py
@@ -116,46 +116,6 @@ def test_make_package_attestations_flagged_but_missing(upload_settings):
         upload._make_package(helpers.NEW_WHEEL_FIXTURE, {}, [], upload_settings)
 
 
-def test_split_inputs():
-    """Split inputs into dists, signatures, and attestations."""
-    inputs = [
-        helpers.WHEEL_FIXTURE,
-        helpers.WHEEL_FIXTURE + ".asc",
-        helpers.WHEEL_FIXTURE + ".build.attestation",
-        helpers.WHEEL_FIXTURE + ".publish.attestation",
-        helpers.SDIST_FIXTURE,
-        helpers.SDIST_FIXTURE + ".asc",
-        helpers.NEW_WHEEL_FIXTURE,
-        helpers.NEW_WHEEL_FIXTURE + ".frob.attestation",
-        helpers.NEW_SDIST_FIXTURE,
-    ]
-
-    inputs = upload._split_inputs(inputs)
-
-    assert inputs.dists == [
-        helpers.WHEEL_FIXTURE,
-        helpers.SDIST_FIXTURE,
-        helpers.NEW_WHEEL_FIXTURE,
-        helpers.NEW_SDIST_FIXTURE,
-    ]
-
-    expected_signatures = {
-        os.path.basename(dist) + ".asc": dist + ".asc"
-        for dist in [helpers.WHEEL_FIXTURE, helpers.SDIST_FIXTURE]
-    }
-    assert inputs.signatures == expected_signatures
-
-    assert inputs.attestations_by_dist == {
-        helpers.WHEEL_FIXTURE: [
-            helpers.WHEEL_FIXTURE + ".build.attestation",
-            helpers.WHEEL_FIXTURE + ".publish.attestation",
-        ],
-        helpers.SDIST_FIXTURE: [],
-        helpers.NEW_WHEEL_FIXTURE: [helpers.NEW_WHEEL_FIXTURE + ".frob.attestation"],
-        helpers.NEW_SDIST_FIXTURE: [],
-    }
-
-
 def test_successs_prints_release_urls(upload_settings, stub_repository, capsys):
     """Print PyPI release URLS for each uploaded package."""
     stub_repository.release_urls = lambda packages: {RELEASE_URL, NEW_RELEASE_URL}

diff --git a/twine/commands/__init__.py b/twine/commands/__init__.py
@@ -17,9 +17,10 @@
 # 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 fnmatch
 import glob
 import os.path
-from typing import List
+from typing import Dict, List, NamedTuple
 
 from twine import exceptions
 
@@ -52,3 +53,41 @@ def _find_dists(dists: List[str]) -> List[str]:
         # Otherwise, files will be filenames that exist
         uploads.extend(files)
     return _group_wheel_files_first(uploads)
+
+
+class Inputs(NamedTuple):
+    """Represents structured user inputs."""
+
+    dists: List[str]
+    signatures: Dict[str, str]
+    attestations_by_dist: Dict[str, List[str]]
+
+
+def _split_inputs(
+    inputs: List[str],
+) -> Inputs:
+    """
+    Split the unstructured list of input files provided by the user into groups.
+
+    Three groups are returned: upload files (i.e. dists), signatures, and attestations.
+
+    Upload files are returned as a linear list, signatures are returned as a
+    dict of ``basename -> path``, and attestations are returned as a dict of
+    ``dist-path -> [attestation-path]``.
+    """
+    signatures = {os.path.basename(i): i for i in fnmatch.filter(inputs, "*.asc")}
+    attestations = fnmatch.filter(inputs, "*.*.attestation")
+    dists = [
+        dist
+        for dist in inputs
+        if dist not in (set(signatures.values()) | set(attestations))
+    ]
+
+    attestations_by_dist = {}
+    for dist in dists:
+        dist_basename = os.path.basename(dist)
+        attestations_by_dist[dist] = [
+            a for a in attestations if os.path.basename(a).startswith(dist_basename)
+        ]
+
+    return Inputs(dists, signatures, attestations_by_dist)
diff --git a/twine/commands/check.py b/twine/commands/check.py
@@ -127,7 +127,7 @@ def check(
     :return:
         ``True`` if there are rendering errors, otherwise ``False``.
     """
-    uploads = [i for i in commands._find_dists(dists) if not i.endswith(".asc")]
+    uploads, _, _ = commands._split_inputs(dists)
     if not uploads:  # Return early, if there are no files to check.
         logger.error("No files to check.")
         return False

diff --git a/twine/commands/upload.py b/twine/commands/upload.py
@@ -14,10 +14,8 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 import argparse
-import fnmatch
 import logging
-import os.path
-from typing import Dict, List, NamedTuple, cast
+from typing import Dict, List, cast
 
 import requests
 from rich import print
@@ -110,44 +108,6 @@ def _make_package(
     return package
 
 
-class Inputs(NamedTuple):
-    """Represents structured user inputs."""
-
-    dists: List[str]
-    signatures: Dict[str, str]
-    attestations_by_dist: Dict[str, List[str]]
-
-
-def _split_inputs(
-    inputs: List[str],
-) -> Inputs:
-    """
-    Split the unstructured list of input files provided by the user into groups.
-
-    Three groups are returned: upload files (i.e. dists), signatures, and attestations.
-
-    Upload files are returned as a linear list, signatures are returned as a
-    dict of ``basename -> path``, and attestations are returned as a dict of
-    ``dist-path -> [attestation-path]``.
-    """
-    signatures = {os.path.basename(i): i for i in fnmatch.filter(inputs, "*.asc")}
-    attestations = fnmatch.filter(inputs, "*.*.attestation")
-    dists = [
-        dist
-        for dist in inputs
-        if dist not in (set(signatures.values()) | set(attestations))
-    ]
-
-    attestations_by_dist = {}
-    for dist in dists:
-        dist_basename = os.path.basename(dist)
-        attestations_by_dist[dist] = [
-            a for a in attestations if os.path.basename(a).startswith(dist_basename)
-        ]
-
-    return Inputs(dists, signatures, attestations_by_dist)
-
-
 def upload(upload_settings: settings.Settings, dists: List[str]) -> None:
     """Upload one or more distributions to a repository, and display the progress.
 
@@ -187,7 +147,7 @@ def upload(upload_settings: settings.Settings, dists: List[str]) -> None:
 
     dists = commands._find_dists(dists)
     # Determine if the user has passed in pre-signed distributions or any attestations.
-    uploads, signatures, attestations_by_dist = _split_inputs(dists)
+    uploads, signatures, attestations_by_dist = commands._split_inputs(dists)
 
     print(f"Uploading distributions to {utils.sanitize_url(repository_url)}")