From 9b6376d461f18e382ae0ca1428685dc3aa09584c Mon Sep 17 00:00:00 2001 From: David Hotham Date: Sun, 2 Apr 2023 12:09:38 +0100 Subject: [PATCH 1/3] check for circular self-dependency --- src/poetry/factory.py | 15 +++++++++++++++ tests/console/commands/test_check.py | 1 + tests/fixtures/invalid_pyproject/pyproject.toml | 1 + tests/fixtures/sample_project/pyproject.toml | 4 ++-- tests/fixtures/with_default_source/pyproject.toml | 4 ++-- .../with_default_source_legacy/pyproject.toml | 4 ++-- .../fixtures/with_explicit_source/pyproject.toml | 2 +- tests/fixtures/with_local_config/pyproject.toml | 4 ++-- .../with_two_default_sources/pyproject.toml | 4 ++-- .../pyproject.toml | 4 ++-- tests/publishing/test_publisher.py | 4 +++- tests/test_factory.py | 3 ++- 12 files changed, 35 insertions(+), 15 deletions(-) diff --git a/src/poetry/factory.py b/src/poetry/factory.py index d1a46a654aa..a9a488d6bf7 100644 --- a/src/poetry/factory.py +++ b/src/poetry/factory.py @@ -9,6 +9,7 @@ from typing import cast from cleo.io.null_io import NullIO +from packaging.utils import canonicalize_name from poetry.core.factory import Factory as BaseFactory from poetry.core.packages.dependency_group import MAIN_GROUP from poetry.core.packages.project_package import ProjectPackage @@ -319,4 +320,18 @@ def validate( results["errors"].extend(validate_object(config)) + # A project should not depend on itself. + dependencies = set(config.get("dependencies", {}).keys()) + dependencies.update(config.get("dev-dependencies", {}).keys()) + groups = config.get("group", {}).values() + for group in groups: + dependencies.update(group.get("dependencies", {}).keys()) + + dependencies = {canonicalize_name(d) for d in dependencies} + + if canonicalize_name(config["name"]) in dependencies: + results["errors"].append( + f"Project name ({config['name']}) is same as one of its dependencies" + ) + return results diff --git a/tests/console/commands/test_check.py b/tests/console/commands/test_check.py index 771f95813dc..323364c8def 100644 --- a/tests/console/commands/test_check.py +++ b/tests/console/commands/test_check.py @@ -43,6 +43,7 @@ def test_check_invalid( expected = """\ Error: 'description' is a required property +Error: Project name (invalid) is same as one of its dependencies Error: Unrecognized classifiers: ['Intended Audience :: Clowns']. Warning: A wildcard Python dependency is ambiguous.\ Consider specifying a more explicit one. diff --git a/tests/fixtures/invalid_pyproject/pyproject.toml b/tests/fixtures/invalid_pyproject/pyproject.toml index dfde9cd449e..55b0b6282af 100644 --- a/tests/fixtures/invalid_pyproject/pyproject.toml +++ b/tests/fixtures/invalid_pyproject/pyproject.toml @@ -15,3 +15,4 @@ classifiers = [ [tool.poetry.dependencies] python = "*" pendulum = {"version" = "^2.0.5", allows-prereleases = true} +invalid = "1.0" diff --git a/tests/fixtures/sample_project/pyproject.toml b/tests/fixtures/sample_project/pyproject.toml index 5a5c1356b52..514235f7921 100644 --- a/tests/fixtures/sample_project/pyproject.toml +++ b/tests/fixtures/sample_project/pyproject.toml @@ -1,5 +1,5 @@ [tool.poetry] -name = "my-package" +name = "sample-project" version = "1.2.3" description = "Some description." authors = [ @@ -51,7 +51,7 @@ pytest = "~3.4" [tool.poetry.scripts] -my-script = "my_package:main" +my-script = "sample_project:main" [tool.poetry.plugins."blogtool.parsers"] diff --git a/tests/fixtures/with_default_source/pyproject.toml b/tests/fixtures/with_default_source/pyproject.toml index 7a274d6201d..b572591b6e1 100644 --- a/tests/fixtures/with_default_source/pyproject.toml +++ b/tests/fixtures/with_default_source/pyproject.toml @@ -1,5 +1,5 @@ [tool.poetry] -name = "my-package" +name = "with-default-source" version = "1.2.3" description = "Some description." authors = [ @@ -48,7 +48,7 @@ pytest = "~3.4" [tool.poetry.scripts] -my-script = "my_package:main" +my-script = "with_default_source:main" [tool.poetry.plugins."blogtool.parsers"] diff --git a/tests/fixtures/with_default_source_legacy/pyproject.toml b/tests/fixtures/with_default_source_legacy/pyproject.toml index 0d639ec25d7..0c8a0977d64 100644 --- a/tests/fixtures/with_default_source_legacy/pyproject.toml +++ b/tests/fixtures/with_default_source_legacy/pyproject.toml @@ -1,5 +1,5 @@ [tool.poetry] -name = "my-package" +name = "default-source-legacy" version = "1.2.3" description = "Some description." authors = [ @@ -48,7 +48,7 @@ pytest = "~3.4" [tool.poetry.scripts] -my-script = "my_package:main" +my-script = "default_source_legacy:main" [tool.poetry.plugins."blogtool.parsers"] diff --git a/tests/fixtures/with_explicit_source/pyproject.toml b/tests/fixtures/with_explicit_source/pyproject.toml index e19ad99bad4..bc0f8e17830 100644 --- a/tests/fixtures/with_explicit_source/pyproject.toml +++ b/tests/fixtures/with_explicit_source/pyproject.toml @@ -1,5 +1,5 @@ [tool.poetry] -name = "my-package" +name = "with-explicit-source" version = "1.2.3" description = "Some description." authors = [ diff --git a/tests/fixtures/with_local_config/pyproject.toml b/tests/fixtures/with_local_config/pyproject.toml index b9c7bbd3862..4b814556799 100644 --- a/tests/fixtures/with_local_config/pyproject.toml +++ b/tests/fixtures/with_local_config/pyproject.toml @@ -1,5 +1,5 @@ [tool.poetry] -name = "my-package" +name = "local-config" version = "1.2.3" description = "Some description." authors = [ @@ -48,7 +48,7 @@ pytest = "~3.4" [tool.poetry.scripts] -my-script = "my_package:main" +my-script = "local_config:main" [tool.poetry.plugins."blogtool.parsers"] diff --git a/tests/fixtures/with_two_default_sources/pyproject.toml b/tests/fixtures/with_two_default_sources/pyproject.toml index fa84b0a40a7..3f91eff4664 100644 --- a/tests/fixtures/with_two_default_sources/pyproject.toml +++ b/tests/fixtures/with_two_default_sources/pyproject.toml @@ -1,5 +1,5 @@ [tool.poetry] -name = "my-package" +name = "two-default-sources" version = "1.2.3" description = "Some description." authors = [ @@ -48,7 +48,7 @@ pytest = "~3.4" [tool.poetry.scripts] -my-script = "my_package:main" +my-script = "two_default_sources:main" [tool.poetry.plugins."blogtool.parsers"] diff --git a/tests/fixtures/with_two_default_sources_legacy/pyproject.toml b/tests/fixtures/with_two_default_sources_legacy/pyproject.toml index 0d0a8a5446b..66a333f543c 100644 --- a/tests/fixtures/with_two_default_sources_legacy/pyproject.toml +++ b/tests/fixtures/with_two_default_sources_legacy/pyproject.toml @@ -1,5 +1,5 @@ [tool.poetry] -name = "my-package" +name = "two-default-sources-legacy" version = "1.2.3" description = "Some description." authors = [ @@ -48,7 +48,7 @@ pytest = "~3.4" [tool.poetry.scripts] -my-script = "my_package:main" +my-script = "two_default_sources_legacy:main" [tool.poetry.plugins."blogtool.parsers"] diff --git a/tests/publishing/test_publisher.py b/tests/publishing/test_publisher.py index 5233785a854..6227cde078c 100644 --- a/tests/publishing/test_publisher.py +++ b/tests/publishing/test_publisher.py @@ -11,6 +11,7 @@ from cleo.io.null_io import NullIO from poetry.factory import Factory +from packaging.utils import canonicalize_name from poetry.publishing.publisher import Publisher @@ -72,7 +73,8 @@ def test_publish_can_publish_to_given_repository( ("http://foo.bar",), {"cert": True, "client_cert": None, "dry_run": False, "skip_existing": False}, ] == uploader_upload.call_args - assert "Publishing my-package (1.2.3) to foo" in io.fetch_output() + project_name = canonicalize_name(fixture_name) + assert f"Publishing {project_name} (1.2.3) to foo" in io.fetch_output() def test_publish_raises_error_for_undefined_repository( diff --git a/tests/test_factory.py b/tests/test_factory.py index d3a39620e86..0530bfee66a 100644 --- a/tests/test_factory.py +++ b/tests/test_factory.py @@ -37,7 +37,7 @@ def test_create_poetry(fixture_dir: FixtureDirGetter) -> None: package = poetry.package - assert package.name == "my-package" + assert package.name == "sample-project" assert package.version.text == "1.2.3" assert package.description == "Some description." assert package.authors == ["Sébastien Eustace "] @@ -418,6 +418,7 @@ def test_create_poetry_fails_on_invalid_configuration( expected = """\ The Poetry configuration is invalid: - 'description' is a required property + - Project name (invalid) is same as one of its dependencies """ assert str(e.value) == expected From aa4b31850166b80b003c93bb5c39110803b0898a Mon Sep 17 00:00:00 2001 From: David Hotham Date: Sun, 2 Apr 2023 12:29:55 +0100 Subject: [PATCH 2/3] Reject circular self-add --- src/poetry/console/commands/add.py | 9 +++++++++ tests/console/commands/test_add.py | 12 ++++++++++++ tests/publishing/test_publisher.py | 2 +- 3 files changed, 22 insertions(+), 1 deletion(-) diff --git a/src/poetry/console/commands/add.py b/src/poetry/console/commands/add.py index 451965cd431..9ee892fd25f 100644 --- a/src/poetry/console/commands/add.py +++ b/src/poetry/console/commands/add.py @@ -126,6 +126,7 @@ def handle(self) -> int: # dictionary. content: dict[str, Any] = self.poetry.file.read() poetry_content = content["tool"]["poetry"] + project_name = canonicalize_name(poetry_content["name"]) if group == MAIN_GROUP: if "dependencies" not in poetry_content: @@ -223,6 +224,14 @@ def handle(self) -> int: canonical_constraint_name = canonicalize_name(constraint_name) + if canonical_constraint_name == project_name: + self.line_error( + f"Cannot add dependency on {_constraint['name']} to" + " project with the same name." + ) + self.line_error("\nNo changes were applied.") + return 1 + for key in section: if canonicalize_name(key) == canonical_constraint_name: section[key] = constraint diff --git a/tests/console/commands/test_add.py b/tests/console/commands/test_add.py index 8daf01ea9d9..4c681b7f0a9 100644 --- a/tests/console/commands/test_add.py +++ b/tests/console/commands/test_add.py @@ -1073,6 +1073,18 @@ def test_add_should_skip_when_adding_canonicalized_existing_package_with_no_cons assert expected in tester.io.fetch_output() +def test_add_should_fail_circular_dependency( + app: PoetryTestApplication, repo: TestRepository, tester: CommandTester +) -> None: + repo.add_package(get_package("simple-project", "1.1.2")) + result = tester.execute("simple-project") + + assert result == 1 + + expected = "Cannot add dependency on simple-project to project with the same name." + assert expected in tester.io.fetch_error() + + def test_add_latest_should_not_create_duplicate_keys( project_factory: ProjectFactory, repo: TestRepository, diff --git a/tests/publishing/test_publisher.py b/tests/publishing/test_publisher.py index 6227cde078c..bf30141541e 100644 --- a/tests/publishing/test_publisher.py +++ b/tests/publishing/test_publisher.py @@ -9,9 +9,9 @@ from cleo.io.buffered_io import BufferedIO from cleo.io.null_io import NullIO +from packaging.utils import canonicalize_name from poetry.factory import Factory -from packaging.utils import canonicalize_name from poetry.publishing.publisher import Publisher From 3b60eb19141868e2d4432cfa07e03c3dcc2f3152 Mon Sep 17 00:00:00 2001 From: David Hotham Date: Tue, 18 Apr 2023 16:40:10 +0100 Subject: [PATCH 3/3] code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Randy Döring <30527984+radoering@users.noreply.github.com> --- src/poetry/console/commands/add.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/poetry/console/commands/add.py b/src/poetry/console/commands/add.py index 9ee892fd25f..f8c82c18c29 100644 --- a/src/poetry/console/commands/add.py +++ b/src/poetry/console/commands/add.py @@ -226,7 +226,7 @@ def handle(self) -> int: if canonical_constraint_name == project_name: self.line_error( - f"Cannot add dependency on {_constraint['name']} to" + f"Cannot add dependency on {constraint_name} to" " project with the same name." ) self.line_error("\nNo changes were applied.")