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

expand: reimplement globstar globbing for correctness #834

Merged
merged 1 commit into from
Mar 31, 2022

Conversation

mvdan
Copy link
Owner

@mvdan mvdan commented Mar 29, 2022

(see commit message)

Fixes #829.

@mvdan mvdan requested a review from riacataquian March 29, 2022 22:07
@mvdan
Copy link
Owner Author

mvdan commented Mar 29, 2022

cc @theclapp

@mvdan mvdan force-pushed the 829-globstar-readdir branch from 352047e to 7206f9b Compare March 29, 2022 22:08
The previous globstar implementation was breadth-first search,
whereas Bash implements depth-first search.

Moreover, when recursing into directories, we could stop early and give
an error to the user when encountering a non-directory.
Instead, we want to simply not recurse into that path.

Add tests for both cases, too.

Fixes #829.
@mvdan mvdan force-pushed the 829-globstar-readdir branch from 7206f9b to 1b937cb Compare March 29, 2022 22:09
@theclapp
Copy link
Collaborator

In this branch, ./gosh -c "shopt -s globstar; cd /tmp ; echo **" works, and doesn't crash on the sockets in my /tmp dir any more.

But running echo * in gosh in interactive mode hangs forever until you press enter (or ^D):

% ./gosh
$ pwd
/Users/lmc/src/goget/src/github.com/mvdan/sh/cmd/gosh
$ echo *
<hangs>
<press ^D>
gosh main.go main_test.go
$ % <gosh prints its prompt and exits without further input after ^D>

Running echo ** requires pressing enter twice:

% ./gosh
$ shopt
expand_aliases	off
globstar	off
nullglob	off
$ echo *
<hangs; press enter>

gosh main.go main_test.go
$ $ echo **
<hangs; press enter twice>


gosh main.go main_test.go
$ $ ls -l
total 3456
-rwxr-xr-x 1 lmc staff 3523536 Mar 30 09:22 gosh
-rw-r--r-- 1 lmc staff    1880 Feb 21  2020 main.go
-rw-r--r-- 1 lmc staff    4201 Feb  3 09:45 main_test.go
$ cd ..
$ echo *
<hangs; press enter>

gosh shfmt
$ $ echo **
<hangs; press enter twice>


gosh shfmt
$ $ shopt -s globstar
$ echo **
<hangs; press enter twice>


gosh gosh/gosh gosh/main.go gosh/main_test.go shfmt shfmt/Dockerfile shfmt/docker-entrypoint.sh shfmt/json.go shfmt/main.go shfmt/main_test.go shfmt/shfmt.1.scd shfmt/testdata shfmt/testdata/scripts shfmt/testdata/scripts/atomic.txt shfmt/testdata/scripts/basic.txt shfmt/testdata/scripts/diff.txt shfmt/testdata/scripts/editorconfig.txt shfmt/testdata/scripts/flags.txt shfmt/testdata/scripts/tojson.txt shfmt/testdata/scripts/walk.txt
$ $

Weird.

@mvdan
Copy link
Owner Author

mvdan commented Mar 30, 2022

Thanks, that's an interesting bug I hadn't noticed. It might be in gosh itself, or elsewhere in the interpreter, but it seems unrelated as it also happens on master. Mind copy-pasting that into a new issue? Because I think we can merge this separately :)

@theclapp
Copy link
Collaborator

Huh. I had not noticed that it also happens on master, as such. I had a previously installed instance of gosh in my bin directory (I assume from a previous master, but I'm not sure exactly what commit) that did not exhibit that behavior, so I thought it must be from this code.

Oh my, I just checked that other version, and the date on the file is Nov 18, 2020, so it's a little out of date. 😆

Anyway, yeah, I'll file a new issue, and I guess you're clear to merge this PR.

@mvdan mvdan merged commit 5790067 into master Mar 31, 2022
@mvdan mvdan deleted the 829-globstar-readdir branch April 3, 2022 05:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

expand: globstar expansion can return errors or leave leading spaces
2 participants