From b7a353e3ea11cda82e00efcae1e856ec562fc9f2 Mon Sep 17 00:00:00 2001 From: Bruno Alla Date: Sat, 12 Oct 2024 18:42:19 +0100 Subject: [PATCH 1/9] Validate Trove classifiers in twine check Fix #976 --- pyproject.toml | 1 + tests/test_check.py | 74 +++++++++++++++++++++++++++++++++++++++++ twine/commands/check.py | 35 ++++++++++++------- 3 files changed, 98 insertions(+), 12 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 52c3de43..4ab30fad 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -40,6 +40,7 @@ dependencies = [ "keyring >= 15.1; platform_machine != 'ppc64le' and platform_machine != 's390x'", "rfc3986 >= 1.4.0", "rich >= 12.0.0", + "trove-classifiers >= 2024.10.12 ", # workaround for #1116 "pkginfo < 1.11", diff --git a/tests/test_check.py b/tests/test_check.py index 1adb5342..b6c0b1c0 100644 --- a/tests/test_check.py +++ b/tests/test_check.py @@ -281,6 +281,80 @@ def test_main(monkeypatch): assert check_stub.calls == [pretend.call(["dist/*"], strict=False)] +def test_fails_invalid_classifiers(tmp_path, capsys, caplog): + sdist = build_sdist( + tmp_path, + { + "setup.cfg": ( + """ + [metadata] + name = test-package + version = 0.0.1 + long_description = file:README.md + long_description_content_type = text/markdown + classifiers = + Framework :: Django :: 5 + """ + ), + "README.md": ( + """ + # test-package + + A test package. + """ + ), + }, + ) + + assert check.check([sdist]) + + assert capsys.readouterr().out == f"Checking {sdist}: FAILED\n" + + assert len(caplog.record_tuples) > 0 + assert caplog.record_tuples == [ + ( + "twine.commands.check", + logging.ERROR, + "`Framework :: Django :: 5` is not a valid classifier" + " and would prevent upload to PyPI.\n", + ) + ] + + +def test_passes_valid_classifiers(tmp_path, capsys, caplog): + sdist = build_sdist( + tmp_path, + { + "setup.cfg": ( + """ + [metadata] + name = test-package + version = 0.0.1 + long_description = file:README.md + long_description_content_type = text/markdown + classifiers = + Programming Language :: Python :: 3 + Framework :: Django + Framework :: Django :: 5.1 + """ + ), + "README.md": ( + """ + # test-package + + A test package. + """ + ), + }, + ) + + assert not check.check([sdist]) + + assert capsys.readouterr().out == f"Checking {sdist}: PASSED\n" + + assert caplog.record_tuples == [] + + # TODO: Test print() color output # TODO: Test log formatting diff --git a/twine/commands/check.py b/twine/commands/check.py index ed9324cb..c32fe977 100644 --- a/twine/commands/check.py +++ b/twine/commands/check.py @@ -22,6 +22,7 @@ import readme_renderer.rst from rich import print +from trove_classifiers import classifiers as all_classifiers from twine import commands from twine import package as package_file @@ -76,14 +77,15 @@ def _parse_content_type(value: str) -> Tuple[str, Dict[str, str]]: def _check_file( filename: str, render_warning_stream: _WarningStream -) -> Tuple[List[str], bool]: +) -> Tuple[List[str], List[str]]: """Check given distribution.""" warnings = [] - is_ok = True + errors = [] package = package_file.PackageFile.from_filename(filename, comment=None) - metadata = package.metadata_dictionary() + + # Check description description = cast(Optional[str], metadata["description"]) description_content_type = cast(Optional[str], metadata["description_content_type"]) @@ -103,9 +105,21 @@ def _check_file( description, stream=render_warning_stream, **params ) if rendering_result is None: - is_ok = False + errors.append( + "`long_description` has syntax errors in markup" + " and would not be rendered on PyPI." + ) - return warnings, is_ok + # Check classifiers + dist_classifiers = cast(Optional[List[str]], metadata["classifiers"]) + for classifier in dist_classifiers: + if classifier not in all_classifiers: + errors.append( + f"`{classifier}` is not a valid classifier" + f" and would prevent upload to PyPI." + ) + + return warnings, errors def check( @@ -137,17 +151,14 @@ def check( for filename in uploads: print(f"Checking {filename}: ", end="") render_warning_stream = _WarningStream() - warnings, is_ok = _check_file(filename, render_warning_stream) + warnings, errors = _check_file(filename, render_warning_stream) # Print the status and/or error - if not is_ok: + if errors: failure = True print("[red]FAILED[/red]") - logger.error( - "`long_description` has syntax errors in markup" - " and would not be rendered on PyPI." - f"\n{render_warning_stream}" - ) + error_mgs = "\n".join(errors) + logger.error(f"{error_mgs}\n" f"{render_warning_stream}") elif warnings: if strict: failure = True From 2e52fbae71a4f2071caff14183f137a5ea10bec4 Mon Sep 17 00:00:00 2001 From: Bruno Alla Date: Sat, 12 Oct 2024 18:49:44 +0100 Subject: [PATCH 2/9] Update documentation --- twine/commands/check.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/twine/commands/check.py b/twine/commands/check.py index c32fe977..22f75932 100644 --- a/twine/commands/check.py +++ b/twine/commands/check.py @@ -126,10 +126,11 @@ def check( dists: List[str], strict: bool = False, ) -> bool: - """Check that a distribution will render correctly on PyPI and display the results. + """Check that a distribution will upload and render correctly on PyPI. - This is currently only validates ``long_description``, but more checks could be - added. + This currently validates + - ``long_description``, to make sure it would render correctly on PyPI + - ``classifiers``, to make sure that all classifiers are valid :param dists: The distribution files to check. From 2867c26863958900f734d57a4c204f2393f7ed7e Mon Sep 17 00:00:00 2001 From: Bruno Alla Date: Sat, 12 Oct 2024 18:50:49 +0100 Subject: [PATCH 3/9] Remove non-existent output_stream parameter from the docs --- twine/commands/check.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/twine/commands/check.py b/twine/commands/check.py index 22f75932..5319b5d2 100644 --- a/twine/commands/check.py +++ b/twine/commands/check.py @@ -134,8 +134,6 @@ def check( :param dists: The distribution files to check. - :param output_stream: - The destination of the resulting output. :param strict: If ``True``, treat warnings as errors. From 1beae6c8d135b73755bd9597fb9f0bcd028290d1 Mon Sep 17 00:00:00 2001 From: Bruno Alla Date: Sat, 12 Oct 2024 19:02:04 +0100 Subject: [PATCH 4/9] Fix type error --- twine/commands/check.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/twine/commands/check.py b/twine/commands/check.py index 5319b5d2..c80fe4b1 100644 --- a/twine/commands/check.py +++ b/twine/commands/check.py @@ -18,7 +18,7 @@ import io import logging import re -from typing import Dict, List, Optional, Tuple, cast +from typing import Dict, List, Optional, Sequence, Tuple, cast import readme_renderer.rst from rich import print @@ -111,7 +111,7 @@ def _check_file( ) # Check classifiers - dist_classifiers = cast(Optional[List[str]], metadata["classifiers"]) + dist_classifiers = cast(Sequence[str], metadata["classifiers"]) for classifier in dist_classifiers: if classifier not in all_classifiers: errors.append( From 6034678c1784605ee8ee93cc9f911359a669940a Mon Sep 17 00:00:00 2001 From: Bruno Alla Date: Sat, 12 Oct 2024 19:23:24 +0100 Subject: [PATCH 5/9] Update check command description --- twine/commands/check.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/twine/commands/check.py b/twine/commands/check.py index c80fe4b1..4ab83c50 100644 --- a/twine/commands/check.py +++ b/twine/commands/check.py @@ -183,7 +183,13 @@ def main(args: List[str]) -> bool: :return: The exit status of the ``check`` command. """ - parser = argparse.ArgumentParser(prog="twine check") + parser = argparse.ArgumentParser( + prog="twine check", + description=( + "Check distribution files and make sure they will upload and render" + " correctly on PyPI. Validates description and all classifiers." + ), + ) parser.add_argument( "dists", nargs="+", From 4f09bce12e46d4232704a0c5ee459106fe0d55fc Mon Sep 17 00:00:00 2001 From: Bruno Alla Date: Sun, 13 Oct 2024 13:55:33 +0100 Subject: [PATCH 6/9] Rename import as to valid_classifiers --- twine/commands/check.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/twine/commands/check.py b/twine/commands/check.py index 4ab83c50..cd0523d6 100644 --- a/twine/commands/check.py +++ b/twine/commands/check.py @@ -22,7 +22,7 @@ import readme_renderer.rst from rich import print -from trove_classifiers import classifiers as all_classifiers +from trove_classifiers import classifiers as valid_classifiers from twine import commands from twine import package as package_file @@ -113,7 +113,7 @@ def _check_file( # Check classifiers dist_classifiers = cast(Sequence[str], metadata["classifiers"]) for classifier in dist_classifiers: - if classifier not in all_classifiers: + if classifier not in valid_classifiers: errors.append( f"`{classifier}` is not a valid classifier" f" and would prevent upload to PyPI." From 444c799dba221540a7f3986da10b2ddbff592c0d Mon Sep 17 00:00:00 2001 From: Bruno Alla Date: Sun, 13 Oct 2024 13:55:59 +0100 Subject: [PATCH 7/9] Remove whitespace --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 4ab30fad..1eaf7bd5 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -40,7 +40,7 @@ dependencies = [ "keyring >= 15.1; platform_machine != 'ppc64le' and platform_machine != 's390x'", "rfc3986 >= 1.4.0", "rich >= 12.0.0", - "trove-classifiers >= 2024.10.12 ", + "trove-classifiers >= 2024.10.12", # workaround for #1116 "pkginfo < 1.11", From 22d72e54d4d3e8c797862ae3cb9926768d9d7a33 Mon Sep 17 00:00:00 2001 From: Bruno Alla Date: Sun, 13 Oct 2024 14:13:25 +0100 Subject: [PATCH 8/9] Change invalid classifier to be more obviously invalid --- tests/test_check.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_check.py b/tests/test_check.py index b6c0b1c0..fb4c242d 100644 --- a/tests/test_check.py +++ b/tests/test_check.py @@ -293,7 +293,7 @@ def test_fails_invalid_classifiers(tmp_path, capsys, caplog): long_description = file:README.md long_description_content_type = text/markdown classifiers = - Framework :: Django :: 5 + Framework | Django | 5 """ ), "README.md": ( @@ -315,7 +315,7 @@ def test_fails_invalid_classifiers(tmp_path, capsys, caplog): ( "twine.commands.check", logging.ERROR, - "`Framework :: Django :: 5` is not a valid classifier" + "`Framework | Django | 5` is not a valid classifier" " and would prevent upload to PyPI.\n", ) ] From 7506995abcb0795323d958aeae89b8dc340b967b Mon Sep 17 00:00:00 2001 From: Bruno Alla Date: Sun, 13 Oct 2024 21:01:15 +0100 Subject: [PATCH 9/9] Add data structure for check results --- twine/commands/check.py | 36 +++++++++++++++++++++++------------- 1 file changed, 23 insertions(+), 13 deletions(-) diff --git a/twine/commands/check.py b/twine/commands/check.py index cd0523d6..028240c8 100644 --- a/twine/commands/check.py +++ b/twine/commands/check.py @@ -14,6 +14,7 @@ # See the License for the specific language governing permissions and # limitations under the License. import argparse +import dataclasses import email.message import io import logging @@ -75,12 +76,21 @@ def _parse_content_type(value: str) -> Tuple[str, Dict[str, str]]: return msg.get_content_type(), msg["content-type"].params +@dataclasses.dataclass +class CheckedFileResults: + warnings: List[str] + errors: List[str] + + @property + def is_ok(self) -> bool: + return len(self.errors) == 0 + + def _check_file( filename: str, render_warning_stream: _WarningStream -) -> Tuple[List[str], List[str]]: +) -> CheckedFileResults: """Check given distribution.""" - warnings = [] - errors = [] + result = CheckedFileResults(warnings=[], errors=[]) package = package_file.PackageFile.from_filename(filename, comment=None) metadata = package.metadata_dictionary() @@ -90,7 +100,7 @@ def _check_file( description_content_type = cast(Optional[str], metadata["description_content_type"]) if description_content_type is None: - warnings.append( + result.warnings.append( "`long_description_content_type` missing. defaulting to `text/x-rst`." ) description_content_type = "text/x-rst" @@ -99,13 +109,13 @@ def _check_file( renderer = _RENDERERS.get(content_type, _RENDERERS[None]) if description is None or description.rstrip() == "UNKNOWN": - warnings.append("`long_description` missing.") + result.warnings.append("`long_description` missing.") elif renderer: rendering_result = renderer.render( description, stream=render_warning_stream, **params ) if rendering_result is None: - errors.append( + result.errors.append( "`long_description` has syntax errors in markup" " and would not be rendered on PyPI." ) @@ -114,12 +124,12 @@ def _check_file( dist_classifiers = cast(Sequence[str], metadata["classifiers"]) for classifier in dist_classifiers: if classifier not in valid_classifiers: - errors.append( + result.errors.append( f"`{classifier}` is not a valid classifier" f" and would prevent upload to PyPI." ) - return warnings, errors + return result def check( @@ -150,15 +160,15 @@ def check( for filename in uploads: print(f"Checking {filename}: ", end="") render_warning_stream = _WarningStream() - warnings, errors = _check_file(filename, render_warning_stream) + check_result = _check_file(filename, render_warning_stream) # Print the status and/or error - if errors: + if not check_result.is_ok: failure = True print("[red]FAILED[/red]") - error_mgs = "\n".join(errors) + error_mgs = "\n".join(check_result.errors) logger.error(f"{error_mgs}\n" f"{render_warning_stream}") - elif warnings: + elif check_result.warnings: if strict: failure = True print("[red]FAILED due to warnings[/red]") @@ -168,7 +178,7 @@ def check( print("[green]PASSED[/green]") # Print warnings after the status and/or error - for message in warnings: + for message in check_result.warnings: logger.warning(message) return failure