Repository: wimglenn/johnnydep

Issue:
Doesn't work with complex projects
```
python3 -m johnnydep 'gftools[qa]'
```

Might be related to #23

Discussion:

Patch:
diff --git a/johnnydep/__init__.py b/johnnydep/__init__.py
index 36988b9..2e2443c 100644
--- a/johnnydep/__init__.py
+++ b/johnnydep/__init__.py
@@ -1,5 +1,5 @@
 """Display dependency tree of Python distribution"""
 
-__version__ = "1.15"
+__version__ = "1.16"
 
 from johnnydep.lib import *
diff --git a/johnnydep/lib.py b/johnnydep/lib.py
index ead0c56..7693619 100644
--- a/johnnydep/lib.py
+++ b/johnnydep/lib.py
@@ -26,6 +26,7 @@ from wimpy import cached_property
 from johnnydep import pipper
 from johnnydep.compat import dict
 from johnnydep.compat import oyaml
+from johnnydep.util import CircularMarker
 
 __all__ = ["JohnnyDist", "gen_table", "flatten_deps", "has_error"]
 
@@ -122,15 +123,22 @@ class JohnnyDist(anytree.NodeMixin):
         """my immediate deps, as a tuple of johnnydists"""
         if not self._recursed:
             self.log.debug("populating dep tree")
-            for dep in self.requires:
-                JohnnyDist(
-                    req_string=dep,
-                    parent=self,
-                    index_url=self.index_url,
-                    env=self.env,
-                    extra_index_url=self.extra_index_url,
-                    ignore_errors=self.ignore_errors,
-                )
+            circular_deps = _detect_circular(self)
+            if circular_deps:
+                chain = " -> ".join([d._name_with_extras() for d in circular_deps])
+                summary = "... <circular dependency marker for {}>".format(chain)
+                self.log.info("pruning circular dependency", chain=chain)
+                _dep = CircularMarker(summary=summary, parent=self)
+            else:
+                for dep in self.requires:
+                    JohnnyDist(
+                        req_string=dep,
+                        parent=self,
+                        index_url=self.index_url,
+                        env=self.env,
+                        extra_index_url=self.extra_index_url,
+                        ignore_errors=self.ignore_errors,
+                    )
             self._recursed = True
         return super(JohnnyDist, self).children
 
@@ -276,14 +284,18 @@ class JohnnyDist(anytree.NodeMixin):
 
     serialize = serialise
 
+    def _name_with_extras(self):
+        result = self.name
+        if self.extras_requested:
+            result += "[{}]".format(",".join(self.extras_requested))
+        return result
+
     def _repr_pretty_(self, p, cycle):
         # hook for IPython's pretty-printer
         if cycle:
             p.text(repr(self))
         else:
-            fullname = self.name + self.specifier
-            if self.extras_requested:
-                fullname += "[{}]".format(",".join(self.extras_requested))
+            fullname = self._name_with_extras() + self.specifier
             p.text("<{} {} at {}>".format(type(self).__name__, fullname, hex(id(self))))
 
 
@@ -291,30 +303,42 @@ def gen_table(johnnydist, extra_cols=()):
     extra_cols = dict.fromkeys(extra_cols)  # de-dupe and preserve ordering
     extra_cols.pop("name", None)  # this is always included anyway, no need to ask for it
     johnnydist.log.debug("generating table")
-    for pre, _fill, node in anytree.RenderTree(johnnydist):
+    for prefix, _fill, dist in anytree.RenderTree(johnnydist):
         row = dict()
-        name = str(node.req)
-        if node.error:
-            name += " (FAILED)"
+        txt = str(dist.req)
+        if dist.error:
+            txt += " (FAILED)"
         if "specifier" in extra_cols:
-            name = wimpy.strip_suffix(name, str(node.specifier))
-        row["name"] = pre + name
+            txt = wimpy.strip_suffix(txt, str(dist.specifier))
+        row["name"] = prefix + txt
         for col in extra_cols:
-            val = getattr(node, col, "")
+            val = getattr(dist, col, "")
             if isinstance(val, list):
                 val = ", ".join(val)
             row[col] = val
         yield row
 
 
+def _detect_circular(dist):
+    # detects a circular dependency when traversing from here to the root node, and returns
+    # a chain of nodes in that case
+    chain = [dist]
+    for ancestor in reversed(dist.ancestors):
+        chain.append(ancestor)
+        if ancestor.name == dist.name:
+            if ancestor.extras_requested == dist.extras_requested:
+                return chain[::-1]
+
+
 def flatten_deps(johnnydist):
-    # TODO: add the check for infinite recursion in here (traverse parents)
     johnnydist.log.debug("resolving dep tree")
     dist_map = OrderedDefaultListDict()
     spec_map = defaultdict(str)
     extra_map = defaultdict(set)
     required_by_map = defaultdict(list)
     for dep in anytree.iterators.LevelOrderIter(johnnydist):
+        if dep.name == CircularMarker.glyph:
+            continue
         dist_map[dep.name].append(dep)
         spec_map[dep.name] = dep.req.specifier & spec_map[dep.name]
         extra_map[dep.name] |= set(dep.extras_requested)
