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.
This commit is contained in:
Barney Gale 2023-12-22 18:09:50 +00:00 committed by GitHub
parent ff5e131df5
commit a0d3d3ec9d
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 87 additions and 60 deletions

View file

@ -59,6 +59,7 @@ class PurePath(_abc.PurePathBase):
# path. It's set when `__hash__()` is called for the first time. # path. It's set when `__hash__()` is called for the first time.
'_hash', '_hash',
) )
pathmod = os.path
def __new__(cls, *args, **kwargs): def __new__(cls, *args, **kwargs):
"""Construct a PurePath from one or several strings and or existing """Construct a PurePath from one or several strings and or existing

View file

@ -1,7 +1,6 @@
import functools import functools
import io import io
import ntpath import ntpath
import os
import posixpath import posixpath
import sys import sys
import warnings import warnings
@ -204,7 +203,7 @@ class PurePathBase:
# work from occurring when `resolve()` calls `stat()` or `readlink()`. # work from occurring when `resolve()` calls `stat()` or `readlink()`.
'_resolving', '_resolving',
) )
pathmod = os.path pathmod = posixpath
def __init__(self, *paths): def __init__(self, *paths):
self._raw_paths = paths self._raw_paths = paths

View file

@ -2,8 +2,10 @@
import os import os
import sys import sys
import errno import errno
import ntpath
import pathlib import pathlib
import pickle import pickle
import posixpath
import socket import socket
import stat import stat
import tempfile import tempfile
@ -39,6 +41,50 @@
class PurePathTest(test_pathlib_abc.DummyPurePathTest): class PurePathTest(test_pathlib_abc.DummyPurePathTest):
cls = pathlib.PurePath 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): def test_constructor_nested(self):
P = self.cls P = self.cls
P(FakePath("a/b/c")) P(FakePath("a/b/c"))
@ -958,6 +1004,19 @@ def tempdir(self):
self.addCleanup(os_helper.rmtree, d) self.addCleanup(os_helper.rmtree, d)
return 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): def test_concrete_class(self):
if self.cls is pathlib.Path: if self.cls is pathlib.Path:
expected = pathlib.WindowsPath if os.name == 'nt' else pathlib.PosixPath expected = pathlib.WindowsPath if os.name == 'nt' else pathlib.PosixPath

View file

