Skip to content

Commit

Permalink
Deprecate follow_symlinks=None
Browse files Browse the repository at this point in the history
  • Loading branch information
barneygale committed Mar 14, 2023
1 parent b2766f8 commit 8704d33
Show file tree
Hide file tree
Showing 3 changed files with 96 additions and 67 deletions.
20 changes: 14 additions & 6 deletions Doc/library/pathlib.rst
Original file line number Diff line number Diff line change
Expand Up @@ -873,9 +873,9 @@ call fails (for example because the path doesn't exist).
PosixPath('setup.py'),
PosixPath('test_pathlib.py')]

By default, :meth:`Path.glob` follows symlinks except when expanding
"``**``" wildcards. Set *follow_symlinks* to true to always follow
symlinks, or false to treat all symlinks as files.
By default, :meth:`Path.glob` emits a deprecation warning and follows
symlinks except when expanding "``**``" wildcards. Set *follow_symlinks*
to true to always follow symlinks, or false to treat all symlinks as files.

.. note::
Using the "``**``" pattern in large directory trees may consume
Expand All @@ -890,6 +890,10 @@ call fails (for example because the path doesn't exist).
.. versionchanged:: 3.12
The *follow_symlinks* parameter was added.

.. deprecated-removed:: 3.12 3.14

Setting *follow_symlinks* to ``None`` (e.g. by omitting it) is deprecated.

.. method:: Path.group()

Return the name of the group owning the file. :exc:`KeyError` is raised
Expand Down Expand Up @@ -1288,9 +1292,9 @@ call fails (for example because the path doesn't exist).
PosixPath('setup.py'),
PosixPath('test_pathlib.py')]

By default, :meth:`Path.rglob` follows symlinks except when expanding
"``**``" wildcards. Set *follow_symlinks* to true to always follow
symlinks, or false to treat all symlinks as files.
By default, :meth:`Path.rglob` emits a deprecation warning and follows
symlinks except when expanding "``**``" wildcards. Set *follow_symlinks*
to true to always follow symlinks, or false to treat all symlinks as files.

.. audit-event:: pathlib.Path.rglob self,pattern pathlib.Path.rglob

Expand All @@ -1301,6 +1305,10 @@ call fails (for example because the path doesn't exist).
.. versionchanged:: 3.12
The *follow_symlinks* parameter was added.

.. deprecated-removed:: 3.12 3.14

Setting *follow_symlinks* to ``None`` (e.g. by omitting it) is deprecated.

.. method:: Path.rmdir()

Remove this directory. The directory must be empty.
Expand Down
14 changes: 14 additions & 0 deletions Lib/pathlib.py
Original file line number Diff line number Diff line change
Expand Up @@ -755,6 +755,13 @@ def glob(self, pattern, *, follow_symlinks=None):
kind, including directories) matching the given relative pattern.
"""
sys.audit("pathlib.Path.glob", self, pattern)
if follow_symlinks is None:
msg = ("pathlib.Path.glob(pattern, follow_symlinks=None) is "
"deprecated and scheduled for removal in Python {remove}. "
"The follow_symlinks keyword-only argument should be set "
"to either True or False.")
warnings._deprecated("pathlib.Path.glob(pattern, follow_symlinks=None)",
msg, remove=(3, 14))
if not pattern:
raise ValueError("Unacceptable pattern: {!r}".format(pattern))
drv, root, pattern_parts = self._parse_parts((pattern,))
Expand All @@ -772,6 +779,13 @@ def rglob(self, pattern, *, follow_symlinks=None):
this subtree.
"""
sys.audit("pathlib.Path.rglob", self, pattern)
if follow_symlinks is None:
msg = ("pathlib.Path.rglob(pattern, follow_symlinks=None) is "
"deprecated and scheduled for removal in Python {remove}. "
"The follow_symlinks keyword-only argument should be set "
"to either True or False.")
warnings._deprecated("pathlib.Path.rglob(pattern, follow_symlinks=None)", msg,
remove=(3, 14))
drv, root, pattern_parts = self._parse_parts((pattern,))
if drv or root:
raise NotImplementedError("Non-relative patterns are unsupported")
Expand Down
129 changes: 68 additions & 61 deletions Lib/test/test_pathlib.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import tempfile
import unittest
from unittest import mock
import warnings

from test.support import import_helper
from test.support import is_emscripten, is_wasi
Expand Down Expand Up @@ -1749,39 +1750,42 @@ def test_iterdir_nodir(self):
errno.ENOENT, errno.EINVAL))

