Track dependencies for local packages that get combined

piptools/repositories/base.py

@@ -47,6 +47,17 @@ def allow_all_wheels(self) -> Iterator[None]:
         Monkey patches pip.Wheel to allow wheels from all platforms and Python versions.
         """
 
+    @abstractmethod
+    def copy_ireq_dependencies(
+        self, source: InstallRequirement, dest: InstallRequirement
+    ) -> None:
+        """
+        Notifies the repository that `dest` is a copy of `source`, and so it
+        has the same dependencies. Otherwise, once we prepare an ireq to assign
+        it its name, we would lose track of those dependencies on combining
+        that ireq with others.
+        """
+
     @property
     @abstractmethod
     def options(self) -> optparse.Values:

piptools/repositories/local.py

@@ -95,3 +95,8 @@ def get_hashes(self, ireq: InstallRequirement) -> Set[str]:
     def allow_all_wheels(self) -> Iterator[None]:
         with self.repository.allow_all_wheels():
             yield
+
+    def copy_ireq_dependencies(
+        self, source: InstallRequirement, dest: InstallRequirement
+    ) -> None:
+        self.repository.copy_ireq_dependencies(source, dest)

piptools/repositories/pypi.py

@@ -241,6 +241,15 @@ def get_dependencies(self, ireq: InstallRequirement) -> Set[InstallRequirement]:
 
         return self._dependencies_cache[ireq]
 
+    def copy_ireq_dependencies(
+        self, source: InstallRequirement, dest: InstallRequirement
+    ) -> None:
+        try:
+            self._dependencies_cache[dest] = self._dependencies_cache[source]
+        except KeyError:
+            # `source` may not be in cache yet.
+            pass
+
     def _get_project(self, ireq: InstallRequirement) -> Any:
         """
         Return a dict of a project info from PyPI JSON API for a given

piptools/resolver.py

