Skip to content

Commit

Permalink
Consolidate the glob case logic
Browse files Browse the repository at this point in the history
Looking at the code as a whole, we ended up with a little to much "buttons". It turns out that doing case insensitive matching (lower both pattern and strings to match) performs just fine. Or at least, it
gives the penalty to the people who uses mixed case filenames.

```
GetGlob/Default_cache-10                          10.6ns ± 2%    10.6ns ± 1%   ~     (p=0.657 n=4+4)
GetGlob/Filenames_cache,_lowercase_searchs-10     10.6ns ± 2%    10.6ns ± 0%   ~     (p=1.000 n=4+4)
GetGlob/Filenames_cache,_mixed_case_searchs-10    29.7ns ± 1%    29.6ns ± 1%   ~     (p=0.886 n=4+4)
GetGlob/GetGlob-10                                13.7ns ± 1%    13.7ns ± 0%   ~     (p=0.429 n=4+4)

name                                            old alloc/op   new alloc/op   delta
GetGlob/Default_cache-10                           0.00B          0.00B        ~     (all equal)
GetGlob/Filenames_cache,_lowercase_searchs-10      0.00B          0.00B        ~     (all equal)
GetGlob/Filenames_cache,_mixed_case_searchs-10     5.00B ± 0%     5.00B ± 0%   ~     (all equal)
GetGlob/GetGlob-10                                 0.00B          0.00B        ~     (all equal)

name                                            old allocs/op  new allocs/op  delta
GetGlob/Default_cache-10                            0.00           0.00        ~     (all equal)
GetGlob/Filenames_cache,_lowercase_searchs-10       0.00           0.00        ~     (all equal)
GetGlob/Filenames_cache,_mixed_case_searchs-10      1.00 ± 0%      1.00 ± 0%   ~     (all equal)
GetGlob/GetGlob-10
```
  • Loading branch information
