Revert "bpo-43105: Importlib now resolves relative paths when creating module spec objects from file locations (pythonGH-25121)"

This reverts commit 04732ca.

Author: kuba2k2

Lib/importlib/_bootstrap_external.py

@@ -45,7 +45,6 @@
 # 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}
 
@@ -92,49 +91,22 @@ def _unpack_uint16(data):
     return int.from_bytes(data, 'little')
 
 
-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_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()."""
-    i = max(path.rfind(p) for p in path_separators)
-    if i < 0:
-        return '', path
-    return path[:i], path[i + 1:]
+    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
 
 
 def _path_stat(path):
@@ -168,18 +140,13 @@ def _path_isdir(path):
     return _path_is_mode_type(path, 0o040000)
 
 
-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('\\'))
+def _path_isabs(path):
+    """Replacement for os.path.isabs.
 
-else:
-    def _path_isabs(path):
-        """Replacement for os.path.isabs."""
-        return path.startswith(path_separators)
+    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
 
 
 def _write_atomic(path, data, mode=0o666):
@@ -743,11 +710,6 @@ 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
@@ -1501,8 +1463,6 @@ 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()
@@ -1568,10 +1528,7 @@ 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:
-            try:
-                full_path = _path_join(self.path, tail_module + suffix)
-            except ValueError:
-                return None
+            full_path = _path_join(self.path, tail_module + suffix)
             _bootstrap._verbose_message('trying {}', full_path, verbosity=2)
             if cache_module + suffix in cache:
                 if _path_isfile(full_path):

Lib/test/test_import/__init__.py

@@ -906,15 +906,15 @@ def test_missing_source_legacy(self):
         m = __import__(TESTFN)
         try:
             self.assertEqual(m.__file__,
-                             os.path.join(os.getcwd(), os.curdir, os.path.relpath(pyc_file)))
+                             os.path.join(os.curdir, os.path.relpath(pyc_file)))
         finally:
             os.remove(pyc_file)
 
     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.getcwd(), os.curdir, pyc_file))
+        self.assertEqual(m.__cached__, os.path.join(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.getcwd(), os.curdir, os.path.relpath(pyc_file)))
+                         os.path.join(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.getcwd(), os.curdir, init_pyc))
+        self.assertEqual(m.__cached__, os.path.join(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.getcwd(), os.curdir, foo_pyc))
+                         os.path.join(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.getcwd(), os.curdir, init_pyc))
+        self.assertEqual(m.__cached__, os.path.join(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.getcwd(), os.curdir, foo_pyc))
+                         os.path.join(os.curdir, foo_pyc))
 
     def test_recompute_pyc_same_second(self):
         # Even when the source file doesn't change timestamp, a change in

Lib/test/test_importlib/test_spec.py

@@ -506,7 +506,7 @@ class FactoryTests:
 
     def setUp(self):
         self.name = 'spam'
-        self.path = os.path.abspath('spam.py')
+        self.path = '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, [os.getcwd()])
+        self.assertEqual(spec.submodule_search_locations, [''])
         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, [os.getcwd()])
+        self.assertEqual(spec.submodule_search_locations, [''])
         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, [os.getcwd()])
+        self.assertEqual(spec.submodule_search_locations, [''])
         self.assertEqual(spec.cached, self.cached)
         self.assertTrue(spec.has_location)
 
@@ -817,17 +817,6 @@ 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

Lib/test/test_importlib/test_windows.py

@@ -144,48 +144,3 @@ 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", "")

Lib/test/test_site.py

@@ -175,7 +175,6 @@ 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):
@@ -411,6 +410,55 @@ 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.