From 639471d110a66bbd412f68282981ff8ef69a18cc Mon Sep 17 00:00:00 2001 From: Pedro Algarvio Date: Wed, 8 Aug 2012 14:43:34 +0100 Subject: [PATCH] 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 | 2 +- salt/fileclient.py | 7 +- salt/utils/__init__.py | 26 +++++++ tests/unit/utils/path_join_test.py | 113 +++++++++++++++++++++++++++++ 4 files changed, 144 insertions(+), 4 deletions(-) create mode 100644 tests/unit/utils/path_join_test.py diff --git a/salt/config.py b/salt/config.py index 71be4ceb18..a082e1d8f9 100644 --- a/salt/config.py +++ b/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): diff --git a/salt/fileclient.py b/salt/fileclient.py index f0ad6de772..c97a0378af 100644 --- a/salt/fileclient.py +++ b/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, diff --git a/salt/utils/__init__.py b/salt/utils/__init__.py index a875a2d6b3..f0b867a1c2 100644 --- a/salt/utils/__init__.py +++ b/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] + )) diff --git a/tests/unit/utils/path_join_test.py b/tests/unit/utils/path_join_test.py new file mode 100644 index 0000000000..31e8a7998d --- /dev/null +++ b/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) \ No newline at end of file