@@ -72,16 +72,26 @@ def combine_install_requirements(
 
     # deepcopy the accumulator so as to not modify the inputs
     combined_ireq = copy.deepcopy(source_ireqs[0])
+    repository.copy_ireq_dependencies(source_ireqs[0], combined_ireq)
 
     for ireq in source_ireqs[1:]:
         # NOTE we may be losing some info on dropped reqs here
         if combined_ireq.req is not None and ireq.req is not None:
             combined_ireq.req.specifier &= ireq.req.specifier
+        if combined_ireq.constraint:
+            # We don't find dependencies for constraint ireqs, so copy them
+            # from non-constraints:
+            repository.copy_ireq_dependencies(ireq, combined_ireq)
         combined_ireq.constraint &= ireq.constraint
         combined_ireq.extras = {*combined_ireq.extras, *ireq.extras}
         if combined_ireq.req is not None:
             combined_ireq.req.extras = set(combined_ireq.extras)
 
+        for attr in ("link", "local_file_path", "original_link"):
+            setattr(
+                combined_ireq, attr, getattr(combined_ireq, attr) or getattr(ireq, attr)
+            )
+
     # InstallRequirements objects are assumed to come from only one source, and
     # so they support only a single comes_from entry. This function breaks this
     # model. As a workaround, we deterministically choose a single source for

tests/conftest.py

@@ -89,6 +89,10 @@ def allow_all_wheels(self):
         # No need to do an actual pip.Wheel mock here.
         yield
 
+    def copy_ireq_dependencies(self, source, dest):
+        # No state to update.
+        pass
+
     @property
     def options(self) -> optparse.Values:
         """Not used"""

tests/test_cli_compile.py

@@ -1727,6 +1727,34 @@ def test_duplicate_reqs_combined(
     assert "test-package-1==0.1" in out.stderr
 
 
+def test_local_duplicate_subdependency_combined(runner, make_package):
+    """
+    Test pip-compile tracks subdependencies properly when install requirements
+    are combined, especially when local paths are passed as urls, and those reqs
+    are combined after getting dependencies.
+
+    Regression test for issue GH-1505.
+    """
+    package_a = make_package("project-a", install_requires=["pip-tools==6.3.0"])
+    package_b = make_package("project-b", install_requires=["project-a"])
+
+    with open("requirements.in", "w") as req_in:
+        req_in.writelines(
+            [
+                f"file://{package_a}#egg=project-a\n",
+                f"file://{package_b}#egg=project-b",
+            ]
+        )
+
+    out = runner.invoke(cli, ["-n"])
+
+    assert out.exit_code == 0
+    assert "project-b" in out.stderr
+    assert "project-a" in out.stderr
+    assert "pip-tools==6.3.0" in out.stderr
+    assert "click" in out.stderr  # dependency of pip-tools
+
+
 def test_combine_extras(pip_conf, runner, make_package):
     """
     Ensure that multiple declarations of a dependency that specify different

tests/test_repository_local.py

@@ -1,7 +1,10 @@
+import copy
+
 import pytest
 
 from piptools.repositories.local import LocalRequirementsRepository
 from piptools.utils import key_from_ireq
+from tests.conftest import FakeRepository
 
 EXPECTED = {"sha256:5e6071ee6e4c59e0d0408d366fe9b66781d2cf01be9a6e19a2433bb3c5336330"}
 
@@ -54,3 +57,25 @@ def test_toggle_reuse_hashes_local_repository(
     captured = capsys.readouterr()
     assert captured.out == ""
     assert captured.err == ""
+
+
+class FakeRepositoryChecksForCopy(FakeRepository):
+    def __init__(self):
+        super().__init__()
+        self.copied = []
+
+    def copy_ireq_dependencies(self, source, dest):
+        self.copied.append(source)
+
+
+def test_local_repository_copy_ireq_dependencies(from_line):
+    # Ensure that local repository forwards any messages to update its state
+    # of ireq dependencies.
+    checker = FakeRepositoryChecksForCopy()
+    local_repository = LocalRequirementsRepository({}, checker)
+
+    src = from_line("small-fake-a==0.1")
+    dest = copy.deepcopy(src)
+    local_repository.copy_ireq_dependencies(src, dest)
+
+    assert src in checker.copied

tests/test_resolver.py

@@ -284,6 +284,51 @@ def test_iter_dependencies_ignores_constraints(resolver, from_line):
         next(res._iter_dependencies(ireq))
 
 
+def test_iter_dependencies_after_combine_install_requirements(
+    pypi_repository, base_resolver, make_package, from_line
+):
+    res = base_resolver([], repository=pypi_repository)
+
+    sub_deps = ["click"]
+    package_a = make_package("package-a", install_requires=sub_deps)
+    package_b = make_package("package-b", install_requires=["package-a"])
+
+    local_package_a = from_line(path_to_url(package_a))
+    assert [dep.name for dep in res._iter_dependencies(local_package_a)] == sub_deps
+
+    package_a_from_b = from_line("package-a", comes_from=path_to_url(package_b))
+    combined = combine_install_requirements(
+        pypi_repository, [local_package_a, package_a_from_b]
+    )
+    assert [dep.name for dep in res._iter_dependencies(combined)] == sub_deps
+
+
+@pytest.mark.xfail(
+    reason="resolver does not yet support combining extras after the first round"
+)
+def test_iter_dependencies_after_combine_install_requirements_extras(
+    pypi_repository, base_resolver, make_package, from_line
+):
+    res = base_resolver([], repository=pypi_repository)
+
+    package_a = make_package(
+        "package-a", extras_require={"click": ["click"], "celery": ["celery"]}
+    )
+    package_b = make_package("package-b", install_requires=["package-a"])
+
+    local_package_a = from_line(path_to_url(package_a))
+    assert [dep.name for dep in res._iter_dependencies(local_package_a)] == []
+
+    package_a_from_b = from_line("package-a[click]", comes_from=path_to_url(package_b))
+    package_a_with_other_extra = from_line("package-a[celery]")
+    combined = combine_install_requirements(
+        pypi_repository, [local_package_a, package_a_from_b, package_a_with_other_extra]
+    )
+
+    dependency_names = {dep.name for dep in res._iter_dependencies(combined)}
+    assert {"celery", "click"}.issubset(dependency_names)
+
+
 def test_combine_install_requirements(repository, from_line):
     celery30 = from_line("celery>3.0", comes_from="-r requirements.in")
     celery31 = from_line("celery==3.1.1", comes_from=from_line("fake-package"))
@@ -341,6 +386,22 @@ def test_combine_install_requirements_extras_no_req(
     )
 
 
+def test_combine_install_requirements_with_paths(repository, from_line, make_package):
+    name = "fake_package_b"
+    version = "1.0.0"
+
+    test_package = make_package(name, version=version)
+    fake_package = from_line(f"{name} @ {path_to_url(test_package)}")
+    fake_package_name = from_line(f"{name}=={version}", comes_from=from_line(name))
+
+    for pair in [(fake_package, fake_package_name), (fake_package_name, fake_package)]:
+        combined = combine_install_requirements(repository, pair)
+        assert str(combined.specifier) == str(fake_package_name.specifier)
+        assert str(combined.link) == str(fake_package.link)
+        assert str(combined.local_file_path) == str(fake_package.local_file_path)
+        assert str(combined.original_link) == str(fake_package.original_link)
+
+
 def test_compile_failure_shows_provenance(resolver, from_line):
     """
     Provenance of conflicting dependencies should be printed on failure.