Adding role deps to v2 Role class and fixing some bugs

v2/ansible/parsing/yaml/__init__.py

@@ -80,7 +80,7 @@ class DataLoader():
         # if the file has already been read in and cached, we'll
         # return those results to avoid more file/vault operations
         if file_name in self._FILE_CACHE:
-            return self._FILE_CACHE
+            return self._FILE_CACHE[file_name]
 
         # read the file contents and load the data structure from them
         (file_data, show_content) = self._get_file_contents(file_name)

v2/ansible/parsing/yaml/objects.py

@@ -37,7 +37,7 @@ class AnsibleBaseYAMLObject:
         self._line_number   = line
         self._column_number = col
 
-    def copy_position_info(obj):
+    def copy_position_info(self, obj):
         ''' copies the position info from another object '''
         assert isinstance(obj, AnsibleBaseYAMLObject)
 

v2/ansible/parsing/yaml/strings.py

@@ -15,10 +15,6 @@
 # You should have received a copy of the GNU General Public License
 # along with Ansible.  If not, see <http://www.gnu.org/licenses/>.
 
-# Make coding more python3-ish
-from __future__ import (absolute_import, division, print_function)
-__metaclass__ = type
-
 __all__ = [
     'YAML_SYNTAX_ERROR',
     'YAML_POSITION_DETAILS',

v2/ansible/playbook/role.py

@@ -93,11 +93,14 @@ class Role(Base):
         if cache_key in _ROLE_CACHE:
             r = _ROLE_CACHE[cache_key]
         else:
-            # load the role
-            r = Role()
-            r.load_data(data)
-            # and cache it for next time
-            _ROLE_CACHE[cache_key] = r
+            try:
+                # load the role
+                r = Role()
+                r.load_data(data)
+                # and cache it for next time
+                _ROLE_CACHE[cache_key] = r
+            except RuntimeError:
+                raise AnsibleError("A recursive loop was detected while loading your roles", obj=data)
 
         # now add the parent to the (new) role
         if parent_role:
@@ -192,17 +195,13 @@ class Role(Base):
 
         # FIXME: this should use unfrackpath once the utils code has been sorted out
         role_path = os.path.normpath(role)
-        print("first role path is %s" % role_path)
         if os.path.exists(role_path):
             role_name = os.path.basename(role)
-            print('returning role path %s' % role_path)
             return (role_name, role_path)
         else:
             for path in ('./roles', '/etc/ansible/roles'):
                 role_path = os.path.join(path, role)
-		print("current role path is %s" % role_path)
                 if os.path.exists(role_path):
-                    print('returning role path %s' % role_path)
                     return (role, role_path)
 
         # FIXME: make the parser smart about list/string entries
@@ -350,12 +349,22 @@ class Role(Base):
             return []
         return self._load_list_of_blocks(ds)
 
+    def _load_dependencies(self, attr, ds):
+        assert type(ds) in (list, type(None))
+
+        deps = []
+        if ds:
+            for role_def in ds:
+                r = Role.load(role_def, parent_role=self)
+                deps.append(r)
+        return deps
+
     #------------------------------------------------------------------------------
     # other functions
 
     def add_parent(self, parent_role):
         ''' adds a role to the list of this roles parents '''
-        assert isinstance(role, Role)
+        assert isinstance(parent_role, Role)
 
         if parent_role not in self._parents:
             self._parents.append(parent_role)

v2/test/playbook/test_role.py

@@ -119,10 +119,23 @@ class TestRole(unittest.TestCase):
     @patch.object(Role, '_load_role_yaml')
     def test_load_role_with_metadata(self, _load_role_yaml, _get_role_path):
 
+        def fake_get_role_path(role):
+            if role == 'foo':
+                return ('foo', '/etc/ansible/roles/foo')
+            elif role == 'bar':
+                return ('bar', '/etc/ansible/roles/bar')
+            elif role == 'bad1':
+                return ('bad1', '/etc/ansible/roles/bad1')
+            elif role == 'bad2':
+                return ('bad2', '/etc/ansible/roles/bad2')
+
         def fake_load_role_yaml(role_path, subdir):
             if role_path == '/etc/ansible/roles/foo':
                 if subdir == 'meta':
                     return dict(dependencies=['bar'], allow_duplicates=True, galaxy_info=dict(a='1', b='2', c='3'))
+            elif role_path == '/etc/ansible/roles/bar':
+                if subdir == 'meta':
+                    return dict()
             elif role_path == '/etc/ansible/roles/bad1':
                 if subdir == 'meta':
                     return 1
@@ -131,19 +144,18 @@ class TestRole(unittest.TestCase):
                     return dict(foo='bar')
             return None
 
+        _get_role_path.side_effect  = fake_get_role_path
         _load_role_yaml.side_effect = fake_load_role_yaml
 
-        _get_role_path.return_value = ('foo', '/etc/ansible/roles/foo')
-
         r = Role.load('foo')
-        self.assertEqual(r.dependencies, ['bar'])
+        self.assertEqual(len(r.dependencies), 1)
+        self.assertEqual(type(r.dependencies[0]), Role)
+        self.assertEqual(len(r.dependencies[0]._parents), 1)
+        self.assertEqual(r.dependencies[0]._parents[0], r)
         self.assertEqual(r.allow_duplicates, True)
         self.assertEqual(r.galaxy_info, dict(a='1', b='2', c='3'))
 
-        _get_role_path.return_value = ('bad1', '/etc/ansible/roles/bad1')
         self.assertRaises(AnsibleParserError, Role.load, 'bad1')
-
-        _get_role_path.return_value = ('bad2', '/etc/ansible/roles/bad2')
         self.assertRaises(AnsibleParserError, Role.load, 'bad2')
 
     @patch.object(Role, '_get_role_path')