canonicalized extra names (#6541)

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

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"

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:

src/poetry/installation/installer.py

@@ -63,7 +63,7 @@ def __init__(
 
  self._whitelist: list[NormalizedName] = []
 
- self._extras: list[str] = []
+ self._extras: list[NormalizedName] = []
 
  if executor is None:
  executor = Executor(
@@ -175,7 +175,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 +259,12 @@ def _do_install(self) -> int:
  "</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 @@ def _get_extra_packages(self, repo: Repository) -> set[NormalizedName]:
 
  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)
 

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
@@ -164,6 +165,7 @@ def locked_repository(self) -> LockfileRepository:
  extras = info.get("extras", {})
  if extras:
  for name, deps in extras.items():
+ name = canonicalize_name(name)
  package.extras[name] = []
 
  for dep in deps:

src/poetry/puzzle/provider.py

@@ -33,7 +33,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
 
 
@@ -580,7 +579,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
 
@@ -615,9 +613,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

src/poetry/utils/extras.py

@@ -14,8 +14,8 @@
 
 def get_extra_package_names(
  packages: Iterable[Package],
- extras: Mapping[str, list[str]],
- extra_names: Collection[str],
+ extras: Mapping[NormalizedName, Iterable[NormalizedName]],
+ extra_names: Collection[NormalizedName],
 ) -> set[NormalizedName]:
  """
  Returns all package names required by the given extras.

src/poetry/utils/helpers.py

@@ -1,7 +1,6 @@
 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.")

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

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"

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_b = ["cachy (>=0.2.0)"]
+extras-a = ["pendulum (>=1.4.4)"]
+extras-b = ["cachy (>=0.2.0)"]
 
 [package.source]
 type = "directory"

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_b = ["cachy (>=0.2.0)"]
+extras-a = ["pendulum (>=1.4.4)"]
+extras-b = ["cachy (>=0.2.0)"]
 
 [package.source]
 type = "directory"

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")],
  }
 
 

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_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", "*")],
  }
 
@@ -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"),

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)
 

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