Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix handling of continuation lines in Description metadata field #1220

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog/1220.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix handling of multiline ``Description`` metadata fields.
55 changes: 55 additions & 0 deletions tests/test_package.py
Original file line number Diff line number Diff line change
Expand Up @@ -463,3 +463,58 @@ def test_setuptools_license_file(read_data, filtered, monkeypatch):
package = package_file.PackageFile.from_filename(filename, comment=None)
meta = package.metadata_dictionary()
assert filtered != ("license_file" in meta)


@pytest.mark.parametrize(
"description,expected",
[
# fmt: off
pytest.param(
"\n"
"Two\n"
"Lines\n",
"Two\nLines\n",
id="body",
),
pytest.param(
"Description: Two\n"
" |Lines\n"
" |\n",
"Two\nLines\n",
id="multiline-header",
),
pytest.param(
"Description: Two\n"
" Lines\n"
" \n",
"Two\nLines\n",
id="multiline-header-setuptools",
),
pytest.param(
"Description: Two\n"
" Lines\n"
" Maybe Three",
"Two\n Lines\n Maybe Three",
id="multiline-inconsistent",
),
pytest.param(
"Description: Two\n"
" |Lines\n"
" Maybe Three",
"Two\n |Lines\n Maybe Three",
id="multiline-mixed",
),
# fmt: on
],
)
def test_description_field_continuation(description, expected, monkeypatch):
"""License-File metadata entries are kept when Metadata-Version is 2.4."""
read_data = (
"Metadata-Version: 2.4\n"
"Name: test-package\n"
"Version: 1.0.0\n" + description
)
monkeypatch.setattr(package_file.wheel.Wheel, "read", lambda _: read_data)
filename = "tests/fixtures/twine-1.5.0-py2.py3-none-any.whl"
package = package_file.PackageFile.from_filename(filename, comment=None)
assert package.metadata["description"] == expected
27 changes: 27 additions & 0 deletions twine/package.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,28 @@ def _safe_name(name: str) -> str:
return re.sub("[^A-Za-z0-9.]+", "-", name)


def _dedent(string: str) -> str:
"""Remove line continuation suffix used for the ``Description`` metadata field.

The metadata standard prescribes that continuation lines should be
prefixed with 7 spaces followed by a pipe character, however, setuptools
used to prefix continuation lines with 8 spaces. Handle both here.

If not all lines following the first start with either of the accepted
prefixes, the string is returned without modifications.
"""
lines = string.splitlines()
if (
False # This is here only to force black into something sensible.
or all(line.startswith(" |") for line in lines[1:])
or all(line.startswith(" ") for line in lines[1:])
Comment on lines +81 to +82
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
or all(line.startswith(" |") for line in lines[1:])
or all(line.startswith(" ") for line in lines[1:])
or all(line.startswith((" |", " ")) for line in lines[1:])

Copy link
Contributor Author

@dnicolodi dnicolodi Jan 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would implement a different check. The code as written tests whether all the line start with 8 spacer or all the lines start with 7 spaced and a pipe. The proposed code tests whether all the lines start with either 8 spaces or 7 spaces and a pipe. I don't think we should treat intermixed continuation style as valid. There is a test that ensures that someone else is not tempted into the same erroneous simplification.

):
for i in range(1, len(lines)):
lines[i] = lines[i][8:]
return "\n".join(lines)
return string


# Map ``metadata.RawMetadata`` fields to ``PackageMetadata`` fields. Some
# fields are renamed to match the names expected in the upload form.
_RAW_TO_PACKAGE_METADATA = {
Expand Down Expand Up @@ -232,6 +254,11 @@ def from_filename(cls, filename: str, comment: Optional[str]) -> "PackageFile":
# than 2.4.
if version.Version(meta.get("metadata_version", "0")) < version.Version("2.4"):
meta.pop("license_files", None)

description = meta.get("description", None)
if description is not None:
meta["description"] = _dedent(description)

try:
metadata.Metadata.from_raw(meta)
except metadata.ExceptionGroup as group:
Expand Down
Loading