Skip to content

Commit 0af99b4

Browse files
authored
bpo-43105: Importlib now resolves relative paths when creating module spec objects from file locations (GH-25121)
1 parent 2df971a commit 0af99b4

File tree

9 files changed

+3031
-2767
lines changed

9 files changed

+3031
-2767
lines changed

Lib/importlib/_bootstrap_external.py

+89-19
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,34 @@
1919
# reference any injected objects! This includes not only global code but also
2020
# anything specified at the class level.
2121

22+
# Import builtin modules
23+
import _imp
24+
import _io
25+
import sys
26+
import _warnings
27+
import marshal
28+
29+
30+
_MS_WINDOWS = (sys.platform == 'win32')
31+
if _MS_WINDOWS:
32+
import nt as _os
33+
import winreg
34+
else:
35+
import posix as _os
36+
37+
38+
if _MS_WINDOWS:
39+
path_separators = ['\\', '/']
40+
else:
41+
path_separators = ['/']
42+
# Assumption made in _path_join()
43+
assert all(len(sep) == 1 for sep in path_separators)
44+
path_sep = path_separators[0]
45+
path_sep_tuple = tuple(path_separators)
46+
path_separators = ''.join(path_separators)
47+
_pathseps_with_colon = {f':{s}' for s in path_separators}
48+
49+
2250
# Bootstrap-related code ######################################################
2351
_CASE_INSENSITIVE_PLATFORMS_STR_KEY = 'win',
2452
_CASE_INSENSITIVE_PLATFORMS_BYTES_KEY = 'cygwin', 'darwin'
@@ -59,22 +87,49 @@ def _unpack_uint16(data):
5987
return int.from_bytes(data, 'little')
6088

6189

62-
def _path_join(*path_parts):
63-
"""Replacement for os.path.join()."""
64-
return path_sep.join([part.rstrip(path_separators)
65-
for part in path_parts if part])
90+
if _MS_WINDOWS:
91+
def _path_join(*path_parts):
92+
"""Replacement for os.path.join()."""
93+
if not path_parts:
94+
return ""
95+
if len(path_parts) == 1:
96+
return path_parts[0]
97+
root = ""
98+
path = []
99+
for new_root, tail in map(_os._path_splitroot, path_parts):
100+
if new_root.startswith(path_sep_tuple) or new_root.endswith(path_sep_tuple):
101+
root = new_root.rstrip(path_separators) or root
102+
path = [path_sep + tail]
103+
elif new_root.endswith(':'):
104+
if root.casefold() != new_root.casefold():
105+
# Drive relative paths have to be resolved by the OS, so we reset the
106+
# tail but do not add a path_sep prefix.
107+
root = new_root
108+
path = [tail]
109+
else:
110+
path.append(tail)
111+
else:
112+
root = new_root or root
113+
path.append(tail)
114+
path = [p.rstrip(path_separators) for p in path if p]
115+
if len(path) == 1 and not path[0]:
116+
# Avoid losing the root's trailing separator when joining with nothing
117+
return root + path_sep
118+
return root + path_sep.join(path)
119+
120+
else:
121+
def _path_join(*path_parts):
122+
"""Replacement for os.path.join()."""
123+
return path_sep.join([part.rstrip(path_separators)
124+
for part in path_parts if part])
66125

67126

68127
def _path_split(path):
69128
"""Replacement for os.path.split()."""
70-
if len(path_separators) == 1:
71-
front, _, tail = path.rpartition(path_sep)
72-
return front, tail
73-
for x in reversed(path):
74-
if x in path_separators:
75-
front, tail = path.rsplit(x, maxsplit=1)
76-
return front, tail
77-
return '', path
129+
i = max(path.rfind(p) for p in path_separators)
130+
if i < 0:
131+
return '', path
132+
return path[:i], path[i + 1:]
78133

79134

80135
def _path_stat(path):
@@ -108,13 +163,18 @@ def _path_isdir(path):
108163
return _path_is_mode_type(path, 0o040000)
109164

110165

111-
def _path_isabs(path):
112-
"""Replacement for os.path.isabs.
166+
if _MS_WINDOWS:
167+
def _path_isabs(path):
168+
"""Replacement for os.path.isabs."""
169+
if not path:
170+
return False
171+
root = _os._path_splitroot(path)[0].replace('/', '\\')
172+
return len(root) > 1 and (root.startswith('\\\\') or root.endswith('\\'))
113173

114-
Considers a Windows drive-relative path (no drive, but starts with slash) to
115-
still be "absolute".
116-
"""
117-
return path.startswith(path_separators) or path[1:3] in _pathseps_with_colon
174+
else:
175+
def _path_isabs(path):
176+
"""Replacement for os.path.isabs."""
177+
return path.startswith(path_separators)
118178

