Rework reverse dep tracking for annotations, now appearing for primar…

…y and VCS reqs; fixes #881 and #293

Change Notes
------------

- Annotations now include reqsin/setup.py/stdin info when relevant
- Always send reverse_dependencies to writer._format_requirement as a dict, even if empty
- Invert checks for primary requirements not getting annotations, as they now do
- Bypass annotation noise in tests which don't target annotations anyway, by passing --no-annotate
- ireqs returned by get_best_match now have the parameter ireq's _source_ireqs, if any
- reverse_dependencies are no longer passed around independently outside the
  resolver/cache, but rather determined from an ireq's comes_from and _source_ireqs
- primary_packages are no longer passed around independently, but rather determined by
  an ireq's constraint
- In tests, reverse dependencies for fake ireqs are now specified by setting the ireq's
  comes_from
- These changes do _not_ improve handling of relative paths to reqsin files in annotations (see #1067)

Co-authored-by: Albert Tugushev <albert@tugushev.ru>

piptools/resolver.py

@@ -318,6 +318,8 @@ def get_best_match(self, ireq):
             )
         )
         best_match.comes_from = ireq.comes_from
+        if hasattr(ireq, "_source_ireqs"):
+            best_match._source_ireqs = ireq._source_ireqs
         return best_match
 
     def _iter_dependencies(self, ireq):

piptools/scripts/compile.py