@ -38,6 +38,9 @@ def test_magic_methods(self):
self.assertIs(P.__gt__, object.__gt__) self.assertIs(P.__gt__, object.__gt__)
self.assertIs(P.__ge__, object.__ge__) self.assertIs(P.__ge__, object.__ge__)
def test_pathmod(self):
self.assertIs(self.cls.pathmod, posixpath)
class DummyPurePath(pathlib._abc.PurePathBase): class DummyPurePath(pathlib._abc.PurePathBase):
def __eq__(self, other): def __eq__(self, other):
@ -52,8 +55,8 @@ def __hash__(self):
class DummyPurePathTest(unittest.TestCase): class DummyPurePathTest(unittest.TestCase):
cls = DummyPurePath cls = DummyPurePath
# Make sure any symbolic links in the base test path are resolved. # Use a base path that's unrelated to any real filesystem path.
base = os.path.realpath(TESTFN) base = f'/this/path/kills/fascists/{TESTFN}'
# Keys are canonical paths, values are list of tuples of arguments # Keys are canonical paths, values are list of tuples of arguments
# supposed to produce equal paths. # supposed to produce equal paths.
@ -86,37 +89,6 @@ def test_constructor_common(self):
P('a/b/c') P('a/b/c')
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): def _check_str_subclass(self, *args):
# Issue #21127: it should be possible to construct a PurePath object # Issue #21127: it should be possible to construct a PurePath object
# from a str subclass instance, and it then gets converted to # 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): def test_as_bytes_common(self):
self.assertRaises(TypeError, bytes, self.cls()) 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): class DummyPathIO(io.BytesIO):
""" """
@ -905,11 +868,13 @@ def assertFileNotFound(self, func, *args, **kwargs):
self.assertEqual(cm.exception.errno, errno.ENOENT) self.assertEqual(cm.exception.errno, errno.ENOENT)
def assertEqualNormCase(self, path_a, path_b): 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): def test_samefile(self):
fileA_path = os.path.join(self.base, 'fileA') pathmod = self.pathmod
fileB_path = os.path.join(self.base, 'dirB', 'fileB') fileA_path = pathmod.join(self.base, 'fileA')
fileB_path = pathmod.join(self.base, 'dirB', 'fileB')
p = self.cls(fileA_path) p = self.cls(fileA_path)
pp = self.cls(fileA_path) pp = self.cls(fileA_path)
q = self.cls(fileB_path) q = self.cls(fileB_path)
@ -918,7 +883,7 @@ def test_samefile(self):
self.assertFalse(p.samefile(fileB_path)) self.assertFalse(p.samefile(fileB_path))
self.assertFalse(p.samefile(q)) self.assertFalse(p.samefile(q))
# Test the non-existent file case # 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) r = self.cls(non_existent)
self.assertRaises(FileNotFoundError, p.samefile, r) self.assertRaises(FileNotFoundError, p.samefile, r)
self.assertRaises(FileNotFoundError, p.samefile, non_existent) self.assertRaises(FileNotFoundError, p.samefile, non_existent)
@ -1379,14 +1344,15 @@ def test_resolve_common(self):
p.resolve(strict=True) p.resolve(strict=True)
self.assertEqual(cm.exception.errno, errno.ENOENT) self.assertEqual(cm.exception.errno, errno.ENOENT)
# Non-strict # Non-strict
pathmod = self.pathmod
self.assertEqualNormCase(str(p.resolve(strict=False)), 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') p = P(self.base, 'foo', 'in', 'spam')
self.assertEqualNormCase(str(p.resolve(strict=False)), 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') p = P(self.base, '..', 'foo', 'in', 'spam')
self.assertEqualNormCase(str(p.resolve(strict=False)), 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. # These are all relative symlinks.
p = P(self.base, 'dirB', 'fileB') p = P(self.base, 'dirB', 'fileB')
self._check_resolve_relative(p, p) 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', self._check_resolve_relative(p, P(self.base, 'dirB', 'fileB', 'foo', 'in',
'spam'), False) 'spam'), False)
p = P(self.base, 'dirA', 'linkC', '..', 'foo', 'in', 'spam') 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\..' # In Windows, if linkY points to dirB, 'dirA\linkY\..'
# resolves to 'dirA' without resolving linkY first. # resolves to 'dirA' without resolving linkY first.
self._check_resolve_relative(p, P(self.base, 'dirA', 'foo', 'in', 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'), self._check_resolve_relative(p, P(self.base, 'dirB', 'foo', 'in', 'spam'),
False) False)
p = P(self.base, 'dirA', 'linkX', 'linkY', '..', 'foo', 'in', 'spam') 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\..' # In Windows, if linkY points to dirB, 'dirA\linkY\..'
# resolves to 'dirA' without resolving linkY first. # resolves to 'dirA' without resolving linkY first.
self._check_resolve_relative(p, P(d, 'foo', 'in', 'spam'), False) 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/ # See http://web.archive.org/web/20200623062557/https://bitbucket.org/pitrou/pathlib/issues/9/
if not self.can_symlink: if not self.can_symlink:
self.skipTest("symlinks required") self.skipTest("symlinks required")
pathmod = self.pathmod
p = self.cls(self.base) p = self.cls(self.base)
p.joinpath('0').symlink_to('.', target_is_directory=True) 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('1').symlink_to(pathmod.join('0', '0'), target_is_directory=True)
p.joinpath('2').symlink_to(os.path.join('1', '1'), target_is_directory=True) p.joinpath('2').symlink_to(pathmod.join('1', '1'), target_is_directory=True)
q = p / '2' q = p / '2'
self.assertEqual(q.resolve(strict=True), p) self.assertEqual(q.resolve(strict=True), p)
r = q / '3' / '4' r = q / '3' / '4'
@ -1454,7 +1421,7 @@ def _check_symlink_loop(self, *args):
def test_resolve_loop(self): def test_resolve_loop(self):
if not self.can_symlink: if not self.can_symlink:
self.skipTest("symlinks required") 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") self.skipTest("symlink loops work differently with concrete Windows paths")
# Loops with relative symlinks. # Loops with relative symlinks.
self.cls(self.base, 'linkX').symlink_to('linkX/inside') self.cls(self.base, 'linkX').symlink_to('linkX/inside')
@ -1657,10 +1624,11 @@ def _check_complex_symlinks(self, link0_target):
self.skipTest("symlinks required") self.skipTest("symlinks required")
# Test solving a non-looping chain of symlinks (issue #19887). # Test solving a non-looping chain of symlinks (issue #19887).
pathmod = self.pathmod
P = self.cls(self.base) P = self.cls(self.base)
P.joinpath('link1').symlink_to(os.path.join('link0', 'link0'), target_is_directory=True) P.joinpath('link1').symlink_to(pathmod.join('link0', 'link0'), target_is_directory=True)
P.joinpath('link2').symlink_to(os.path.join('link1', 'link1'), target_is_directory=True) P.joinpath('link2').symlink_to(pathmod.join('link1', 'link1'), target_is_directory=True)
P.joinpath('link3').symlink_to(os.path.join('link2', 'link2'), 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) P.joinpath('link0').symlink_to(link0_target, target_is_directory=True)
# Resolve absolute paths. # Resolve absolute paths.
@ -1707,7 +1675,7 @@ def test_complex_symlinks_relative(self):
self._check_complex_symlinks('.') self._check_complex_symlinks('.')
def test_complex_symlinks_relative_dot_dot(self): 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): def setUpWalk(self):
# Build: # Build: