Skip to content

GH-77609: Add follow_symlinks argument to pathlib.Path.glob() #102616

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
barneygale merged 17 commits into from
May 29, 2023
Merged

GH-77609: Add follow_symlinks argument to pathlib.Path.glob() #102616

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
59dcdb1
GH-77609: Support following symlinks in `pathlib.Path.glob()`
barneygale Mar 12, 2023
e850bde
Fix top-level error handling.
barneygale Mar 12, 2023
4f4ffd3
Always follow top-level symlink
barneygale Mar 12, 2023
b6c019e
Simplify test diff slightly
barneygale Mar 12, 2023
8d657ee
Clarify docs for *follow_symlinks* argument
barneygale Mar 14, 2023
b600363
Backwards compatibility
barneygale Mar 14, 2023
b2766f8
Fix handling of '..' in patterns.
barneygale Mar 14, 2023
8704d33
Deprecate follow_symlinks=None
barneygale Mar 14, 2023
6f7a83a
Fix news
barneygale Mar 14, 2023
5fb7d28
Merge branch 'main' into gh-77609-glob-follow-symlinks
barneygale May 3, 2023
ce7973f
Revert "Fix news"
barneygale May 10, 2023
2db28d9
Revert "Deprecate follow_symlinks=None"
barneygale May 10, 2023
d3a3396
Merge branch 'main' into gh-77609-glob-follow-symlinks
barneygale May 10, 2023
6c61822
Merge branch 'main' into gh-77609-glob-follow-symlinks
barneygale May 11, 2023
3722b64
Merge branch 'main' into gh-77609-glob-follow-symlinks
barneygale May 15, 2023
9700dd1
Merge branch 'main' into gh-77609-glob-follow-symlinks
barneygale May 23, 2023
4f2a492
Re-target to 3.13
barneygale May 23, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Deprecate follow_symlinks=None
  • Loading branch information
@barneygale
barneygale committed Mar 14, 2023
commit 8704d33e0d36ba895cf540a3a49c1984c49ac133
20 changes: 14 additions & 6 deletions Doc/library/pathlib.rst
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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