From 3b8eaf6128b08c00b9097b76854e19f3bc980b62 Mon Sep 17 00:00:00 2001
From: James Cammarata <jimi@sngx.net>
Date: Mon, 5 Oct 2015 22:59:08 -0400
Subject: [PATCH] Cleaning up some ansible-galaxy stuff

---
 lib/ansible/cli/galaxy.py                | 35 ++++-----
 lib/ansible/galaxy/role.py               | 10 +--
 lib/ansible/playbook/role/requirement.py | 91 +++++-------------------
 3 files changed, 40 insertions(+), 96 deletions(-)

diff --git a/lib/ansible/cli/galaxy.py b/lib/ansible/cli/galaxy.py
index 3422815735c..1ce3bb3b810 100644
--- a/lib/ansible/cli/galaxy.py
+++ b/lib/ansible/cli/galaxy.py
@@ -135,7 +135,7 @@ class GalaxyCLI(CLI):
     def _display_role_info(self, role_info):
 
         text = "\nRole: %s \n" % role_info['name']
-        text += "\tdescription: %s \n" % role_info['description']
+        text += "\tdescription: %s \n" % role_info.get('description', '')
 
         for k in sorted(role_info.keys()):
 
@@ -249,7 +249,7 @@ class GalaxyCLI(CLI):
         data = ''
         for role in self.args:
 
-            role_info = {'role_path': roles_path}
+            role_info = {'path': roles_path}
             gr = GalaxyRole(self.galaxy, role)
 
             install_info = gr.install_info
@@ -270,7 +270,7 @@ class GalaxyCLI(CLI):
                 role_info.update(gr.metadata)
 
             req = RoleRequirement()
-            __, __, role_spec= req.parse({'role': role})
+            role_spec= req.role_yaml_parse({'role': role})
             if role_spec:
                 role_info.update(role_spec)
 
@@ -308,19 +308,17 @@ class GalaxyCLI(CLI):
                 f = open(role_file, 'r')
                 if role_file.endswith('.yaml') or role_file.endswith('.yml'):
                     for role in yaml.safe_load(f.read()):
+                        role = RoleRequirement.role_yaml_parse(role)
                         self.display.debug('found role %s in yaml file' % str(role))
-                        if 'name' not in role:
-                            if 'src' in role:
-                                role['name'] = RoleRequirement.repo_url_to_role_name(role['src'])
-                            else:
-                                raise AnsibleError("Must specify name or src for role")
+                        if 'name' not in role and 'scm' not in role:
+                            raise AnsibleError("Must specify name or src for role")
                         roles_left.append(GalaxyRole(self.galaxy, **role))
                 else:
                     self.display.deprecated("going forward only the yaml format will be supported")
                     # roles listed in a file, one per line
                     for rline in f.readlines():
                         self.display.debug('found role %s in text file' % str(rline))
-                        roles_left.append(GalaxyRole(self.galaxy, **RoleRequirement.role_spec_parse(rline)))
+                        roles_left.append(GalaxyRole(self.galaxy, **RoleRequirement.role_yaml_parse(rline)))
                 f.close()
             except (IOError, OSError) as e:
                 self.display.error('Unable to open %s: %s' % (role_file, str(e)))
@@ -334,7 +332,6 @@ class GalaxyCLI(CLI):
             self.display.debug('Installing role %s ' % role.name)
             # query the galaxy API for the role data
             role_data = None
-            role = roles_left.pop(0)
 
             if role.install_info is not None and not force:
                 self.display.display('- %s is already installed, skipping.' % role.name)
@@ -353,16 +350,20 @@ class GalaxyCLI(CLI):
                 for dep in role_dependencies:
                     self.display.debug('Installing dep %s' % dep)
                     dep_req = RoleRequirement()
-                    __, dep_name, __ = dep_req.parse(dep)
-                    dep_role = GalaxyRole(self.galaxy, name=dep_name)
+                    dep_info = dep_req.role_yaml_parse(dep)
+                    dep_role = GalaxyRole(self.galaxy, **dep_info)
+                    if '.' not in dep_role.name and '.' not in dep_role.src and dep_role.scm is None:
+                        # we know we can skip this, as it's not going to
+                        # be found on galaxy.ansible.com
+                        continue
                     if dep_role.install_info is None or force:
                         if dep_role not in roles_left:
-                            self.display.display('- adding dependency: %s' % dep_name)
-                            roles_left.append(GalaxyRole(self.galaxy, name=dep_name))
+                            self.display.display('- adding dependency: %s' % dep_role.name)
+                            roles_left.append(dep_role)
                         else:
-                            self.display.display('- dependency %s already pending installation.' % dep_name)
+                            self.display.display('- dependency %s already pending installation.' % dep_role.name)
                     else:
-                        self.display.display('- dependency %s is already installed, skipping.' % dep_name)
+                        self.display.display('- dependency %s is already installed, skipping.' % dep_role.name)
 
             if not installed:
                 self.display.warning("- %s was NOT installed successfully." % role.name)
@@ -429,7 +430,7 @@ class GalaxyCLI(CLI):
             for path_file in path_files:
                 gr = GalaxyRole(self.galaxy, path_file)
                 if gr.metadata:
-                    install_info = gr.metadata
+                    install_info = gr.install_info
                     version = None
                     if install_info:
                         version = install_info.get("version", None)
diff --git a/lib/ansible/galaxy/role.py b/lib/ansible/galaxy/role.py
index f4f3776c8a9..4e6cdc9d151 100644
--- a/lib/ansible/galaxy/role.py
+++ b/lib/ansible/galaxy/role.py
@@ -46,7 +46,7 @@ class GalaxyRole(object):
     ROLE_DIRS = ('defaults','files','handlers','meta','tasks','templates','vars')
 
 
