canonicalized extra names (#6541)

Co-authored-by: Randy Döring <30527984+radoering@users.noreply.github.com>

canonicalized extra names (#6541)
Co-authored-by: Randy Döring <30527984+radoering@users.noreply.github.com>
44a89cbd · David Hotham · GitHub · 85a0913c · 44a89cbd · 44a89cbd
Commit 44a89cbd authored Oct 03, 2022 by David Hotham Committed by GitHub Oct 03, 2022
16 changed files
--- 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
@@ -951,7 +958,7 @@ testing = ["func-timeout", "jaraco.itertools", "pytest (>=6)", "pytest-black (>=
 [metadata]
 lock-version = "1.1"
 python-versions = "^3.7"
-content-hash = "d7ae3f4917162a35d94dee690a627a70505794ba345b39703ba10e08d7064dac"
+content-hash = "1bde59127c0958c1a4b0d097e29478841141558832d05e70b6dd97a8d2388364"
 [metadata.files]
 attrs = [
@@ -1359,13 +1366,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 = [
+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-plugin-export = [
-    {file = "poetry-plugin-export-1.0.7.tar.gz", hash = "sha256:f6ac707ae227b06b2481249ed2678ff6b810b3487cac0fbb66eb0dc2bfd6ecf1"},
+    {file = "poetry-plugin-export-1.1.1.tar.gz", hash = "sha256:23e3e512a609b54ef5ac441339fc9e68fd41e61d15bd924eb0094b4fda1e30d0"},
-    {file = "poetry_plugin_export-1.0.7-py3-none-any.whl", hash = "sha256:dd9d4552e7113a86c97908c13b9a439cb46830f247c7e4969e46a0d8d70e4d3f"},
+    {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"},

--- 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"
@@ -77,6 +77,8 @@ urllib3 = "^1.26.0"
 pre-commit = "^2.6"
 [tool.poetry.group.test.dependencies]
+# 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"
 flatdict = "^4.0.1"
 httpretty = "^1.0"

--- a/src/poetry/console/commands/install.py
+++ b/src/poetry/console/commands/install.py
@@ -115,6 +115,7 @@ dependencies and not including the current project, run the command with the
            )
            return 1
+        extras: list[str]
        if self.option("all-extras"):
            extras = list(self.poetry.package.extras.keys())
        else:

--- a/src/poetry/installation/installer.py
+++ b/src/poetry/installation/installer.py
@@ -63,7 +63,7 @@ class Installer:
        self._whitelist: list[NormalizedName] = []
-        self._extras: list[str] = []
+        self._extras: list[NormalizedName] = []
        if executor is None:
            executor = Executor(
@@ -175,7 +175,7 @@ class 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 +259,12 @@ class Installer:
                    "</warning>"
                )
+            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
@@ -538,11 +542,17 @@ class Installer:
        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 get_extra_package_names(repo.packages, extras, self._extras)

--- a/src/poetry/packages/locker.py
+++ b/src/poetry/packages/locker.py
@@ -11,6 +11,7 @@ from typing import TYPE_CHECKING
 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
@@ -164,6 +165,7 @@ class Locker:
            extras = info.get("extras", {})
            if extras:
                for name, deps in extras.items():
+                    name = canonicalize_name(name)
                    package.extras[name] = []
                    for dep in deps:

--- a/src/poetry/puzzle/provider.py
+++ b/src/poetry/puzzle/provider.py
@@ -33,7 +33,6 @@ from poetry.packages.package_collection import PackageCollection
 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
@@ -580,7 +579,6 @@ class Provider:
        # to the current package
        if dependency.extras:
            for extra in dependency.extras:
-                extra = safe_extra(extra)
                if extra not in package.extras:
                    continue
@@ -615,9 +613,7 @@ class Provider:
                (dep.is_optional() and dep.name not in optional_dependencies)
                or (
                    dep.in_extras
-                    and not set(dep.in_extras).intersection(
+                    and not set(dep.in_extras).intersection(dependency.extras)
-                        {safe_extra(extra) for extra in dependency.extras}
-                    )
                )
            ):
                continue

--- a/src/poetry/utils/extras.py
+++ b/src/poetry/utils/extras.py
@@ -14,8 +14,8 @@ if TYPE_CHECKING:
 def get_extra_package_names(
    packages: Iterable[Package],
-    extras: Mapping[str, list[str]],
+    extras: Mapping[NormalizedName, Iterable[NormalizedName]],
-    extra_names: Collection[str],
+    extra_names: Collection[NormalizedName],
 ) -> set[NormalizedName]:
    """
    Returns all package names required by the given extras.

--- a/src/poetry/utils/helpers.py
+++ b/src/poetry/utils/helpers.py
 from __future__ import annotations
 import os
-import re
 import shutil
 import stat
 import sys
@@ -171,18 +170,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.")

--- 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.
    """

--- a/tests/installation/fixtures/with-dependencies-nested-extras.test
+++ b/tests/installation/fixtures/with-dependencies-nested-extras.test
@@ -8,10 +8,10 @@ python-versions = "*"
 files = []
 [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"

--- a/tests/installation/fixtures/with-directory-dependency-poetry-transitive.test
+++ b/tests/installation/fixtures/with-directory-dependency-poetry-transitive.test
@@ -55,8 +55,8 @@ files = []
 version = "1.2.3"
 [package.extras]
-extras_a = ["pendulum (>=1.4.4)"]
+extras-a = ["pendulum (>=1.4.4)"]
-extras_b = ["cachy (>=0.2.0)"]
+extras-b = ["cachy (>=0.2.0)"]
 [package.source]
 type = "directory"

--- a/tests/installation/fixtures/with-directory-dependency-poetry.test
+++ b/tests/installation/fixtures/with-directory-dependency-poetry.test
@@ -21,8 +21,8 @@ version = "1.2.3"
 pendulum = {version = ">=1.4.4", optional = true}
 [package.extras]
-extras_a = ["pendulum (>=1.4.4)"]
+extras-a = ["pendulum (>=1.4.4)"]
-extras_b = ["cachy (>=0.2.0)"]
+extras-b = ["cachy (>=0.2.0)"]
 [package.source]
 type = "directory"

--- 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-a": [get_dependency("pendulum", ">=1.4.4")],
-        "extras_b": [get_dependency("cachy", ">=0.2.0")],
+        "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-a": [get_dependency("pendulum", ">=1.4.4")],
-        "extras_b": [get_dependency("cachy", ">=0.2.0")],
+        "extras-b": [get_dependency("cachy", ">=0.2.0")],
    }

--- a/tests/repositories/test_pypi_repository.py
+++ b/tests/repositories/test_pypi_repository.py
@@ -238,14 +238,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-pymssql": [Dependency("pymssql", "*")],
-        "mssql_pyodbc": [Dependency("pyodbc", "*")],
+        "mssql-pyodbc": [Dependency("pyodbc", "*")],
        "mysql": [Dependency("mysqlclient", "*")],
        "oracle": [Dependency("cx_oracle", "*")],
        "postgresql": [Dependency("psycopg2", "*")],
-        "postgresql_pg8000": [Dependency("pg8000", "*")],
+        "postgresql-pg8000": [Dependency("pg8000", "*")],
-        "postgresql_psycopg2binary": [Dependency("psycopg2-binary", "*")],
+        "postgresql-psycopg2binary": [Dependency("psycopg2-binary", "*")],
-        "postgresql_psycopg2cffi": [Dependency("psycopg2cffi", "*")],
+        "postgresql-psycopg2cffi": [Dependency("psycopg2cffi", "*")],
        "pymysql": [Dependency("pymysql", "*")],
    }
@@ -270,7 +270,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"),

--- a/tests/test_factory.py
+++ b/tests/test_factory.py
@@ -6,6 +6,7 @@ from typing import TYPE_CHECKING
 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)

--- a/tests/utils/test_helpers.py
+++ b/tests/utils/test_helpers.py
@@ -2,8 +2,6 @@ from __future__ import annotations
 from poetry.core.utils.helpers import parse_requires
-from poetry.utils.helpers import safe_extra
 def test_parse_requires():
    requires = """\
@@ -59,10 +57,3 @@ isort@ git+git://github.com/timothycrosley/isort.git@e63ae06ec7d70b06df9e5283576
    ]
    # fmt: on
    assert result == expected
-def test_safe_extra():
-    extra = "pandas.CSVDataSet"
-    result = safe_extra(extra)
-    expected = "pandas.csvdataset"
-    assert result == expected