Merge pull request #15256 from alikins/galaxy_roles_path_fix_15255

Fix galaxy roles_path cli usage. (#15255)

lib/ansible/cli/__init__.py

@@ -227,6 +227,18 @@ class CLI(object):
     def expand_tilde(option, opt, value, parser):
         setattr(parser.values, option.dest, os.path.expanduser(value))
 
+    @staticmethod
+    def expand_paths(option, opt, value, parser):
+        """optparse action callback to convert a PATH style string arg to a list of path strings.
+
+        For ex, cli arg of '-p /blip/foo:/foo/bar' would be split on the
+        default os.pathsep and the option value would be set to
+        the list ['/blip/foo', '/foo/bar']. Each path string in the list
+        will also have '~/' values expand via os.path.expanduser()."""
+        path_entries = value.split(os.pathsep)
+        expanded_path_entries = [os.path.expanduser(path_entry) for path_entry in path_entries]
+        setattr(parser.values, option.dest, expanded_path_entries)
+
     @staticmethod
     def base_parser(usage="", output_opts=False, runas_opts=False, meta_opts=False, runtask_opts=False, vault_opts=False, module_opts=False,
             async_opts=False, connect_opts=False, subset_opts=False, check_opts=False, inventory_opts=False, epilog=None, fork_opts=False, runas_prompt_opts=False):
@@ -548,6 +560,8 @@ class CLI(object):
             data = getattr(self.options, k)
         except:
             return defval
+        # FIXME: Can this be removed if cli and/or constants ensures it's a
+        # list?
         if k == "roles_path":
             if os.pathsep in data:
                 data = data.split(os.pathsep)[0]

lib/ansible/cli/galaxy.py

@@ -120,7 +120,11 @@ class GalaxyCLI(CLI):
 
         # options that apply to more than one action
         if not self.action in ("delete","import","init","login","setup"):
-            self.parser.add_option('-p', '--roles-path', dest='roles_path', default=C.DEFAULT_ROLES_PATH,
+            # NOTE: while the option type=str, the default is a list, and the
+            # callback will set the value to a list.
+            self.parser.add_option('-p', '--roles-path', dest='roles_path',
+                                   action="callback", callback=CLI.expand_paths,
+                                   type=str, default=C.DEFAULT_ROLES_PATH,
                 help='The path to the directory containing your roles. '
                      'The default is the roles_path configured in your '
                      'ansible.cfg file (/etc/ansible/roles if not configured)')

lib/ansible/galaxy/__init__.py

@@ -39,9 +39,10 @@ class Galaxy(object):
     def __init__(self, options):
 
         self.options = options
-        roles_paths = getattr(self.options, 'roles_path', [])
+        # self.options.roles_path needs to be a list and will be by default
-        if isinstance(roles_paths, string_types):
+        roles_path = getattr(self.options, 'roles_path', [])
-            self.roles_paths = [os.path.expanduser(roles_path) for roles_path in roles_paths.split(os.pathsep)]
+        # cli option handling is responsible for making roles_path a list
+        self.roles_paths = roles_path
 
         self.roles =  {}
 

lib/ansible/galaxy/role.py

@@ -67,8 +67,8 @@ class GalaxyRole(object):
                 path = os.path.join(path, self.name)
             self.path = path
         else:
-            for path in galaxy.roles_paths:
+            for role_path_dir in galaxy.roles_paths:
-                role_path = os.path.join(path, self.name)
+                role_path = os.path.join(role_path_dir, self.name)
                 if os.path.exists(role_path):
                     self.path = role_path
                     break