Skip to content

Commit

Permalink
canonicalized extra names (python-poetry#6541)
Browse files Browse the repository at this point in the history
(cherry picked from commit 44a89cb)

Co-authored-by: Randy Döring <30527984+radoering@users.noreply.github.com>
  • Loading branch information
dimbleby and radoering committed Oct 3, 2022
1 parent 7cd761a commit b2e9d3b
Show file tree
Hide file tree
Showing 16 changed files with 68 additions and 63 deletions.
22 changes: 13 additions & 9 deletions poetry.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 3 additions & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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" }
Expand Down
1 change: 1 addition & 0 deletions src/poetry/console/commands/install.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
21 changes: 16 additions & 5 deletions src/poetry/installation/installer.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -63,7 +64,7 @@ def __init__(

self._whitelist: list[str] = []

self._extras: list[str] = []
self._extras: list[NormalizedName] = []

if executor is None:
executor = Executor(
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -259,8 +260,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
Expand Down Expand Up @@ -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))

Expand Down
2 changes: 2 additions & 0 deletions src/poetry/packages/locker.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down
6 changes: 1 addition & 5 deletions src/poetry/puzzle/provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down Expand Up @@ -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

Expand All @@ -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
Expand Down
7 changes: 4 additions & 3 deletions src/poetry/utils/extras.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from __future__ import annotations

from typing import TYPE_CHECKING
from typing import Collection


if TYPE_CHECKING:
Expand All @@ -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.
Expand Down
13 changes: 0 additions & 13 deletions src/poetry/utils/helpers.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
from __future__ import annotations

import os
import re
import shutil
import stat
import sys
Expand Down Expand Up @@ -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.")
Expand Down
4 changes: 2 additions & 2 deletions tests/console/commands/test_install.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
"""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
8 changes: 4 additions & 4 deletions tests/puzzle/test_provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")],
}


Expand Down Expand Up @@ -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")],
}


Expand Down
12 changes: 6 additions & 6 deletions tests/repositories/test_pypi_repository.py
Original file line number Diff line number Diff line change
Expand Up @@ -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", "*")],
}

Expand All @@ -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"),
Expand Down
10 changes: 10 additions & 0 deletions tests/test_factory.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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)

Expand Down
9 changes: 0 additions & 9 deletions tests/utils/test_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@

from poetry.core.utils.helpers import parse_requires

from poetry.utils.helpers import safe_extra


def test_parse_requires():
requires = """\
Expand Down Expand Up @@ -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

0 comments on commit b2e9d3b

Please sign in to comment.