From a76b6d2d0f3ef8405c28df3000420b031dc88399 Mon Sep 17 00:00:00 2001 From: Arun Babu Neelicattu Date: Fri, 16 Oct 2020 23:42:10 +0200 Subject: [PATCH] locker: handle nested extras requirement Previously, when using locked repository, incorrect dependency instance was created when a dependency's extra requirement activated a nested extra. This change ensures that these are correctly loaded. As part of this change new lock files write PEP 508 serialised form of extra dependencies in order to reuse core logic to parse specification of extra requirement. Resolves: #3224 --- poetry/packages/locker.py | 23 +++- .../fixtures/with-dependencies-extras.test | 2 +- .../with-dependencies-nested-extras.test | 45 ++++++ tests/installation/test_installer.py | 29 ++++ tests/packages/test_locker.py | 130 ++++++++++++++++++ 5 files changed, 222 insertions(+), 7 deletions(-) create mode 100644 tests/installation/fixtures/with-dependencies-nested-extras.test diff --git a/poetry/packages/locker.py b/poetry/packages/locker.py index 9dd75e66519..ac791537450 100644 --- a/poetry/packages/locker.py +++ b/poetry/packages/locker.py @@ -24,12 +24,14 @@ import poetry.repositories +from poetry.core.packages import dependency_from_pep_508 from poetry.core.packages.package import Dependency from poetry.core.packages.package import Package from poetry.core.semver import parse_constraint from poetry.core.semver.version import Version from poetry.core.toml.file import TOMLFile from poetry.core.version.markers import parse_marker +from poetry.core.version.requirements import InvalidRequirement from poetry.packages import DependencyPackage from poetry.utils._compat import OrderedDict from poetry.utils._compat import Path @@ -142,11 +144,18 @@ def locked_repository( package.extras[name] = [] for dep in deps: - m = re.match(r"^(.+?)(?:\s+\((.+)\))?$", dep) - dep_name = m.group(1) - constraint = m.group(2) or "*" - - package.extras[name].append(Dependency(dep_name, constraint)) + try: + dependency = dependency_from_pep_508(dep) + except InvalidRequirement: + # handle lock files with invalid PEP 508 + m = re.match(r"^(.+?)(?:\[(.+?)])?(?:\s+\((.+)\))?$", dep) + dep_name = m.group(1) + extras = m.group(2) or "" + constraint = m.group(3) or "*" + dependency = Dependency( + dep_name, constraint, extras=extras.split(",") + ) + package.extras[name].append(dependency) if "marker" in info: package.marker = parse_marker(info["marker"]) @@ -543,8 +552,10 @@ def _dump_package(self, package): # type: (Package) -> dict if package.extras: extras = {} for name, deps in package.extras.items(): + # TODO: This should use dep.to_pep_508() once this is fixed + # https://github.com/python-poetry/poetry-core/pull/102 extras[name] = [ - str(dep) if not dep.constraint.is_any() else dep.name + dep.base_pep_508_name if not dep.constraint.is_any() else dep.name for dep in deps ] diff --git a/tests/installation/fixtures/with-dependencies-extras.test b/tests/installation/fixtures/with-dependencies-extras.test index 63560bb4793..042e29670e1 100644 --- a/tests/installation/fixtures/with-dependencies-extras.test +++ b/tests/installation/fixtures/with-dependencies-extras.test @@ -18,7 +18,7 @@ python-versions = "*" C = {version = "^1.0", optional = true} [package.extras] -foo = ["C (^1.0)"] +foo = ["C (>=1.0,<2.0)"] [[package]] name = "C" diff --git a/tests/installation/fixtures/with-dependencies-nested-extras.test b/tests/installation/fixtures/with-dependencies-nested-extras.test new file mode 100644 index 00000000000..48a22a7c7f3 --- /dev/null +++ b/tests/installation/fixtures/with-dependencies-nested-extras.test @@ -0,0 +1,45 @@ +[[package]] +name = "A" +version = "1.0" +description = "" +category = "main" +optional = false +python-versions = "*" + +[package.dependencies] +B = {version = "^1.0", optional = true, extras = ["C"]} + +[package.extras] +B = ["B[C] (>=1.0,<2.0)"] + +[[package]] +name = "B" +version = "1.0" +description = "" +category = "main" +optional = false +python-versions = "*" + +[package.dependencies] +C = {version = "^1.0", optional = true} + +[package.extras] +C = ["C (>=1.0,<2.0)"] + +[[package]] +name = "C" +version = "1.0" +description = "" +category = "main" +optional = false +python-versions = "*" + +[metadata] +python-versions = "*" +lock-version = "1.1" +content-hash = "123456789" + +[metadata.files] +"A" = [] +"B" = [] +"C" = [] diff --git a/tests/installation/test_installer.py b/tests/installation/test_installer.py index 7a1660670ee..106efde6e9c 100644 --- a/tests/installation/test_installer.py +++ b/tests/installation/test_installer.py @@ -639,6 +639,35 @@ def test_run_with_dependencies_extras(installer, locker, repo, package): assert locker.written_data == expected +def test_run_with_dependencies_nested_extras(installer, locker, repo, package): + package_a = get_package("A", "1.0") + package_b = get_package("B", "1.0") + package_c = get_package("C", "1.0") + + dependency_c = Factory.create_dependency("C", {"version": "^1.0", "optional": True}) + dependency_b = Factory.create_dependency( + "B", {"version": "^1.0", "optional": True, "extras": ["C"]} + ) + dependency_a = Factory.create_dependency("A", {"version": "^1.0", "extras": ["B"]}) + + package_b.extras = {"C": [dependency_c]} + package_b.add_dependency(dependency_c) + + package_a.add_dependency(dependency_b) + package_a.extras = {"B": [dependency_b]} + + repo.add_package(package_a) + repo.add_package(package_b) + repo.add_package(package_c) + + package.add_dependency(dependency_a) + + installer.run() + expected = fixture("with-dependencies-nested-extras") + + assert locker.written_data == expected + + def test_run_does_not_install_extras_if_not_requested(installer, locker, repo, package): package.extras["foo"] = [get_dependency("D")] package_a = get_package("A", "1.0") diff --git a/tests/packages/test_locker.py b/tests/packages/test_locker.py index 35c584f2767..a4aa17971f8 100644 --- a/tests/packages/test_locker.py +++ b/tests/packages/test_locker.py @@ -142,6 +142,136 @@ def test_locker_properly_loads_extras(locker): assert lockfile_dep.name == "lockfile" +def test_locker_properly_loads_nested_extras(locker): + content = """\ +[[package]] +name = "a" +version = "1.0" +description = "" +category = "main" +optional = false +python-versions = "*" + +[package.dependencies] +b = {version = "^1.0", optional = true, extras = "c"} + +[package.extras] +b = ["b[c] (>=1.0,<2.0)"] + +[[package]] +name = "b" +version = "1.0" +description = "" +category = "main" +optional = false +python-versions = "*" + +[package.dependencies] +c = {version = "^1.0", optional = true} + +[package.extras] +c = ["c (>=1.0,<2.0)"] + +[[package]] +name = "c" +version = "1.0" +description = "" +category = "main" +optional = false +python-versions = "*" + +[metadata] +python-versions = "*" +lock-version = "1.1" +content-hash = "123456789" + +[metadata.files] +"a" = [] +"b" = [] +"c" = [] +""" + + locker.lock.write(tomlkit.parse(content)) + + repository = locker.locked_repository() + assert 3 == len(repository.packages) + + packages = repository.find_packages(get_dependency("a", "1.0")) + assert len(packages) == 1 + + package = packages[0] + assert len(package.requires) == 1 + assert len(package.extras) == 1 + + dependency_b = package.extras["b"][0] + assert dependency_b.name == "b" + assert dependency_b.extras == frozenset({"c"}) + + packages = repository.find_packages(dependency_b) + assert len(packages) == 1 + + package = packages[0] + assert len(package.requires) == 1 + assert len(package.extras) == 1 + + dependency_c = package.extras["c"][0] + assert dependency_c.name == "c" + assert dependency_c.extras == frozenset() + + packages = repository.find_packages(dependency_c) + assert len(packages) == 1 + + +def test_locker_properly_loads_extras_legacy(locker): + content = """\ +[[package]] +name = "a" +version = "1.0" +description = "" +category = "main" +optional = false +python-versions = "*" + +[package.dependencies] +b = {version = "^1.0", optional = true} + +[package.extras] +b = ["b (^1.0)"] + +[[package]] +name = "b" +version = "1.0" +description = "" +category = "main" +optional = false +python-versions = "*" + +[metadata] +python-versions = "*" +lock-version = "1.1" +content-hash = "123456789" + +[metadata.files] +"a" = [] +"b" = [] +""" + + locker.lock.write(tomlkit.parse(content)) + + repository = locker.locked_repository() + assert 2 == len(repository.packages) + + packages = repository.find_packages(get_dependency("a", "1.0")) + assert len(packages) == 1 + + package = packages[0] + assert len(package.requires) == 1 + assert len(package.extras) == 1 + + dependency_b = package.extras["b"][0] + assert dependency_b.name == "b" + + def test_lock_packages_with_null_description(locker, root): package_a = get_package("A", "1.0.0") package_a.description = None