Friendly Role Names and roles from URLs

* Roles can now be given a friendly name as third field in role spec csv
* Roles can be installed from URL (not just from archived SCMs)
* Integration tests to demonstrate this
* Unit tests to ensure that role spec parsing works as expected

bin/ansible-galaxy

@@ -332,13 +332,12 @@ def api_get_list(api_server, what):
 # scm repo utility functions
 #-------------------------------------------------------------------------------------
 
-def scm_archive_role(scm, role_url, role_version):
+def scm_archive_role(scm, role_url, role_version, role_name):
     if scm not in ['hg', 'git']:
         print "SCM %s is not currently supported" % scm
         return False
     tempdir = tempfile.mkdtemp()
-    role_name = ansible.utils.repo_url_to_role_name(role_url)
-    clone_cmd = [scm, 'clone', role_url]
+    clone_cmd = [scm, 'clone', role_url, role_name]
     with open('/dev/null', 'w') as devnull:
         popen = subprocess.Popen(clone_cmd, cwd=tempdir, stdout=devnull, stderr=devnull)
         rc = popen.wait()
@@ -464,7 +463,10 @@ def fetch_role(role_name, target, role_data, options):
     """
 
     # first grab the file and save it to a temp location
-    archive_url = 'https://github.com/%s/%s/archive/%s.tar.gz' % (role_data["github_user"], role_data["github_repo"], target)
+    if '://' in role_name:
+        archive_url = role_name
+    else: 
+        archive_url = 'https://github.com/%s/%s/archive/%s.tar.gz' % (role_data["github_user"], role_data["github_repo"], target)
     print " - downloading role from %s" % archive_url
 
     try:
@@ -706,39 +708,24 @@ def execute_install(args, options, parser):
 
     while len(roles_left) > 0:
         # query the galaxy API for the role data
-        role_name = roles_left.pop(0).strip()
-        role_version = None
+        (scm, role_src, role_version, role_name) = ansible.utils.role_spec_parse(roles_left.pop(0))
+        role_data = None
 
-        if role_name == "" or role_name.startswith("#"):
-            continue
-        elif ',' in role_name:
-            role_name,role_version = role_name.split(',',1)
-            role_name = role_name.strip()
-            role_version = role_version.strip()
-
-        if os.path.isfile(role_name):
+        if os.path.isfile(role_src):
             # installing a local tar.gz
-            tar_file = role_name
-            role_name = os.path.basename(role_name).replace('.tar.gz','')
-            if tarfile.is_tarfile(tar_file):
-                print " - installing %s as %s" % (tar_file, role_name)
-                if not install_role(role_name, role_version, tar_file, options):
-                    exit_without_ignore(options)
-            else:
-                print "%s (%s) was NOT installed successfully." % (role_name,tar_file)
-                exit_without_ignore(options)
+            tmp_file = role_src
         else:
-            if '+' in role_name:
-                (scm, role_url) = role_name.split('+',1)
-                role_name = ansible.utils.repo_url_to_role_name(role_name)
+            if scm:
                 # create tar file from scm url
-                tmp_file = scm_archive_role(scm, role_url, role_version)
-                role_data = None
+                tmp_file = scm_archive_role(scm, role_src, role_version, role_name)
+            elif '://' in role_src:
+                # just download a URL - version will probably be in the URL
+                tmp_file = fetch_role(role_src, None, None, options)
             else:
-                # installing remotely
-                role_data = api_lookup_role_by_name(api_server, role_name)
+                # installing from galaxy
+                role_data = api_lookup_role_by_name(api_server, role_src)
                 if not role_data:
-                    print "Sorry, %s was not found on %s." % (role_name, api_server)
+                    print "Sorry, %s was not found on %s." % (role_src, api_server)
                     continue
 
                 role_versions = api_fetch_role_related(api_server, 'versions', role_data['id'])
@@ -762,29 +749,29 @@ def execute_install(args, options, parser):
 
                 # download the role. if --no-deps was specified, we stop here, 
                 # otherwise we recursively grab roles and all of their deps.
-                tmp_file = fetch_role(role_name, role_version, role_data, options)
-            if tmp_file and install_role(role_name, role_version, tmp_file, options):
-                # we're done with the temp file, clean it up
-                os.unlink(tmp_file)
-                # install dependencies, if we want them
-                if not no_deps:
-                    if not role_data:
-                        role_data = get_role_metadata(role_name, options)
-                        role_dependencies = role_data['dependencies']
+                tmp_file = fetch_role(role_src, role_version, role_data, options)
+        if tmp_file and install_role(role_name, role_version, tmp_file, options):
+            # we're done with the temp file, clean it up
+            os.unlink(tmp_file)
+            # install dependencies, if we want them
+            if not no_deps:
+                if not role_data:
+                    role_data = get_role_metadata(role_name, options)
+                    role_dependencies = role_data['dependencies']
+                else:
+                    role_dependencies = role_data['summary_fields']['dependencies'] # api_fetch_role_related(api_server, 'dependencies', role_data['id'])
+                for dep_name in role_dependencies:
+                    #dep_name = "%s.%s" % (dep['owner'], dep['name'])
+                    if not get_role_metadata(dep_name.split('/')[-1], options):
+                        print ' adding dependency: %s' % dep_name
+                        roles_left.append(dep_name)
                     else:
-                        role_dependencies = role_data['summary_fields']['dependencies'] # api_fetch_role_related(api_server, 'dependencies', role_data['id'])
-                    for dep_name in role_dependencies:
-                        #dep_name = "%s.%s" % (dep['owner'], dep['name'])
-                        if not get_role_metadata(dep_name.split('/')[-1], options):
-                            print ' adding dependency: %s' % dep_name
-                            roles_left.append(dep_name)
-                        else:
-                            print ' dependency %s is already installed, skipping.' % dep_name
-            else:
-                if tmp_file:
-                    os.unlink(tmp_file)
-                print "%s was NOT installed successfully." % role_name
-                exit_without_ignore(options)
+                        print ' dependency %s is already installed, skipping.' % dep_name
+        else:
+            if tmp_file:
+                os.unlink(tmp_file)
+            print "%s was NOT installed successfully." % role_name
+            exit_without_ignore(options)
     sys.exit(0)
 
 def execute_remove(args, options, parser):

lib/ansible/playbook/play.py

@@ -178,13 +178,6 @@ class Play(object):
         returns any variables that were included with the role
         """
         orig_path = template(self.basedir,role,self.vars)