-    def __init__(self, galaxy, name, src=None, version=None, scm=None, role_path=None):
+    def __init__(self, galaxy, name, src=None, version=None, scm=None, path=None):
 
         self._metadata = None
         self._install_info = None
@@ -59,8 +59,10 @@ class GalaxyRole(object):
         self.src = src or name
         self.scm = scm
 
-        if role_path is not None:
-            self.path = role_path
+        if path is not None:
+            if self.name not in path:
+                path = os.path.join(path, self.name)
+            self.path = path
         else:
             for path in galaxy.roles_paths:
                 role_path = os.path.join(path, self.name)
@@ -222,7 +224,7 @@ class GalaxyRole(object):
 
         if tmp_file:
 
-            display.display("installing from %s" % tmp_file)
+            display.debug("installing from %s" % tmp_file)
 
             if not tarfile.is_tarfile(tmp_file):
                 raise AnsibleError("the file downloaded was not a tar.gz")
diff --git a/lib/ansible/playbook/role/requirement.py b/lib/ansible/playbook/role/requirement.py
index a0c3f6ddac0..3facfdd2c4d 100644
--- a/lib/ansible/playbook/role/requirement.py
+++ b/lib/ansible/playbook/role/requirement.py
@@ -32,6 +32,14 @@ from ansible.playbook.role.definition import RoleDefinition
 __all__ = ['RoleRequirement']
 
 
+VALID_SPEC_KEYS = [
+    'name',
+    'role',
+    'scm',
+    'src',
+    'version',
+]
+
 class RoleRequirement(RoleDefinition):
 
     """
@@ -41,72 +49,6 @@ class RoleRequirement(RoleDefinition):
     def __init__(self):
         pass
 
-    def _get_valid_spec_keys(self):
-        return (
-            'name',
-            'role',
-            'scm',
-            'src',
-            'version',
-        )
-
-    def parse(self, ds):
-        '''
-        FIXME: docstring
-        '''
-
-        assert type(ds) == dict or isinstance(ds, string_types)
-
-        role_name    = None
-        role_params  = dict()
-        new_ds       = dict()
-
-        if isinstance(ds, string_types):
-            role_name = ds
-        else:
-            (new_ds, role_params) = self._split_role_params(self._preprocess_role_spec(ds))
-
-            # pull the role name out of the ds
-            role_name = new_ds.pop('role_name', new_ds.pop('role', None))
-
-        if role_name is None:
-            raise AnsibleError("Role requirement did not contain a role name!", obj=ds)
-        return (new_ds, role_name, role_params)
-
-    def _preprocess_role_spec(self, ds):
-        if 'role' in ds:
-            # Old style: {role: "galaxy.role,version,name", other_vars: "here" }
-            role_info = RoleRequirement.role_spec_parse(ds['role'])
-            if isinstance(role_info, dict):
-                # Warning: Slight change in behaviour here.  name may be being
-                # overloaded.  Previously, name was only a parameter to the role.
-                # Now it is both a parameter to the role and the name that
-                # ansible-galaxy will install under on the local system.
-                if 'name' in ds and 'name' in role_info:
-                    del role_info['name']
-                ds.update(role_info)
-        else:
-            # New style: { src: 'galaxy.role,version,name', other_vars: "here" }
-            if 'github.com' in ds["src"] and 'http' in ds["src"] and '+' not in ds["src"] and not ds["src"].endswith('.tar.gz'):
-                ds["src"] = "git+" + ds["src"]
-
-            if '+' in ds["src"]:
-                (scm, src) = ds["src"].split('+')
-                ds["scm"] = scm
-                ds["src"] = src
-
-            if 'name' in ds:
-                ds["role"] = ds["name"]
-                del ds["name"]
-            else:
-                ds["role"] = RoleRequirement.repo_url_to_role_name(ds["src"])
-
-            # set some values to a default value, if none were specified
-            ds.setdefault('version', '')
-            ds.setdefault('scm', None)
-
-        return ds
-
     @staticmethod
     def repo_url_to_role_name(repo_url):
         # gets the role name out of a repo like
@@ -173,16 +115,11 @@ class RoleRequirement(RoleDefinition):
 
         if 'role' in role:
             # Old style: {role: "galaxy.role,version,name", other_vars: "here" }
-            role_info = RoleRequirement.role_spec_parse(role['role'])
-            if isinstance(role_info, dict):
-                # Warning: Slight change in behaviour here.  name may be being
-                # overloaded.  Previously, name was only a parameter to the role.
-                # Now it is both a parameter to the role and the name that
-                # ansible-galaxy will install under on the local system.
-                if 'name' in role and 'name' in role_info:
-                    del role_info['name']
-                role.update(role_info)
+            role = RoleRequirement.role_spec_parse(role['role'])
+            #if 'name' in role:
+            #    del role['name']
         else:
+            role = role.copy()
             # New style: { src: 'galaxy.role,version,name', other_vars: "here" }
             if 'github.com' in role["src"] and 'http' in role["src"] and '+' not in role["src"] and not role["src"].endswith('.tar.gz'):
                 role["src"] = "git+" + role["src"]
@@ -201,6 +138,10 @@ class RoleRequirement(RoleDefinition):
             if 'scm' not in role:
                 role['scm'] = None
 
+        for key in role.keys():
+            if key not in VALID_SPEC_KEYS:
+                role.pop(key)
+
         return role
 
     @staticmethod