mvdan / sh

A shell parser, formatter, and interpreter with bash support; includes shfmt
https://pkg.go.dev/mvdan.cc/sh/v3
BSD 3-Clause "New" or "Revised" License
7.14k stars 338 forks source link

expand: reimplement globstar globbing for correctness #834

Closed mvdan closed 2 years ago

mvdan commented 2 years ago

(see commit message)

Fixes #829.

mvdan commented 2 years ago

cc @theclapp

theclapp commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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.