Multi-platform path joiner(appender)

This commit kind of reverts the previous "no more os.sep.join ..." commit by solving the issue which led to switching that code to `os.path.join`. Tested both on linux and windows.

salt/config.py

@@ -139,7 +139,7 @@ def prepend_root_dir(opts, path_options):
     root_dir = os.path.abspath(opts['root_dir'])
     for path_option in path_options:
         if path_option in opts:
-            opts[path_option] = os.path.join(root_dir, opts[path_option])
+            opts[path_option] = salt.utils.path_join(root_dir, opts[path_option])
 
 
 def minion_config(path):

salt/fileclient.py

@@ -21,6 +21,7 @@ import salt.crypt
 import salt.loader
 import salt.utils
 import salt.payload
+import salt.utils
 import salt.utils.templates
 from salt._compat import (
     URLError, HTTPError, BaseHTTPServer, urlparse, url_open)
@@ -156,7 +157,7 @@ class Client(object):
                 prefix = separated[0]
             for fn_ in self.file_list_emptydirs(env):
                 if fn_.startswith(path):
-                    dest = os.path.join(
+                    dest = salt.utils.path_join(
                         self.opts['cachedir'],
                         'files',
                         env
@@ -296,7 +297,7 @@ class Client(object):
                 else:
                     return ''
         else:
-            dest = os.path.join(
+            dest = salt.utils.path_join(
                 self.opts['cachedir'],
                 'extrn_files',
                 env,
@@ -354,7 +355,7 @@ class Client(object):
             return ''
         if not dest:
             # No destination passed, set the dest as an extrn_files cache
-            dest = os.path.join(
+            dest = salt.utils.path_join(
                 self.opts['cachedir'],
                 'extrn_files',
                 env,

salt/utils/__init__.py

@@ -15,6 +15,7 @@ import datetime
 import tempfile
 import shutil
 import time
+import platform
 from calendar import month_abbr as months
 
 # Import Salt libs
@@ -468,3 +469,28 @@ def copyfile(source, dest, backup_mode='', cachedir=''):
         # TODO, backup to master
         pass
     shutil.move(tgt, dest)
+
+
+def path_join(*parts):
+    """
+    This functions tries to solve some issues when joining multiple absolute
+    paths on both *nix and windows platforms.
+
+    See tests/unit/utils/path_join_test.py for some examples on what's being
+    talked about here.
+    """
+    # Normalize path converting any os.sep as needed
+    parts = [os.path.normpath(p) for p in parts]
+
+    root = parts.pop(0)
+    if not parts:
+        return root
+
+    if platform.system().lower() == 'windows':
+        if len(root) == 1:
+            root += ':'
+        root = root.rstrip(os.sep) + os.sep
+
+    return os.path.normpath(os.path.join(
+        root, *[p.lstrip(os.sep) for p in parts]
+    ))

tests/unit/utils/path_join_test.py

@@ -0,0 +1,113 @@
+# -*- coding: utf-8 -*-
+"""
+    tests.unit.utils.path_join_test
+    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+    :copyright: © 2012 UfSoft.org - :email:`Pedro Algarvio (pedro@algarvio.me)`
+    :license: Apache 2.0, see LICENSE for more details.
+"""
+
+import os
+import sys
+import posixpath
+import ntpath
+import platform
+import tempfile
+
+if __name__ == "__main__":
+    sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.dirname(__file__))))
+
+from saltunittest import TestCase, TestLoader, TextTestRunner
+from salt.utils import path_join
+
+class PathJoinTestCase(TestCase):
+
+    PLATFORM_FUNC = platform.system
+    BUILTIN_MODULES = sys.builtin_module_names
+
+    NIX_PATHS = (
+        (('/', 'key'), '/key'),
+        (('/usr/local', '/etc/salt/pki'), '/usr/local/etc/salt/pki')
+    )
+
+    WIN_PATHS = (
+        (('c:', 'temp', 'foo'), 'c:\\temp\\foo'),
+        (('c:', r'\temp', r'\foo'), 'c:\\temp\\foo'),
+        (('c:\\', r'\temp', r'\foo'), 'c:\\temp\\foo'),
+        ((r'c:\\', r'\temp', r'\foo'), 'c:\\temp\\foo'),
+        (('c:', r'\temp', r'\foo', 'bar'), 'c:\\temp\\foo\\bar'),
+        (('c:', r'\temp', r'\foo\bar'), 'c:\\temp\\foo\\bar'),
+        (('c', r'\temp', r'\foo\bar'), 'c:\\temp\\foo\\bar')
+    )
+
+    def test_nix_paths(self):
+        if platform.system().lower() == "windows":
+            self.skipTest(
+                "Windows platform found. not running *nix path_join tests"
+            )
+        for idx, (parts, expected) in enumerate(self.NIX_PATHS):
+            path = path_join(*parts)
+            self.assertEqual(
+                '{0}: {1}'.format(idx, path),
+                '{0}: {1}'.format(idx, expected)
+            )
+
+    def test_windows_paths(self):
+        if platform.system().lower() != "windows":
+            self.skipTest(
+                "Non windows platform found. not running non patched os.path path_join tests"
+            )
+
+        for idx, (parts, expected) in enumerate(self.WIN_PATHS):
+            path = path_join(*parts)
+            self.assertEqual(
+                '{0}: {1}'.format(idx, path),
+                '{0}: {1}'.format(idx, expected)
+            )
+
+    def test_windows_paths_patched_path_module(self):
+        if platform.system().lower() == "windows":
+            self.skipTest(
+                "Windows platform found. not running patched os.path path_join tests"
+            )
+
+        self.__patch_path()
+
+        for idx, (parts, expected) in enumerate(self.WIN_PATHS):
+            path = path_join(*parts)
+            self.assertEqual(
+                '{0}: {1}'.format(idx, path),
+                '{0}: {1}'.format(idx, expected)
+            )
+
+        self.__unpatch_path()
+
+    def __patch_path(self):
+        import imp
+        modules = list(self.BUILTIN_MODULES[:])
+        modules.pop(modules.index('posix'))
+        modules.append('nt')
+
+        code = """'''Salt unittest loaded NT module'''"""
+        module = imp.new_module('nt')
+        exec code in module.__dict__
+        sys.modules['nt'] = module
+
+        sys.builtin_module_names = modules
+        platform.system = lambda: "windows"
+
+        for module in (ntpath, os, os.path, tempfile):
+            reload(module)
+
+    def __unpatch_path(self):
+        del sys.modules['nt']
+        sys.builtin_module_names = self.BUILTIN_MODULES[:]
+        platform.system = self.PLATFORM_FUNC
+
+        for module in (posixpath, os, os.path, tempfile, platform):
+            reload(module)
+
+if __name__ == "__main__":
+    loader = PathJoinTestCase()
+    tests = loader.loadTestsFromTestCase(PathJoinTestCase)
+    TextTestRunner(verbosity=1).run(tests)