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

glob.glob("**", root_dir=root, recursive=True) doesn't return root #116320

Closed
barneygale opened this issue Mar 4, 2024 · 7 comments
Closed

glob.glob("**", root_dir=root, recursive=True) doesn't return root #116320

barneygale opened this issue Mar 4, 2024 · 7 comments
Labels
type-bug An unexpected behavior, bug, or error

Comments

@barneygale
Copy link
Contributor

barneygale commented Mar 4, 2024

Bug report

Bug description:

$ mkdir my_dir
$ touch my_dir/my_file
>>> import glob
>>> glob.glob('my_dir/**', recursive=True)
['my_dir/', 'my_dir/my_file']
>>> glob.glob('**', root_dir='my_dir/', recursive=True)
['my_file']  # should include './'

I think the ** wildcard should match any number of file or directory segments, including zero. And it usually does! Unless the pattern begins with **, in which case this wildcard seems to match at least one file or directory segments.

CPython versions tested on:

CPython main branch

Operating systems tested on:

Linux

@barneygale barneygale added the type-bug An unexpected behavior, bug, or error label Mar 4, 2024
@Privat33r-dev
Copy link
Contributor

Privat33r-dev commented Mar 4, 2024

>>> glob.glob('**', root_dir='my_dir/', recursive=True)
['./', './my_file']

If recursive is true, the pattern “**” will match any files and zero or more directories, subdirectories and symbolic links to directories

It functions according to Docs. In case if you prefix ** with ./, it will return ./ as well. I find it useful for the developers to have a choice, not everyone would want a current directory in a list of files.

Btw, it seems to be related to #106747

@barneygale
Copy link
Contributor Author

I find it useful for the developers to have a choice, not everyone would want a current directory in a list of files.

If you want at least one segment, you can spell that with **/*, right?

@Privat33r-dev
Copy link
Contributor

Privat33r-dev commented Mar 4, 2024

I find it useful for the developers to have a choice, not everyone would want a current directory in a list of files.

If you want at least one segment, you can spell that with **/*, right?

Right. Currently did some testing, Python's behaviour doesn't much Linux's.

And judging by previous issues, you are likely going to stand hard on your opinion. I would prefer to bail out in this case since I don't find it rational to spend a lot of time for research and discussion of such a minor and unnecessary change (no offence).

@barneygale
Copy link
Contributor Author

If you think my conduct isn't good enough you can share specific information in confidence with the PSF. Why are you making nonspecific allegations in public? Doing so makes me feel pretty shitty about my work on CPython but without any way to refute what you're saying. I don't think I'm uncompromising or that I don't listen to feedback, but now anyone reading this thread might think so and I can't really defend myself. It's not a nice feeling.

@serhiy-storchaka
Copy link
Member

glob.glob(pattern, root_dir=root_dir) is equivalent to os.chdir(root_dir); glob.glob(pattern), except that it does not actually change the process-wide current working directory.

In Bash, ** does not produce the current directory (or empty string). You can check it with (shopt -s globstar; echo **). glob.glob() was intended to produce the same result.

So this is a documentation issue at most.

@Privat33r-dev
Copy link
Contributor

Privat33r-dev commented Mar 5, 2024

If you think my conduct isn't good enough you can share specific information in confidence with the PSF. Why are you making nonspecific allegations in public? Doing so makes me feel pretty shitty about my work on CPython but without any way to refute what you're saying. I don't think I'm uncompromising or that I don't listen to feedback, but now anyone reading this thread might think so and I can't really defend myself. It's not a nice feeling.

I apologize for any unintended offense caused by my previous comment. I wanted to say that my expertise in path resolving is not good enough and it would take hours to conduct tests and read man pages and code only likely to stay on the same point regarding necessity of the '.' since it's likely unintuitive and undesirable in the first place (I might even suggest to conduct a poll).

More than that, it will likely lead to the situation when some developers rely on this functions to check whether folder is empty prior to doing something and a new behavior might be a breaking change for them.

files = glob.glob('**')
if not files: # logic works without `.` being present
  # do_x

Regarding your point about unconstructive feedback from me, I referred to the previous topic where @jaraco expressed valid concerns. And for me it didn't seem like his points were properly addressed.

I didn't want to just stop commenting which I would find disrespectful, so I preferred switch to other issues where I can contribute more meaningfully (e.g. improve someone's code quality or maybe fix a bug or two myself). I see now that blatantly putting my idea without sharing the context was even more so, and I've learned from it.

Btw, I noticed now that my first comment is ambiguous: in this case is not related to your behaviour, it's about this issue (as a case).

@barneygale
Copy link
Contributor Author

I'm sorry I took your comment the wrong way, thanks for your feedback.

In Bash, ** does not produce the current directory (or empty string). You can check it with (shopt -s globstar; echo **). glob.glob() was intended to produce the same result.

I tested this... or at least I thought I had. Re-testing shows you're absolutely right.

Sorry for the noise everyone.

@serhiy-storchaka serhiy-storchaka closed this as not planned Won't fix, can't repro, duplicate, stale Mar 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

3 participants