-        if '+' in orig_path and '://' in orig_path:
-            # dependency name pointing to SCM URL
-            # assume role name is last part of the URL
-            orig_path = utils.repo_url_to_role_name(orig_path)
-        if ',' in orig_path:
-            # version information for role dependency used by galaxy
-            orig_path = orig_path.split(',')[0]
 
         role_vars = {}
         if type(orig_path) == dict:
@@ -193,20 +186,21 @@ class Play(object):
             if role_name is None:
                 raise errors.AnsibleError("expected a role name in dictionary: %s" % orig_path)
             role_vars = orig_path
-            orig_path = role_name
+        else:
+            (scm, role_src, role_version, role_name) = utils.role_spec_parse(orig_path)
 
         role_path = None
 
         possible_paths = [
-            utils.path_dwim(self.basedir, os.path.join('roles', orig_path)),
-            utils.path_dwim(self.basedir, orig_path)
+            utils.path_dwim(self.basedir, os.path.join('roles', role_name)),
+            utils.path_dwim(self.basedir, role_name)
         ]
 
         if C.DEFAULT_ROLES_PATH:
             search_locations = C.DEFAULT_ROLES_PATH.split(os.pathsep)
             for loc in search_locations:
                 loc = os.path.expanduser(loc)
-                possible_paths.append(utils.path_dwim(loc, orig_path))
+                possible_paths.append(utils.path_dwim(loc, role_name))
 
         for path_option in possible_paths:
             if os.path.isdir(path_option):
@@ -419,8 +413,7 @@ class Play(object):
                 role_name = role['role']
             else:
                 role_name = role
-            if '+' in role_name and '://' in role_name:
-                role_name = utils.repo_url_to_role_name(role_name)
+            (scm, role_src, role_version, role_name) = utils.role_spec_parse(role_name)
 
             role_names.append(role_name)
             if os.path.isfile(task):

lib/ansible/utils/__init__.py

@@ -356,10 +356,33 @@ def repo_url_to_role_name(repo_url):
     trailing_path = repo_url.split('/')[-1]
     if trailing_path.endswith('.git'):
         trailing_path = trailing_path[:-4]
+    if trailing_path.endswith('.tar.gz'):
+        trailing_path = trailing_path[:-7]
     if ',' in trailing_path:
         trailing_path = trailing_path.split(',')[0]
     return trailing_path
 
+
+def role_spec_parse(role_spec):
+    role_spec = role_spec.strip()
+    role_version = ''
+    if role_spec == "" or role_spec.startswith("#"):
+        return (None, None, None, None)
+    tokens = map(lambda s: s.strip(), role_spec.split(','))
+    if '+' in tokens[0]:
+        (scm, role_url) = tokens[0].split('+')
+    else:
+        scm = None
+        role_url = tokens[0]
+    if len(tokens) >= 2:
+        role_version = tokens[1]
+    if len(tokens) == 3:
+        role_name = tokens[2]
+    else:
+        role_name = repo_url_to_role_name(tokens[0])
+    return (scm, role_url, role_version, role_name)
+
+
 def json_loads(data):
     ''' parse a JSON string and return a data structure '''
 

test/integration/galaxy_playbook.yml

@@ -3,3 +3,4 @@
 
   roles:
   - "git-ansible-galaxy"
+  - "http-role"

test/integration/galaxy_rolesfile

@@ -1,2 +1,3 @@
-git+http://bitbucket.org/willthames/git-ansible-galaxy,v1.3
+git+http://bitbucket.org/willthames/git-ansible-galaxy,v1.4
 hg+ssh://hg@bitbucket.org/willthames/hg-ansible-galaxy
+https://bitbucket.org/willthames/http-ansible-galaxy/get/master.tar.gz,,http-role

test/units/TestUtils.py

@@ -784,3 +784,12 @@ class TestUtils(unittest.TestCase):
                  ("ssh://git@git.example.com:repos/role-name,v0.1", "role-name")]
         for (url, result) in tests:
             self.assertEqual(ansible.utils.repo_url_to_role_name(url), result)
+
+    def test_role_spec_parse(self):
+        tests = [("git+http://git.example.com/repos/repo.git,v1.0", ('git', 'http://git.example.com/repos/repo.git', 'v1.0', 'repo')),
+                 ("http://repo.example.com/download/tarfile.tar.gz", (None, 'http://repo.example.com/download/tarfile.tar.gz', '', 'tarfile')),
+                 ("http://repo.example.com/download/tarfile.tar.gz,,nicename", (None, 'http://repo.example.com/download/tarfile.tar.gz', '', 'nicename')),
+                 ("git+http://git.example.com/repos/repo.git,v1.0,awesome", ('git', 'http://git.example.com/repos/repo.git', 'v1.0', 'awesome'))]
+        for (spec, result) in tests:
+            self.assertEqual(ansible.utils.role_spec_parse(spec), result)
+