bep committed Sep 23, 2022
1 parent 281554e commit 5c41653
Show file tree
Hide file tree
Showing 7 changed files with 25 additions and 51 deletions.
7 changes: 4 additions & 3 deletions hugofs/glob.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,19 +26,20 @@ import (
// Glob walks the fs and passes all matches to the handle func.
// The handle func can return true to signal a stop.
func Glob(fs afero.Fs, pattern string, handle func(fi FileMetaInfo) (bool, error)) error {
pattern = glob.NormalizePathCaseSensitive(pattern)
pattern = glob.NormalizePathNoLower(pattern)
if pattern == "" {
return nil
}
root := glob.ResolveRootDir(pattern)
pattern = strings.ToLower(pattern)

g, err := glob.GetFilenamesGlob(pattern)
g, err := glob.GetGlob(pattern)
if err != nil {
return err
}

hasSuperAsterisk := strings.Contains(pattern, "**")
levels := strings.Count(pattern, "/")
root := glob.ResolveRootDir(pattern)

// Signals that we're done.
done := errors.New("done")
Expand Down
8 changes: 4 additions & 4 deletions hugofs/glob/filename_filter.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ func NewFilenameFilter(inclusions, exclusions []string) (*FilenameFilter, error)

for _, include := range inclusions {
include = normalizeFilenameGlobPattern(include)
g, err := filenamesGlobCache.GetGlob(include)
g, err := GetGlob(include)
if err != nil {
return nil, err
}
Expand All @@ -60,9 +60,9 @@ func NewFilenameFilter(inclusions, exclusions []string) (*FilenameFilter, error)
// gets included.
dir := path.Dir(include)
parts := strings.Split(dir, "/")
for i, _ := range parts {
for i := range parts {
pattern := "/" + filepath.Join(parts[:i+1]...)
g, err := filenamesGlobCache.GetGlob(pattern)
g, err := GetGlob(pattern)
if err != nil {
return nil, err
}
Expand All @@ -72,7 +72,7 @@ func NewFilenameFilter(inclusions, exclusions []string) (*FilenameFilter, error)

for _, exclude := range exclusions {
exclude = normalizeFilenameGlobPattern(exclude)
g, err := filenamesGlobCache.GetGlob(exclude)
g, err := GetGlob(exclude)
if err != nil {
return nil, err
}
Expand Down
5 changes: 5 additions & 0 deletions hugofs/glob/filename_filter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,11 @@ func TestFilenameFilter(t *testing.T) {
c.Assert(excludeAllButFooJSON.Match(filepath.FromSlash("/a/b/"), true), qt.Equals, true)
c.Assert(excludeAllButFooJSON.Match(filepath.FromSlash("/"), true), qt.Equals, true)
c.Assert(excludeAllButFooJSON.Match(filepath.FromSlash("/b"), true), qt.Equals, false)

excludeAllButFooJSONMixedCasePattern, err := NewFilenameFilter([]string{"/**/Foo.json"}, nil)
c.Assert(excludeAllButFooJSONMixedCasePattern.Match(filepath.FromSlash("/a/b/c/d/e/foo.json"), false), qt.Equals, true)
c.Assert(excludeAllButFooJSONMixedCasePattern.Match(filepath.FromSlash("/a/b/c/d/e/FOO.json"), false), qt.Equals, true)

c.Assert(err, qt.IsNil)

nopFilter, err := NewFilenameFilter(nil, nil)
Expand Down
42 changes: 4 additions & 38 deletions hugofs/glob/glob.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,6 @@ var (
isWindows: isWindows,
cache: make(map[string]globErr),
}

filenamesGlobCache = &globCache{
isWindows: isWindows,
cache: make(map[string]globErr),
}
)

type globErr struct {
Expand Down Expand Up @@ -86,10 +81,6 @@ func (gc *globCache) GetGlob(pattern string) (glob.Glob, error) {
}

type globDecorator struct {
// Whether both pattern and the strings to match will be matched
// by their original case.
isCaseSensitive bool

// On Windows we may get filenames with Windows slashes to match,
// which wee need to normalize.
isWindows bool
Expand All @@ -101,14 +92,12 @@ func (g globDecorator) Match(s string) bool {
if g.isWindows {
s = filepath.ToSlash(s)
}
if !g.isCaseSensitive {
s = strings.ToLower(s)
}
s = strings.ToLower(s)
return g.g.Match(s)
}

type globDecoratorDouble struct {
lowerCase glob.Glob
lowerCase glob.Glob
originalCase glob.Glob
}

Expand All @@ -120,34 +109,11 @@ func GetGlob(pattern string) (glob.Glob, error) {
return defaultGlobCache.GetGlob(pattern)
}

func GetFilenamesGlob(pattern string) (glob.Glob, error) {
lowered := strings.ToLower(pattern)
hasUpperCase := pattern != lowered
gLowered, err := filenamesGlobCache.GetGlob(lowered)
if err != nil {
return nil, err
}

if !hasUpperCase {
return gLowered, nil
}

gSensitive, err := filenamesGlobCache.GetGlob(pattern)
if err != nil {
return nil, err
}
return globDecoratorDouble{
lowerCase: gLowered,
originalCase: gSensitive,
}, nil

}

func NormalizePath(p string) string {
return strings.Trim(path.Clean(filepath.ToSlash(strings.ToLower(p))), "/.")
return strings.ToLower(NormalizePathNoLower(p))
}

func NormalizePathCaseSensitive(p string) string {
func NormalizePathNoLower(p string) string {
return strings.Trim(path.Clean(filepath.ToSlash(p)), "/.")
}

Expand Down
6 changes: 3 additions & 3 deletions hugofs/glob/glob_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ func TestNormalizePath(t *testing.T) {
}

func TestGetGlob(t *testing.T) {
for _, cache := range []*globCache{defaultGlobCache, filenamesGlobCache} {
for _, cache := range []*globCache{defaultGlobCache} {
c := qt.New(t)
g, err := cache.GetGlob("**.JSON")
c.Assert(err, qt.IsNil)
Expand All @@ -89,8 +89,8 @@ func BenchmarkGetGlob(b *testing.B) {
}

runBench("Default cache", defaultGlobCache, "abcde")
runBench("Filenames cache, lowercase searchs", filenamesGlobCache, "abcde")
runBench("Filenames cache, mixed case searchs", filenamesGlobCache, "abCDe")
runBench("Filenames cache, lowercase searchs", defaultGlobCache, "abcde")
runBench("Filenames cache, mixed case searchs", defaultGlobCache, "abCDe")

b.Run("GetGlob", func(b *testing.B) {
for i := 0; i < b.N; i++ {
Expand Down
6 changes: 4 additions & 2 deletions hugofs/glob_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,14 +52,16 @@ func TestGlob(t *testing.T) {
create("UPPER/sub/style.css")
create("root/UPPER/sub/style.css")

c.Assert(collect(filepath.FromSlash("/jsonfiles/*.json")), qt.HasLen, 2)

c.Assert(collect("**.json"), qt.HasLen, 5)
c.Assert(collect("**"), qt.HasLen, 8)
c.Assert(collect(""), qt.HasLen, 0)
c.Assert(collect("jsonfiles/*.json"), qt.HasLen, 2)
c.Assert(collect("*.json"), qt.HasLen, 1)
c.Assert(collect("**.xml"), qt.HasLen, 1)
c.Assert(collect(filepath.FromSlash("/jsonfiles/*.json")), qt.HasLen, 2)
c.Assert(collect("UPPER/sub/style.css"), qt.HasLen, 1)

c.Assert(collect("root/UPPER/sub/style.css"), qt.HasLen, 1)
c.Assert(collect("UPPER/sub/style.css"), qt.HasLen, 1)

}
2 changes: 1 addition & 1 deletion resources/resource_factories/create/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ func (c *Client) GetMatch(pattern string) (resource.Resource, error) {
}

func (c *Client) match(name, pattern string, matchFunc func(r resource.Resource) bool, firstOnly bool) (resource.Resources, error) {
pattern = glob.NormalizePathCaseSensitive(pattern)
pattern = glob.NormalizePath(pattern)
partitions := glob.FilterGlobParts(strings.Split(pattern, "/"))
if len(partitions) == 0 {
partitions = []string{resources.CACHE_OTHER}
Expand Down

0 comments on commit 5c41653

Please sign in to comment.