From eb8883ebb387bae79a52f3f0d110675f008e2e3c Mon Sep 17 00:00:00 2001 From: Wagner Macedo Date: Tue, 16 Nov 2021 23:38:44 +0100 Subject: [PATCH 1/3] Add support for multiple README files This code reuses the `"readme"` entry in a way to allow the user to declare in the old way or the new way. With the changes, the two following declarations are valid: **Single file** ```toml readme = "README.rst" ``` **Multiple files** ```toml readme = [ "README.rst", "HISTORY.rst" ] ``` If the user declares files in different formats, the strict validation will issue. NOTICE: The class `Package` suffered an important change: `readme` was renamed to the plural `readmes`. Properties for the single form were introduced to ensure retrocompatibility. --- src/poetry/core/factory.py | 25 ++++++++++++++++++- .../core/json/schemas/poetry-schema.json | 15 +++++++++-- src/poetry/core/masonry/metadata.py | 18 ++++++------- src/poetry/core/packages/package.py | 24 +++++++++++++++++- tests/fixtures/with_readme_files/README-1.rst | 2 ++ tests/fixtures/with_readme_files/README-2.rst | 2 ++ .../fixtures/with_readme_files/pyproject.toml | 19 ++++++++++++++ .../with_readme_files/single_python.py | 3 +++ tests/masonry/builders/test_builder.py | 13 ++++++++++ tests/packages/test_package.py | 19 ++++++++++++++ tests/test_factory.py | 22 +++++++++++++++- 11 files changed, 147 insertions(+), 15 deletions(-) create mode 100644 tests/fixtures/with_readme_files/README-1.rst create mode 100644 tests/fixtures/with_readme_files/README-2.rst create mode 100644 tests/fixtures/with_readme_files/pyproject.toml create mode 100644 tests/fixtures/with_readme_files/single_python.py diff --git a/src/poetry/core/factory.py b/src/poetry/core/factory.py index 2bee9896a..e9a3582f1 100644 --- a/src/poetry/core/factory.py +++ b/src/poetry/core/factory.py @@ -93,7 +93,12 @@ def configure_package( package.classifiers = config.get("classifiers", []) if "readme" in config: - package.readme = root / config["readme"] + if isinstance(config["readme"], str): + package.readmes = (root / config["readme"],) + else: + package.readmes = tuple(root / readme for readme in config["readme"]) + + package.description_type = cls._readme_content_type(package.readmes[0]) if "platform" in config: package.platform = config["platform"] @@ -421,6 +426,14 @@ def validate(cls, config: dict, strict: bool = False) -> Dict[str, List[str]]: ) ) + # Checking types of all readme files (must match) + if "readme" in config and not isinstance(config["readme"], str): + readme_types = [cls._readme_content_type(r) for r in config["readme"]] + if len(set(readme_types)) > 1: + result["errors"].append( + f"Declared README files must be of same type: found {', '.join(readme_types)}" + ) + return result @classmethod @@ -441,3 +454,13 @@ def locate(cls, cwd: Optional[Path] = None) -> Path: cwd ) ) + + @staticmethod + def _readme_content_type(path: Union[str, Path]) -> str: + suffix = Path(path).suffix + if suffix == ".rst": + return "text/x-rst" + elif suffix in [".md", ".markdown"]: + return "text/markdown" + else: + return "text/plain" diff --git a/src/poetry/core/json/schemas/poetry-schema.json b/src/poetry/core/json/schemas/poetry-schema.json index 519f7b9be..1a9484431 100644 --- a/src/poetry/core/json/schemas/poetry-schema.json +++ b/src/poetry/core/json/schemas/poetry-schema.json @@ -55,8 +55,19 @@ "$ref": "#/definitions/maintainers" }, "readme": { - "type": "string", - "description": "The path to the README file" + "anyOf": [ + { + "type": "string", + "description": "The path to the README file." + }, + { + "type": "array", + "description": "A list of paths to the readme files.", + "items": { + "type": "string" + } + } + ] }, "classifiers": { "type": "array", diff --git a/src/poetry/core/masonry/metadata.py b/src/poetry/core/masonry/metadata.py index 74178942a..1a37e6e31 100644 --- a/src/poetry/core/masonry/metadata.py +++ b/src/poetry/core/masonry/metadata.py @@ -53,9 +53,12 @@ def from_package(cls, package: "Package") -> "Metadata": meta.name = canonicalize_name(package.name) meta.version = normalize_version(package.version.text) meta.summary = package.description - if package.readme: - with package.readme.open(encoding="utf-8") as f: - meta.description = f.read() + if package.readmes: + descriptions = [] + for readme in package.readmes: + with readme.open(encoding="utf-8") as f: + descriptions.append(f.read()) + meta.description = "\n".join(descriptions) meta.keywords = ",".join(package.keywords) meta.home_page = package.homepage or package.repository_url @@ -78,13 +81,8 @@ def from_package(cls, package: "Package") -> "Metadata": meta.requires_dist = [d.to_pep_508() for d in package.requires] # Version 2.1 - if package.readme: - if package.readme.suffix == ".rst": - meta.description_content_type = "text/x-rst" - elif package.readme.suffix in [".md", ".markdown"]: - meta.description_content_type = "text/markdown" - else: - meta.description_content_type = "text/plain" + if package.description_type: + meta.description_content_type = package.description_type meta.provides_extra = list(package.extras) diff --git a/src/poetry/core/packages/package.py b/src/poetry/core/packages/package.py index 861df411b..fea8b1065 100644 --- a/src/poetry/core/packages/package.py +++ b/src/poetry/core/packages/package.py @@ -7,6 +7,7 @@ from typing import Dict from typing import List from typing import Optional +from typing import Tuple from typing import Union from poetry.core.packages.specification import PackageSpecification @@ -87,7 +88,8 @@ def __init__( self.documentation_url = None self.keywords = [] self._license = None - self.readme = None + self.readmes: Tuple[Path, ...] = () + self.description_type: Optional[str] = None self.extras = {} self.requires_extras = [] @@ -347,6 +349,26 @@ def urls(self) -> Dict[str, str]: return urls + @property + def readme(self) -> Path: + import warnings + + warnings.warn( + "`readme` is deprecated: you are getting only the first readme file. Please use the plural form `readmes`", + DeprecationWarning, + ) + return next(iter(self.readmes), None) + + @readme.setter + def readme(self, path: Path) -> None: + import warnings + + warnings.warn( + "`readme` is deprecated. Please assign a tuple to the plural form `readmes`", + DeprecationWarning, + ) + self.readmes = (path,) + def is_prerelease(self) -> bool: return self._version.is_unstable() diff --git a/tests/fixtures/with_readme_files/README-1.rst b/tests/fixtures/with_readme_files/README-1.rst new file mode 100644 index 000000000..265d70d6a --- /dev/null +++ b/tests/fixtures/with_readme_files/README-1.rst @@ -0,0 +1,2 @@ +Single Python +============= diff --git a/tests/fixtures/with_readme_files/README-2.rst b/tests/fixtures/with_readme_files/README-2.rst new file mode 100644 index 000000000..a5693d973 --- /dev/null +++ b/tests/fixtures/with_readme_files/README-2.rst @@ -0,0 +1,2 @@ +Changelog +========= diff --git a/tests/fixtures/with_readme_files/pyproject.toml b/tests/fixtures/with_readme_files/pyproject.toml new file mode 100644 index 000000000..850e51174 --- /dev/null +++ b/tests/fixtures/with_readme_files/pyproject.toml @@ -0,0 +1,19 @@ +[tool.poetry] +name = "single-python" +version = "0.1" +description = "Some description." +authors = [ + "Wagner Macedo " +] +license = "MIT" + +readme = [ + "README-1.rst", + "README-2.rst" +] + +homepage = "https://python-poetry.org/" + + +[tool.poetry.dependencies] +python = "2.7.15" diff --git a/tests/fixtures/with_readme_files/single_python.py b/tests/fixtures/with_readme_files/single_python.py new file mode 100644 index 000000000..7ef41c5d4 --- /dev/null +++ b/tests/fixtures/with_readme_files/single_python.py @@ -0,0 +1,3 @@ +"""Example module""" + +__version__ = "0.1" diff --git a/tests/masonry/builders/test_builder.py b/tests/masonry/builders/test_builder.py index a9189d3ee..1e45c9cd4 100644 --- a/tests/masonry/builders/test_builder.py +++ b/tests/masonry/builders/test_builder.py @@ -267,3 +267,16 @@ def test_builder_convert_script_files(fixture: str, result: List[Path]): project_root = Path(__file__).parent / "fixtures" / fixture script_files = Builder(Factory().create_poetry(project_root)).convert_script_files() assert [p.relative_to(project_root) for p in script_files] == result + + +def test_metadata_with_readme_files(): + test_path = Path(__file__).parent.parent.parent / "fixtures" / "with_readme_files" + builder = Builder(Factory().create_poetry(test_path)) + + metadata = Parser().parsestr(builder.get_metadata_content()) + + readme1 = test_path / "README-1.rst" + readme2 = test_path / "README-2.rst" + description = "\n".join([readme1.read_text(), readme2.read_text(), ""]) + + assert metadata.get_payload() == description diff --git a/tests/packages/test_package.py b/tests/packages/test_package.py index 760906a17..394d9e656 100644 --- a/tests/packages/test_package.py +++ b/tests/packages/test_package.py @@ -408,3 +408,22 @@ def test_only_with_dependency_groups(package_with_groups: Package): assert len(package.requires) == 2 assert len(package.all_requires) == 2 + + +def test_get_readme_property_with_multiple_readme_files(): + package = Package("foo", "0.1.0") + + package.readmes = ("README.md", "HISTORY.md") + with pytest.deprecated_call(): + assert package.readme == "README.md" + + +def test_set_readme_property(): + package = Package("foo", "0.1.0") + + with pytest.deprecated_call(): + package.readme = "README.md" + + assert package.readmes == ("README.md",) + with pytest.deprecated_call(): + assert package.readme == "README.md" diff --git a/tests/test_factory.py b/tests/test_factory.py index c29590415..3a9d80d39 100644 --- a/tests/test_factory.py +++ b/tests/test_factory.py @@ -20,7 +20,7 @@ def test_create_poetry(): assert package.authors == ["Sébastien Eustace "] assert package.license.id == "MIT" assert ( - package.readme.relative_to(fixtures_dir).as_posix() + package.readmes[0].relative_to(fixtures_dir).as_posix() == "sample_project/README.rst" ) assert package.homepage == "https://python-poetry.org" @@ -182,6 +182,26 @@ def test_validate_fails(): assert Factory.validate(content) == {"errors": [expected], "warnings": []} +def test_strict_validation_success_on_multiple_readme_files(): + with_readme_files = TOMLFile(fixtures_dir / "with_readme_files" / "pyproject.toml") + content = with_readme_files.read()["tool"]["poetry"] + + assert Factory.validate(content, strict=True) == {"errors": [], "warnings": []} + + +def test_strict_validation_fails_on_readme_files_with_unmatching_types(): + with_readme_files = TOMLFile(fixtures_dir / "with_readme_files" / "pyproject.toml") + content = with_readme_files.read()["tool"]["poetry"] + content["readme"][0] = "README.md" + + assert Factory.validate(content, strict=True) == { + "errors": [ + "Declared README files must be of same type: found text/markdown, text/x-rst" + ], + "warnings": [], + } + + def test_create_poetry_fails_on_invalid_configuration(): with pytest.raises(RuntimeError) as e: Factory().create_poetry( From 6b98c3e78d280d83f8a9cfefd2b81d0bd74730a0 Mon Sep 17 00:00:00 2001 From: Wagner Macedo Date: Sat, 20 Nov 2021 07:33:50 +0100 Subject: [PATCH 2/3] Refactoring in order to remove Package.description_type --- src/poetry/core/factory.py | 16 +++------------- src/poetry/core/masonry/metadata.py | 6 ++++-- src/poetry/core/packages/package.py | 1 - src/poetry/core/utils/helpers.py | 10 ++++++++++ tests/utils/test_helpers.py | 18 ++++++++++++++++++ 5 files changed, 35 insertions(+), 16 deletions(-) diff --git a/src/poetry/core/factory.py b/src/poetry/core/factory.py index e9a3582f1..be22a6d9b 100644 --- a/src/poetry/core/factory.py +++ b/src/poetry/core/factory.py @@ -9,6 +9,8 @@ from typing import Union from warnings import warn +from poetry.core.utils.helpers import readme_content_type + if TYPE_CHECKING: from poetry.core.packages.project_package import ProjectPackage @@ -98,8 +100,6 @@ def configure_package( else: package.readmes = tuple(root / readme for readme in config["readme"]) - package.description_type = cls._readme_content_type(package.readmes[0]) - if "platform" in config: package.platform = config["platform"] @@ -428,7 +428,7 @@ def validate(cls, config: dict, strict: bool = False) -> Dict[str, List[str]]: # Checking types of all readme files (must match) if "readme" in config and not isinstance(config["readme"], str): - readme_types = [cls._readme_content_type(r) for r in config["readme"]] + readme_types = [readme_content_type(r) for r in config["readme"]] if len(set(readme_types)) > 1: result["errors"].append( f"Declared README files must be of same type: found {', '.join(readme_types)}" @@ -454,13 +454,3 @@ def locate(cls, cwd: Optional[Path] = None) -> Path: cwd ) ) - - @staticmethod - def _readme_content_type(path: Union[str, Path]) -> str: - suffix = Path(path).suffix - if suffix == ".rst": - return "text/x-rst" - elif suffix in [".md", ".markdown"]: - return "text/markdown" - else: - return "text/plain" diff --git a/src/poetry/core/masonry/metadata.py b/src/poetry/core/masonry/metadata.py index 1a37e6e31..082bf6b54 100644 --- a/src/poetry/core/masonry/metadata.py +++ b/src/poetry/core/masonry/metadata.py @@ -2,6 +2,8 @@ from typing import List from typing import Tuple +from poetry.core.utils.helpers import readme_content_type + if TYPE_CHECKING: from poetry.core.packages.package import Package @@ -81,8 +83,8 @@ def from_package(cls, package: "Package") -> "Metadata": meta.requires_dist = [d.to_pep_508() for d in package.requires] # Version 2.1 - if package.description_type: - meta.description_content_type = package.description_type + if package.readmes: + meta.description_content_type = readme_content_type(package.readmes[0]) meta.provides_extra = list(package.extras) diff --git a/src/poetry/core/packages/package.py b/src/poetry/core/packages/package.py index fea8b1065..bbbf1433a 100644 --- a/src/poetry/core/packages/package.py +++ b/src/poetry/core/packages/package.py @@ -89,7 +89,6 @@ def __init__( self.keywords = [] self._license = None self.readmes: Tuple[Path, ...] = () - self.description_type: Optional[str] = None self.extras = {} self.requires_extras = [] diff --git a/src/poetry/core/utils/helpers.py b/src/poetry/core/utils/helpers.py index fd57f0ed3..7a2842c67 100644 --- a/src/poetry/core/utils/helpers.py +++ b/src/poetry/core/utils/helpers.py @@ -101,3 +101,13 @@ def merge_dicts(d1: dict, d2: dict) -> None: merge_dicts(d1[k], d2[k]) else: d1[k] = d2[k] + + +def readme_content_type(path: Union[str, Path]) -> str: + suffix = Path(path).suffix + if suffix == ".rst": + return "text/x-rst" + elif suffix in [".md", ".markdown"]: + return "text/markdown" + else: + return "text/plain" diff --git a/tests/utils/test_helpers.py b/tests/utils/test_helpers.py index eeac12797..d407ba679 100644 --- a/tests/utils/test_helpers.py +++ b/tests/utils/test_helpers.py @@ -1,11 +1,14 @@ import os +from pathlib import Path from stat import S_IREAD +from typing import Union import pytest from poetry.core.utils.helpers import canonicalize_name from poetry.core.utils.helpers import parse_requires +from poetry.core.utils.helpers import readme_content_type from poetry.core.utils.helpers import temporary_directory @@ -76,3 +79,18 @@ def test_utils_helpers_temporary_directory_readonly_file(): assert not os.path.exists(temp_dir) assert not os.path.exists(readonly_filename) + + +@pytest.mark.parametrize( + "readme, content_type", + [ + ("README.rst", "text/x-rst"), + ("README.md", "text/markdown"), + ("README", "text/plain"), + (Path("README.rst"), "text/x-rst"), + (Path("README.md"), "text/markdown"), + (Path("README"), "text/plain"), + ], +) +def test_utils_helpers_readme_content_type(readme: Union[str, Path], content_type: str): + assert readme_content_type(readme) == content_type From b1971c3590772b317757f1443d3a75f4245f0bea Mon Sep 17 00:00:00 2001 From: Wagner Macedo Date: Sun, 21 Nov 2021 00:56:43 +0100 Subject: [PATCH 3/3] Apply suggestions from code review (@neersighted) --- src/poetry/core/factory.py | 6 +++--- src/poetry/core/packages/package.py | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/poetry/core/factory.py b/src/poetry/core/factory.py index be22a6d9b..d383037d1 100644 --- a/src/poetry/core/factory.py +++ b/src/poetry/core/factory.py @@ -428,10 +428,10 @@ def validate(cls, config: dict, strict: bool = False) -> Dict[str, List[str]]: # Checking types of all readme files (must match) if "readme" in config and not isinstance(config["readme"], str): - readme_types = [readme_content_type(r) for r in config["readme"]] - if len(set(readme_types)) > 1: + readme_types = {readme_content_type(r) for r in config["readme"]} + if len(readme_types) > 1: result["errors"].append( - f"Declared README files must be of same type: found {', '.join(readme_types)}" + f"Declared README files must be of same type: found {', '.join(sorted(readme_types))}" ) return result diff --git a/src/poetry/core/packages/package.py b/src/poetry/core/packages/package.py index bbbf1433a..7a77e2bee 100644 --- a/src/poetry/core/packages/package.py +++ b/src/poetry/core/packages/package.py @@ -353,7 +353,7 @@ def readme(self) -> Path: import warnings warnings.warn( - "`readme` is deprecated: you are getting only the first readme file. Please use the plural form `readmes`", + "`readme` is deprecated: you are getting only the first readme file. Please use the plural form `readmes`.", DeprecationWarning, ) return next(iter(self.readmes), None) @@ -363,7 +363,7 @@ def readme(self, path: Path) -> None: import warnings warnings.warn( - "`readme` is deprecated. Please assign a tuple to the plural form `readmes`", + "`readme` is deprecated. Please assign a tuple to the plural form `readmes`.", DeprecationWarning, ) self.readmes = (path,)