119179

120180
def _write_atomic(path, data, mode=0o666):
@@ -658,6 +718,11 @@ def spec_from_file_location(name, location=None, *, loader=None,
658718
pass
659719
else:
660720
location = _os.fspath(location)
721+
if not _path_isabs(location):
722+
try:
723+
location = _path_join(_os.getcwd(), location)
724+
except OSError:
725+
pass
661726

662727
# If the location is on the filesystem, but doesn't actually exist,
663728
# we could return None here, indicating that the location is not
@@ -1408,6 +1473,8 @@ def __init__(self, path, *loader_details):
14081473
self._loaders = loaders
14091474
# Base (directory) path
14101475
self.path = path or '.'
1476+
if not _path_isabs(self.path):
1477+
self.path = _path_join(_os.getcwd(), self.path)
14111478
self._path_mtime = -1
14121479
self._path_cache = set()
14131480
self._relaxed_path_cache = set()
@@ -1470,7 +1537,10 @@ def find_spec(self, fullname, target=None):
14701537
is_namespace = _path_isdir(base_path)
14711538
# Check for a file w/ a proper suffix exists.
14721539
for suffix, loader_class in self._loaders:
1473-
full_path = _path_join(self.path, tail_module + suffix)
1540+
try:
1541+
full_path = _path_join(self.path, tail_module + suffix)
1542+
except ValueError:
1543+
return None
14741544
_bootstrap._verbose_message('trying {}', full_path, verbosity=2)
14751545
if cache_module + suffix in cache:
14761546
if _path_isfile(full_path):

Lib/test/test_import/__init__.py

+7-7
Original file line numberDiff line numberDiff line change
@@ -904,15 +904,15 @@ def test_missing_source_legacy(self):
904904
m = __import__(TESTFN)
905905
try:
906906
self.assertEqual(m.__file__,
907-
os.path.join(os.curdir, os.path.relpath(pyc_file)))
907+
os.path.join(os.getcwd(), os.curdir, os.path.relpath(pyc_file)))
908908
finally:
909909
os.remove(pyc_file)
910910

911911
def test___cached__(self):
912912
# Modules now also have an __cached__ that points to the pyc file.
913913
m = __import__(TESTFN)
914914
pyc_file = importlib.util.cache_from_source(TESTFN + '.py')
915-
self.assertEqual(m.__cached__, os.path.join(os.curdir, pyc_file))
915+
self.assertEqual(m.__cached__, os.path.join(os.getcwd(), os.curdir, pyc_file))
916916

917917
@skip_if_dont_write_bytecode
918918
def test___cached___legacy_pyc(self):
@@ -928,7 +928,7 @@ def test___cached___legacy_pyc(self):
928928
importlib.invalidate_caches()
929929
m = __import__(TESTFN)
930930
self.assertEqual(m.__cached__,
931-
os.path.join(os.curdir, os.path.relpath(pyc_file)))
931+
os.path.join(os.getcwd(), os.curdir, os.path.relpath(pyc_file)))
932932

933933
@skip_if_dont_write_bytecode
934934
def test_package___cached__(self):
@@ -948,10 +948,10 @@ def cleanup():
948948
m = __import__('pep3147.foo')
949949
init_pyc = importlib.util.cache_from_source(
950950
os.path.join('pep3147', '__init__.py'))
951-
self.assertEqual(m.__cached__, os.path.join(os.curdir, init_pyc))
951+
self.assertEqual(m.__cached__, os.path.join(os.getcwd(), os.curdir, init_pyc))
952952
foo_pyc = importlib.util.cache_from_source(os.path.join('pep3147', 'foo.py'))
953953
self.assertEqual(sys.modules['pep3147.foo'].__cached__,
954-
os.path.join(os.curdir, foo_pyc))
954+
os.path.join(os.getcwd(), os.curdir, foo_pyc))
955955

956956
def test_package___cached___from_pyc(self):
957957
# Like test___cached__ but ensuring __cached__ when imported from a
@@ -975,10 +975,10 @@ def cleanup():
975975
m = __import__('pep3147.foo')
976976
init_pyc = importlib.util.cache_from_source(
977977
os.path.join('pep3147', '__init__.py'))
978-
self.assertEqual(m.__cached__, os.path.join(os.curdir, init_pyc))
978+
self.assertEqual(m.__cached__, os.path.join(os.getcwd(), os.curdir, init_pyc))
979979
foo_pyc = importlib.util.cache_from_source(os.path.join('pep3147', 'foo.py'))
980980
self.assertEqual(sys.modules['pep3147.foo'].__cached__,
981-
os.path.join(os.curdir, foo_pyc))
981+
os.path.join(os.getcwd(), os.curdir, foo_pyc))
982982