@@ -433,4 +457,3 @@ def _get_info(dist_name, index_url=None, env=None, extra_index_url=None):
 
 # TODO: multi-line progress bar?
 # TODO: upload test dists to test PyPI index, document pip existing failure modes
-# TODO: don't infinitely recurse on circular dep tree
diff --git a/johnnydep/util.py b/johnnydep/util.py
index f0478b8..291adc5 100644
--- a/johnnydep/util.py
+++ b/johnnydep/util.py
@@ -1,8 +1,13 @@
+# coding: utf-8
+from __future__ import unicode_literals
+
 import json
 from argparse import ArgumentTypeError
 from subprocess import check_output
 from subprocess import CalledProcessError
 
+import anytree
+
 from johnnydep import env_check
 from johnnydep.compat import JSONDecodeError
 
@@ -18,3 +23,29 @@ def python_interpreter(path):
         raise ArgumentTypeError("Invalid python env output")
     frozen = tuple(map(tuple, env))
     return frozen
+
+
+class CircularMarker(anytree.NodeMixin):
+    """
+    This is like a "fake" JohnnyDist instance which is used
+    to render a node in circular dep trees like:
+
+    a
+    └── b
+        └── c
+            └── ...
+
+    Everything is null except the req/name which is "..." and
+    the metadata summary, which can be provided by the caller
+    """
+    glyph = "..."
+
+    def __init__(self, summary, parent):
+        self.req = CircularMarker.glyph
+        self.name = CircularMarker.glyph
+        self.summary = summary
+        self.parent = parent
+
+    def __getattr__(self, name):
+        if name.startswith("_"):
+            return super(CircularMarker, self).__getattr__(self, name)
diff --git a/setup.py b/setup.py
index f0ac65b..47f42fc 100644
--- a/setup.py
+++ b/setup.py
@@ -2,7 +2,7 @@ from setuptools import setup
 
 setup(
     name="johnnydep",
-    version="1.15",
+    version="1.16",
     description="Display dependency tree of Python distribution",
     long_description=open("README.md").read(),
     long_description_content_type="text/markdown",


Test patch:
diff --git a/tests/test_cli.py b/tests/test_cli.py
index 485ee08..4320b29 100644
--- a/tests/test_cli.py
+++ b/tests/test_cli.py
@@ -211,3 +211,45 @@ def test_no_deps(mocker, capsys, make_dist):
         ------
         distA
     """)
+
+
+def test_circular_deptree(mocker, capsys, make_dist):
+    make_dist(name="pkg0", install_requires=["pkg1"], version="0.1")
+    make_dist(name="pkg1", install_requires=["pkg2", "quux"], version="0.2")
+    make_dist(name="pkg2", install_requires=["pkg3"], version="0.3")
+    make_dist(name="pkg3", install_requires=["pkg1"], version="0.4")
+    make_dist(name="quux")
+    mocker.patch("sys.argv", "johnnydep pkg0".split())
+    main()
+    out, err = capsys.readouterr()
+    assert out == dedent(
+        """\
+        name                     summary
+        -----------------------  -----------------------------------------------------------------
+        pkg0                     default text for metadata summary
+        └── pkg1                 default text for metadata summary
+            ├── pkg2             default text for metadata summary
+            │   └── pkg3         default text for metadata summary
+            │       └── pkg1     default text for metadata summary
+            │           └── ...  ... <circular dependency marker for pkg1 -> pkg2 -> pkg3 -> pkg1>
+            └── quux             default text for metadata summary
+    """)
+
+
+def test_circular_deptree_resolve(mocker, capsys, make_dist):
+    make_dist(name="pkg0", install_requires=["pkg1"], version="0.1")
+    make_dist(name="pkg1", install_requires=["pkg2", "quux"], version="0.2")
+    make_dist(name="pkg2", install_requires=["pkg3"], version="0.3")
+    make_dist(name="pkg3", install_requires=["pkg1"], version="0.4")
+    make_dist(name="quux")
+    mocker.patch("sys.argv", "johnnydep pkg0 -o pinned".split())
+    main()
+    out, err = capsys.readouterr()
+    assert out == dedent(
+        """\
+        pkg0==0.1
+        pkg1==0.2
+        pkg2==0.3
+        quux==0.1.2
+        pkg3==0.4
+    """)
diff --git a/tests/test_lib.py b/tests/test_lib.py
index b641a2d..9c78a2f 100644
--- a/tests/test_lib.py
+++ b/tests/test_lib.py
@@ -416,7 +416,7 @@ def test_pprint(make_dist, mocker):
     mock_printer.text.reset_mock()
     jdist = JohnnyDist("jdtest[a]~=0.1.2")
     jdist._repr_pretty_(mock_printer, cycle=False)
-    pretty = "<JohnnyDist jdtest~=0.1.2[a] at 0xcafe>"
+    pretty = "<JohnnyDist jdtest[a]~=0.1.2 at 0xcafe>"
     mock_printer.text.assert_called_once_with(pretty)
     mock_printer.text.reset_mock()
     jdist._repr_pretty_(mock_printer, cycle=True)