bpo-43105: Importlib now resolves relative paths when creating module spec objects from file locations (GH-25121)
diff --git a/Lib/importlib/_bootstrap_external.py b/Lib/importlib/_bootstrap_external.py
index be11cfe..0b6cc8a 100644
--- a/Lib/importlib/_bootstrap_external.py
+++ b/Lib/importlib/_bootstrap_external.py
@@ -45,6 +45,7 @@
# Assumption made in _path_join()
assert all(len(sep) == 1 for sep in path_separators)
path_sep = path_separators[0]
+path_sep_tuple = tuple(path_separators)
path_separators = ''.join(path_separators)
_pathseps_with_colon = {f':{s}' for s in path_separators}
@@ -91,22 +92,49 @@ def _unpack_uint16(data):
return int.from_bytes(data, 'little')
-def _path_join(*path_parts):
- """Replacement for os.path.join()."""
- return path_sep.join([part.rstrip(path_separators)
- for part in path_parts if part])
+if _MS_WINDOWS:
+ def _path_join(*path_parts):
+ """Replacement for os.path.join()."""
+ if not path_parts:
+ return ""
+ if len(path_parts) == 1:
+ return path_parts[0]
+ root = ""
+ path = []
+ for new_root, tail in map(_os._path_splitroot, path_parts):
+ if new_root.startswith(path_sep_tuple) or new_root.endswith(path_sep_tuple):
+ root = new_root.rstrip(path_separators) or root
+ path = [path_sep + tail]
+ elif new_root.endswith(':'):
+ if root.casefold() != new_root.casefold():
+ # Drive relative paths have to be resolved by the OS, so we reset the
+ # tail but do not add a path_sep prefix.
+ root = new_root
+ path = [tail]
+ else:
+ path.append(tail)
+ else:
+ root = new_root or root
+ path.append(tail)
+ path = [p.rstrip(path_separators) for p in path if p]
+ if len(path) == 1 and not path[0]:
+ # Avoid losing the root's trailing separator when joining with nothing
+ return root + path_sep
+ return root + path_sep.join(path)
+
+else:
+ def _path_join(*path_parts):
+ """Replacement for os.path.join()."""
+ return path_sep.join([part.rstrip(path_separators)
+ for part in path_parts if part])
def _path_split(path):
"""Replacement for os.path.split()."""
- if len(path_separators) == 1:
- front, _, tail = path.rpartition(path_sep)
- return front, tail
- for x in reversed(path):
- if x in path_separators:
- front, tail = path.rsplit(x, maxsplit=1)
- return front, tail
- return '', path
+ i = max(path.rfind(p) for p in path_separators)
+ if i < 0:
+ return '', path
+ return path[:i], path[i + 1:]
def _path_stat(path):
@@ -140,13 +168,18 @@ def _path_isdir(path):
return _path_is_mode_type(path, 0o040000)
-def _path_isabs(path):
- """Replacement for os.path.isabs.
+if _MS_WINDOWS:
+ def _path_isabs(path):
+ """Replacement for os.path.isabs."""
+ if not path:
+ return False
+ root = _os._path_splitroot(path)[0].replace('/', '\\')
+ return len(root) > 1 and (root.startswith('\\\\') or root.endswith('\\'))
- Considers a Windows drive-relative path (no drive, but starts with slash) to
- still be "absolute".
- """
- return path.startswith(path_separators) or path[1:3] in _pathseps_with_colon
+else:
+ def _path_isabs(path):
+ """Replacement for os.path.isabs."""
+ return path.startswith(path_separators)
def _write_atomic(path, data, mode=0o666):
@@ -707,6 +740,11 @@ def spec_from_file_location(name, location=None, *, loader=None,
pass
else:
location = _os.fspath(location)
+ if not _path_isabs(location):
+ try:
+ location = _path_join(_os.getcwd(), location)
+ except OSError:
+ pass
# If the location is on the filesystem, but doesn't actually exist,
# we could return None here, indicating that the location is not
@@ -1451,6 +1489,8 @@ def __init__(self, path, *loader_details):
self._loaders = loaders
# Base (directory) path
self.path = path or '.'
+ if not _path_isabs(self.path):
+ self.path = _path_join(_os.getcwd(), self.path)
self._path_mtime = -1
self._path_cache = set()
self._relaxed_path_cache = set()
@@ -1516,7 +1556,10 @@ def find_spec(self, fullname, target=None):
is_namespace = _path_isdir(base_path)
# Check for a file w/ a proper suffix exists.
for suffix, loader_class in self._loaders:
- full_path = _path_join(self.path, tail_module + suffix)
+ try:
+ full_path = _path_join(self.path, tail_module + suffix)
+ except ValueError:
+ return None
_bootstrap._verbose_message('trying {}', full_path, verbosity=2)
if cache_module + suffix in cache:
if _path_isfile(full_path):
diff --git a/Lib/test/test_import/__init__.py b/Lib/test/test_import/__init__.py
index dd832a1..8fe3e1d 100644
--- a/Lib/test/test_import/__init__.py
+++ b/Lib/test/test_import/__init__.py
@@ -906,7 +906,7 @@ def test_missing_source_legacy(self):
m = __import__(TESTFN)
try:
self.assertEqual(m.__file__,
- os.path.join(os.curdir, os.path.relpath(pyc_file)))
+ os.path.join(os.getcwd(), os.curdir, os.path.relpath(pyc_file)))
finally:
os.remove(pyc_file)
@@ -914,7 +914,7 @@ def test___cached__(self):
# Modules now also have an __cached__ that points to the pyc file.
m = __import__(TESTFN)
pyc_file = importlib.util.cache_from_source(TESTFN + '.py')
- self.assertEqual(m.__cached__, os.path.join(os.curdir, pyc_file))
+ self.assertEqual(m.__cached__, os.path.join(os.getcwd(), os.curdir, pyc_file))
@skip_if_dont_write_bytecode
def test___cached___legacy_pyc(self):
@@ -930,7 +930,7 @@ def test___cached___legacy_pyc(self):
importlib.invalidate_caches()
m = __import__(TESTFN)
self.assertEqual(m.__cached__,
- os.path.join(os.curdir, os.path.relpath(pyc_file)))
+ os.path.join(os.getcwd(), os.curdir, os.path.relpath(pyc_file)))
@skip_if_dont_write_bytecode
def test_package___cached__(self):
@@ -950,10 +950,10 @@ def cleanup():
m = __import__('pep3147.foo')
init_pyc = importlib.util.cache_from_source(
os.path.join('pep3147', '__init__.py'))
- self.assertEqual(m.__cached__, os.path.join(os.curdir, init_pyc))
+ self.assertEqual(m.__cached__, os.path.join(os.getcwd(), os.curdir, init_pyc))
foo_pyc = importlib.util.cache_from_source(os.path.join('pep3147', 'foo.py'))
self.assertEqual(sys.modules['pep3147.foo'].__cached__,
- os.path.join(os.curdir, foo_pyc))
+ os.path.join(os.getcwd(), os.curdir, foo_pyc))
def test_package___cached___from_pyc(self):
# Like test___cached__ but ensuring __cached__ when imported from a
@@ -977,10 +977,10 @@ def cleanup():
m = __import__('pep3147.foo')
init_pyc = importlib.util.cache_from_source(
os.path.join('pep3147', '__init__.py'))
- self.assertEqual(m.__cached__, os.path.join(os.curdir, init_pyc))
+ self.assertEqual(m.__cached__, os.path.join(os.getcwd(), os.curdir, init_pyc))
foo_pyc = importlib.util.cache_from_source(os.path.join('pep3147', 'foo.py'))
self.assertEqual(sys.modules['pep3147.foo'].__cached__,
- os.path.join(os.curdir, foo_pyc))
+ os.path.join(os.getcwd(), os.curdir, foo_pyc))
def test_recompute_pyc_same_second(self):
# Even when the source file doesn't change timestamp, a change in
diff --git a/Lib/test/test_importlib/test_spec.py b/Lib/test/test_importlib/test_spec.py
index b57eb6c..dcb0527 100644
--- a/Lib/test/test_importlib/test_spec.py
+++ b/Lib/test/test_importlib/test_spec.py
@@ -506,7 +506,7 @@ class FactoryTests:
def setUp(self):
self.name = 'spam'
- self.path = 'spam.py'
+ self.path = os.path.abspath('spam.py')
self.cached = self.util.cache_from_source(self.path)
self.loader = TestLoader()
self.fileloader = TestLoader(self.path)
@@ -645,7 +645,7 @@ def test_spec_from_loader_is_package_true_with_fileloader(self):
self.assertEqual(spec.loader, self.fileloader)
self.assertEqual(spec.origin, self.path)
self.assertIs(spec.loader_state, None)
- self.assertEqual(spec.submodule_search_locations, [''])
+ self.assertEqual(spec.submodule_search_locations, [os.getcwd()])
self.assertEqual(spec.cached, self.cached)
self.assertTrue(spec.has_location)
@@ -744,7 +744,7 @@ def test_spec_from_file_location_smsl_empty(self):
self.assertEqual(spec.loader, self.fileloader)
self.assertEqual(spec.origin, self.path)
self.assertIs(spec.loader_state, None)
- self.assertEqual(spec.submodule_search_locations, [''])
+ self.assertEqual(spec.submodule_search_locations, [os.getcwd()])
self.assertEqual(spec.cached, self.cached)
self.assertTrue(spec.has_location)
@@ -769,7 +769,7 @@ def test_spec_from_file_location_smsl_default(self):
self.assertEqual(spec.loader, self.pkgloader)
self.assertEqual(spec.origin, self.path)
self.assertIs(spec.loader_state, None)
- self.assertEqual(spec.submodule_search_locations, [''])
+ self.assertEqual(spec.submodule_search_locations, [os.getcwd()])
self.assertEqual(spec.cached, self.cached)
self.assertTrue(spec.has_location)
@@ -817,6 +817,17 @@ def is_package(self, name):
self.assertEqual(spec.cached, self.cached)
self.assertTrue(spec.has_location)
+ def test_spec_from_file_location_relative_path(self):
+ spec = self.util.spec_from_file_location(self.name,
+ os.path.basename(self.path), loader=self.fileloader)
+
+ self.assertEqual(spec.name, self.name)
+ self.assertEqual(spec.loader, self.fileloader)
+ self.assertEqual(spec.origin, self.path)
+ self.assertIs(spec.loader_state, None)
+ self.assertIs(spec.submodule_search_locations, None)
+ self.assertEqual(spec.cached, self.cached)
+ self.assertTrue(spec.has_location)
(Frozen_FactoryTests,
Source_FactoryTests
diff --git a/Lib/test/test_importlib/test_windows.py b/Lib/test/test_importlib/test_windows.py
index 64ffe10..802fb34 100644
--- a/Lib/test/test_importlib/test_windows.py
+++ b/Lib/test/test_importlib/test_windows.py
@@ -126,3 +126,48 @@ def test_tagged_suffix(self):
(Frozen_WindowsExtensionSuffixTests,
Source_WindowsExtensionSuffixTests
) = test_util.test_both(WindowsExtensionSuffixTests, machinery=machinery)
+
+
+@unittest.skipUnless(sys.platform.startswith('win'), 'requires Windows')
+class WindowsBootstrapPathTests(unittest.TestCase):
+ def check_join(self, expected, *inputs):
+ from importlib._bootstrap_external import _path_join
+ actual = _path_join(*inputs)
+ if expected.casefold() == actual.casefold():
+ return
+ self.assertEqual(expected, actual)
+
+ def test_path_join(self):
+ self.check_join(r"C:\A\B", "C:\\", "A", "B")
+ self.check_join(r"C:\A\B", "D:\\", "D", "C:\\", "A", "B")
+ self.check_join(r"C:\A\B", "C:\\", "A", "C:B")
+ self.check_join(r"C:\A\B", "C:\\", "A\\B")
+ self.check_join(r"C:\A\B", r"C:\A\B")
+
+ self.check_join("D:A", r"D:", "A")
+ self.check_join("D:A", r"C:\B\C", "D:", "A")
+ self.check_join("D:A", r"C:\B\C", r"D:A")
+
+ self.check_join(r"A\B\C", "A", "B", "C")
+ self.check_join(r"A\B\C", "A", r"B\C")
+ self.check_join(r"A\B/C", "A", "B/C")
+ self.check_join(r"A\B\C", "A/", "B\\", "C")
+
+ # Dots are not normalised by this function
+ self.check_join(r"A\../C", "A", "../C")
+ self.check_join(r"A.\.\B", "A.", ".", "B")
+
+ self.check_join(r"\\Server\Share\A\B\C", r"\\Server\Share", "A", "B", "C")
+ self.check_join(r"\\Server\Share\A\B\C", r"\\Server\Share", "D", r"\A", "B", "C")
+ self.check_join(r"\\Server\Share\A\B\C", r"\\Server2\Share2", "D",
+ r"\\Server\Share", "A", "B", "C")
+ self.check_join(r"\\Server\Share\A\B\C", r"\\Server", r"\Share", "A", "B", "C")
+ self.check_join(r"\\Server\Share", r"\\Server\Share")
+ self.check_join(r"\\Server\Share\\", r"\\Server\Share\\")
+
+ # Handle edge cases with empty segments
+ self.check_join("C:\\A", "C:/A", "")
+ self.check_join("C:\\", "C:/", "")
+ self.check_join("C:", "C:", "")
+ self.check_join("//Server/Share\\", "//Server/Share/", "")
+ self.check_join("//Server/Share\\", "//Server/Share", "")
diff --git a/Lib/test/test_site.py b/Lib/test/test_site.py
index 6060288..9b4ab42 100644
--- a/Lib/test/test_site.py
+++ b/Lib/test/test_site.py
@@ -173,6 +173,7 @@ def test_addpackage_import_bad_pth_file(self):
pth_dir, pth_fn = self.make_pth("abc\x00def\n")
with captured_stderr() as err_out:
self.assertFalse(site.addpackage(pth_dir, pth_fn, set()))
+ self.maxDiff = None
self.assertEqual(err_out.getvalue(), "")
for path in sys.path:
if isinstance(path, str):
@@ -408,55 +409,6 @@ def tearDown(self):
"""Restore sys.path"""
sys.path[:] = self.sys_path
- def test_abs_paths(self):
- # Make sure all imported modules have their __file__ and __cached__
- # attributes as absolute paths. Arranging to put the Lib directory on
- # PYTHONPATH would cause the os module to have a relative path for
- # __file__ if abs_paths() does not get run. sys and builtins (the
- # only other modules imported before site.py runs) do not have
- # __file__ or __cached__ because they are built-in.
- try:
- parent = os.path.relpath(os.path.dirname(os.__file__))
- cwd = os.getcwd()
- except ValueError:
- # Failure to get relpath probably means we need to chdir
- # to the same drive.
- cwd, parent = os.path.split(os.path.dirname(os.__file__))
- with change_cwd(cwd):
- env = os.environ.copy()
- env['PYTHONPATH'] = parent
- code = ('import os, sys',
- # use ASCII to avoid locale issues with non-ASCII directories
- 'os_file = os.__file__.encode("ascii", "backslashreplace")',
- r'sys.stdout.buffer.write(os_file + b"\n")',
- 'os_cached = os.__cached__.encode("ascii", "backslashreplace")',
- r'sys.stdout.buffer.write(os_cached + b"\n")')
- command = '\n'.join(code)
- # First, prove that with -S (no 'import site'), the paths are
- # relative.
- proc = subprocess.Popen([sys.executable, '-S', '-c', command],
- env=env,
- stdout=subprocess.PIPE)
- stdout, stderr = proc.communicate()
-
- self.assertEqual(proc.returncode, 0)
- os__file__, os__cached__ = stdout.splitlines()[:2]
- self.assertFalse(os.path.isabs(os__file__))
- self.assertFalse(os.path.isabs(os__cached__))
- # Now, with 'import site', it works.
- proc = subprocess.Popen([sys.executable, '-c', command],
- env=env,
- stdout=subprocess.PIPE)
- stdout, stderr = proc.communicate()
- self.assertEqual(proc.returncode, 0)
- os__file__, os__cached__ = stdout.splitlines()[:2]
- self.assertTrue(os.path.isabs(os__file__),
- "expected absolute path, got {}"
- .format(os__file__.decode('ascii')))
- self.assertTrue(os.path.isabs(os__cached__),
- "expected absolute path, got {}"
- .format(os__cached__.decode('ascii')))
-
def test_abs_paths_cached_None(self):
"""Test for __cached__ is None.