bpo-22276: Fix pathlib.Path.glob not to ignore trailing path separator

[e-kwsm]

See also Issue 33392: pathlib .glob('*/') returns files as well as directories, closed as duplicate of 22276.

At present pathlib.Path.glob returns directories and FILES even if glob pattern ends with the path separator (/ in Linux), while glob.glob works as expected.
This PR fixes the behaviour.

$ uname -s
Linux
$ mkdir /tmp/test-glob
$ cd /tmp/test-glob
$ mkdir dir
$ touch file
$ echo *  # shell glob
dir file
$ echo */
dir/

>>> from glob import glob
>>> glob("*")
['file', 'dir']
>>> glob("*/")
['dir/']
>>> 
>>> from pathlib import Path
>>> list(Path(".").glob("*"))
[PosixPath('file'), PosixPath('dir')]
>>> list(Path(".").glob("*/"))
[PosixPath('file'), PosixPath('dir')]

https://bugs.python.org/issue22276

[serhiy-storchaka]

Needed tests, the documentation update, a news and What's New entries.

And don't forget about an alternate separator on Windows.

[bedevere-bot]

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

[e-kwsm]

I didn't expect the Spanish Inquisition.

[bedevere-bot]

Nobody expects the Spanish Inquisition!

@serhiy-storchaka: please review the changes made to this pull request.

[serhiy-storchaka]

I think it is better to use sep and altsep (and corresponding pathlib attributes) instead of hardcoding '/' in the general code.

[e-kwsm]

@serhiy-storchaka Thank you for your advice.

[e-kwsm]

BTW, I address bpo-33392 (Path.glob and Path.rglob) rather than [bpo-22276](https://bugs.python.org/issue) (PurePath.match) in this PR; the former is closed as duplicate of the latter.

[e-kwsm]

ping

[brettcannon]

/cc @barneygale

[barneygale]

Hey @e-kwsm, sorry for the long wait. Are you still interested in working on this PR? If so I can help review.

[e-kwsm]

Are you still interested in working on this PR?

Yes.

[ghost]

All commit authors signed the Contributor License Agreement.

[arhadthedev]

@e-kwsm The bot is broken and is being fixed so no worries.

[barneygale]

I don't think the code should make additional is_dir() calls, as those cause a stat() under the hood. The "selector" classes already have this information, it's just a case of feeding it through to the _TerminatingSelector. What do you think @e-kwsm?

[serhiy-storchaka]

You can also press "Update branch" to trigger testing.

[serhiy-storchaka]

I propose simpler patch:

diff --git a/Lib/pathlib.py b/Lib/pathlib.py
index 4763ab54f6..a759db1684 100644
--- a/Lib/pathlib.py
+++ b/Lib/pathlib.py
@@ -281,7 +281,9 @@ def make_uri(self, path):
 def _make_selector(pattern_parts, flavour):
     pat = pattern_parts[0]
     child_parts = pattern_parts[1:]
-    if pat == '**':
+    if not pat:
+        return _TerminatingSelector()
+    elif pat == '**':
         cls = _RecursiveWildcardSelector
     elif '**' in pat:
         raise ValueError("Invalid pattern: '**' can only be an entire path component")
@@ -943,6 +945,8 @@ def glob(self, pattern):
         drv, root, pattern_parts = self._flavour.parse_parts((pattern,))
         if drv or root:
             raise NotImplementedError("Non-relative patterns are unsupported")
+        if pattern[-1] in (self._flavour.sep, self._flavour.altsep):
+            pattern_parts.append('')
         selector = _make_selector(tuple(pattern_parts), self._flavour)
         for p in selector.select_from(self):
             yield p
@@ -956,6 +960,8 @@ def rglob(self, pattern):
         drv, root, pattern_parts = self._flavour.parse_parts((pattern,))
         if drv or root:
             raise NotImplementedError("Non-relative patterns are unsupported")
+        if pattern[-1] in (self._flavour.sep, self._flavour.altsep):
+            pattern_parts.append('')
         selector = _make_selector(("**",) + tuple(pattern_parts), self._flavour)
         for p in selector.select_from(self):
             yield p

[brettcannon]

You can also press "Update branch" to trigger testing.

You're right, but I was also trying to get rid of the old CLA check.

[e-kwsm]

@serhiy-storchaka Thank you for your review. I have reflected your suggestion.

[barneygale]

LGTM

[brettcannon]

@e-kwsm would you mind signing our new digital CLA (see the "details" link in the failing status check)?

[e-kwsm]

would you mind signing our new digital CLA (see the "details" link in the failing status check)?

Done.

[brettcannon]

Thanks!