Merge pull request #3229 from abn/issue/3224

locker: handle nested extras requirement

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"])
@@ -541,8 +550,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
                 ]
 

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"

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" = []

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

tests/packages/test_locker.py

@@ -143,6 +143,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