From 05b46091e4fb8466544718cf428cc3deca03e439 Mon Sep 17 00:00:00 2001
From: Adrian Likins <alikins@redhat.com>
Date: Sat, 2 Apr 2016 20:22:00 -0400
Subject: [PATCH] Fix galaxy roles_path cli usage.

If we specify a roles_path from the cli, use a
optparse action callback to make sure the roles_path
is set to a path list.

Fixes #15255
---
 lib/ansible/cli/__init__.py    | 14 ++++++++++++++
 lib/ansible/cli/galaxy.py      |  6 +++++-
 lib/ansible/galaxy/__init__.py |  7 ++++---
 lib/ansible/galaxy/role.py     |  4 ++--
 4 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/lib/ansible/cli/__init__.py b/lib/ansible/cli/__init__.py
index e901940f039..c66c9c5b45d 100644
--- a/lib/ansible/cli/__init__.py
+++ b/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):
@@ -544,6 +556,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]
diff --git a/lib/ansible/cli/galaxy.py b/lib/ansible/cli/galaxy.py
index 17f06409bb5..822492cd1d3 100644
--- a/lib/ansible/cli/galaxy.py
+++ b/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)')
diff --git a/lib/ansible/galaxy/__init__.py b/lib/ansible/galaxy/__init__.py
index e526b0aa873..8fdd24e9109 100644
--- a/lib/ansible/galaxy/__init__.py
+++ b/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', [])
-        if isinstance(roles_paths, string_types):
-            self.roles_paths = [os.path.expanduser(roles_path) for roles_path in roles_paths.split(os.pathsep)]
+        # self.options.roles_path needs to be a list and will be by default
+        roles_path = getattr(self.options, 'roles_path', [])
+        # cli option handling is responsible for making roles_path a list
+        self.roles_paths = roles_path
 
         self.roles =  {}
 
diff --git a/lib/ansible/galaxy/role.py b/lib/ansible/galaxy/role.py
index 5472255480c..7657e12f4c0 100644
--- a/lib/ansible/galaxy/role.py
+++ b/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:
-                role_path = os.path.join(path, self.name)
+            for role_path_dir in galaxy.roles_paths:
+                role_path = os.path.join(role_path_dir, self.name)
                 if os.path.exists(role_path):
                     self.path = role_path
                     break