@@ -327,17 +327,24 @@ def cli(
 
                 dist = run_setup(src_file)
                 tmpfile.write("\n".join(dist.install_requires))
+                comes_from = "{name} ({filename})".format(
+                    name=dist.get_name(), filename=src_file
+                )
             else:
                 tmpfile.write(sys.stdin.read())
+                comes_from = "-r -"
             tmpfile.flush()
-            constraints.extend(
+            reqs = list(
                 parse_requirements(
                     tmpfile.name,
                     finder=repository.finder,
                     session=repository.session,
                     options=repository.options,
                 )
             )
+            for req in reqs:
+                req.comes_from = comes_from
+            constraints.extend(reqs)
         else:
             constraints.extend(
                 parse_requirements(
@@ -396,33 +403,6 @@ def cli(
     # Output
     ##
 
-    # Compute reverse dependency annotations statically, from the
-    # dependency cache that the resolver has populated by now.
-    #
-    # TODO (1a): reverse deps for any editable package are lost
-    #            what SHOULD happen is that they are cached in memory, just
-    #            not persisted to disk!
-    #
-    # TODO (1b): perhaps it's easiest if the dependency cache has an API
-    #            that could take InstallRequirements directly, like:
-    #
-    #              cache.set(ireq, ...)
-    #
-    #            then, when ireq is editable, it would store in
-    #
-    #              editables[egg_name][link_without_fragment] = deps
-    #              editables['pip-tools']['git+...ols.git@future'] = {
-    #                  'click>=3.0', 'six'
-    #              }
-    #
-    #            otherwise:
-    #
-    #              self[as_name_version_tuple(ireq)] = {'click>=3.0', 'six'}
-    #
-    reverse_dependencies = None
-    if annotate:
-        reverse_dependencies = resolver.reverse_dependencies(results)
-
     writer = OutputWriter(
         src_files,
         output_file,
@@ -444,8 +424,6 @@ def cli(
     writer.write(
         results=results,
         unsafe_requirements=resolver.unsafe_constraints,
-        reverse_dependencies=reverse_dependencies,
-        primary_packages=primary_packages,
         markers={
             key_from_ireq(ireq): ireq.markers for ireq in constraints if ireq.markers
         },

piptools/writer.py

@@ -3,6 +3,8 @@
 import os
 from itertools import chain
 
+import six
+
 from .click import unstyle
 from .logging import log
 from .utils import (
@@ -36,6 +38,12 @@
 )
 
 
+def _comes_from_as_string(ireq):
+    if isinstance(ireq.comes_from, six.string_types):
+        return ireq.comes_from
+    return key_from_ireq(ireq.comes_from)
+
+
 class OutputWriter(object):
     def __init__(
         self,
@@ -125,19 +133,9 @@ def write_flags(self):
         if emitted:
             yield ""
 
-    def _iter_lines(
-        self,
-        results,
-        unsafe_requirements=None,
-        reverse_dependencies=None,
-        primary_packages=None,
-        markers=None,
-        hashes=None,
-    ):
+    def _iter_lines(self, results, unsafe_requirements=None, markers=None, hashes=None):
         # default values
         unsafe_requirements = unsafe_requirements or []
-        reverse_dependencies = reverse_dependencies or {}
-        primary_packages = primary_packages or []
         markers = markers or {}
         hashes = hashes or {}
 
@@ -169,11 +167,7 @@ def _iter_lines(
                     yield MESSAGE_UNHASHED_PACKAGE
                     warn_uninstallable = True
                 line = self._format_requirement(
-                    ireq,
-                    reverse_dependencies,
-                    primary_packages,
-                    markers.get(key_from_ireq(ireq)),
-                    hashes=hashes,
+                    ireq, markers.get(key_from_ireq(ireq)), hashes=hashes
                 )
                 yield line
             yielded = True
@@ -194,11 +188,7 @@ def _iter_lines(
                     yield comment("# {}".format(ireq_key))
                 else:
                     line = self._format_requirement(
-                        ireq,
-                        reverse_dependencies,
-                        primary_packages,
-                        marker=markers.get(ireq_key),
-                        hashes=hashes,
+                        ireq, marker=markers.get(ireq_key), hashes=hashes
                     )
                     yield line
 
@@ -209,41 +199,32 @@ def _iter_lines(
         if warn_uninstallable:
             log.warning(MESSAGE_UNINSTALLABLE)
 
-    def write(
-        self,
-        results,
-        unsafe_requirements,
-        reverse_dependencies,
-        primary_packages,
-        markers,
-        hashes,
-    ):
+    def write(self, results, unsafe_requirements, markers, hashes):
 
-        for line in self._iter_lines(
-            results,
-            unsafe_requirements,
-            reverse_dependencies,
-            primary_packages,
-            markers,
-            hashes,
-        ):
+        for line in self._iter_lines(results, unsafe_requirements, markers, hashes):
             log.info(line)
             if not self.dry_run:
                 self.dst_file.write(unstyle(line).encode("utf-8"))
                 self.dst_file.write(os.linesep.encode("utf-8"))
 
-    def _format_requirement(
-        self, ireq, reverse_dependencies, primary_packages, marker=None, hashes=None
-    ):
+    def _format_requirement(self, ireq, marker=None, hashes=None):
         ireq_hashes = (hashes if hashes is not None else {}).get(ireq)
 
         line = format_requirement(ireq, marker=marker, hashes=ireq_hashes)
 
-        if not self.annotate or key_from_ireq(ireq) in primary_packages:
+        if not self.annotate:
             return line
 
-        # Annotate what packages this package is required by
-        required_by = reverse_dependencies.get(ireq.name.lower(), [])
+        # Annotate what packages or reqs-ins this package is required by
+        required_by = set()
+        if hasattr(ireq, "_source_ireqs"):
+            required_by |= {
+                _comes_from_as_string(src_ireq)
+                for src_ireq in ireq._source_ireqs
+                if src_ireq.comes_from
+            }
+        elif ireq.comes_from:
+            required_by.add(_comes_from_as_string(ireq))
         if required_by:
             annotation = ", ".join(sorted(required_by))
             line = "{:24}{}{}".format(

tests/test_cli_compile.py

@@ -41,20 +41,27 @@ def test_command_line_overrides_pip_conf(pip_with_index_conf, runner):
 
 
 def test_command_line_setuptools_read(pip_conf, runner):
-    package = open("setup.py", "w")
-    package.write(
-        dedent(
-            """\
-            from setuptools import setup
-            setup(install_requires=[])
-            """
+    with open("setup.py", "w") as package:
+        package.write(
+            dedent(
+                """\
+                from setuptools import setup
+                setup(
+                    name="fake-setuptools-a",
+                    install_requires=["small-fake-a==0.1"]
+                )
+                """
+            )
         )
-    )
-    package.close()
     out = runner.invoke(cli)
 
-    # check that pip-compile generated a configuration
     assert "This file is autogenerated by pip-compile" in out.stderr
+    assert (
+        "small-fake-a==0.1         # via fake-setuptools-a (setup.py)"
+        in out.stderr.splitlines()
+    )
+
+    # check that pip-compile generated a configuration file
     assert os.path.exists("requirements.txt")
 
 
@@ -363,11 +370,11 @@ def test_upgrade_packages_option(pip_conf, runner):
     with open("requirements.txt", "w") as req_in:
         req_in.write("small-fake-a==0.1\nsmall-fake-b==0.1")
 
-    out = runner.invoke(cli, ["-P", "small-fake-b"])
+    out = runner.invoke(cli, ["--no-annotate", "-P", "small-fake-b"])
 
     assert out.exit_code == 0
-    assert "small-fake-a==0.1" in out.stderr
-    assert "small-fake-b==0.3" in out.stderr
+    assert "small-fake-a==0.1" in out.stderr.splitlines()
+    assert "small-fake-b==0.3" in out.stderr.splitlines()
 
 
 def test_upgrade_packages_option_irrelevant(pip_conf, runner):
@@ -379,11 +386,11 @@ def test_upgrade_packages_option_irrelevant(pip_conf, runner):
     with open("requirements.txt", "w") as req_in:
         req_in.write("small-fake-a==0.1")
 
-    out = runner.invoke(cli, ["--upgrade-package", "small-fake-b"])
+    out = runner.invoke(cli, ["--no-annotate", "--upgrade-package", "small-fake-b"])
 
     assert out.exit_code == 0
     assert "small-fake-a==0.1" in out.stderr.splitlines()
-    assert "small-fake-b==0.3" not in out.stderr
+    assert "small-fake-b==0.3" not in out.stderr.splitlines()
 
 
 def test_upgrade_packages_option_no_existing_file(pip_conf, runner):
@@ -394,11 +401,11 @@ def test_upgrade_packages_option_no_existing_file(pip_conf, runner):
     with open("requirements.in", "w") as req_in:
         req_in.write("small-fake-a\nsmall-fake-b")
 
-    out = runner.invoke(cli, ["-P", "small-fake-b"])
+    out = runner.invoke(cli, ["--no-annotate", "-P", "small-fake-b"])
 
     assert out.exit_code == 0
-    assert "small-fake-a==0.2" in out.stderr
-    assert "small-fake-b==0.3" in out.stderr
+    assert "small-fake-a==0.2" in out.stderr.splitlines()
+    assert "small-fake-b==0.3" in out.stderr.splitlines()
 
 
 @pytest.mark.parametrize(
@@ -502,7 +509,7 @@ def test_generate_hashes_with_editable(pip_conf, runner):
     small_fake_package_url = path_to_url(small_fake_package_dir)
     with open("requirements.in", "w") as fp:
         fp.write("-e {}\n".format(small_fake_package_url))
-    out = runner.invoke(cli, ["--generate-hashes"])
+    out = runner.invoke(cli, ["--no-annotate", "--generate-hashes"])
     expected = (
         "-e {}\n"
         "small-fake-a==0.1 \\\n"
@@ -523,7 +530,7 @@ def test_generate_hashes_with_url(runner):
             "https://github.com/jazzband/pip-tools/archive/"
             "7d86c8d3ecd1faa6be11c7ddc6b29a30ffd1dae3.zip#egg=pip-tools\n"
         )
-    out = runner.invoke(cli, ["--generate-hashes"])
+    out = runner.invoke(cli, ["--no-annotate", "--generate-hashes"])
     expected = (
         "https://github.com/jazzband/pip-tools/archive/"
         "7d86c8d3ecd1faa6be11c7ddc6b29a30ffd1dae3.zip#egg=pip-tools \\\n"
@@ -628,7 +635,7 @@ def test_stdin(pip_conf, runner):
         cli, ["-", "--output-file", "requirements.txt", "-n"], input="small-fake-a==0.1"
     )
 
-    assert "small-fake-a==0.1" in out.stderr
+    assert "small-fake-a==0.1         # via -r -" in out.stderr.splitlines()
 
 
 def test_multiple_input_files_without_output_file(runner):
@@ -652,15 +659,22 @@ def test_multiple_input_files_without_output_file(runner):
 @pytest.mark.parametrize(
     "option, expected",
     [
-        ("--annotate", "small-fake-a==0.1         # via small-fake-with-deps\n"),
+        (
+            "--annotate",
+            "small-fake-a==0.1         "
+            "# via -c constraints.txt (line 1), small-fake-with-deps\n",
+        ),
         ("--no-annotate", "small-fake-a==0.1\n"),
     ],
 )
 def test_annotate_option(pip_conf, runner, option, expected):
     """
     The output lines has have annotations if option is turned on.
     """
+    with open("constraints.txt", "w") as constraints_in:
+        constraints_in.write("small-fake-a==0.1")
     with open("requirements.in", "w") as req_in:
+        req_in.write("-c constraints.txt\n")
         req_in.write("small_fake_with_deps")
 
     out = runner.invoke(cli, [option, "-n"])
@@ -681,7 +695,7 @@ def test_allow_unsafe_option(pip_conf, monkeypatch, runner, option, expected):
     with open("requirements.in", "w") as req_in:
         req_in.write(path_to_url(os.path.join(PACKAGES_PATH, "small_fake_with_deps")))
 
-    out = runner.invoke(cli, [option] if option else [])
+    out = runner.invoke(cli, ["--no-annotate", option] if option else [])
 
     assert expected in out.stderr.splitlines()
     assert out.exit_code == 0
@@ -746,7 +760,7 @@ def test_pre_option(pip_conf, runner, cli_option, infile_option, expected_packag
             req_in.write("--pre\n")
         req_in.write("small-fake-a\n")
 
-    out = runner.invoke(cli, ["-n"] + (["-p"] if cli_option else []))
+    out = runner.invoke(cli, ["--no-annotate", "-n"] + (["-p"] if cli_option else []))
 
     assert out.exit_code == 0, out.stderr
     assert expected_package in out.stderr.splitlines(), out.stderr
@@ -770,7 +784,7 @@ def test_dry_run_option(pip_conf, runner, add_options):
     with open("requirements.in", "w") as req_in:
         req_in.write("small-fake-a\n")
 
-    out = runner.invoke(cli, ["--dry-run"] + add_options)
+    out = runner.invoke(cli, ["--no-annotate", "--dry-run"] + add_options)
 
     assert out.exit_code == 0, out.stderr
     assert "small-fake-a==0.2" in out.stderr.splitlines()
@@ -805,7 +819,7 @@ def test_dry_run_doesnt_touch_output_file(
 
     before_compile_mtime = os.stat("requirements.txt").st_mtime
 
-    out = runner.invoke(cli, ["--dry-run"] + add_options)
+    out = runner.invoke(cli, ["--no-annotate", "--dry-run"] + add_options)
 
     assert out.exit_code == 0, out.stderr
     assert expected_cli_output_package in out.stderr.splitlines()
@@ -863,7 +877,6 @@ def test_upgrade_package_doesnt_remove_annotation(pip_conf, runner):
         )
 
     runner.invoke(cli, ["-P", "small-fake-a"])
-
     with open("requirements.txt", "r") as req_txt:
         assert (
             "small-fake-a==0.1         # via small-fake-with-deps"
@@ -1029,14 +1042,14 @@ def test_sub_dependencies_with_constraints(pip_conf, runner):
         req_in.write("-c constraints.txt\n")
         req_in.write("small_fake_with_deps_and_sub_deps")  # require fake package
 
-    out = runner.invoke(cli)
+    out = runner.invoke(cli, ["--no-annotate"])
 
     assert out.exit_code == 0
 
     req_out_lines = set(out.stderr.splitlines())
     assert {
-        "small-fake-a==0.1         # via small-fake-with-unpinned-deps",
-        "small-fake-b==0.2         # via small-fake-with-unpinned-deps",
+        "small-fake-a==0.1",
+        "small-fake-b==0.2",
         "small-fake-with-deps-and-sub-deps==0.1",
-        "small-fake-with-unpinned-deps==0.1  # via small-fake-with-deps-and-sub-deps",
+        "small-fake-with-unpinned-deps==0.1",
     }.issubset(req_out_lines)