Skip to content

Conversation

@mmxmb
Copy link
Contributor

@mmxmb mmxmb commented Oct 18, 2023

Addressing the issue raised in #88

Summary of the issue

In the current implementation, doublestar.Match("a/**/", "a") and doublestar.Match("a/**/", "a/") both return false.

Bash shopt docs say that ** followed by a / will match zero or more directories and subdirectories.

globstar
If set, the pattern ‘**’ used in a filename expansion context will match all files and zero or more directories and subdirectories. If the pattern is followed by a ‘/’, only directories and subdirectories match.

Bash 5.2 implementation confirms this behavior:

bash-5.2$ bash --version
GNU bash, version 5.2.15(1)-release (aarch64-apple-darwin23.0.0)
Copyright (C) 2022 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>

This is free software; you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
bash-5.2$ mkdir a
bash-5.2$ shopt -s globstar
bash-5.2$ ls -d a/**/
a/

What changed?

Changed isZeroLengthPattern in match.go to recognize **/ and /**/ as zero length patterns, in addition to /**, *, and **.

How was this tested?

Added two new test cases to matchTests in doublestar_test.go:

  • {"a/**/", "a", true, true, nil, false, false, false, false, 4, 4}. This test case verifies that /**/ is a zero length pattern.
  • {"a/**/", "a/", true, true, nil, false, false, false, false, 4, 4} This test case verifies that **/ is a zero length pattern. I was less certain whether **/ should be considered a zero length pattern; but it's the only way I came up with to make this test case pass. And it felt wrong to omit this test case, since there's already a very similar test case with a successful match {"a/**", "a/", true, true, nil, false, false, false, false, 7, 7}

I wanted to also run these tests on disk but the current implementation of FilepathGlob in utils.go makes it impossible and changing its implementation is outside of the scope of this change. The issue there is on this line:

pattern = filepath.Clean(pattern)

This line removes the trailing / from a pattern like a/**/ which changes its meaning. a/**/ matches zero or more directories and subdirectories at path a/. a/** matches all files, and zero or more directories and subdirectories at path a/.

Luckily, with GlobWalk I can add an additional filtering function for each match so that I can ensure that the pattern that I use matches only directories and not regular files.

@codecov
Copy link

codecov bot commented Oct 21, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (465a339) 85.42% compared to head (09b9aee) 85.50%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master      #89      +/-   ##
==========================================
+ Coverage   85.42%   85.50%   +0.07%     
==========================================
  Files           5        5              
  Lines         940      945       +5     
==========================================
+ Hits          803      808       +5     
  Misses        106      106              
  Partials       31       31              
Files Coverage Δ
match.go 90.00% <100.00%> (+0.21%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@bmatcuk bmatcuk changed the title Globstar matches zero directories Oct 21, 2023
@bmatcuk bmatcuk merged commit 5df0d9d into bmatcuk:master Oct 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants