From b5493702188199a9a1f3f51cb39acbdd4e7bee7e Mon Sep 17 00:00:00 2001 From: finswimmer Date: Mon, 10 Feb 2025 07:21:25 +0100 Subject: [PATCH 01/22] feat: include groups into groups --- src/poetry/core/packages/dependency.py | 11 +- src/poetry/core/packages/dependency_group.py | 105 ++++++++++++------- tests/packages/test_dependency_group.py | 62 +++++++++++ 3 files changed, 136 insertions(+), 42 deletions(-) diff --git a/src/poetry/core/packages/dependency.py b/src/poetry/core/packages/dependency.py index 53ea864a0..22e2b2d0c 100644 --- a/src/poetry/core/packages/dependency.py +++ b/src/poetry/core/packages/dependency.py @@ -73,7 +73,7 @@ def __init__( if not groups: groups = [MAIN_GROUP] - self._groups = frozenset(canonicalize_name(g) for g in groups) + self.groups = frozenset(canonicalize_name(g) for g in groups) self._allows_prereleases = allows_prereleases # "_develop" is only required for enriching [project] dependencies self._develop = False @@ -115,10 +115,6 @@ def pretty_constraint(self) -> str: def pretty_name(self) -> str: return self._pretty_name - @property - def groups(self) -> frozenset[NormalizedName]: - return self._groups - @property def python_versions(self) -> str: return self._python_versions @@ -332,6 +328,11 @@ def with_constraint(self: T, constraint: str | VersionConstraint) -> T: dependency.constraint = constraint # type: ignore[assignment] return dependency + def with_groups(self, groups: Iterable[str]) -> Dependency: + dependency = self.clone() + dependency.groups = frozenset(canonicalize_name(g) for g in groups) + return dependency + @classmethod def create_from_pep_508( cls, name: str, relative_to: Path | None = None diff --git a/src/poetry/core/packages/dependency_group.py b/src/poetry/core/packages/dependency_group.py index 24dfded08..25f83dfd8 100644 --- a/src/poetry/core/packages/dependency_group.py +++ b/src/poetry/core/packages/dependency_group.py @@ -26,6 +26,7 @@ def __init__( self._mixed_dynamic = mixed_dynamic self._dependencies: list[Dependency] = [] self._poetry_dependencies: list[Dependency] = [] + self._included_dependency_groups: dict[NormalizedName, DependencyGroup] = {} @property def name(self) -> NormalizedName: @@ -37,59 +38,79 @@ def pretty_name(self) -> str: @property def dependencies(self) -> list[Dependency]: - if not self._dependencies: + group_dependencies = self._dependencies + included_group_dependencies = self._resolve_included_dependency_groups() + + if not group_dependencies: # legacy mode - return self._poetry_dependencies - if self._mixed_dynamic and self._poetry_dependencies: + group_dependencies = self._poetry_dependencies + elif self._mixed_dynamic and self._poetry_dependencies: if all(dep.is_optional() for dep in self._dependencies): - return [ + group_dependencies = [ *self._dependencies, *(d for d in self._poetry_dependencies if not d.is_optional()), ] - if all(not dep.is_optional() for dep in self._dependencies): - return [ + elif all(not dep.is_optional() for dep in self._dependencies): + group_dependencies = [ *self._dependencies, *(d for d in self._poetry_dependencies if d.is_optional()), ] - return self._dependencies + + return group_dependencies + included_group_dependencies @property def dependencies_for_locking(self) -> list[Dependency]: - if not self._poetry_dependencies: - return self._dependencies - if not self._dependencies: - return self._poetry_dependencies + included_group_dependencies = self._resolve_included_dependency_groups() - poetry_dependencies_by_name = defaultdict(list) - for dep in self._poetry_dependencies: - poetry_dependencies_by_name[dep.name].append(dep) - - dependencies = [] - for dep in self.dependencies: - if dep.name in poetry_dependencies_by_name: - enriched = False - dep_marker = dep.marker - if dep.in_extras: - dep_marker = dep.marker.intersect( - parse_marker( - " or ".join( - f"extra == '{extra}'" for extra in dep.in_extras + if not self._poetry_dependencies: + dependencies = self._dependencies + elif not self._dependencies: + dependencies = self._poetry_dependencies + else: + poetry_dependencies_by_name = defaultdict(list) + for dep in self._poetry_dependencies: + poetry_dependencies_by_name[dep.name].append(dep) + + dependencies = [] + for dep in self.dependencies: + if dep.name in poetry_dependencies_by_name: + enriched = False + dep_marker = dep.marker + if dep.in_extras: + dep_marker = dep.marker.intersect( + parse_marker( + " or ".join( + f"extra == '{extra}'" for extra in dep.in_extras + ) ) ) - ) - for poetry_dep in poetry_dependencies_by_name[dep.name]: - marker = dep_marker.intersect(poetry_dep.marker) - if not marker.is_empty(): - if marker == dep_marker: - marker = dep.marker - enriched = True - dependencies.append(_enrich_dependency(dep, poetry_dep, marker)) - if not enriched: + for poetry_dep in poetry_dependencies_by_name[dep.name]: + marker = dep_marker.intersect(poetry_dep.marker) + if not marker.is_empty(): + if marker == dep_marker: + marker = dep.marker + enriched = True + dependencies.append( + _enrich_dependency(dep, poetry_dep, marker) + ) + if not enriched: + dependencies.append(dep) + else: dependencies.append(dep) - else: - dependencies.append(dep) - return dependencies + return dependencies + included_group_dependencies + + def _resolve_included_dependency_groups(self) -> list[Dependency]: + """Resolves and returns the dependencies from included dependency groups. + + This method iterates over all included dependency groups and collects + their dependencies, associating them with the current group. + """ + return [ + dependency.with_groups([self.name]) + for dependency_group in self._included_dependency_groups.values() + for dependency in dependency_group.dependencies + ] def is_optional(self) -> bool: return self._optional @@ -122,6 +143,16 @@ def remove_dependency(self, name: str) -> None: dependencies.append(dependency) self._poetry_dependencies = dependencies + def include_dependency_group(self, dependency_group: DependencyGroup) -> None: + if dependency_group.name == self.name: + raise ValueError("Cannot include the dependency group to itself.") + if dependency_group.name in self._included_dependency_groups: + raise ValueError( + f"Dependency group {dependency_group.pretty_name} is already included" + ) + + self._included_dependency_groups[dependency_group.name] = dependency_group + def __eq__(self, other: object) -> bool: if not isinstance(other, DependencyGroup): return NotImplemented diff --git a/tests/packages/test_dependency_group.py b/tests/packages/test_dependency_group.py index 065f16eed..f7a542e2a 100644 --- a/tests/packages/test_dependency_group.py +++ b/tests/packages/test_dependency_group.py @@ -544,3 +544,65 @@ def test_dependency_group_use_canonicalize_name( group = DependencyGroup(pretty_name) assert group.name == canonicalized_name assert group.pretty_name == pretty_name + + +def test_include_dependency_groups() -> None: + group = DependencyGroup(name="group") + group.add_poetry_dependency( + Dependency(name="foo", constraint="*", groups=["group"]) + ) + + group_2 = DependencyGroup(name="group2") + group_2.add_poetry_dependency( + Dependency(name="bar", constraint="*", groups=["group2"]) + ) + + group.include_dependency_group(group_2) + + assert [dep.name for dep in group.dependencies] == ["foo", "bar"] + assert [dep.name for dep in group.dependencies_for_locking] == ["foo", "bar"] + for dep in group.dependencies: + assert dep.groups == {"group"} + + assert [dep.name for dep in group_2.dependencies] == ["bar"] + assert [dep.name for dep in group_2.dependencies_for_locking] == ["bar"] + for dep in group_2.dependencies: + assert dep.groups == {"group2"} + + +@pytest.mark.parametrize("group_name", ["group_2", "Group-2", "group-2"]) +def test_include_dependency_group_raise_if_including_itself(group_name: str) -> None: + group = DependencyGroup(name="group-2") + group.add_poetry_dependency( + Dependency(name="foo", constraint="*", groups=["group"]) + ) + + with pytest.raises( + ValueError, match="Cannot include the dependency group to itself" + ): + group.include_dependency_group(DependencyGroup(name=group_name)) + + +@pytest.mark.parametrize("group_name", ["group_2", "Group-2", "group-2"]) +def test_include_dependency_group_raise_if_already_included(group_name: str) -> None: + group = DependencyGroup(name="group") + group.add_poetry_dependency( + Dependency(name="foo", constraint="*", groups=["group"]) + ) + + group_2 = DependencyGroup(name="group_2") + group_2.add_poetry_dependency( + Dependency(name="bar", constraint="*", groups=["group2"]) + ) + + group.include_dependency_group(group_2) + + group_3 = DependencyGroup(name=group_name) + group_3.add_poetry_dependency( + Dependency(name="bar", constraint="*", groups=["group2"]) + ) + + with pytest.raises( + ValueError, match=f"Dependency group {group_name} is already included" + ): + group.include_dependency_group(group_3) From 4c1721b20f5bdb9cc2283be01a67b8fe9cd6134f Mon Sep 17 00:00:00 2001 From: finswimmer Date: Mon, 10 Feb 2025 10:26:04 +0100 Subject: [PATCH 02/22] feat: parse `include-groups` --- src/poetry/core/factory.py | 45 ++++ .../core/json/schemas/poetry-schema.json | 7 + tests/test_factory.py | 235 ++++++++++++++++++ 3 files changed, 287 insertions(+) diff --git a/src/poetry/core/factory.py b/src/poetry/core/factory.py index eb2fb60eb..687fbcf10 100644 --- a/src/poetry/core/factory.py +++ b/src/poetry/core/factory.py @@ -350,6 +350,20 @@ def _configure_package_dependencies( dependencies=group_config["dependencies"], ) + for group_name, group_config in tool_poetry["group"].items(): + if include_groups := group_config.get("include-groups", []): + current_group = package.dependency_group(group_name) + for name in include_groups: + try: + group_to_include = package.dependency_group(name) + except ValueError as e: + raise ValueError( + f"Group '{group_name}' includes group '{name}'" + " which is not defined." + ) from e + + current_group.include_dependency_group(group_to_include) + if with_groups and "dev-dependencies" in tool_poetry: cls._add_package_group_dependencies( package=package, @@ -614,6 +628,8 @@ def validate( ' Use "poetry.group.dev.dependencies" instead.' ) + cls._validate_dependency_groups_includes(toml_data, result) + if strict: # Validate relation between [project] and [tool.poetry] cls._validate_legacy_vs_project(toml_data, result) @@ -622,6 +638,35 @@ def validate( return result + @classmethod + def _validate_dependency_groups_includes( + cls, toml_data: dict[str, Any], result: dict[str, list[str]] + ) -> None: + """Ensure that dependency groups do not include themselves.""" + config = toml_data.setdefault("tool", {}).setdefault("poetry", {}) + + group_includes = {} + for group_name, group_config in config.get("group", {}).items(): + if include_groups := group_config.get("include-groups", []): + group_includes[canonicalize_name(group_name)] = [ + canonicalize_name(name) for name in include_groups + ] + + for group_name in group_includes: + visited = set() + stack = [group_name] + while stack: + group = stack.pop() + visited.add(group) + for include in group_includes.get(group, []): + if include in visited: + result["errors"].append( + f"Cyclic dependency group include in {group_name}:" + f" {group} -> {include}" + ) + continue + stack.append(include) + @classmethod def _validate_legacy_vs_project( cls, toml_data: dict[str, Any], result: dict[str, list[str]] diff --git a/src/poetry/core/json/schemas/poetry-schema.json b/src/poetry/core/json/schemas/poetry-schema.json index a746328a1..13cfed94e 100644 --- a/src/poetry/core/json/schemas/poetry-schema.json +++ b/src/poetry/core/json/schemas/poetry-schema.json @@ -179,6 +179,13 @@ "type": "boolean", "description": "Whether the dependency group is optional or not" }, + "include-groups": { + "type": "array", + "description": "List of dependency group names included in this group.", + "items": { + "type": "string" + } + }, "dependencies": { "type": "object", "description": "The dependencies of this dependency group", diff --git a/tests/test_factory.py b/tests/test_factory.py index 2ddf2aad1..f22d41a6f 100644 --- a/tests/test_factory.py +++ b/tests/test_factory.py @@ -1072,3 +1072,238 @@ def test_poetry_build_system_dependencies( poetry = Factory().create_poetry(temporary_directory) assert set(poetry.build_system_dependencies) == expected + + +def test_create_poetry_with_nested_dependency_groups(temporary_directory: Path) -> None: + pyproject_toml = temporary_directory / "pyproject.toml" + content = """\ +[project] +name = "my-package" +version = "1.2.3" + +[tool.poetry.group.testing.dependencies] +pytest = "*" +pytest-cov ="*" + +[tool.poetry.group.dev] +include-groups = [ + "testing", +] +[tool.poetry.group.dev.dependencies] +black = "*" +""" + pyproject_toml.write_text(content) + poetry = Factory().create_poetry(temporary_directory) + + assert len(poetry.package.all_requires) == 5 + assert [ + (dep.name, ",".join(dep.groups)) for dep in poetry.package.all_requires + ] == [ + ("pytest", "testing"), + ("pytest-cov", "testing"), + ("black", "dev"), + ("pytest", "dev"), + ("pytest-cov", "dev"), + ] + + +def assert_invalid_group_including( + toml_data: str, + expected_error: str, + error_type: type[Exception], + temporary_directory: Path, +) -> None: + pyproject_toml = temporary_directory / "pyproject.toml" + pyproject_toml.write_text(toml_data) + + with pytest.raises(error_type) as error: + _ = Factory().create_poetry(temporary_directory) + + assert str(error.value) == expected_error + + +@pytest.mark.parametrize( + "include_group_name", ["testing_group", "Testing-Group", "testing-group"] +) +def test_create_poetry_with_self_referenced_dependency_groups( + include_group_name: str, + temporary_directory: Path, +) -> None: + content = f"""\ +[project] +name = "my-package" +version = "1.2.3" + +[tool.poetry.group.testing-group] +include-groups = [ + "{include_group_name}", +] + +[tool.poetry.group.testing-group.dependencies] +pytest = "*" +pytest-cov ="*" +""" + + expected = """\ +The Poetry configuration is invalid: + - Cyclic dependency group include in testing-group: testing-group -> testing-group +""" + assert_invalid_group_including( + toml_data=content, + expected_error=expected, + error_type=RuntimeError, + temporary_directory=temporary_directory, + ) + + +def test_create_poetry_with_direct_cyclic_dependency_groups( + temporary_directory: Path, +) -> None: + content = """\ +[project] +name = "my-package" +version = "1.2.3" + +[tool.poetry.group.testing] +include-groups = [ + "dev", +] + +[tool.poetry.group.testing.dependencies] +pytest = "*" +pytest-cov ="*" + +[tool.poetry.group.dev] +include-groups = [ + "testing", +] +[tool.poetry.group.dev.dependencies] +black = "*" +""" + + expected = """\ +The Poetry configuration is invalid: + - Cyclic dependency group include in testing: dev -> testing + - Cyclic dependency group include in dev: testing -> dev +""" + assert_invalid_group_including( + toml_data=content, + expected_error=expected, + error_type=RuntimeError, + temporary_directory=temporary_directory, + ) + + +def test_create_poetry_with_indirect_full_cyclic_dependency_groups( + temporary_directory: Path, +) -> None: + content = """\ +[project] +name = "my-package" +version = "1.2.3" + +[tool.poetry.group.group_1] +include-groups = [ + "group_3", +] + +[tool.poetry.group.group_1.dependencies] +foo = "*" + +[tool.poetry.group.group_2] +include-groups = [ + "group_1", +] +[tool.poetry.group.group_2.dependencies] +bar = "*" + +[tool.poetry.group.group_3] +include-groups = [ + "group_2", +] +[tool.poetry.group.group_3.dependencies] +baz = "*" +""" + + expected = """\ +The Poetry configuration is invalid: + - Cyclic dependency group include in group-1: group-2 -> group-1 + - Cyclic dependency group include in group-2: group-3 -> group-2 + - Cyclic dependency group include in group-3: group-1 -> group-3 +""" + assert_invalid_group_including( + toml_data=content, + expected_error=expected, + error_type=RuntimeError, + temporary_directory=temporary_directory, + ) + + +def test_create_poetry_with_indirect_partial_cyclic_dependency_groups( + temporary_directory: Path, +) -> None: + content = """\ +[project] +name = "my-package" +version = "1.2.3" + +[tool.poetry.group.group_1] +include-groups = [ + "group_2", +] + +[tool.poetry.group.group_1.dependencies] +foo = "*" + +[tool.poetry.group.group_2] +include-groups = [ + "group_1", +] +[tool.poetry.group.group_2.dependencies] +bar = "*" + +[tool.poetry.group.group_3] +include-groups = [ + "group_2", +] +[tool.poetry.group.group_3.dependencies] +baz = "*" +""" + + expected = """\ +The Poetry configuration is invalid: + - Cyclic dependency group include in group-1: group-2 -> group-1 + - Cyclic dependency group include in group-2: group-1 -> group-2 + - Cyclic dependency group include in group-3: group-1 -> group-2 +""" + assert_invalid_group_including( + toml_data=content, + expected_error=expected, + error_type=RuntimeError, + temporary_directory=temporary_directory, + ) + + +def test_create_poetry_with_unknown_nested_dependency_groups( + temporary_directory: Path, +) -> None: + content = """\ +[project] +name = "my-package" +version = "1.2.3" + +[tool.poetry.group.dev] +include-groups = [ + "testing", +] +[tool.poetry.group.dev.dependencies] +black = "*" +""" + expected = "Group 'dev' includes group 'testing' which is not defined." + + assert_invalid_group_including( + toml_data=content, + expected_error=expected, + error_type=ValueError, + temporary_directory=temporary_directory, + ) From c01b6407f1fc9217fc73fe28c25d68e6cfae8125 Mon Sep 17 00:00:00 2001 From: finswimmer Date: Mon, 10 Feb 2025 16:40:02 +0100 Subject: [PATCH 03/22] feat: Add support for dependency groups which include other groups only --- src/poetry/core/factory.py | 2 +- .../core/json/schemas/poetry-schema.json | 13 ++++++-- tests/test_factory.py | 33 +++++++++++++++++++ 3 files changed, 45 insertions(+), 3 deletions(-) diff --git a/src/poetry/core/factory.py b/src/poetry/core/factory.py index 687fbcf10..741b276a6 100644 --- a/src/poetry/core/factory.py +++ b/src/poetry/core/factory.py @@ -347,7 +347,7 @@ def _configure_package_dependencies( cls._add_package_group_dependencies( package=package, group=group, - dependencies=group_config["dependencies"], + dependencies=group_config.get("dependencies", {}), ) for group_name, group_config in tool_poetry["group"].items(): diff --git a/src/poetry/core/json/schemas/poetry-schema.json b/src/poetry/core/json/schemas/poetry-schema.json index 13cfed94e..55385b92f 100644 --- a/src/poetry/core/json/schemas/poetry-schema.json +++ b/src/poetry/core/json/schemas/poetry-schema.json @@ -171,8 +171,17 @@ "^[a-zA-Z-_.0-9]+$": { "type": "object", "description": "This represents a single dependency group", - "required": [ - "dependencies" + "anyOf": [ + { + "required": [ + "dependencies" + ] + }, + { + "required": [ + "include-groups" + ] + } ], "properties": { "optional": { diff --git a/tests/test_factory.py b/tests/test_factory.py index f22d41a6f..1dc281e56 100644 --- a/tests/test_factory.py +++ b/tests/test_factory.py @@ -1307,3 +1307,36 @@ def test_create_poetry_with_unknown_nested_dependency_groups( error_type=ValueError, temporary_directory=temporary_directory, ) + + +def test_create_poetry_with_included_groups_only(temporary_directory: Path) -> None: + pyproject_toml = temporary_directory / "pyproject.toml" + content = """\ +[project] +name = "my-package" +version = "1.2.3" + +[tool.poetry.group.lint.dependencies] +black = "*" + +[tool.poetry.group.testing.dependencies] +pytest = "*" + +[tool.poetry.group.all] +include-groups = [ + "lint", + "testing", +] +""" + pyproject_toml.write_text(content) + + poetry = Factory().create_poetry(temporary_directory) + assert len(poetry.package.all_requires) == 4 + assert [ + (dep.name, ",".join(dep.groups)) for dep in poetry.package.all_requires + ] == [ + ("black", "lint"), + ("pytest", "testing"), + ("black", "all"), + ("pytest", "all"), + ] From df1b22b82b68a242adcfce76fe5f43b792918bca Mon Sep 17 00:00:00 2001 From: Ben Stolovitz Date: Sun, 4 May 2025 15:49:01 -0400 Subject: [PATCH 04/22] test out-of-order dependencies --- tests/test_factory.py | 44 ++++++++++++++++++++++++++++++++++--------- 1 file changed, 35 insertions(+), 9 deletions(-) diff --git a/tests/test_factory.py b/tests/test_factory.py index 1dc281e56..057abceed 100644 --- a/tests/test_factory.py +++ b/tests/test_factory.py @@ -1074,9 +1074,11 @@ def test_poetry_build_system_dependencies( assert set(poetry.build_system_dependencies) == expected -def test_create_poetry_with_nested_dependency_groups(temporary_directory: Path) -> None: - pyproject_toml = temporary_directory / "pyproject.toml" - content = """\ +@pytest.mark.parametrize( + ("content"), + [ + ( + """\ [project] name = "my-package" version = "1.2.3" @@ -1092,18 +1094,42 @@ def test_create_poetry_with_nested_dependency_groups(temporary_directory: Path) [tool.poetry.group.dev.dependencies] black = "*" """ + ), + ( + """\ +[project] +name = "my-package" +version = "1.2.3" + +[tool.poetry.group.dev] +include-groups = [ + "testing", +] +[tool.poetry.group.dev.dependencies] +black = "*" + +[tool.poetry.group.testing.dependencies] +pytest = "*" +pytest-cov ="*" +""" + ), + ], + ids=["in-order", "out-of-order"]) +def test_create_poetry_with_nested_dependency_groups(content: str, temporary_directory: Path) -> None: + pyproject_toml = temporary_directory / "pyproject.toml" pyproject_toml.write_text(content) poetry = Factory().create_poetry(temporary_directory) assert len(poetry.package.all_requires) == 5 - assert [ - (dep.name, ",".join(dep.groups)) for dep in poetry.package.all_requires - ] == [ - ("pytest", "testing"), - ("pytest-cov", "testing"), + assert sorted( + [(dep.name, ",".join(dep.groups)) for dep in poetry.package.all_requires], + key = lambda x: x[0] + x[1], + ) == [ ("black", "dev"), - ("pytest", "dev"), ("pytest-cov", "dev"), + ("pytest-cov", "testing"), + ("pytest", "dev"), + ("pytest", "testing"), ] From 04233c0d2db71bf75ee9a08a487d5e5ea5c4f8b3 Mon Sep 17 00:00:00 2001 From: Ben Stolovitz Date: Sun, 4 May 2025 15:56:53 -0400 Subject: [PATCH 05/22] clear comment --- tests/test_factory.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/test_factory.py b/tests/test_factory.py index 057abceed..108c7dea6 100644 --- a/tests/test_factory.py +++ b/tests/test_factory.py @@ -1096,6 +1096,7 @@ def test_poetry_build_system_dependencies( """ ), ( + # Ensure that we can reference groups that are defined later. """\ [project] name = "my-package" From 9cbcd76101e2225d969d2da29f8b69462666e5d2 Mon Sep 17 00:00:00 2001 From: Ben Stolovitz Date: Sun, 4 May 2025 18:37:24 -0400 Subject: [PATCH 06/22] handle diamond dependency deduping --- src/poetry/core/factory.py | 14 ++++++++-- tests/test_factory.py | 56 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+), 3 deletions(-) diff --git a/src/poetry/core/factory.py b/src/poetry/core/factory.py index 741b276a6..ce0f30ba2 100644 --- a/src/poetry/core/factory.py +++ b/src/poetry/core/factory.py @@ -653,18 +653,26 @@ def _validate_dependency_groups_includes( ] for group_name in group_includes: - visited = set() + ancestors = defaultdict(set[str]) stack = [group_name] while stack: group = stack.pop() - visited.add(group) + + canon_group = canonicalize_name(group) + current_ancestors = ancestors.get(canon_group, set()) + child_ancestors = current_ancestors.union({canon_group}) + for include in group_includes.get(group, []): - if include in visited: + canon_include = canonicalize_name(include) + if canon_include in child_ancestors: result["errors"].append( f"Cyclic dependency group include in {group_name}:" f" {group} -> {include}" ) + # Avoid infinite loop; we've already found an error. continue + + ancestors[canon_include] = child_ancestors stack.append(include) @classmethod diff --git a/tests/test_factory.py b/tests/test_factory.py index 108c7dea6..9607c3151 100644 --- a/tests/test_factory.py +++ b/tests/test_factory.py @@ -1310,6 +1310,62 @@ def test_create_poetry_with_indirect_partial_cyclic_dependency_groups( temporary_directory=temporary_directory, ) +def test_create_poetry_with_shared_dependency_groups( + temporary_directory: Path, +) -> None: + content = """\ +[project] +name = "my-package" +version = "1.2.3" + +[tool.poetry.group.root] +include-groups = [ + "child_1", + "child_2", +] + +[tool.poetry.group.root.dependencies] +foo = "*" + +[tool.poetry.group.child_1] +include-groups = [ + "shared", +] +[tool.poetry.group.child_1.dependencies] +bar = "*" + +[tool.poetry.group.child_2] +include-groups = [ + "shared", +] +[tool.poetry.group.child_2.dependencies] +baz = "*" + +[tool.poetry.group.shared.dependencies] +quux = "*" +""" + pyproject_toml = temporary_directory / "pyproject.toml" + pyproject_toml.write_text(content) + poetry = Factory().create_poetry(temporary_directory) + + assert len(poetry.package.all_requires) == 10 + assert sorted( + [(dep.name, ",".join(dep.groups)) for dep in poetry.package.all_requires], + key = lambda x: x[0] + x[1], + ) == [ + ("bar", "child_1"), + ("bar", "root"), + ("baz", "child_2"), + ("baz", "root"), + ("foo", "root"), + ("quux", "child_1"), + ("quux", "child_2"), + ("quux", "root"), + ("quux", "root"), # TODO: is this expected? + ("quux", "shared"), + ] + +# non-canonical names still can cycle. def test_create_poetry_with_unknown_nested_dependency_groups( temporary_directory: Path, From 3293fc28d9404b27f22ba30d2c8e14e31e32e034 Mon Sep 17 00:00:00 2001 From: Ben Stolovitz Date: Sun, 4 May 2025 18:41:00 -0400 Subject: [PATCH 07/22] more complex test --- tests/test_factory.py | 66 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/tests/test_factory.py b/tests/test_factory.py index 9607c3151..273c2bbd6 100644 --- a/tests/test_factory.py +++ b/tests/test_factory.py @@ -1365,6 +1365,72 @@ def test_create_poetry_with_shared_dependency_groups( ("quux", "shared"), ] +def test_create_poetry_with_shared_dependency_groups_more_complicated( + temporary_directory: Path, +) -> None: + content = """\ +[project] +name = "my-package" +version = "1.2.3" + +[tool.poetry.group.root] +include-groups = [ + "child_1", + "child_2", +] + +[tool.poetry.group.root.dependencies] +foo = "*" + +[tool.poetry.group.child_1] +include-groups = [ + "shared", +] +[tool.poetry.group.child_1.dependencies] +bar = "*" + +[tool.poetry.group.child_2] +include-groups = [ + "grandchild", +] +[tool.poetry.group.child_2.dependencies] +baz = "*" + +[tool.poetry.group.grandchild] +include-groups = [ + "shared", +] +[tool.poetry.group.grandchild.dependencies] +bax = "*" + +[tool.poetry.group.shared.dependencies] +quux = "*" +""" + pyproject_toml = temporary_directory / "pyproject.toml" + pyproject_toml.write_text(content) + poetry = Factory().create_poetry(temporary_directory) + + assert len(poetry.package.all_requires) == 14 + assert sorted( + [(dep.name, ",".join(dep.groups)) for dep in poetry.package.all_requires], + key = lambda x: x[0] + x[1], + ) == [ + ("bar", "child_1"), + ("bar", "root"), + ("bax", "child_2"), + ("bax", "grandchild"), + ("bax", "root"), + ("baz", "child_2"), + ("baz", "root"), + ("foo", "root"), + ("quux", "child_1"), + ("quux", "child_2"), + ("quux", "grandchild"), + ("quux", "root"), + ("quux", "root"), # TODO: is this expected? + ("quux", "shared"), + ] + # non-canonical names still can cycle. def test_create_poetry_with_unknown_nested_dependency_groups( From eaf2d2733cd81576fa688181dfe2aa4b3d60a531 Mon Sep 17 00:00:00 2001 From: Ben Stolovitz Date: Sun, 4 May 2025 18:42:59 -0400 Subject: [PATCH 08/22] test canonical name --- tests/test_factory.py | 43 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/tests/test_factory.py b/tests/test_factory.py index 273c2bbd6..9974b99e0 100644 --- a/tests/test_factory.py +++ b/tests/test_factory.py @@ -1432,6 +1432,49 @@ def test_create_poetry_with_shared_dependency_groups_more_complicated( ] # non-canonical names still can cycle. +def test_create_poetry_with_noncanonical_names_cyclic_dependency_groups( + temporary_directory: Path, +) -> None: + content = """\ +[project] +name = "my-package" +version = "1.2.3" + +[tool.poetry.group.GROUP_1] +include-groups = [ + "gRoup_2", +] + +[tool.poetry.group.GROUP_1.dependencies] +foo = "*" + +[tool.poetry.group.group_2] +include-groups = [ + "groUp_1", +] +[tool.poetry.group.group_2.dependencies] +bar = "*" + +[tool.poetry.group.group_3] +include-groups = [ + "group_2", +] +[tool.poetry.group.group_3.dependencies] +baz = "*" +""" + + expected = """\ +The Poetry configuration is invalid: + - Cyclic dependency group include in group-1: group-2 -> group-1 + - Cyclic dependency group include in group-2: group-1 -> group-2 + - Cyclic dependency group include in group-3: group-1 -> group-2 +""" + assert_invalid_group_including( + toml_data=content, + expected_error=expected, + error_type=RuntimeError, + temporary_directory=temporary_directory, + ) def test_create_poetry_with_unknown_nested_dependency_groups( temporary_directory: Path, From 30141e1b83d3d8f1b20fb41361035ba89a629ba2 Mon Sep 17 00:00:00 2001 From: Ben Stolovitz Date: Sun, 4 May 2025 19:40:33 -0400 Subject: [PATCH 09/22] working on it... --- src/poetry/core/factory.py | 13 ++++---- tests/test_factory.py | 65 +++++++++++++++++++++++++++++++++++++- 2 files changed, 70 insertions(+), 8 deletions(-) diff --git a/src/poetry/core/factory.py b/src/poetry/core/factory.py index ce0f30ba2..c9784aa6a 100644 --- a/src/poetry/core/factory.py +++ b/src/poetry/core/factory.py @@ -657,14 +657,11 @@ def _validate_dependency_groups_includes( stack = [group_name] while stack: group = stack.pop() - - canon_group = canonicalize_name(group) - current_ancestors = ancestors.get(canon_group, set()) - child_ancestors = current_ancestors.union({canon_group}) + current_ancestors = ancestors.get(group, set()) + child_ancestors = current_ancestors.union({group}) for include in group_includes.get(group, []): - canon_include = canonicalize_name(include) - if canon_include in child_ancestors: + if include in child_ancestors: result["errors"].append( f"Cyclic dependency group include in {group_name}:" f" {group} -> {include}" @@ -672,7 +669,9 @@ def _validate_dependency_groups_includes( # Avoid infinite loop; we've already found an error. continue - ancestors[canon_include] = child_ancestors + # TODO: test this scenario + ancestors[include] = ancestors.get(include, set()).union(child_ancestors) + # ancestors[include] = child_ancestors stack.append(include) @classmethod diff --git a/tests/test_factory.py b/tests/test_factory.py index 9974b99e0..451f2aaac 100644 --- a/tests/test_factory.py +++ b/tests/test_factory.py @@ -1431,7 +1431,70 @@ def test_create_poetry_with_shared_dependency_groups_more_complicated( ("quux", "shared"), ] -# non-canonical names still can cycle. +def test_create_poetry_with_complicated_cyclic_diamond_dependency_groups( + temporary_directory: Path, +) -> None: + content = """\ +[project] +name = "my-package" +version = "1.2.3" + +[tool.poetry.group.root] +include-groups = [ + "child_1", + "child_2", +] + +[tool.poetry.group.root.dependencies] +foo = "*" + +[tool.poetry.group.child_1] +include-groups = [ + "shared", +] +[tool.poetry.group.child_1.dependencies] +bar = "*" + +[tool.poetry.group.child_2] +include-groups = [ + "shared", +] +[tool.poetry.group.child_2.dependencies] +baz = "*" + +[tool.poetry.group.shared] +include-groups = [ + "grandchild", +] +[tool.poetry.group.shared.dependencies] +quux = "*" + +[tool.poetry.group.grandchild] +include-groups = [ + "child_1", +] +[tool.poetry.group.grandchild.dependencies] +bar = "*" +""" + + expected = """\ +The Poetry configuration is invalid: + - Cyclic dependency group include in root: child-1 -> shared + - Cyclic dependency group include in root: child-1 -> shared + - Cyclic dependency group include in child-1: grandchild -> child-1 + - Cyclic dependency group include in child-2: child-1 -> shared + - Cyclic dependency group include in shared: child-1 -> shared + - Cyclic dependency group include in grandchild: shared -> grandchild +""" + + assert_invalid_group_including( + toml_data=content, + expected_error=expected, + error_type=RuntimeError, + temporary_directory=temporary_directory, + ) + + def test_create_poetry_with_noncanonical_names_cyclic_dependency_groups( temporary_directory: Path, ) -> None: From bdb2d935d4a6703d10e999b7d678b41d63162ab4 Mon Sep 17 00:00:00 2001 From: Ben Stolovitz Date: Sun, 4 May 2025 19:40:36 -0400 Subject: [PATCH 10/22] done! --- src/poetry/core/factory.py | 7 +++++-- tests/test_factory.py | 12 ++++++------ 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/src/poetry/core/factory.py b/src/poetry/core/factory.py index c9784aa6a..40c073d22 100644 --- a/src/poetry/core/factory.py +++ b/src/poetry/core/factory.py @@ -669,9 +669,12 @@ def _validate_dependency_groups_includes( # Avoid infinite loop; we've already found an error. continue - # TODO: test this scenario + # This must be a union with any existing known ancestors. + # Otherwise, we might accidentally miss a cycle (since we'd + # be tossing out known dependencies). That cycle will be + # caught when we use the share dependency as `group_name`, + # but best to be safe. ancestors[include] = ancestors.get(include, set()).union(child_ancestors) - # ancestors[include] = child_ancestors stack.append(include) @classmethod diff --git a/tests/test_factory.py b/tests/test_factory.py index 451f2aaac..b7d4bcecc 100644 --- a/tests/test_factory.py +++ b/tests/test_factory.py @@ -1471,7 +1471,7 @@ def test_create_poetry_with_complicated_cyclic_diamond_dependency_groups( [tool.poetry.group.grandchild] include-groups = [ - "child_1", + "child_2", ] [tool.poetry.group.grandchild.dependencies] bar = "*" @@ -1479,11 +1479,11 @@ def test_create_poetry_with_complicated_cyclic_diamond_dependency_groups( expected = """\ The Poetry configuration is invalid: - - Cyclic dependency group include in root: child-1 -> shared - - Cyclic dependency group include in root: child-1 -> shared - - Cyclic dependency group include in child-1: grandchild -> child-1 - - Cyclic dependency group include in child-2: child-1 -> shared - - Cyclic dependency group include in shared: child-1 -> shared + - Cyclic dependency group include in root: grandchild -> child-2 + - Cyclic dependency group include in root: grandchild -> child-2 + - Cyclic dependency group include in child-1: child-2 -> shared + - Cyclic dependency group include in child-2: grandchild -> child-2 + - Cyclic dependency group include in shared: child-2 -> shared - Cyclic dependency group include in grandchild: shared -> grandchild """ From e2c439c9c853a8d8282fa821964a27f1085efa9c Mon Sep 17 00:00:00 2001 From: Ben Stolovitz Date: Sun, 4 May 2025 21:50:01 -0400 Subject: [PATCH 11/22] better parameterization --- tests/test_factory.py | 34 ++++++++++++++++++---------------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/tests/test_factory.py b/tests/test_factory.py index b7d4bcecc..4d0ed6464 100644 --- a/tests/test_factory.py +++ b/tests/test_factory.py @@ -1075,49 +1075,51 @@ def test_poetry_build_system_dependencies( @pytest.mark.parametrize( - ("content"), + ("group_name", "included_group_name", "in_order"), [ - ( - """\ + ("testing", "testing", True), + ("testing", "testing", False) + ]) +def test_create_poetry_with_nested_dependency_groups(group_name: str, included_group_name: str, in_order: bool, temporary_directory: Path) -> None: + pyproject_toml = temporary_directory / "pyproject.toml" + + replace_group_name = "%REPLACE_GROUP_NAME%" + replace_included_group_name = "%REPLACE_INCLUDED_GROUP_NAME%" + in_order_content = """\ [project] name = "my-package" version = "1.2.3" -[tool.poetry.group.testing.dependencies] +[tool.poetry.group.%REPLACE_GROUP_NAME%.dependencies] pytest = "*" pytest-cov ="*" [tool.poetry.group.dev] include-groups = [ - "testing", + "%REPLACE_INCLUDED_GROUP_NAME%", ] [tool.poetry.group.dev.dependencies] black = "*" """ - ), - ( - # Ensure that we can reference groups that are defined later. - """\ + out_of_order_content = """\ [project] name = "my-package" version = "1.2.3" [tool.poetry.group.dev] include-groups = [ - "testing", + "%REPLACE_INCLUDED_GROUP_NAME%", ] [tool.poetry.group.dev.dependencies] black = "*" -[tool.poetry.group.testing.dependencies] +[tool.poetry.group.%REPLACE_GROUP_NAME%.dependencies] pytest = "*" pytest-cov ="*" """ - ), - ], - ids=["in-order", "out-of-order"]) -def test_create_poetry_with_nested_dependency_groups(content: str, temporary_directory: Path) -> None: - pyproject_toml = temporary_directory / "pyproject.toml" + + base_content = in_order_content if in_order else out_of_order_content + content = base_content.replace(replace_group_name, group_name).replace(replace_included_group_name, included_group_name) pyproject_toml.write_text(content) poetry = Factory().create_poetry(temporary_directory) From b0b74f869f8d4a7f1d4dd212d2cbb214c5bfee16 Mon Sep 17 00:00:00 2001 From: Ben Stolovitz Date: Sun, 4 May 2025 22:13:24 -0400 Subject: [PATCH 12/22] test non-normalized names --- src/poetry/core/factory.py | 12 ++++- src/poetry/core/packages/dependency_group.py | 2 +- tests/test_factory.py | 53 +++++++++++++++----- 3 files changed, 52 insertions(+), 15 deletions(-) diff --git a/src/poetry/core/factory.py b/src/poetry/core/factory.py index 40c073d22..c88a660e6 100644 --- a/src/poetry/core/factory.py +++ b/src/poetry/core/factory.py @@ -355,7 +355,17 @@ def _configure_package_dependencies( current_group = package.dependency_group(group_name) for name in include_groups: try: - group_to_include = package.dependency_group(name) + # Neither the current name nor the package's + # dependency group names are guaranteed to be + # normalized. Compare the normalized names to find + # the actual key to use. + all_group_names = package.dependency_group_names() + group_name_to_use = next( + (n + for n in all_group_names + if canonicalize_name(n) == canonicalize_name(name))) + + group_to_include = package.dependency_group(group_name_to_use) except ValueError as e: raise ValueError( f"Group '{group_name}' includes group '{name}'" diff --git a/src/poetry/core/packages/dependency_group.py b/src/poetry/core/packages/dependency_group.py index 25f83dfd8..a5f2e106d 100644 --- a/src/poetry/core/packages/dependency_group.py +++ b/src/poetry/core/packages/dependency_group.py @@ -107,7 +107,7 @@ def _resolve_included_dependency_groups(self) -> list[Dependency]: their dependencies, associating them with the current group. """ return [ - dependency.with_groups([self.name]) + dependency.with_groups([canonicalize_name(self.name)]) for dependency_group in self._included_dependency_groups.values() for dependency in dependency_group.dependencies ] diff --git a/tests/test_factory.py b/tests/test_factory.py index 4d0ed6464..013a99c71 100644 --- a/tests/test_factory.py +++ b/tests/test_factory.py @@ -1077,8 +1077,35 @@ def test_poetry_build_system_dependencies( @pytest.mark.parametrize( ("group_name", "included_group_name", "in_order"), [ - ("testing", "testing", True), - ("testing", "testing", False) + ("testing", "testing", True), + ("testing", "testing", False), + ("testing", "TESTING", True), + ("testing", "TESTING", False), + ("group_a", "group-a", True), + ("group_a", "group-a", False), + + # https://packaging.python.org/en/latest/specifications/name-normalization/#valid-non-normalized-names + # friendly-bard + # Friendly-Bard + # FRIENDLY-BARD + # friendly.bard + # friendly_bard + # friendly--bard + # FrIeNdLy-._.-bArD + ("friendly-bard", "friendly-bard", True), + ("friendly-bard", "friendly-bard", False), + ("friendly-bard", "Friendly-Bard", True), + ("friendly-bard", "Friendly-Bard", False), + ("friendly-bard", "FRIENDLY-BARD", True), + ("friendly-bard", "FRIENDLY-BARD", False), + ("friendly-bard", "friendly.bard", True), + ("friendly-bard", "friendly.bard", False), + ("friendly-bard", "friendly_bard", True), + ("friendly-bard", "friendly_bard", False), + ("friendly-bard", "friendly--bard", True), + ("friendly-bard", "friendly--bard", False), + ("friendly-bard", "FrIeNdLy-._.-bArD", True), + ("friendly-bard", "FrIeNdLy-._.-bArD", False), ]) def test_create_poetry_with_nested_dependency_groups(group_name: str, included_group_name: str, in_order: bool, temporary_directory: Path) -> None: pyproject_toml = temporary_directory / "pyproject.toml" @@ -1130,9 +1157,9 @@ def test_create_poetry_with_nested_dependency_groups(group_name: str, included_g ) == [ ("black", "dev"), ("pytest-cov", "dev"), - ("pytest-cov", "testing"), + ("pytest-cov", canonicalize_name(group_name)), ("pytest", "dev"), - ("pytest", "testing"), + ("pytest", canonicalize_name(group_name)), ] @@ -1355,13 +1382,13 @@ def test_create_poetry_with_shared_dependency_groups( [(dep.name, ",".join(dep.groups)) for dep in poetry.package.all_requires], key = lambda x: x[0] + x[1], ) == [ - ("bar", "child_1"), + ("bar", "child-1"), ("bar", "root"), - ("baz", "child_2"), + ("baz", "child-2"), ("baz", "root"), ("foo", "root"), - ("quux", "child_1"), - ("quux", "child_2"), + ("quux", "child-1"), + ("quux", "child-2"), ("quux", "root"), ("quux", "root"), # TODO: is this expected? ("quux", "shared"), @@ -1417,16 +1444,16 @@ def test_create_poetry_with_shared_dependency_groups_more_complicated( [(dep.name, ",".join(dep.groups)) for dep in poetry.package.all_requires], key = lambda x: x[0] + x[1], ) == [ - ("bar", "child_1"), + ("bar", "child-1"), ("bar", "root"), - ("bax", "child_2"), + ("bax", "child-2"), ("bax", "grandchild"), ("bax", "root"), - ("baz", "child_2"), + ("baz", "child-2"), ("baz", "root"), ("foo", "root"), - ("quux", "child_1"), - ("quux", "child_2"), + ("quux", "child-1"), + ("quux", "child-2"), ("quux", "grandchild"), ("quux", "root"), ("quux", "root"), # TODO: is this expected? From c832471e9b35557214676e7dd33a1d072eba1038 Mon Sep 17 00:00:00 2001 From: Ben Stolovitz Date: Sun, 4 May 2025 22:20:04 -0400 Subject: [PATCH 13/22] use specified names, handle errors --- src/poetry/core/factory.py | 5 +++ src/poetry/core/packages/dependency_group.py | 2 +- tests/test_factory.py | 42 ++++++++++++++------ 3 files changed, 35 insertions(+), 14 deletions(-) diff --git a/src/poetry/core/factory.py b/src/poetry/core/factory.py index c88a660e6..de27307fe 100644 --- a/src/poetry/core/factory.py +++ b/src/poetry/core/factory.py @@ -371,6 +371,11 @@ def _configure_package_dependencies( f"Group '{group_name}' includes group '{name}'" " which is not defined." ) from e + except StopIteration as e: + raise ValueError( + f"Group '{group_name}' includes group '{name}'" + " which is not defined." + ) from e current_group.include_dependency_group(group_to_include) diff --git a/src/poetry/core/packages/dependency_group.py b/src/poetry/core/packages/dependency_group.py index a5f2e106d..25f83dfd8 100644 --- a/src/poetry/core/packages/dependency_group.py +++ b/src/poetry/core/packages/dependency_group.py @@ -107,7 +107,7 @@ def _resolve_included_dependency_groups(self) -> list[Dependency]: their dependencies, associating them with the current group. """ return [ - dependency.with_groups([canonicalize_name(self.name)]) + dependency.with_groups([self.name]) for dependency_group in self._included_dependency_groups.values() for dependency in dependency_group.dependencies ] diff --git a/tests/test_factory.py b/tests/test_factory.py index 013a99c71..a908c9246 100644 --- a/tests/test_factory.py +++ b/tests/test_factory.py @@ -1106,6 +1106,22 @@ def test_poetry_build_system_dependencies( ("friendly-bard", "friendly--bard", False), ("friendly-bard", "FrIeNdLy-._.-bArD", True), ("friendly-bard", "FrIeNdLy-._.-bArD", False), + + ("friendly-bard", "friendly-bard", True), + ("friendly-bard", "friendly-bard", False), + ("friendly-Bard", "friendly-bard", True), + ("friendly-Bard", "friendly-bard", False), + ("FRIENDLY-BARD", "friendly-bard", True), + ("FRIENDLY-BARD", "friendly-bard", False), + # ("friendly.bard", "friendly-bard", True), + # ("friendly.bard", "friendly-bard", False), + ("friendly_bard", "friendly-bard", True), + ("friendly_bard", "friendly-bard", False), + ("friendly--bard", "friendly-bard", True), + ("friendly--bard", "friendly-bard", False), + # ("FrIeNdLy-._.-bArD", "friendly-bard", True), + # ("FrIeNdLy-._.-bArD", "friendly-bard", False), + ]) def test_create_poetry_with_nested_dependency_groups(group_name: str, included_group_name: str, in_order: bool, temporary_directory: Path) -> None: pyproject_toml = temporary_directory / "pyproject.toml" @@ -1154,13 +1170,13 @@ def test_create_poetry_with_nested_dependency_groups(group_name: str, included_g assert sorted( [(dep.name, ",".join(dep.groups)) for dep in poetry.package.all_requires], key = lambda x: x[0] + x[1], - ) == [ + ) == sorted([ ("black", "dev"), ("pytest-cov", "dev"), - ("pytest-cov", canonicalize_name(group_name)), + ("pytest-cov", group_name), ("pytest", "dev"), - ("pytest", canonicalize_name(group_name)), - ] + ("pytest", group_name), + ], key = lambda x: x[0] + x[1]) def assert_invalid_group_including( @@ -1382,13 +1398,13 @@ def test_create_poetry_with_shared_dependency_groups( [(dep.name, ",".join(dep.groups)) for dep in poetry.package.all_requires], key = lambda x: x[0] + x[1], ) == [ - ("bar", "child-1"), + ("bar", "child_1"), ("bar", "root"), - ("baz", "child-2"), + ("baz", "child_2"), ("baz", "root"), ("foo", "root"), - ("quux", "child-1"), - ("quux", "child-2"), + ("quux", "child_1"), + ("quux", "child_2"), ("quux", "root"), ("quux", "root"), # TODO: is this expected? ("quux", "shared"), @@ -1444,16 +1460,16 @@ def test_create_poetry_with_shared_dependency_groups_more_complicated( [(dep.name, ",".join(dep.groups)) for dep in poetry.package.all_requires], key = lambda x: x[0] + x[1], ) == [ - ("bar", "child-1"), + ("bar", "child_1"), ("bar", "root"), - ("bax", "child-2"), + ("bax", "child_2"), ("bax", "grandchild"), ("bax", "root"), - ("baz", "child-2"), + ("baz", "child_2"), ("baz", "root"), ("foo", "root"), - ("quux", "child-1"), - ("quux", "child-2"), + ("quux", "child_1"), + ("quux", "child_2"), ("quux", "grandchild"), ("quux", "root"), ("quux", "root"), # TODO: is this expected? From cb4c16ea2d16e1b8fd2607886fe52dd53b2c1a0c Mon Sep 17 00:00:00 2001 From: Ben Stolovitz Date: Sun, 4 May 2025 22:25:55 -0400 Subject: [PATCH 14/22] test dots too --- tests/test_factory.py | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/tests/test_factory.py b/tests/test_factory.py index a908c9246..9115f4d8e 100644 --- a/tests/test_factory.py +++ b/tests/test_factory.py @@ -1084,14 +1084,8 @@ def test_poetry_build_system_dependencies( ("group_a", "group-a", True), ("group_a", "group-a", False), + # Examples from the PEP 508 spec # https://packaging.python.org/en/latest/specifications/name-normalization/#valid-non-normalized-names - # friendly-bard - # Friendly-Bard - # FRIENDLY-BARD - # friendly.bard - # friendly_bard - # friendly--bard - # FrIeNdLy-._.-bArD ("friendly-bard", "friendly-bard", True), ("friendly-bard", "friendly-bard", False), ("friendly-bard", "Friendly-Bard", True), @@ -1113,15 +1107,14 @@ def test_poetry_build_system_dependencies( ("friendly-Bard", "friendly-bard", False), ("FRIENDLY-BARD", "friendly-bard", True), ("FRIENDLY-BARD", "friendly-bard", False), - # ("friendly.bard", "friendly-bard", True), - # ("friendly.bard", "friendly-bard", False), + ("friendly.bard", "friendly-bard", True), + ("friendly.bard", "friendly-bard", False), ("friendly_bard", "friendly-bard", True), ("friendly_bard", "friendly-bard", False), ("friendly--bard", "friendly-bard", True), ("friendly--bard", "friendly-bard", False), - # ("FrIeNdLy-._.-bArD", "friendly-bard", True), - # ("FrIeNdLy-._.-bArD", "friendly-bard", False), - + ("FrIeNdLy-._.-bArD", "friendly-bard", True), + ("FrIeNdLy-._.-bArD", "friendly-bard", False), ]) def test_create_poetry_with_nested_dependency_groups(group_name: str, included_group_name: str, in_order: bool, temporary_directory: Path) -> None: pyproject_toml = temporary_directory / "pyproject.toml" @@ -1144,6 +1137,7 @@ def test_create_poetry_with_nested_dependency_groups(group_name: str, included_g [tool.poetry.group.dev.dependencies] black = "*" """ + # The dev group refers to a group that is defined after it. out_of_order_content = """\ [project] name = "my-package" @@ -1161,8 +1155,14 @@ def test_create_poetry_with_nested_dependency_groups(group_name: str, included_g pytest-cov ="*" """ + # Generate the content. If `group_name` has a `.` in it, we "escape" it with + # quotes to make it a valid TOML key. base_content = in_order_content if in_order else out_of_order_content - content = base_content.replace(replace_group_name, group_name).replace(replace_included_group_name, included_group_name) + group_name_to_use = ( + group_name if "." not in group_name else f'"{group_name}"' + ) + content = base_content.replace(replace_group_name, group_name_to_use).replace(replace_included_group_name, included_group_name) + pyproject_toml.write_text(content) poetry = Factory().create_poetry(temporary_directory) From 06cb85bc13cef1062062cd6660a8b125b90735ab Mon Sep 17 00:00:00 2001 From: Ben Stolovitz Date: Sun, 4 May 2025 22:34:15 -0400 Subject: [PATCH 15/22] simplify params --- tests/test_factory.py | 59 +++++++++++++++---------------------------- 1 file changed, 21 insertions(+), 38 deletions(-) diff --git a/tests/test_factory.py b/tests/test_factory.py index 9115f4d8e..de6fb87fd 100644 --- a/tests/test_factory.py +++ b/tests/test_factory.py @@ -1073,49 +1073,32 @@ def test_poetry_build_system_dependencies( assert set(poetry.build_system_dependencies) == expected - +@pytest.mark.parametrize("in_order", [True, False]) @pytest.mark.parametrize( - ("group_name", "included_group_name", "in_order"), + ("group_name", "included_group_name"), [ - ("testing", "testing", True), - ("testing", "testing", False), - ("testing", "TESTING", True), - ("testing", "TESTING", False), - ("group_a", "group-a", True), - ("group_a", "group-a", False), + ("testing", "testing"), + ("testing", "TESTING"), + ("group_a", "group-a"), # Examples from the PEP 508 spec # https://packaging.python.org/en/latest/specifications/name-normalization/#valid-non-normalized-names - ("friendly-bard", "friendly-bard", True), - ("friendly-bard", "friendly-bard", False), - ("friendly-bard", "Friendly-Bard", True), - ("friendly-bard", "Friendly-Bard", False), - ("friendly-bard", "FRIENDLY-BARD", True), - ("friendly-bard", "FRIENDLY-BARD", False), - ("friendly-bard", "friendly.bard", True), - ("friendly-bard", "friendly.bard", False), - ("friendly-bard", "friendly_bard", True), - ("friendly-bard", "friendly_bard", False), - ("friendly-bard", "friendly--bard", True), - ("friendly-bard", "friendly--bard", False), - ("friendly-bard", "FrIeNdLy-._.-bArD", True), - ("friendly-bard", "FrIeNdLy-._.-bArD", False), - - ("friendly-bard", "friendly-bard", True), - ("friendly-bard", "friendly-bard", False), - ("friendly-Bard", "friendly-bard", True), - ("friendly-Bard", "friendly-bard", False), - ("FRIENDLY-BARD", "friendly-bard", True), - ("FRIENDLY-BARD", "friendly-bard", False), - ("friendly.bard", "friendly-bard", True), - ("friendly.bard", "friendly-bard", False), - ("friendly_bard", "friendly-bard", True), - ("friendly_bard", "friendly-bard", False), - ("friendly--bard", "friendly-bard", True), - ("friendly--bard", "friendly-bard", False), - ("FrIeNdLy-._.-bArD", "friendly-bard", True), - ("FrIeNdLy-._.-bArD", "friendly-bard", False), - ]) + ("friendly-bard", "friendly-bard"), + ("friendly-bard", "Friendly-Bard"), + ("friendly-bard", "FRIENDLY-BARD"), + ("friendly-bard", "friendly.bard"), + ("friendly-bard", "friendly_bard"), + ("friendly-bard", "friendly--bard"), + ("friendly-bard", "FrIeNdLy-._.-bArD"), + + ("friendly-Bard", "friendly-bard"), + ("FRIENDLY-BARD", "friendly-bard"), + ("friendly.bard", "friendly-bard"), + ("friendly_bard", "friendly-bard"), + ("friendly--bard", "friendly-bard"), + ("FrIeNdLy-._.-bArD", "friendly-bard"), + ] +) def test_create_poetry_with_nested_dependency_groups(group_name: str, included_group_name: str, in_order: bool, temporary_directory: Path) -> None: pyproject_toml = temporary_directory / "pyproject.toml" From 053c87cb313b3ea50da6a09e9344a78713ad3f30 Mon Sep 17 00:00:00 2001 From: Ben Stolovitz Date: Sun, 4 May 2025 22:35:50 -0400 Subject: [PATCH 16/22] nominally handle locking deps --- src/poetry/core/packages/dependency_group.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/poetry/core/packages/dependency_group.py b/src/poetry/core/packages/dependency_group.py index 25f83dfd8..b2739d4d0 100644 --- a/src/poetry/core/packages/dependency_group.py +++ b/src/poetry/core/packages/dependency_group.py @@ -39,7 +39,7 @@ def pretty_name(self) -> str: @property def dependencies(self) -> list[Dependency]: group_dependencies = self._dependencies - included_group_dependencies = self._resolve_included_dependency_groups() + included_group_dependencies = self._resolve_included_dependency_groups(dependencies_for_locking=False) if not group_dependencies: # legacy mode @@ -60,7 +60,7 @@ def dependencies(self) -> list[Dependency]: @property def dependencies_for_locking(self) -> list[Dependency]: - included_group_dependencies = self._resolve_included_dependency_groups() + included_group_dependencies = self._resolve_included_dependency_groups(dependencies_for_locking=True) if not self._poetry_dependencies: dependencies = self._dependencies @@ -100,7 +100,7 @@ def dependencies_for_locking(self) -> list[Dependency]: return dependencies + included_group_dependencies - def _resolve_included_dependency_groups(self) -> list[Dependency]: + def _resolve_included_dependency_groups(self, dependencies_for_locking: bool = False) -> list[Dependency]: """Resolves and returns the dependencies from included dependency groups. This method iterates over all included dependency groups and collects @@ -109,7 +109,7 @@ def _resolve_included_dependency_groups(self) -> list[Dependency]: return [ dependency.with_groups([self.name]) for dependency_group in self._included_dependency_groups.values() - for dependency in dependency_group.dependencies + for dependency in (dependency_group.dependencies_for_locking if dependencies_for_locking else dependency_group.dependencies) ] def is_optional(self) -> bool: From a63171d36c57f168ef9949b7e6654e0501765528 Mon Sep 17 00:00:00 2001 From: Ben Stolovitz Date: Sun, 4 May 2025 23:03:08 -0400 Subject: [PATCH 17/22] test locking deps properly --- tests/packages/test_dependency_group.py | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/tests/packages/test_dependency_group.py b/tests/packages/test_dependency_group.py index f7a542e2a..b8a4421d7 100644 --- a/tests/packages/test_dependency_group.py +++ b/tests/packages/test_dependency_group.py @@ -557,10 +557,24 @@ def test_include_dependency_groups() -> None: Dependency(name="bar", constraint="*", groups=["group2"]) ) + # This will resolve to a more precise constraint in dependencies_for_locking. + group_3 = DependencyGroup(name="group3") + group_3._dependencies = [ + Dependency(name="baz", constraint="<2", groups=["group3"]) + ] + group_3._poetry_dependencies = [ + Dependency(name="baz", constraint=">=1", groups=["group3"]) + ] + group.include_dependency_group(group_2) + group.include_dependency_group(group_3) - assert [dep.name for dep in group.dependencies] == ["foo", "bar"] - assert [dep.name for dep in group.dependencies_for_locking] == ["foo", "bar"] + assert [dep.name for dep in group.dependencies] == ["foo", "bar", "baz"] + assert [dep.name for dep in group.dependencies_for_locking] == ["foo", "bar", "baz"] + assert [dep.pretty_constraint for dep in group.dependencies] == ["*", "*", "<2"] + assert [dep.pretty_constraint for dep in group.dependencies_for_locking] == [ + "*", "*", ">=1,<2" + ] for dep in group.dependencies: assert dep.groups == {"group"} @@ -569,6 +583,13 @@ def test_include_dependency_groups() -> None: for dep in group_2.dependencies: assert dep.groups == {"group2"} + assert [dep.name for dep in group_3.dependencies] == ["baz"] + assert [dep.name for dep in group_3.dependencies_for_locking] == ["baz"] + assert [dep.pretty_constraint for dep in group_3.dependencies] == ["<2"] + assert [dep.pretty_constraint for dep in group_3.dependencies_for_locking] == [">=1,<2"] + for dep in group_3.dependencies: + assert dep.groups == {"group3"} + @pytest.mark.parametrize("group_name", ["group_2", "Group-2", "group-2"]) def test_include_dependency_group_raise_if_including_itself(group_name: str) -> None: From 83eb6784a5c4df321c7c05b5e3ec11f8b222f27d Mon Sep 17 00:00:00 2001 From: Ben Stolovitz Date: Tue, 27 May 2025 21:19:19 -0400 Subject: [PATCH 18/22] fix failing tests... --- tests/test_factory.py | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/tests/test_factory.py b/tests/test_factory.py index de6fb87fd..57ce0b90e 100644 --- a/tests/test_factory.py +++ b/tests/test_factory.py @@ -1149,6 +1149,9 @@ def test_create_poetry_with_nested_dependency_groups(group_name: str, included_g pyproject_toml.write_text(content) poetry = Factory().create_poetry(temporary_directory) + # Groups are reported internally using their canonical names. + canonical_name = canonicalize_name(group_name) + assert len(poetry.package.all_requires) == 5 assert sorted( [(dep.name, ",".join(dep.groups)) for dep in poetry.package.all_requires], @@ -1156,9 +1159,9 @@ def test_create_poetry_with_nested_dependency_groups(group_name: str, included_g ) == sorted([ ("black", "dev"), ("pytest-cov", "dev"), - ("pytest-cov", group_name), + ("pytest-cov", canonical_name), ("pytest", "dev"), - ("pytest", group_name), + ("pytest", canonical_name), ], key = lambda x: x[0] + x[1]) @@ -1381,13 +1384,13 @@ def test_create_poetry_with_shared_dependency_groups( [(dep.name, ",".join(dep.groups)) for dep in poetry.package.all_requires], key = lambda x: x[0] + x[1], ) == [ - ("bar", "child_1"), + ("bar", "child-1"), ("bar", "root"), - ("baz", "child_2"), + ("baz", "child-2"), ("baz", "root"), ("foo", "root"), - ("quux", "child_1"), - ("quux", "child_2"), + ("quux", "child-1"), + ("quux", "child-2"), ("quux", "root"), ("quux", "root"), # TODO: is this expected? ("quux", "shared"), @@ -1443,16 +1446,16 @@ def test_create_poetry_with_shared_dependency_groups_more_complicated( [(dep.name, ",".join(dep.groups)) for dep in poetry.package.all_requires], key = lambda x: x[0] + x[1], ) == [ - ("bar", "child_1"), + ("bar", "child-1"), ("bar", "root"), - ("bax", "child_2"), + ("bax", "child-2"), ("bax", "grandchild"), ("bax", "root"), - ("baz", "child_2"), + ("baz", "child-2"), ("baz", "root"), ("foo", "root"), - ("quux", "child_1"), - ("quux", "child_2"), + ("quux", "child-1"), + ("quux", "child-2"), ("quux", "grandchild"), ("quux", "root"), ("quux", "root"), # TODO: is this expected? From 560c2530a85fc48b1d1ee3118742c994d337896a Mon Sep 17 00:00:00 2001 From: Ben Stolovitz Date: Tue, 27 May 2025 21:20:48 -0400 Subject: [PATCH 19/22] unnec normalization. --- src/poetry/core/factory.py | 19 +++---------------- 1 file changed, 3 insertions(+), 16 deletions(-) diff --git a/src/poetry/core/factory.py b/src/poetry/core/factory.py index de27307fe..b955c0b58 100644 --- a/src/poetry/core/factory.py +++ b/src/poetry/core/factory.py @@ -355,27 +355,14 @@ def _configure_package_dependencies( current_group = package.dependency_group(group_name) for name in include_groups: try: - # Neither the current name nor the package's - # dependency group names are guaranteed to be - # normalized. Compare the normalized names to find - # the actual key to use. - all_group_names = package.dependency_group_names() - group_name_to_use = next( - (n - for n in all_group_names - if canonicalize_name(n) == canonicalize_name(name))) - - group_to_include = package.dependency_group(group_name_to_use) + # `name` isn't normalized, but `.dependency_group()` + # handles that. + group_to_include = package.dependency_group(name) except ValueError as e: raise ValueError( f"Group '{group_name}' includes group '{name}'" " which is not defined." ) from e - except StopIteration as e: - raise ValueError( - f"Group '{group_name}' includes group '{name}'" - " which is not defined." - ) from e current_group.include_dependency_group(group_to_include) From 691d454f6ef90e39c84995687d298f90b63159eb Mon Sep 17 00:00:00 2001 From: Ben Stolovitz Date: Tue, 27 May 2025 21:28:13 -0400 Subject: [PATCH 20/22] type-check... --- src/poetry/core/factory.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/poetry/core/factory.py b/src/poetry/core/factory.py index b955c0b58..a6b70165d 100644 --- a/src/poetry/core/factory.py +++ b/src/poetry/core/factory.py @@ -647,7 +647,7 @@ def _validate_dependency_groups_includes( """Ensure that dependency groups do not include themselves.""" config = toml_data.setdefault("tool", {}).setdefault("poetry", {}) - group_includes = {} + group_includes: dict[NormalizedName, list[NormalizedName]] = {} for group_name, group_config in config.get("group", {}).items(): if include_groups := group_config.get("include-groups", []): group_includes[canonicalize_name(group_name)] = [ @@ -655,7 +655,7 @@ def _validate_dependency_groups_includes( ] for group_name in group_includes: - ancestors = defaultdict(set[str]) + ancestors = defaultdict(set[NormalizedName]) stack = [group_name] while stack: group = stack.pop() From e8507b4df735c4ab462cb6f73a0c8325a3ee7903 Mon Sep 17 00:00:00 2001 From: Ben Stolovitz Date: Tue, 27 May 2025 22:02:48 -0400 Subject: [PATCH 21/22] fix type error runnign tests --- src/poetry/core/factory.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/poetry/core/factory.py b/src/poetry/core/factory.py index a6b70165d..1b4719668 100644 --- a/src/poetry/core/factory.py +++ b/src/poetry/core/factory.py @@ -655,7 +655,7 @@ def _validate_dependency_groups_includes( ] for group_name in group_includes: - ancestors = defaultdict(set[NormalizedName]) + ancestors: defaultdict[NormalizedName, set[NormalizedName]] = defaultdict(set) stack = [group_name] while stack: group = stack.pop() From 1377bfa58755db2cc9c2c608a1383cdfb1eeb286 Mon Sep 17 00:00:00 2001 From: Ben Stolovitz Date: Tue, 27 May 2025 22:02:57 -0400 Subject: [PATCH 22/22] test another thing that brings the dupe --- tests/test_factory.py | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/tests/test_factory.py b/tests/test_factory.py index 57ce0b90e..62618eee5 100644 --- a/tests/test_factory.py +++ b/tests/test_factory.py @@ -1626,3 +1626,35 @@ def test_create_poetry_with_included_groups_only(temporary_directory: Path) -> N ("black", "all"), ("pytest", "all"), ] + +def test_create_poetry_with_nested_similar_dependencies(temporary_directory: Path) -> None: + pyproject_toml = temporary_directory / "pyproject.toml" + content = """\ +[project] +name = "my-package" +version = "1.2.3" + +[tool.poetry.group.parent.dependencies] +foo = "*" + +[tool.poetry.group.parent] +include-groups = [ + "child", +] + +[tool.poetry.group.child.dependencies] +foo = "*" + +""" + + pyproject_toml.write_text(content) + + poetry = Factory().create_poetry(temporary_directory) + assert len(poetry.package.all_requires) == 3 + assert [ + (dep.name, ",".join(dep.groups)) for dep in poetry.package.all_requires + ] == [ + ("foo", "parent"), + ("foo", "parent"), # TODO: dupe! + ("foo", "child"), + ] \ No newline at end of file