From 14ad0b7cf984516b2c3565c7bd7e0d6b7488a144 Mon Sep 17 00:00:00 2001 From: Barney Gale Date: Fri, 22 Dec 2023 18:09:50 +0000 Subject: [PATCH] GH-110109: pathlib ABCs: do not vary path syntax by host OS. (#113219) Change the value of `pathlib._abc.PurePathBase.pathmod` from `os.path` to `posixpath`. User subclasses of `PurePathBase` and `PathBase` previously used the host OS's path syntax, e.g. backslashes as separators on Windows. This is wrong in most use cases, and likely to catch developers out unless they test on both Windows and non-Windows machines. In this patch we change the default to POSIX syntax, regardless of OS. This is somewhat arguable (why not make all aspects of syntax abstract and individually configurable?) but an improvement all the same. This change has no effect on `PurePath`, `Path`, nor their subclasses. Only private APIs are affected. --- Lib/pathlib/__init__.py | 1 + Lib/pathlib/_abc.py | 3 +- Lib/test/test_pathlib/test_pathlib.py | 59 ++++++++++++++++ Lib/test/test_pathlib/test_pathlib_abc.py | 84 +++++++---------------- 4 files changed, 87 insertions(+), 60 deletions(-) diff --git a/Lib/pathlib/__init__.py b/Lib/pathlib/__init__.py index 4a8bee601225ed9..bfd2a9249797466 100644 --- a/Lib/pathlib/__init__.py +++ b/Lib/pathlib/__init__.py @@ -59,6 +59,7 @@ class PurePath(_abc.PurePathBase): # path. It's set when `__hash__()` is called for the first time. '_hash', ) + pathmod = os.path def __new__(cls, *args, **kwargs): """Construct a PurePath from one or several strings and or existing diff --git a/Lib/pathlib/_abc.py b/Lib/pathlib/_abc.py index fc4b6443448a5d3..43e2670c4d0258b 100644 --- a/Lib/pathlib/_abc.py +++ b/Lib/pathlib/_abc.py @@ -1,7 +1,6 @@ import functools import io import ntpath -import os import posixpath import sys import warnings @@ -204,7 +203,7 @@ class PurePathBase: # work from occurring when `resolve()` calls `stat()` or `readlink()`. '_resolving', ) - pathmod = os.path + pathmod = posixpath def __init__(self, *paths): self._raw_paths = paths diff --git a/Lib/test/test_pathlib/test_pathlib.py b/Lib/test/test_pathlib/test_pathlib.py index a448f6a1adefce1..db5f3b2634be97e 100644 --- a/Lib/test/test_pathlib/test_pathlib.py +++ b/Lib/test/test_pathlib/test_pathlib.py @@ -2,8 +2,10 @@ import os import sys import errno +import ntpath import pathlib import pickle +import posixpath import socket import stat import tempfile @@ -39,6 +41,50 @@ class PurePathTest(test_pathlib_abc.DummyPurePathTest): cls = pathlib.PurePath + # Make sure any symbolic links in the base test path are resolved. + base = os.path.realpath(TESTFN) + + def test_concrete_class(self): + if self.cls is pathlib.PurePath: + expected = pathlib.PureWindowsPath if os.name == 'nt' else pathlib.PurePosixPath + else: + expected = self.cls + p = self.cls('a') + self.assertIs(type(p), expected) + + def test_concrete_pathmod(self): + if self.cls is pathlib.PurePosixPath: + expected = posixpath + elif self.cls is pathlib.PureWindowsPath: + expected = ntpath + else: + expected = os.path + p = self.cls('a') + self.assertIs(p.pathmod, expected) + + def test_different_pathmods_unequal(self): + p = self.cls('a') + if p.pathmod is posixpath: + q = pathlib.PureWindowsPath('a') + else: + q = pathlib.PurePosixPath('a') + self.assertNotEqual(p, q) + + def test_different_pathmods_unordered(self): + p = self.cls('a') + if p.pathmod is posixpath: + q = pathlib.PureWindowsPath('a') + else: + q = pathlib.PurePosixPath('a') + with self.assertRaises(TypeError): + p < q + with self.assertRaises(TypeError): + p <= q + with self.assertRaises(TypeError): + p > q + with self.assertRaises(TypeError): + p >= q + def test_constructor_nested(self): P = self.cls P(FakePath("a/b/c")) @@ -958,6 +1004,19 @@ def tempdir(self): self.addCleanup(os_helper.rmtree, d) return d + def test_matches_pathbase_api(self): + our_names = {name for name in dir(self.cls) if name[0] != '_'} + path_names = {name for name in dir(pathlib._abc.PathBase) if name[0] != '_'} + self.assertEqual(our_names, path_names) + for attr_name in our_names: + if attr_name == 'pathmod': + # On Windows, Path.pathmod is ntpath, but PathBase.pathmod is + # posixpath, and so their docstrings differ. + continue + our_attr = getattr(self.cls, attr_name) + path_attr = getattr(pathlib._abc.PathBase, attr_name) + self.assertEqual(our_attr.__doc__, path_attr.__doc__) + def test_concrete_class(self): if self.cls is pathlib.Path: expected = pathlib.WindowsPath if os.name == 'nt' else pathlib.PosixPath diff --git a/Lib/test/test_pathlib/test_pathlib_abc.py b/Lib/test/test_pathlib/test_pathlib_abc.py index 42575a5e640d65e..37f4caa4da10282 100644 --- a/Lib/test/test_pathlib/test_pathlib_abc.py +++ b/Lib/test/test_pathlib/test_pathlib_abc.py @@ -38,6 +38,9 @@ def test_magic_methods(self): self.assertIs(P.__gt__, object.__gt__) self.assertIs(P.__ge__, object.__ge__) + def test_pathmod(self): + self.assertIs(self.cls.pathmod, posixpath) + class DummyPurePath(pathlib._abc.PurePathBase): def __eq__(self, other): @@ -52,8 +55,8 @@ def __hash__(self): class DummyPurePathTest(unittest.TestCase): cls = DummyPurePath - # Make sure any symbolic links in the base test path are resolved. - base = os.path.realpath(TESTFN) + # Use a base path that's unrelated to any real filesystem path. + base = f'/this/path/kills/fascists/{TESTFN}' # Keys are canonical paths, values are list of tuples of arguments # supposed to produce equal paths. @@ -86,37 +89,6 @@ def test_constructor_common(self): P('a/b/c') P('/a/b/c') - def test_concrete_class(self): - if self.cls is pathlib.PurePath: - expected = pathlib.PureWindowsPath if os.name == 'nt' else pathlib.PurePosixPath - else: - expected = self.cls - p = self.cls('a') - self.assertIs(type(p), expected) - - def test_different_pathmods_unequal(self): - p = self.cls('a') - if p.pathmod is posixpath: - q = pathlib.PureWindowsPath('a') - else: - q = pathlib.PurePosixPath('a') - self.assertNotEqual(p, q) - - def test_different_pathmods_unordered(self): - p = self.cls('a') - if p.pathmod is posixpath: - q = pathlib.PureWindowsPath('a') - else: - q = pathlib.PurePosixPath('a') - with self.assertRaises(TypeError): - p < q - with self.assertRaises(TypeError): - p <= q - with self.assertRaises(TypeError): - p > q - with self.assertRaises(TypeError): - p >= q - def _check_str_subclass(self, *args): # Issue #21127: it should be possible to construct a PurePath object # from a str subclass instance, and it then gets converted to @@ -721,15 +693,6 @@ def test_fspath_common(self): def test_as_bytes_common(self): self.assertRaises(TypeError, bytes, self.cls()) - def test_matches_path_api(self): - our_names = {name for name in dir(self.cls) if name[0] != '_'} - path_names = {name for name in dir(pathlib.Path) if name[0] != '_'} - self.assertEqual(our_names, path_names) - for attr_name in our_names: - our_attr = getattr(self.cls, attr_name) - path_attr = getattr(pathlib.Path, attr_name) - self.assertEqual(our_attr.__doc__, path_attr.__doc__) - class DummyPathIO(io.BytesIO): """ @@ -905,11 +868,13 @@ def assertFileNotFound(self, func, *args, **kwargs): self.assertEqual(cm.exception.errno, errno.ENOENT) def assertEqualNormCase(self, path_a, path_b): - self.assertEqual(os.path.normcase(path_a), os.path.normcase(path_b)) + normcase = self.pathmod.normcase + self.assertEqual(normcase(path_a), normcase(path_b)) def test_samefile(self): - fileA_path = os.path.join(self.base, 'fileA') - fileB_path = os.path.join(self.base, 'dirB', 'fileB') + pathmod = self.pathmod + fileA_path = pathmod.join(self.base, 'fileA') + fileB_path = pathmod.join(self.base, 'dirB', 'fileB') p = self.cls(fileA_path) pp = self.cls(fileA_path) q = self.cls(fileB_path) @@ -918,7 +883,7 @@ def test_samefile(self): self.assertFalse(p.samefile(fileB_path)) self.assertFalse(p.samefile(q)) # Test the non-existent file case - non_existent = os.path.join(self.base, 'foo') + non_existent = pathmod.join(self.base, 'foo') r = self.cls(non_existent) self.assertRaises(FileNotFoundError, p.samefile, r) self.assertRaises(FileNotFoundError, p.samefile, non_existent) @@ -1379,14 +1344,15 @@ def test_resolve_common(self): p.resolve(strict=True) self.assertEqual(cm.exception.errno, errno.ENOENT) # Non-strict + pathmod = self.pathmod self.assertEqualNormCase(str(p.resolve(strict=False)), - os.path.join(self.base, 'foo')) + pathmod.join(self.base, 'foo')) p = P(self.base, 'foo', 'in', 'spam') self.assertEqualNormCase(str(p.resolve(strict=False)), - os.path.join(self.base, 'foo', 'in', 'spam')) + pathmod.join(self.base, 'foo', 'in', 'spam')) p = P(self.base, '..', 'foo', 'in', 'spam') self.assertEqualNormCase(str(p.resolve(strict=False)), - os.path.abspath(os.path.join('foo', 'in', 'spam'))) + pathmod.join(pathmod.dirname(self.base), 'foo', 'in', 'spam')) # These are all relative symlinks. p = P(self.base, 'dirB', 'fileB') self._check_resolve_relative(p, p) @@ -1401,7 +1367,7 @@ def test_resolve_common(self): self._check_resolve_relative(p, P(self.base, 'dirB', 'fileB', 'foo', 'in', 'spam'), False) p = P(self.base, 'dirA', 'linkC', '..', 'foo', 'in', 'spam') - if os.name == 'nt' and isinstance(p, pathlib.Path): + if self.cls.pathmod is not posixpath: # In Windows, if linkY points to dirB, 'dirA\linkY\..' # resolves to 'dirA' without resolving linkY first. self._check_resolve_relative(p, P(self.base, 'dirA', 'foo', 'in', @@ -1421,7 +1387,7 @@ def test_resolve_common(self): self._check_resolve_relative(p, P(self.base, 'dirB', 'foo', 'in', 'spam'), False) p = P(self.base, 'dirA', 'linkX', 'linkY', '..', 'foo', 'in', 'spam') - if os.name == 'nt' and isinstance(p, pathlib.Path): + if self.cls.pathmod is not posixpath: # In Windows, if linkY points to dirB, 'dirA\linkY\..' # resolves to 'dirA' without resolving linkY first. self._check_resolve_relative(p, P(d, 'foo', 'in', 'spam'), False) @@ -1434,10 +1400,11 @@ def test_resolve_dot(self): # See http://web.archive.org/web/20200623062557/https://bitbucket.org/pitrou/pathlib/issues/9/ if not self.can_symlink: self.skipTest("symlinks required") + pathmod = self.pathmod p = self.cls(self.base) p.joinpath('0').symlink_to('.', target_is_directory=True) - p.joinpath('1').symlink_to(os.path.join('0', '0'), target_is_directory=True) - p.joinpath('2').symlink_to(os.path.join('1', '1'), target_is_directory=True) + p.joinpath('1').symlink_to(pathmod.join('0', '0'), target_is_directory=True) + p.joinpath('2').symlink_to(pathmod.join('1', '1'), target_is_directory=True) q = p / '2' self.assertEqual(q.resolve(strict=True), p) r = q / '3' / '4' @@ -1454,7 +1421,7 @@ def _check_symlink_loop(self, *args): def test_resolve_loop(self): if not self.can_symlink: self.skipTest("symlinks required") - if os.name == 'nt' and issubclass(self.cls, pathlib.Path): + if self.cls.pathmod is not posixpath: self.skipTest("symlink loops work differently with concrete Windows paths") # Loops with relative symlinks. self.cls(self.base, 'linkX').symlink_to('linkX/inside') @@ -1657,10 +1624,11 @@ def _check_complex_symlinks(self, link0_target): self.skipTest("symlinks required") # Test solving a non-looping chain of symlinks (issue #19887). + pathmod = self.pathmod P = self.cls(self.base) - P.joinpath('link1').symlink_to(os.path.join('link0', 'link0'), target_is_directory=True) - P.joinpath('link2').symlink_to(os.path.join('link1', 'link1'), target_is_directory=True) - P.joinpath('link3').symlink_to(os.path.join('link2', 'link2'), target_is_directory=True) + P.joinpath('link1').symlink_to(pathmod.join('link0', 'link0'), target_is_directory=True) + P.joinpath('link2').symlink_to(pathmod.join('link1', 'link1'), target_is_directory=True) + P.joinpath('link3').symlink_to(pathmod.join('link2', 'link2'), target_is_directory=True) P.joinpath('link0').symlink_to(link0_target, target_is_directory=True) # Resolve absolute paths. @@ -1707,7 +1675,7 @@ def test_complex_symlinks_relative(self): self._check_complex_symlinks('.') def test_complex_symlinks_relative_dot_dot(self): - self._check_complex_symlinks(os.path.join('dirA', '..')) + self._check_complex_symlinks(self.pathmod.join('dirA', '..')) def setUpWalk(self): # Build: