From 06fb7491faf69135f5e765c3ab09caf4daba01b7 Mon Sep 17 00:00:00 2001
From: Steven Armstrong <steven@icarus.ethz.ch>
Date: Fri, 18 Jan 2013 15:02:43 +0100
Subject: [PATCH] prevent requirements from leaking into autorequired objects
 and creating circular references

Signed-off-by: Steven Armstrong <steven@icarus.ethz.ch>

Conflicts:
	cdist/test/autorequire/__init__.py
---
 cdist/resolver.py                             | 21 ++++++++++++-------
 cdist/test/autorequire/__init__.py            |  6 ++++++
 .../fixtures/conf/manifest/recursive_type     |  2 ++
 .../fixtures/conf/type/__git/manifest         |  1 +
 4 files changed, 23 insertions(+), 7 deletions(-)
 create mode 100755 cdist/test/autorequire/fixtures/conf/manifest/recursive_type
 create mode 100644 cdist/test/autorequire/fixtures/conf/type/__git/manifest

diff --git a/cdist/resolver.py b/cdist/resolver.py
index c1b2c292..ddcf382e 100644
--- a/cdist/resolver.py
+++ b/cdist/resolver.py
@@ -23,6 +23,7 @@ import logging
 import os
 import itertools
 import fnmatch
+import pprint
 
 import cdist
 
@@ -77,14 +78,15 @@ class DependencyResolver(object):
         lists of all dependencies including the key object itself.
         """
         if self._dependencies is None:
-            log.info("Resolving dependencies...")
-            self._dependencies = d = {}
+            self.log.info("Resolving dependencies...")
+            self._dependencies = {}
             self._preprocess_requirements()
             for name,cdist_object in self.objects.items():
                 resolved = []
                 unresolved = []
                 self._resolve_object_dependencies(cdist_object, resolved, unresolved)
-                d[name] = resolved
+                self._dependencies[name] = resolved
+            self.log.debug(self._dependencies)
         return self._dependencies
 
     def find_requirements_by_name(self, requirements):
@@ -120,9 +122,12 @@ class DependencyResolver(object):
                 # that the parent has so that user defined requirements are 
                 # fullfilled and processed in the expected order.
                 for auto_requirement in self.find_requirements_by_name(cdist_object.autorequire):
-                    for requirement in cdist_object.requirements:
-                        if requirement not in auto_requirement.requirements:
-                            auto_requirement.requirements.append(requirement)
+                    for requirement in self.find_requirements_by_name(cdist_object.requirements):
+                        requirement_object_all_requirements = list(requirement.requirements) + list(requirement.autorequire)
+                        if (requirement.name not in auto_requirement.requirements
+                            and auto_requirement.name not in requirement_object_all_requirements):
+                            self.log.debug('Adding %s to %s.requirements', requirement.name, auto_requirement)
+                            auto_requirement.requirements.append(requirement.name)
                 # On the other hand the parent shall depend on all the children
                 # it created so that the user can setup dependencies on it as a 
                 # whole without having to know anything about the parents 
@@ -149,7 +154,9 @@ class DependencyResolver(object):
                 self.log.debug("Object %s requires %s", cdist_object, required_object)
                 if required_object not in resolved:
                     if required_object in unresolved:
-                        raise CircularReferenceError(cdist_object, required_object)
+                        error = CircularReferenceError(cdist_object, required_object)
+                        self.log.error('%s: %s', error, pprint.pformat(self._dependencies))
+                        raise error
                     self._resolve_object_dependencies(required_object, resolved, unresolved)
             resolved.append(cdist_object)
             unresolved.remove(cdist_object)
diff --git a/cdist/test/autorequire/__init__.py b/cdist/test/autorequire/__init__.py
index 330680df..714a7d4b 100644
--- a/cdist/test/autorequire/__init__.py
+++ b/cdist/test/autorequire/__init__.py
@@ -83,3 +83,9 @@ class AutorequireTestCase(test.CdistTestCase):
         self.config.stage_prepare()
         # raises CircularDependecyError
         self.config.stage_run()
+
+    def test_recursive_type(self):
+        self.context.initial_manifest = os.path.join(self.config.local.manifest_path, 'recursive_type')
+        self.config.stage_prepare()
+        # raises CircularDependecyError
+        self.config.stage_run()
diff --git a/cdist/test/autorequire/fixtures/conf/manifest/recursive_type b/cdist/test/autorequire/fixtures/conf/manifest/recursive_type
new file mode 100755
index 00000000..c32b7710
--- /dev/null
+++ b/cdist/test/autorequire/fixtures/conf/manifest/recursive_type
@@ -0,0 +1,2 @@
+__git foo
+require="__git/foo" __git bar
diff --git a/cdist/test/autorequire/fixtures/conf/type/__git/manifest b/cdist/test/autorequire/fixtures/conf/type/__git/manifest
new file mode 100644
index 00000000..616ecca9
--- /dev/null
+++ b/cdist/test/autorequire/fixtures/conf/type/__git/manifest
@@ -0,0 +1 @@
+__package git