Skip to content

Commit

Permalink
expand: reimplement globstar globbing for correctness
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
mvdan committed Mar 29, 2022
1 parent 07eccc2 commit 7206f9b
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 23 deletions.
55 changes: 32 additions & 23 deletions expand/expand.go
Original file line number Diff line number Diff line change
Expand Up @@ -773,6 +773,9 @@ func (cfg *Config) glob(base, pat string) ([]string, error) {
// ReadDir("/foo") glob "*"

for i, part := range parts {
// Keep around for debugging.
// log.Printf("matches %q part %d %q", matches, i, part)

wantDir := i < len(parts)-1
switch {
case part == "", part == ".", part == "..":
Expand Down Expand Up @@ -816,30 +819,35 @@ func (cfg *Config) glob(base, pat string) ([]string, error) {
matches = newMatches
continue
case part == "**" && cfg.GlobStar:
for i, match := range matches {
// "a/**" should match "a/ a/b a/b/cfg ..."; note
// how the zero-match case has a trailing
// separator.
matches[i] = pathJoin2(match, "")
}
// expand all the possible levels of **
latest := matches
for {
var newMatches []string
for _, dir := range latest {
var err error
newMatches, err = cfg.globDir(base, dir, rxGlobStar, wantDir, newMatches)
if err != nil {
return nil, err
}
// Find all recursive matches for "**".
// Note that we need the results to be in depth-first order,
// and to avoid recursion, we use a slice as a stack.
// Since we pop from the back, we populate the stack backwards.
stack := make([]string, 0, len(matches))
for i := len(matches) - 1; i >= 0; i-- {
match := matches[i]
// "a/**" should match "a/ a/b a/b/cfg ...";
// note how the zero-match case has a trailing separator.
match = pathJoin2(match, "")
stack = append(stack, match)
}
matches = matches[:0]
var newMatches []string // to reuse its capacity
for len(stack) > 0 {
dir := stack[len(stack)-1]
stack = stack[:len(stack)-1]

// Don't include the original "" match as it's not a valid path.
if dir != "" {
matches = append(matches, dir)
}
if len(newMatches) == 0 {
// not another level of directories to
// try; stop
break

// If dir is not a directory, we keep the stack as-is and continue.
newMatches = newMatches[:0]
newMatches, _ = cfg.globDir(base, dir, rxGlobStar, wantDir, newMatches)
for i := len(newMatches) - 1; i >= 0; i-- {
stack = append(stack, newMatches[i])
}
matches = append(matches, newMatches...)
latest = newMatches
}
continue
}
Expand Down Expand Up @@ -868,7 +876,8 @@ func (cfg *Config) globDir(base, dir string, rx *regexp.Regexp, wantDir bool, ma
}
infos, err := cfg.ReadDir(fullDir)
if err != nil {
return nil, err
// We still want to return matches, for the sake of reusing slices.
return matches, err
}
for _, info := range infos {
name := info.Name()
Expand Down
12 changes: 12 additions & 0 deletions interp/interp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2557,6 +2557,18 @@ set +o pipefail
"shopt -s globstar; mkdir -p a/b/c; echo **/c | sed 's@\\\\@/@g'",
"a/b/c\n",
},
{
"shopt -s globstar; mkdir -p a/b; touch c; echo ** | sed 's@\\\\@/@g'",
"a a/b c\n",
},
{
"shopt -s globstar; mkdir -p a/b; touch c; echo **/ | sed 's@\\\\@/@g'",
"a/ a/b/\n",
},
{
"shopt -s globstar; mkdir -p a/b/c a/d; echo ** | sed 's@\\\\@/@g'",
"a a/b a/b/c a/d\n",
},
{
"mkdir foo; touch foo/bar; echo */bar */bar/ | sed 's@\\\\@/@g'",
"foo/bar */bar/\n",
Expand Down

0 comments on commit 7206f9b

Please sign in to comment.