Skip to content

Commit

Permalink
Print a message for cycles in the graph when computing the target fin…
Browse files Browse the repository at this point in the history
…gerprint

While hand editing a BUILD file I introduced a cycle in the graph. The error was:

```
21:35:47 00:03   [complete]
               FAILURE
Exception caught: (<class 'pants.invalidation.cache_manager.CacheValidationError'>)

Exception message: Problem validating target project.tests.src.test.proto.proto in project/tests/src/test/proto: maximum recursion depth exceeded
```

The stack trace showed:

```
  ...
  File "pantsbuild.pants-1.1.0_pre6_square_20160707_01-py2-none-any.whl/pan
ts/build_graph/target.py", line 464, in dep_hash_iter
    dep_hash = dep.transitive_invalidation_hash(fingerprint_strategy)
  File "pantsbuild.pants-1.1.0_pre6_square_20160707_01-py2-none-any.whl/pan
ts/build_graph/target.py", line 467, in transitive_invalidation_hash
    dep_hashes = sorted(list(dep_hash_iter()))
  ...
```

This patch fixes it by trakcing the depth of these recursive calls and abort with an AddressLookupError so that we can see a trace. Now the error looks like:

```
Exception caught: (<class 'pants.invalidation.cache_manager.CacheValidationError'>)

Exception message: Problem validating target service.container.tests.src.test.proto.proto in service/container/tests/src/test/proto: Max depth of 300 exceeded.
  referenced from 3rdparty:com.google.inject.guice
  referenced from otherproject/components/cronjobs/annotations/src/main/java:lib
  referenced from otherproject/components/cronjobs/annotations/src/main/java:cron-jobs-processor
  referenced from otherproject/components/cronjobs/annotations/src/main/java:lib
  referenced from otherproject/components/cronjobs/annotations/src/main/java:cron-jobs-processor
  referenced from otherproject/components/cronjobs/annotations/src/main/java:lib
  ...
```

Testing Done:
Added unit test.

CI running at https://travis-ci.org/pantsbuild/pants/builds/145436454

Bugs closed: 3681

Reviewed at https://rbcommons.com/s/twitter/r/4087/
  • Loading branch information
ericzundel committed Jul 21, 2016
1 parent 8720a3c commit a2fe1dc
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 4 deletions.
27 changes: 23 additions & 4 deletions src/python/pants/build_graph/target.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
from pants.base.payload_field import PrimitiveField
from pants.base.validation import assert_list
from pants.build_graph.address import Address, Addresses
from pants.build_graph.address_lookup_error import AddressLookupError
from pants.build_graph.target_addressable import TargetAddressable
from pants.build_graph.target_scopes import Scope
from pants.source.payload_fields import DeferredSourcesField, SourcesField
Expand Down Expand Up @@ -130,6 +131,14 @@ class Target(AbstractTarget):
:API: public
"""


class RecursiveDepthError(AddressLookupError):
"""Raised when there are too many recursive calls to calculate the fingerprint."""
pass


_MAX_RECURSION_DEPTH=300

class WrongNumberOfAddresses(Exception):
"""Internal error, too many elements in Addresses
Expand Down Expand Up @@ -444,7 +453,7 @@ def mark_invalidation_hash_dirty(self):
self.mark_extra_invalidation_hash_dirty()
self.payload.mark_dirty()

def transitive_invalidation_hash(self, fingerprint_strategy=None):
def transitive_invalidation_hash(self, fingerprint_strategy=None, depth=0):
"""
:API: public
Expand All @@ -455,15 +464,25 @@ def transitive_invalidation_hash(self, fingerprint_strategy=None):
did not contribute to the fingerprint, according to the provided FingerprintStrategy.
:rtype: string
"""
if depth > self._MAX_RECURSION_DEPTH:
# NB(zundel) without this catch, we'll eventually hit the python stack limit
# RuntimeError: maximum recursion depth exceeded while calling a Python object
raise self.RecursiveDepthError("Max depth of {} exceeded.".format(self._MAX_RECURSION_DEPTH))

fingerprint_strategy = fingerprint_strategy or DefaultFingerprintStrategy()
if fingerprint_strategy not in self._cached_transitive_fingerprint_map:
hasher = sha1()

def dep_hash_iter():
for dep in self.dependencies:
dep_hash = dep.transitive_invalidation_hash(fingerprint_strategy)
if dep_hash is not None:
yield dep_hash
try:
dep_hash = dep.transitive_invalidation_hash(fingerprint_strategy, depth=depth+1)
if dep_hash is not None:
yield dep_hash
except self.RecursiveDepthError as e:
raise self.RecursiveDepthError("{message}\n referenced from {spec}"
.format(message=e, spec=dep.address.spec))

dep_hashes = sorted(list(dep_hash_iter()))
for dep_hash in dep_hashes:
hasher.update(dep_hash)
Expand Down
8 changes: 8 additions & 0 deletions tests/python/pants_test/build_graph/test_target.py
Original file line number Diff line number Diff line change
Expand Up @@ -138,3 +138,11 @@ def test_sources_with_more_than_one_address_fails(self):
t.create_sources_field(sources=addresses, sources_rel_path='', key_arg='cool_field')
self.assertIn("Expected 'cool_field' to be a single address to from_target() as argument",
str(cm.exception))

def test_max_recursion(self):
target_a = self.make_target('a', Target)
target_b = self.make_target('b', Target, dependencies=[target_a])
self.make_target('c', Target, dependencies=[target_b])
target_a.inject_dependency(Address.parse('c'))
with self.assertRaises(Target.RecursiveDepthError):
target_a.transitive_invalidation_hash()

0 comments on commit a2fe1dc

Please sign in to comment.