def test_glob_common(self):
def _check(glob, expected):
self.assertEqual(set(glob), { P(BASE, q) for q in expected })
def _check(path, glob, expected):
with self.assertWarns(DeprecationWarning):
actual = {q for q in path.glob(glob)}
self.assertEqual(actual, { P(BASE, q) for q in expected })
P = self.cls
p = P(BASE)
it = p.glob("fileA")
self.assertIsInstance(it, collections.abc.Iterator)
_check(it, ["fileA"])
_check(p.glob("fileB"), [])
_check(p.glob("dir*/file*"), ["dirB/fileB", "dirC/fileC"])
with self.assertWarns(DeprecationWarning):
it = p.glob("fileA")
self.assertIsInstance(it, collections.abc.Iterator)
self.assertEqual(set(it), { P(BASE, "fileA") })
_check(p, "fileB", [])
_check(p, "dir*/file*", ["dirB/fileB", "dirC/fileC"])
if not os_helper.can_symlink():
_check(p.glob("*A"), ['dirA', 'fileA'])
_check(p, "*A", ['dirA', 'fileA'])
else:
_check(p.glob("*A"), ['dirA', 'fileA', 'linkA'])
_check(p, "*A", ['dirA', 'fileA', 'linkA'])
if not os_helper.can_symlink():
_check(p.glob("*B/*"), ['dirB/fileB'])
_check(p, "*B/*", ['dirB/fileB'])
else:
_check(p.glob("*B/*"), ['dirB/fileB', 'dirB/linkD',
_check(p, "*B/*", ['dirB/fileB', 'dirB/linkD',
'linkB/fileB', 'linkB/linkD'])
if not os_helper.can_symlink():
_check(p.glob("*/fileB"), ['dirB/fileB'])
_check(p, "*/fileB", ['dirB/fileB'])
else:
_check(p.glob("*/fileB"), ['dirB/fileB', 'linkB/fileB'])
_check(p, "*/fileB", ['dirB/fileB', 'linkB/fileB'])

if not os_helper.can_symlink():
_check(p.glob("*/"), ["dirA", "dirB", "dirC", "dirE"])
_check(p, "*/", ["dirA", "dirB", "dirC", "dirE"])
else:
_check(p.glob("*/"), ["dirA", "dirB", "dirC", "dirE", "linkB"])
_check(p, "*/", ["dirA", "dirB", "dirC", "dirE", "linkB"])

@os_helper.skip_unless_symlink
def test_glob_follow_symlinks_common(self):
def _check(path, glob, expected):
actual = {path for path in path.glob(glob, follow_symlinks=True)
if "linkD" not in path.parent.parts} # exclude symlink loop.
actual = {q for q in path.glob(glob, follow_symlinks=True)
if "linkD" not in q.parent.parts} # exclude symlink loop.
self.assertEqual(actual, { P(BASE, q) for q in expected })
P = self.cls
p = P(BASE)
Expand All @@ -1795,7 +1799,7 @@ def _check(path, glob, expected):
@os_helper.skip_unless_symlink
def test_glob_no_follow_symlinks_common(self):
def _check(path, glob, expected):
actual = {path for path in path.glob(glob, follow_symlinks=False)}
actual = {q for q in path.glob(glob, follow_symlinks=False)}
self.assertEqual(actual, { P(BASE, q) for q in expected })
P = self.cls
p = P(BASE)
Expand All @@ -1807,43 +1811,46 @@ def _check(path, glob, expected):
_check(p, "*/", ["dirA", "dirB", "dirC", "dirE"])

def test_rglob_common(self):
def _check(glob, expected):
self.assertEqual(set(glob), { P(BASE, q) for q in expected })
def _check(path, glob, expected):
with self.assertWarns(DeprecationWarning):
actual = {q for q in path.rglob(glob)}
self.assertEqual(actual, { P(BASE, q) for q in expected })
P = self.cls
p = P(BASE)
it = p.rglob("fileA")
self.assertIsInstance(it, collections.abc.Iterator)
_check(it, ["fileA"])
_check(p.rglob("fileB"), ["dirB/fileB"])
_check(p.rglob("*/fileA"), [])
with self.assertWarns(DeprecationWarning):
it = p.rglob("fileA")
self.assertIsInstance(it, collections.abc.Iterator)
self.assertEqual(set(it), { P(BASE, "fileA") })
_check(p, "fileB", ["dirB/fileB"])
_check(p, "*/fileA", [])
if not os_helper.can_symlink():
_check(p.rglob("*/fileB"), ["dirB/fileB"])
_check(p, "*/fileB", ["dirB/fileB"])
else:
_check(p.rglob("*/fileB"), ["dirB/fileB", "dirB/linkD/fileB",
_check(p, "*/fileB", ["dirB/fileB", "dirB/linkD/fileB",
"linkB/fileB", "dirA/linkC/fileB"])
_check(p.rglob("file*"), ["fileA", "dirB/fileB",
_check(p, "file*", ["fileA", "dirB/fileB",
"dirC/fileC", "dirC/dirD/fileD"])
if not os_helper.can_symlink():
_check(p.rglob("*/"), [
_check(p, "*/", [
"dirA", "dirB", "dirC", "dirC/dirD", "dirE",
])
else:
_check(p.rglob("*/"), [
_check(p, "*/", [
"dirA", "dirA/linkC", "dirB", "dirB/linkD", "dirC",
"dirC/dirD", "dirE", "linkB",
])
_check(p.rglob(""), ["", "dirA", "dirB", "dirC", "dirE", "dirC/dirD"])
_check(p, "", ["", "dirA", "dirB", "dirC", "dirE", "dirC/dirD"])

p = P(BASE, "dirC")
_check(p.rglob("*"), ["dirC/fileC", "dirC/novel.txt",
_check(p, "*", ["dirC/fileC", "dirC/novel.txt",
"dirC/dirD", "dirC/dirD/fileD"])
_check(p.rglob("file*"), ["dirC/fileC", "dirC/dirD/fileD"])
_check(p.rglob("*/*"), ["dirC/dirD/fileD"])
_check(p.rglob("*/"), ["dirC/dirD"])
_check(p.rglob(""), ["dirC", "dirC/dirD"])
_check(p, "file*", ["dirC/fileC", "dirC/dirD/fileD"])
_check(p, "*/*", ["dirC/dirD/fileD"])
_check(p, "*/", ["dirC/dirD"])
_check(p, "", ["dirC", "dirC/dirD"])
# gh-91616, a re module regression
_check(p.rglob("*.txt"), ["dirC/novel.txt"])
_check(p.rglob("*.*"), ["dirC/novel.txt"])
_check(p, "*.txt", ["dirC/novel.txt"])
_check(p, "*.*", ["dirC/novel.txt"])

@os_helper.skip_unless_symlink
def test_rglob_follow_symlinks_common(self):
Expand Down Expand Up @@ -1904,7 +1911,7 @@ def test_rglob_symlink_loop(self):
# Don't get fooled by symlink loops (Issue #26012).
P = self.cls
p = P(BASE)
given = set(p.rglob('*'))
given = set(p.rglob('*', follow_symlinks=False))
expect = {'brokenLink',
'dirA', 'dirA/linkC',
'dirB', 'dirB/fileB', 'dirB/linkD',
Expand All @@ -1925,10 +1932,10 @@ def test_glob_many_open_files(self):
p = P(base, *(['d']*depth))
p.mkdir(parents=True)
pattern = '/'.join(['*'] * depth)
iters = [base.glob(pattern) for j in range(100)]
iters = [base.glob(pattern, follow_symlinks=True) for j in range(100)]
for it in iters:
self.assertEqual(next(it), p)
iters = [base.rglob('d') for j in range(100)]
iters = [base.rglob('d', follow_symlinks=True) for j in range(100)]
p = base
for i in range(depth):
p = p / 'd'
Expand All @@ -1939,9 +1946,9 @@ def test_glob_dotdot(self):
# ".." is not special in globs.
P = self.cls
p = P(BASE)
self.assertEqual(set(p.glob("..")), { P(BASE, "..") })
self.assertEqual(set(p.glob("dirA/../file*")), { P(BASE, "dirA/../fileA") })
self.assertEqual(set(p.glob("../xyzzy")), set())
self.assertEqual(set(p.glob("..", follow_symlinks=True)), { P(BASE, "..") })
self.assertEqual(set(p.glob("dirA/../file*", follow_symlinks=True)), { P(BASE, "dirA/../fileA") })
self.assertEqual(set(p.glob("../xyzzy", follow_symlinks=True)), set())

@os_helper.skip_unless_symlink
def test_glob_permissions(self):
Expand Down Expand Up @@ -1971,11 +1978,11 @@ def my_scandir(path):
return contextlib.nullcontext(entries)

with mock.patch("os.scandir", my_scandir):
self.assertEqual(len(set(base.glob("*"))), 3)
self.assertEqual(len(set(base.glob("*", follow_symlinks=True))), 3)
subdir.mkdir()
self.assertEqual(len(set(base.glob("*"))), 4)
self.assertEqual(len(set(base.glob("*", follow_symlinks=True))), 4)
subdir.chmod(000)
self.assertEqual(len(set(base.glob("*"))), 4)
self.assertEqual(len(set(base.glob("*", follow_symlinks=True))), 4)

def _check_resolve(self, p, expected, strict=True):
q = p.resolve(strict)
Expand Down Expand Up @@ -2894,7 +2901,7 @@ def test_unsupported_flavour(self):
def test_glob_empty_pattern(self):
p = self.cls()
with self.assertRaisesRegex(ValueError, 'Unacceptable pattern'):
list(p.glob(''))
list(p.glob('', follow_symlinks=True))


@only_posix
Expand Down Expand Up @@ -2987,18 +2994,18 @@ def test_resolve_loop(self):
def test_glob(self):
P = self.cls
p = P(BASE)
given = set(p.glob("FILEa"))
given = set(p.glob("FILEa", follow_symlinks=True))
expect = set() if not os_helper.fs_is_case_insensitive(BASE) else given
self.assertEqual(given, expect)
self.assertEqual(set(p.glob("FILEa*")), set())
self.assertEqual(set(p.glob("FILEa*", follow_symlinks=True)), set())

def test_rglob(self):
P = self.cls
p = P(BASE, "dirC")
given = set(p.rglob("FILEd"))
given = set(p.rglob("FILEd", follow_symlinks=True))
expect = set() if not os_helper.fs_is_case_insensitive(BASE) else given
self.assertEqual(given, expect)
self.assertEqual(set(p.rglob("FILEd*")), set())
self.assertEqual(set(p.rglob("FILEd*", follow_symlinks=True)), set())

@unittest.skipUnless(hasattr(pwd, 'getpwall'),
'pwd module does not expose getpwall()')
Expand Down Expand Up @@ -3061,7 +3068,7 @@ def test_expanduser(self):
"Bad file descriptor in /dev/fd affects only macOS")
def test_handling_bad_descriptor(self):
try:
file_descriptors = list(pathlib.Path('/dev/fd').rglob("*"))[3:]
file_descriptors = list(pathlib.Path('/dev/fd').rglob("*", follow_symlinks=True))[3:]
if not file_descriptors:
self.skipTest("no file descriptors - issue was not reproduced")
# Checking all file descriptors because there is no guarantee
Expand Down Expand Up @@ -3133,18 +3140,18 @@ def test_absolute(self):
def test_glob(self):
P = self.cls
p = P(BASE)
self.assertEqual(set(p.glob("FILEa")), { P(BASE, "fileA") })
self.assertEqual(set(p.glob("*a\\")), { P(BASE, "dirA") })
self.assertEqual(set(p.glob("F*a")), { P(BASE, "fileA") })
self.assertEqual(set(map(str, p.glob("FILEa"))), {f"{p}\\fileA"})
self.assertEqual(set(map(str, p.glob("F*a"))), {f"{p}\\fileA"})
self.assertEqual(set(p.glob("FILEa", follow_symlinks=True)), { P(BASE, "fileA") })
self.assertEqual(set(p.glob("*a\\", follow_symlinks=True)), { P(BASE, "dirA") })
self.assertEqual(set(p.glob("F*a", follow_symlinks=True)), { P(BASE, "fileA") })
self.assertEqual(set(map(str, p.glob("FILEa", follow_symlinks=True))), {f"{p}\\fileA"})
self.assertEqual(set(map(str, p.glob("F*a", follow_symlinks=True))), {f"{p}\\fileA"})

def test_rglob(self):
P = self.cls
p = P(BASE, "dirC")
self.assertEqual(set(p.rglob("FILEd")), { P(BASE, "dirC/dirD/fileD") })
self.assertEqual(set(p.rglob("*\\")), { P(BASE, "dirC/dirD") })
self.assertEqual(set(map(str, p.rglob("FILEd"))), {f"{p}\\dirD\\fileD"})
self.assertEqual(set(p.rglob("FILEd", follow_symlinks=True)), { P(BASE, "dirC/dirD/fileD") })
self.assertEqual(set(p.rglob("*\\", follow_symlinks=True)), { P(BASE, "dirC/dirD") })
self.assertEqual(set(map(str, p.rglob("FILEd", follow_symlinks=True))), {f"{p}\\dirD\\fileD"})

def test_expanduser(self):
P = self.cls
Expand Down

0 comments on commit 8704d33

Please sign in to comment.