983983
def test_recompute_pyc_same_second(self):
984984
# Even when the source file doesn't change timestamp, a change in

Lib/test/test_importlib/test_spec.py

+15-4
Original file line numberDiff line numberDiff line change
@@ -498,7 +498,7 @@ class FactoryTests:
498498

499499
def setUp(self):
500500
self.name = 'spam'
501-
self.path = 'spam.py'
501+
self.path = os.path.abspath('spam.py')
502502
self.cached = self.util.cache_from_source(self.path)
503503
self.loader = TestLoader()
504504
self.fileloader = TestLoader(self.path)
@@ -637,7 +637,7 @@ def test_spec_from_loader_is_package_true_with_fileloader(self):
637637
self.assertEqual(spec.loader, self.fileloader)
638638
self.assertEqual(spec.origin, self.path)
639639
self.assertIs(spec.loader_state, None)
640-
self.assertEqual(spec.submodule_search_locations, [''])
640+
self.assertEqual(spec.submodule_search_locations, [os.getcwd()])
641641
self.assertEqual(spec.cached, self.cached)
642642
self.assertTrue(spec.has_location)
643643

@@ -736,7 +736,7 @@ def test_spec_from_file_location_smsl_empty(self):
736736
self.assertEqual(spec.loader, self.fileloader)
737737
self.assertEqual(spec.origin, self.path)
738738
self.assertIs(spec.loader_state, None)
739-
self.assertEqual(spec.submodule_search_locations, [''])
739+
self.assertEqual(spec.submodule_search_locations, [os.getcwd()])
740740
self.assertEqual(spec.cached, self.cached)
741741
self.assertTrue(spec.has_location)
742742

@@ -761,7 +761,7 @@ def test_spec_from_file_location_smsl_default(self):
761761
self.assertEqual(spec.loader, self.pkgloader)
762762
self.assertEqual(spec.origin, self.path)
763763
self.assertIs(spec.loader_state, None)
764-
self.assertEqual(spec.submodule_search_locations, [''])
764+
self.assertEqual(spec.submodule_search_locations, [os.getcwd()])
765765
self.assertEqual(spec.cached, self.cached)
766766
self.assertTrue(spec.has_location)
767767

@@ -809,6 +809,17 @@ def is_package(self, name):
809809
self.assertEqual(spec.cached, self.cached)
810810
self.assertTrue(spec.has_location)
811811

812+
def test_spec_from_file_location_relative_path(self):
813+
spec = self.util.spec_from_file_location(self.name,
814+
os.path.basename(self.path), loader=self.fileloader)
815+
816+
self.assertEqual(spec.name, self.name)
817+
self.assertEqual(spec.loader, self.fileloader)
818+
self.assertEqual(spec.origin, self.path)
819+
self.assertIs(spec.loader_state, None)
820+
self.assertIs(spec.submodule_search_locations, None)
821+
self.assertEqual(spec.cached, self.cached)
822+
self.assertTrue(spec.has_location)
812823

813824
(Frozen_FactoryTests,
814825
Source_FactoryTests

Lib/test/test_importlib/test_windows.py

+45
Original file line numberDiff line numberDiff line change
@@ -107,3 +107,48 @@ def test_tagged_suffix(self):
107107
(Frozen_WindowsExtensionSuffixTests,
108108
Source_WindowsExtensionSuffixTests
109109
) = test_util.test_both(WindowsExtensionSuffixTests, machinery=machinery)
110+
111+
112+
@unittest.skipUnless(sys.platform.startswith('win'), 'requires Windows')
113+
class WindowsBootstrapPathTests(unittest.TestCase):
114+
def check_join(self, expected, *inputs):
115+
from importlib._bootstrap_external import _path_join
116+
actual = _path_join(*inputs)
117+
if expected.casefold() == actual.casefold():
118+
return
119+
self.assertEqual(expected, actual)
120+
121+
def test_path_join(self):
122+
self.check_join(r"C:\A\B", "C:\\", "A", "B")
123+
self.check_join(r"C:\A\B", "D:\\", "D", "C:\\", "A", "B")
124+
self.check_join(r"C:\A\B", "C:\\", "A", "C:B")
125+
self.check_join(r"C:\A\B", "C:\\", "A\\B")
126+
self.check_join(r"C:\A\B", r"C:\A\B")
127+
128+
self.check_join("D:A", r"D:", "A")
129+
self.check_join("D:A", r"C:\B\C", "D:", "A")
130+
self.check_join("D:A", r"C:\B\C", r"D:A")
131+
132+
self.check_join(r"A\B\C", "A", "B", "C")
133+
self.check_join(r"A\B\C", "A", r"B\C")
134+
self.check_join(r"A\B/C", "A", "B/C")
135+
self.check_join(r"A\B\C", "A/", "B\\", "C")
136+
137+
# Dots are not normalised by this function
138+
self.check_join(r"A\../C", "A", "../C")
139+
self.check_join(r"A.\.\B", "A.", ".", "B")
140+
141+
self.check_join(r"\\Server\Share\A\B\C", r"\\Server\Share", "A", "B", "C")
142+
self.check_join(r"\\Server\Share\A\B\C", r"\\Server\Share", "D", r"\A", "B", "C")
143+
self.check_join(r"\\Server\Share\A\B\C", r"\\Server2\Share2", "D",
144+
r"\\Server\Share", "A", "B", "C")
145+
self.check_join(r"\\Server\Share\A\B\C", r"\\Server", r"\Share", "A", "B", "C")
146+
self.check_join(r"\\Server\Share", r"\\Server\Share")
147+
self.check_join(r"\\Server\Share\\", r"\\Server\Share\\")
148+
149+
# Handle edge cases with empty segments
150+
self.check_join("C:\\A", "C:/A", "")
151+
self.check_join("C:\\", "C:/", "")
152+
self.check_join("C:", "C:", "")
153+
self.check_join("//Server/Share\\", "//Server/Share/", "")
154+
self.check_join("//Server/Share\\", "//Server/Share", "")

Lib/test/test_site.py

+1-49
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,7 @@ def test_addpackage_import_bad_pth_file(self):
164164
pth_dir, pth_fn = self.make_pth("abc\x00def\n")
165165
with captured_stderr() as err_out:
166166
self.assertFalse(site.addpackage(pth_dir, pth_fn, set()))
167+
self.maxDiff = None
167168
self.assertEqual(err_out.getvalue(), "")
168169
for path in sys.path:
169170
if isinstance(path, str):
@@ -387,55 +388,6 @@ def tearDown(self):
387388
"""Restore sys.path"""
388389
sys.path[:] = self.sys_path
389390

390-
def test_abs_paths(self):
391-
# Make sure all imported modules have their __file__ and __cached__
392-
# attributes as absolute paths. Arranging to put the Lib directory on
393-
# PYTHONPATH would cause the os module to have a relative path for
394-
# __file__ if abs_paths() does not get run. sys and builtins (the
395-
# only other modules imported before site.py runs) do not have
396-
# __file__ or __cached__ because they are built-in.
397-
try:
398-
parent = os.path.relpath(os.path.dirname(os.__file__))
399-
cwd = os.getcwd()
400-
except ValueError:
401-
# Failure to get relpath probably means we need to chdir
402-
# to the same drive.
403-
cwd, parent = os.path.split(os.path.dirname(os.__file__))
404-
with change_cwd(cwd):
405-
env = os.environ.copy()
406-
env['PYTHONPATH'] = parent
407-
code = ('import os, sys',
408-
# use ASCII to avoid locale issues with non-ASCII directories
409-
'os_file = os.__file__.encode("ascii", "backslashreplace")',
410-
r'sys.stdout.buffer.write(os_file + b"\n")',
411-
'os_cached = os.__cached__.encode("ascii", "backslashreplace")',
412-
r'sys.stdout.buffer.write(os_cached + b"\n")')
413-
command = '\n'.join(code)
414-
# First, prove that with -S (no 'import site'), the paths are
415-
# relative.
416-
proc = subprocess.Popen([sys.executable, '-S', '-c', command],
417-
env=env,
418-
stdout=subprocess.PIPE)
419-
stdout, stderr = proc.communicate()
420-
421-
self.assertEqual(proc.returncode, 0)
422-
os__file__, os__cached__ = stdout.splitlines()[:2]
423-
self.assertFalse(os.path.isabs(os__file__))
424-
self.assertFalse(os.path.isabs(os__cached__))
425-
# Now, with 'import site', it works.
426-
proc = subprocess.Popen([sys.executable, '-c', command],
427-
env=env,
428-
stdout=subprocess.PIPE)
429-
stdout, stderr = proc.communicate()
430-
self.assertEqual(proc.returncode, 0)
431-
os__file__, os__cached__ = stdout.splitlines()[:2]
432-
self.assertTrue(os.path.isabs(os__file__),
433-
"expected absolute path, got {}"
434-
.format(os__file__.decode('ascii')))
435-
self.assertTrue(os.path.isabs(os__cached__),
436-
"expected absolute path, got {}"
437-
.format(os__cached__.decode('ascii')))
438-
439391
def test_abs_paths_cached_None(self):
440392
"""Test for __cached__ is None.
441393
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Importlib now resolves relative paths when creating module spec objects from
2+
file locations.

0 commit comments

Comments
 (0)