From 28830960355882551d98d812d183d00cb20c94c2 Mon Sep 17 00:00:00 2001 From: Daniel Hahler Date: Thu, 29 Oct 2020 12:57:29 +0100 Subject: [PATCH 1/2] pathlib: _Selector: factor out _skip_entry Refs: - bpo-24120 (https://github.com/python/cpython/commit/6c2d33a258bb) uses to broad try/catch for PermissionError, which led to bpo-38894 in the first place - bpo-38894 (https://github.com/python/cpython/pull/18815) fixed handling only for `_WildcardSelector` TODO: tests for others? --- Lib/pathlib.py | 89 +++++++++++++++++++++++++------------------------- 1 file changed, 45 insertions(+), 44 deletions(-) diff --git a/Lib/pathlib.py b/Lib/pathlib.py index 178c5b981d8e50..d207610c6e2a9a 100644 --- a/Lib/pathlib.py +++ b/Lib/pathlib.py @@ -502,6 +502,21 @@ def __init__(self, child_parts, flavour): self.successor = _TerminatingSelector() self.dironly = False + def _skip_entry(self, entry): + if self.dironly: + try: + return not entry.is_dir() + except PermissionError: + # "entry.is_dir()" can raise PermissionError + # in some cases (see bpo-38894), which is not + # among the errors ignored by _ignore_error(). + return True + except OSError as e: + if not _ignore_error(e): + raise + return True + return False + def select_from(self, parent_path): """Iterate over all child paths of `parent_path` matched by this selector. This can contain parent_path itself.""" @@ -527,14 +542,16 @@ def __init__(self, name, child_parts, flavour): _Selector.__init__(self, child_parts, flavour) def _select_from(self, parent_path, is_dir, exists, scandir): + path = parent_path._make_child_relpath(self.name) try: - path = parent_path._make_child_relpath(self.name) - if (is_dir if self.dironly else exists)(path): - for p in self.successor._select_from(path, is_dir, exists, scandir): - yield p + if not (is_dir if self.dironly else exists)(path): + return except PermissionError: return + for p in self.successor._select_from(path, is_dir, exists, scandir): + yield p + class _WildcardSelector(_Selector): @@ -546,25 +563,16 @@ def _select_from(self, parent_path, is_dir, exists, scandir): try: with scandir(parent_path) as scandir_it: entries = list(scandir_it) - for entry in entries: - if self.dironly: - try: - # "entry.is_dir()" can raise PermissionError - # in some cases (see bpo-38894), which is not - # among the errors ignored by _ignore_error() - if not entry.is_dir(): - continue - except OSError as e: - if not _ignore_error(e): - raise - continue - name = entry.name - if self.match(name): - path = parent_path._make_child_relpath(name) - for p in self.successor._select_from(path, is_dir, exists, scandir): - yield p except PermissionError: return + for entry in entries: + if self._skip_entry(entry): + continue + name = entry.name + if self.match(name): + path = parent_path._make_child_relpath(name) + for p in self.successor._select_from(path, is_dir, exists, scandir): + yield p class _RecursiveWildcardSelector(_Selector): @@ -577,34 +585,27 @@ def _iterate_directories(self, parent_path, is_dir, scandir): try: with scandir(parent_path) as scandir_it: entries = list(scandir_it) - for entry in entries: - entry_is_dir = False - try: - entry_is_dir = entry.is_dir() - except OSError as e: - if not _ignore_error(e): - raise - if entry_is_dir and not entry.is_symlink(): - path = parent_path._make_child_relpath(entry.name) - for p in self._iterate_directories(path, is_dir, scandir): - yield p except PermissionError: return + for entry in entries: + if self._skip_entry(entry): + continue + if not entry.is_symlink(): + path = parent_path._make_child_relpath(entry.name) + for p in self._iterate_directories(path, is_dir, scandir): + yield p def _select_from(self, parent_path, is_dir, exists, scandir): + yielded = set() try: - yielded = set() - try: - successor_select = self.successor._select_from - for starting_point in self._iterate_directories(parent_path, is_dir, scandir): - for p in successor_select(starting_point, is_dir, exists, scandir): - if p not in yielded: - yield p - yielded.add(p) - finally: - yielded.clear() - except PermissionError: - return + successor_select = self.successor._select_from + for starting_point in self._iterate_directories(parent_path, is_dir, scandir): + for p in successor_select(starting_point, is_dir, exists, scandir): + if p not in yielded: + yield p + yielded.add(p) + finally: + yielded.clear() # From ff099bc51227bda527e8dade45168cee74105e60 Mon Sep 17 00:00:00 2001 From: Daniel Hahler Date: Thu, 29 Oct 2020 13:03:46 +0100 Subject: [PATCH 2/2] use "yield from" --- Lib/pathlib.py | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/Lib/pathlib.py b/Lib/pathlib.py index d207610c6e2a9a..339b70af20a370 100644 --- a/Lib/pathlib.py +++ b/Lib/pathlib.py @@ -548,9 +548,7 @@ def _select_from(self, parent_path, is_dir, exists, scandir): return except PermissionError: return - - for p in self.successor._select_from(path, is_dir, exists, scandir): - yield p + yield from self.successor._select_from(path, is_dir, exists, scandir) class _WildcardSelector(_Selector): @@ -571,8 +569,7 @@ def _select_from(self, parent_path, is_dir, exists, scandir): name = entry.name if self.match(name): path = parent_path._make_child_relpath(name) - for p in self.successor._select_from(path, is_dir, exists, scandir): - yield p + yield from self.successor._select_from(path, is_dir, exists, scandir) class _RecursiveWildcardSelector(_Selector): @@ -592,8 +589,7 @@ def _iterate_directories(self, parent_path, is_dir, scandir): continue if not entry.is_symlink(): path = parent_path._make_child_relpath(entry.name) - for p in self._iterate_directories(path, is_dir, scandir): - yield p + yield from self._iterate_directories(path, is_dir, scandir) def _select_from(self, parent_path, is_dir, exists, scandir): yielded = set() @@ -1164,8 +1160,7 @@ def glob(self, pattern): if drv or root: raise NotImplementedError("Non-relative patterns are unsupported") selector = _make_selector(tuple(pattern_parts), self._flavour) - for p in selector.select_from(self): - yield p + yield from selector.select_from(self) def rglob(self, pattern): """Recursively yield all existing files (of any kind, including @@ -1177,8 +1172,7 @@ def rglob(self, pattern): if drv or root: raise NotImplementedError("Non-relative patterns are unsupported") selector = _make_selector(("**",) + tuple(pattern_parts), self._flavour) - for p in selector.select_from(self): - yield p + yield from selector.select_from(self) def absolute(self): """Return an absolute version of this path. This function works