From b2e9d3b946054c71ec6cb027f6a84e67076798e4 Mon Sep 17 00:00:00 2001 From: David Hotham Date: Mon, 3 Oct 2022 18:50:32 +0100 Subject: [PATCH] canonicalized extra names (#6541) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit (cherry picked from commit 44a89cbd7530bc3c50c87f672c221f61125faf54) Co-authored-by: Randy Döring <30527984+radoering@users.noreply.github.com> --- poetry.lock | 22 +++++++++++-------- pyproject.toml | 4 +++- src/poetry/console/commands/install.py | 1 + src/poetry/installation/installer.py | 21 +++++++++++++----- src/poetry/packages/locker.py | 2 ++ src/poetry/puzzle/provider.py | 6 +---- src/poetry/utils/extras.py | 7 +++--- src/poetry/utils/helpers.py | 13 ----------- tests/console/commands/test_install.py | 4 ++-- .../with-dependencies-nested-extras.test | 4 ++-- ...irectory-dependency-poetry-transitive.test | 4 ++-- .../with-directory-dependency-poetry.test | 4 ++-- tests/puzzle/test_provider.py | 8 +++---- tests/repositories/test_pypi_repository.py | 12 +++++----- tests/test_factory.py | 10 +++++++++ tests/utils/test_helpers.py | 9 -------- 16 files changed, 68 insertions(+), 63 deletions(-) diff --git a/poetry.lock b/poetry.lock index 26316b7197a..6a7fc262afc 100644 --- a/poetry.lock +++ b/poetry.lock @@ -517,14 +517,21 @@ version = "1.2.0" description = "Poetry PEP 517 Build Backend" category = "main" optional = false -python-versions = ">=3.7,<4.0" +python-versions = "^3.7" +develop = false [package.dependencies] importlib-metadata = {version = ">=1.7.0", markers = "python_version < \"3.8\""} +[package.source] +type = "git" +url = "https://github.com/dimbleby/poetry-core.git" +reference = "canonicalize-extras" +resolved_reference = "9f58ecf8dddbafd38a2b4f2367a5f0f1d8ff25f4" + [[package]] name = "poetry-plugin-export" -version = "1.0.7" +version = "1.1.1" description = "Poetry plugin to export the dependencies to various formats" category = "main" optional = false @@ -976,7 +983,7 @@ testing = ["func-timeout", "jaraco.itertools", "pytest (>=6)", "pytest-black (>= [metadata] lock-version = "1.1" python-versions = "^3.7" -content-hash = "a3480636766bb3c5c00e1ed48b6b70136726754001837ef7515c6b355fa35812" +content-hash = "d16cc698c71f2387eb8ce7c658ccc77830ac8aee4b6085bbe0e01119a50c2be6" [metadata.files] attrs = [ @@ -1379,13 +1386,10 @@ pluggy = [ {file = "pluggy-1.0.0-py2.py3-none-any.whl", hash = "sha256:74134bbf457f031a36d68416e1509f34bd5ccc019f0bcc952c7b909d06b37bd3"}, {file = "pluggy-1.0.0.tar.gz", hash = "sha256:4224373bacce55f955a878bf9cfa763c1e360858e330072059e10bad68531159"}, ] -poetry-core = [ - {file = "poetry-core-1.2.0.tar.gz", hash = "sha256:ceccec95487e46c63a41761fbac5211b809bca22658e25a049f4c7da96269f71"}, - {file = "poetry_core-1.2.0-py3-none-any.whl", hash = "sha256:e248d36c1314dd60fbc66390791923ad8b58c629d3e587080b7c1537a1c0d30f"}, -] +poetry-core = [] poetry-plugin-export = [ - {file = "poetry-plugin-export-1.0.7.tar.gz", hash = "sha256:f6ac707ae227b06b2481249ed2678ff6b810b3487cac0fbb66eb0dc2bfd6ecf1"}, - {file = "poetry_plugin_export-1.0.7-py3-none-any.whl", hash = "sha256:dd9d4552e7113a86c97908c13b9a439cb46830f247c7e4969e46a0d8d70e4d3f"}, + {file = "poetry-plugin-export-1.1.1.tar.gz", hash = "sha256:23e3e512a609b54ef5ac441339fc9e68fd41e61d15bd924eb0094b4fda1e30d0"}, + {file = "poetry_plugin_export-1.1.1-py3-none-any.whl", hash = "sha256:170fa367794d2385975d75298fe5509f772d35216ee36b8fa50c0350a064b761"}, ] pre-commit = [ {file = "pre_commit-2.20.0-py2.py3-none-any.whl", hash = "sha256:51a5ba7c480ae8072ecdb6933df22d2f812dc897d5fe848778116129a681aac7"}, diff --git a/pyproject.toml b/pyproject.toml index c38762d3838..f7cb9a55f87 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -45,7 +45,7 @@ generate-setup-file = false python = "^3.7" poetry-core = "1.2.0" -poetry-plugin-export = "^1.0.7" +poetry-plugin-export = "^1.1.1" "backports.cached-property" = { version = "^1.0.2", python = "<3.8" } cachecontrol = { version = "^0.12.9", extras = ["filecache"] } cachy = "^0.3.0" @@ -81,6 +81,8 @@ pytest-randomly = "^3.10" pytest-sugar = "^0.9" pytest-xdist = { version = "^2.5", extras = ["psutil"] } pre-commit = "^2.6" +# TODO: remove as soon as poetry-core with poetry-core#476 is available +poetry-core = { git = "https://github.com/dimbleby/poetry-core.git", branch = "canonicalize-extras" } deepdiff = "^5.0" httpretty = "^1.0" typing-extensions = { version = "^4.0.0", python = "<3.8" } diff --git a/src/poetry/console/commands/install.py b/src/poetry/console/commands/install.py index 7ce0776a237..f2060033253 100644 --- a/src/poetry/console/commands/install.py +++ b/src/poetry/console/commands/install.py @@ -115,6 +115,7 @@ def handle(self) -> int: ) return 1 + extras: list[str] if self.option("all-extras"): extras = list(self.poetry.package.extras.keys()) else: diff --git a/src/poetry/installation/installer.py b/src/poetry/installation/installer.py index 38127feb423..ed015952e4e 100644 --- a/src/poetry/installation/installer.py +++ b/src/poetry/installation/installer.py @@ -3,6 +3,7 @@ from typing import TYPE_CHECKING from cleo.io.null_io import NullIO +from packaging.utils import NormalizedName from packaging.utils import canonicalize_name from poetry.installation.executor import Executor @@ -63,7 +64,7 @@ def __init__( self._whitelist: list[str] = [] - self._extras: list[str] = [] + self._extras: list[NormalizedName] = [] if executor is None: executor = Executor( @@ -175,7 +176,7 @@ def whitelist(self, packages: Iterable[str]) -> Installer: return self def extras(self, extras: list[str]) -> Installer: - self._extras = extras + self._extras = [canonicalize_name(extra) for extra in extras] return self @@ -259,8 +260,12 @@ def _do_install(self) -> int: "" ) + locker_extras = { + canonicalize_name(extra) + for extra in self._locker.lock_data.get("extras", {}) + } for extra in self._extras: - if extra not in self._locker.lock_data.get("extras", {}): + if extra not in locker_extras: raise ValueError(f"Extra [{extra}] is not specified.") # If we are installing from lock @@ -547,11 +552,17 @@ def _get_extra_packages(self, repo: Repository) -> list[str]: Maybe we just let the solver handle it? """ - extras: dict[str, list[str]] + extras: dict[NormalizedName, list[NormalizedName]] if self._update: extras = {k: [d.name for d in v] for k, v in self._package.extras.items()} else: - extras = self._locker.lock_data.get("extras", {}) + raw_extras = self._locker.lock_data.get("extras", {}) + extras = { + canonicalize_name(extra): [ + canonicalize_name(dependency) for dependency in dependencies + ] + for extra, dependencies in raw_extras.items() + } return list(get_extra_package_names(repo.packages, extras, self._extras)) diff --git a/src/poetry/packages/locker.py b/src/poetry/packages/locker.py index ceadc1febf0..78607fd16b4 100644 --- a/src/poetry/packages/locker.py +++ b/src/poetry/packages/locker.py @@ -11,6 +11,7 @@ from typing import Any from typing import cast +from packaging.utils import canonicalize_name from poetry.core.packages.dependency import Dependency from poetry.core.packages.package import Package from poetry.core.semver.helpers import parse_constraint @@ -145,6 +146,7 @@ def locked_repository(self) -> Repository: extras = info.get("extras", {}) if extras: for name, deps in extras.items(): + name = canonicalize_name(name) package.extras[name] = [] for dep in deps: diff --git a/src/poetry/puzzle/provider.py b/src/poetry/puzzle/provider.py index 92db9901251..04eed064833 100644 --- a/src/poetry/puzzle/provider.py +++ b/src/poetry/puzzle/provider.py @@ -32,7 +32,6 @@ from poetry.puzzle.exceptions import OverrideNeeded from poetry.repositories.exceptions import PackageNotFound from poetry.utils.helpers import download_file -from poetry.utils.helpers import safe_extra from poetry.vcs.git import Git @@ -563,7 +562,6 @@ def complete_package( # to the current package if dependency.extras: for extra in dependency.extras: - extra = safe_extra(extra) if extra not in package.extras: continue @@ -590,9 +588,7 @@ def complete_package( (dep.is_optional() and dep.name not in optional_dependencies) or ( dep.in_extras - and not set(dep.in_extras).intersection( - {safe_extra(extra) for extra in dependency.extras} - ) + and not set(dep.in_extras).intersection(dependency.extras) ) ): continue diff --git a/src/poetry/utils/extras.py b/src/poetry/utils/extras.py index e0681d46b76..651204f2f80 100644 --- a/src/poetry/utils/extras.py +++ b/src/poetry/utils/extras.py @@ -1,6 +1,7 @@ from __future__ import annotations from typing import TYPE_CHECKING +from typing import Collection if TYPE_CHECKING: @@ -15,9 +16,9 @@ def get_extra_package_names( packages: Sequence[Package], - extras: Mapping[str, list[str]], - extra_names: Sequence[str], -) -> Iterable[str]: + extras: Mapping[NormalizedName, Iterable[NormalizedName]], + extra_names: Collection[NormalizedName], +) -> set[NormalizedName]: """ Returns all package names required by the given extras. diff --git a/src/poetry/utils/helpers.py b/src/poetry/utils/helpers.py index caee1b279bf..1f38d20e4b7 100644 --- a/src/poetry/utils/helpers.py +++ b/src/poetry/utils/helpers.py @@ -1,7 +1,6 @@ from __future__ import annotations import os -import re import shutil import stat import sys @@ -152,18 +151,6 @@ def pluralize(count: int, word: str = "") -> str: return word + "s" -def safe_extra(extra: str) -> str: - """Convert an arbitrary string to a standard 'extra' name. - - Any runs of non-alphanumeric characters are replaced with a single '_', - and the result is always lowercased. - - See - https://github.com/pypa/setuptools/blob/452e13c/pkg_resources/__init__.py#L1423-L1431. - """ - return re.sub("[^A-Za-z0-9.-]+", "_", extra).lower() - - def _get_win_folder_from_registry(csidl_name: str) -> str: if sys.platform != "win32": raise RuntimeError("Method can only be called on Windows.") diff --git a/tests/console/commands/test_install.py b/tests/console/commands/test_install.py index 09efe8ecbcd..6ba6417f761 100644 --- a/tests/console/commands/test_install.py +++ b/tests/console/commands/test_install.py @@ -166,10 +166,10 @@ def test_all_extras_populates_installer(tester: CommandTester, mocker: MockerFix tester.execute("--all-extras") - assert tester.command.installer._extras == ["extras_a", "extras_b"] + assert tester.command.installer._extras == ["extras-a", "extras-b"] -def test_extras_conlicts_all_extras(tester: CommandTester, mocker: MockerFixture): +def test_extras_conflicts_all_extras(tester: CommandTester, mocker: MockerFixture): """ The --extras doesn't make sense with --all-extras. """ diff --git a/tests/installation/fixtures/with-dependencies-nested-extras.test b/tests/installation/fixtures/with-dependencies-nested-extras.test index 369aa3cd74b..b21750d0f71 100644 --- a/tests/installation/fixtures/with-dependencies-nested-extras.test +++ b/tests/installation/fixtures/with-dependencies-nested-extras.test @@ -7,10 +7,10 @@ optional = false python-versions = "*" [package.dependencies] -B = {version = "^1.0", optional = true, extras = ["C"]} +B = {version = "^1.0", optional = true, extras = ["c"]} [package.extras] -b = ["B[C] (>=1.0,<2.0)"] +b = ["B[c] (>=1.0,<2.0)"] [[package]] name = "B" diff --git a/tests/installation/fixtures/with-directory-dependency-poetry-transitive.test b/tests/installation/fixtures/with-directory-dependency-poetry-transitive.test index fb10b1accea..a7fef5fe68d 100644 --- a/tests/installation/fixtures/with-directory-dependency-poetry-transitive.test +++ b/tests/installation/fixtures/with-directory-dependency-poetry-transitive.test @@ -48,8 +48,8 @@ python-versions = "*" version = "1.2.3" [package.extras] -extras_a = ["pendulum (>=1.4.4)"] -extras_b = ["cachy (>=0.2.0)"] +extras-a = ["pendulum (>=1.4.4)"] +extras-b = ["cachy (>=0.2.0)"] [package.source] type = "directory" diff --git a/tests/installation/fixtures/with-directory-dependency-poetry.test b/tests/installation/fixtures/with-directory-dependency-poetry.test index 12431f62185..d96812b9fea 100644 --- a/tests/installation/fixtures/with-directory-dependency-poetry.test +++ b/tests/installation/fixtures/with-directory-dependency-poetry.test @@ -19,8 +19,8 @@ version = "1.2.3" pendulum = {version = ">=1.4.4", optional = true} [package.extras] -extras_a = ["pendulum (>=1.4.4)"] -extras_b = ["cachy (>=0.2.0)"] +extras-a = ["pendulum (>=1.4.4)"] +extras-b = ["cachy (>=0.2.0)"] [package.source] type = "directory" diff --git a/tests/puzzle/test_provider.py b/tests/puzzle/test_provider.py index 60a45e9fca6..1bbbeacd48d 100644 --- a/tests/puzzle/test_provider.py +++ b/tests/puzzle/test_provider.py @@ -462,8 +462,8 @@ def test_search_for_directory_poetry(provider: Provider): get_dependency("pendulum", ">=1.4.4"), ] assert package.extras == { - "extras_a": [get_dependency("pendulum", ">=1.4.4")], - "extras_b": [get_dependency("cachy", ">=0.2.0")], + "extras-a": [get_dependency("pendulum", ">=1.4.4")], + "extras-b": [get_dependency("cachy", ">=0.2.0")], } @@ -491,8 +491,8 @@ def test_search_for_directory_poetry_with_extras(provider: Provider): get_dependency("pendulum", ">=1.4.4"), ] assert package.extras == { - "extras_a": [get_dependency("pendulum", ">=1.4.4")], - "extras_b": [get_dependency("cachy", ">=0.2.0")], + "extras-a": [get_dependency("pendulum", ">=1.4.4")], + "extras-b": [get_dependency("cachy", ">=0.2.0")], } diff --git a/tests/repositories/test_pypi_repository.py b/tests/repositories/test_pypi_repository.py index eda05c1e88f..a93f40f3411 100644 --- a/tests/repositories/test_pypi_repository.py +++ b/tests/repositories/test_pypi_repository.py @@ -237,14 +237,14 @@ def test_fallback_can_read_setup_to_get_dependencies() -> None: assert len([r for r in package.requires if r.is_optional()]) == 9 assert package.extras == { - "mssql_pymssql": [Dependency("pymssql", "*")], - "mssql_pyodbc": [Dependency("pyodbc", "*")], + "mssql-pymssql": [Dependency("pymssql", "*")], + "mssql-pyodbc": [Dependency("pyodbc", "*")], "mysql": [Dependency("mysqlclient", "*")], "oracle": [Dependency("cx_oracle", "*")], "postgresql": [Dependency("psycopg2", "*")], - "postgresql_pg8000": [Dependency("pg8000", "*")], - "postgresql_psycopg2binary": [Dependency("psycopg2-binary", "*")], - "postgresql_psycopg2cffi": [Dependency("psycopg2cffi", "*")], + "postgresql-pg8000": [Dependency("pg8000", "*")], + "postgresql-psycopg2binary": [Dependency("psycopg2-binary", "*")], + "postgresql-psycopg2cffi": [Dependency("psycopg2cffi", "*")], "pymysql": [Dependency("pymysql", "*")], } @@ -269,7 +269,7 @@ def test_pypi_repository_supports_reading_bz2_files() -> None: ] expected_extras = { - "all_non_platform": [ + "all-non-platform": [ Dependency("appdirs", ">=1.4.0"), Dependency("cryptography", ">=1.5"), Dependency("h2", ">=3.0,<4.0"), diff --git a/tests/test_factory.py b/tests/test_factory.py index 63eb150e413..348de29ff11 100644 --- a/tests/test_factory.py +++ b/tests/test_factory.py @@ -6,6 +6,7 @@ import pytest from deepdiff import DeepDiff +from packaging.utils import canonicalize_name from poetry.core.semver.helpers import parse_constraint from poetry.core.toml.file import TOMLFile @@ -152,6 +153,15 @@ def test_create_pyproject_from_package(project: str): result = pyproject["tool"]["poetry"] expected = poetry.pyproject.poetry_config + # Extras are normalized as they are read. + extras = expected.pop("extras", None) + if extras is not None: + normalized_extras = { + canonicalize_name(extra): dependencies + for extra, dependencies in extras.items() + } + expected["extras"] = normalized_extras + # packages do not support this at present expected.pop("scripts", None) diff --git a/tests/utils/test_helpers.py b/tests/utils/test_helpers.py index 6aacaa18f48..b5ab3cfebdc 100644 --- a/tests/utils/test_helpers.py +++ b/tests/utils/test_helpers.py @@ -2,8 +2,6 @@ from poetry.core.utils.helpers import parse_requires -from poetry.utils.helpers import safe_extra - def test_parse_requires(): requires = """\ @@ -59,10 +57,3 @@ def test_parse_requires(): ] # fmt: on assert result == expected - - -def test_safe_extra(): - extra = "pandas.CSVDataSet" - result = safe_extra(extra) - expected = "pandas.csvdataset" - assert result == expected