Skip to content
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.

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

gh-129350: Make tests for glob with trailing slash more strict #129376

Merged

Conversation

serhiy-storchaka
Copy link
Member

@serhiy-storchaka serhiy-storchaka commented Jan 27, 2025

Test that the trailing pathname separator is preserved.

Multiple trailing pathname separators are only preserved if the pattern does not contain metacharacters, otherwise only one trailing pathname separator is preserved. This is rather an implementation detail.

Test that the trailing pathname separator is preserved.

Multiple trailing pathname separators are only preserved if the pattern
does not contain metacharacters, otherwise only one trailing pathname
separator is preserved. This is rather an implementation detail.
Copy link
Contributor

@barneygale barneygale left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Interesting that the results for patterns ending sep*2 differ depending on whether the preceding part involves a wildcard or not - is there another bug there do you think?

@encukou
Copy link
Member

encukou commented Jan 28, 2025

Normalizing separators to os.sep is desirable, right?
Maybe use self.subTest for the for loop.

@serhiy-storchaka
Copy link
Member Author

Stripping or preserving the redundant pathname separators is an implementation detail. User code can rely on that details, so it is better to test them to avoid unintentional changes.

There are other implementation details (normalizing the separator on Windows, the case of the names on Windows). There is an open issue for some of them, but I do not think that we can do something without sacrificing performance or breaking user code.

I'll add comments clarifying what is an implementation detail.

@barneygale
Copy link
Contributor

It strikes me as an (extremely minor) bug rather than an implementation detail, especially as it differs from shell globbing behaviour. But probably not worth fixing.

@serhiy-storchaka
Copy link
Member Author

This is the same in shell:

$ echo /usr/bin//
/usr/bin//
$ echo /usr/b*//
/usr/bin/
$ echo /u*/bin//
/usr/bin/

@barneygale
Copy link
Contributor

This is the same in shell:

$ echo /usr/bin//
/usr/bin//
$ echo /usr/b*//
/usr/bin/
$ echo /u*/bin//
/usr/bin/

There are some cases that aren't consistent:

$ echo /usr//b*
/usr//bin
$ python3 -c 'import glob; print(glob.glob("/usr//b*"))'
['/usr/bin']

@encukou
Copy link
Member

encukou commented Jan 28, 2025

Which shell?

$ fish
Welcome to fish, the friendly interactive shell
Type help for instructions on how to use fish
encukou@blackbox ~> echo /usr//b*
/usr/bin

@serhiy-storchaka
Copy link
Member Author

This is a different implementation detail, not related to this PR. For /usr//bin//p?thon3, the shell preserves both double slashes, Python -- only the first one. It is related to the implementation of path manipulating routines (os.path.split() and os.path.join()) in Python and analogues in the shell. pathlib.Path does not preserve double slashes at all.

@serhiy-storchaka serhiy-storchaka merged commit 8b5c850 into python:main Feb 4, 2025
37 checks passed
@miss-islington-app
Copy link

Thanks @serhiy-storchaka for the PR 🌮🎉.. I'm working now to backport this PR to: 3.12, 3.13.
🐍🍒⛏🤖

@serhiy-storchaka serhiy-storchaka deleted the test_glob_trailing_slash branch February 4, 2025 14:24
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Feb 4, 2025
…ythonGH-129376)

Test that the trailing pathname separator is preserved.

Multiple trailing pathname separators are only preserved if the pattern
does not contain metacharacters, otherwise only one trailing pathname
separator is preserved. This is rather an implementation detail.
(cherry picked from commit 8b5c850)

Co-authored-by: Serhiy Storchaka <[email protected]>
@bedevere-app
Copy link

bedevere-app bot commented Feb 4, 2025

GH-129651 is a backport of this pull request to the 3.13 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.13 bugs and security fixes label Feb 4, 2025
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Feb 4, 2025
…ythonGH-129376)

Test that the trailing pathname separator is preserved.

Multiple trailing pathname separators are only preserved if the pattern
does not contain metacharacters, otherwise only one trailing pathname
separator is preserved. This is rather an implementation detail.
(cherry picked from commit 8b5c850)

Co-authored-by: Serhiy Storchaka <[email protected]>
@bedevere-app
Copy link

bedevere-app bot commented Feb 4, 2025

GH-129652 is a backport of this pull request to the 3.12 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.12 bug and security fixes label Feb 4, 2025
serhiy-storchaka added a commit that referenced this pull request Feb 4, 2025
…GH-129376) (GH-129652)

Test that the trailing pathname separator is preserved.

Multiple trailing pathname separators are only preserved if the pattern
does not contain metacharacters, otherwise only one trailing pathname
separator is preserved. This is rather an implementation detail.
(cherry picked from commit 8b5c850)

Co-authored-by: Serhiy Storchaka <[email protected]>
serhiy-storchaka added a commit that referenced this pull request Feb 4, 2025
…GH-129376) (GH-129651)

Test that the trailing pathname separator is preserved.

Multiple trailing pathname separators are only preserved if the pattern
does not contain metacharacters, otherwise only one trailing pathname
separator is preserved. This is rather an implementation detail.
(cherry picked from commit 8b5c850)

Co-authored-by: Serhiy Storchaka <[email protected]>
srinivasreddy pushed a commit to srinivasreddy/cpython that referenced this pull request Feb 7, 2025
…ythonGH-129376)

Test that the trailing pathname separator is preserved.

Multiple trailing pathname separators are only preserved if the pattern
does not contain metacharacters, otherwise only one trailing pathname
separator is preserved. This is rather an implementation detail.
cmaloney pushed a commit to cmaloney/cpython that referenced this pull request Feb 8, 2025
…ythonGH-129376)

Test that the trailing pathname separator is preserved.

Multiple trailing pathname separators are only preserved if the pattern
does not contain metacharacters, otherwise only one trailing pathname
separator is preserved. This is rather an implementation detail.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip news tests Tests in the Lib/test dir
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants