GTTF: Print much better error message for dependency cycles.

pylib/gyp/input.py

Index: pylib/gyp/input.py
===================================================================
--- pylib/gyp/input.py	(revision 1694)
+++ pylib/gyp/input.py	(working copy)
@@ -1443,6 +1443,9 @@
     self.dependencies = []
     self.dependents = []
 
+  def __repr__(self):
+    return '<DependencyGraphNode: %r>' % self.ref
+
   def FlattenToList(self):
     # flat_list is the sorted list of dependencies - actually, the list items
     # are the "ref" attributes of DependencyGraphNodes.  Every target will
@@ -1485,6 +1488,27 @@
 
     return flat_list
 
+  def FindCycles(self, path=None):
+    """
+    Returns a list of cycles in the graph, where each cycle is its own list.
+    """
+    if path is None:
+      path = [self]
+
+    results = []
+    for node in self.dependents:
+      if node in path:
+        cycle = [node]
+        for part in path:
+          cycle.append(part)
+          if part == node:
+            break
+        results.append(tuple(cycle))
+      else:
+        results.extend(node.FindCycles([node] + path))
+
+    return sorted(list(set(results)))
+
   def DirectDependencies(self, dependencies=None):
     """Returns a list of just direct dependencies."""
     if dependencies == None:
@@ -1717,10 +1741,16 @@
     for file in dependency_nodes.iterkeys():
       if not file in flat_list:
         bad_files.append(file)
+    common_path_prefix = os.path.commonprefix(dependency_nodes)
+    cycles = []
+    for num, cycle in enumerate(root_node.FindCycles(), 1):
+      simplified_paths = []
+      for node in cycle:
+        assert(node.ref.startswith(common_path_prefix))
+        simplified_paths.append(node.ref[len(common_path_prefix):])
+      cycles.append('Cycle #%d: %s' % (num, ' -> '.join(simplified_paths)))

[Mark Mentovai, 2013/08/16 19:32:46]

I don’t think you really need to print |num|.

[Paweł Hajdan Jr., 2013/08/16 22:39:42]

I think it can be useful for long cycles, to make it really clear where one
cycle begins and ends.

I can remove it if it's a really strong opinion.

[Mark Mentovai, 2013/08/17 03:59:02]

Paweł Hajdan Jr. wrote:
I think it kind of implies that there’s some ordering (which is why I asked you
to adjust the test to not expect an ordering).

If you wanted to indent the beginning of each cycle to make them stand out,
that’d be OK, but I think it’s probably already fine if it just says Cycle: at
the beginning of each cycle line.

[Paweł Hajdan Jr., 2013/08/19 18:59:49]

Done.

     raise DependencyGraphNode.CircularException, \
-        'Some files not reachable, cycle in .gyp file dependency graph ' + \
-        'detected involving some or all of: ' + \
-        ' '.join(bad_files)
+        'Cycles in .gyp file dependency graph detected:\n' + '\n'.join(cycles)
 
 
 def DoDependentSettings(key, flat_list, targets, dependency_nodes):