Skip to content

Commit

Permalink
perf: do not revalidate glob expressions on each invocation (#1964)
Browse files Browse the repository at this point in the history
doublestar now has an option to match without re-validating every time,
see bmatcuk/doublestar#92

**What type of PR is this?**

> Other

**What package or component does this PR mostly affect?**

> all

**What does this PR do? Why is it needed?**

Performance
  • Loading branch information
jbedard authored Oct 21, 2024
1 parent 2a65ce8 commit 2a84bad
Show file tree
Hide file tree
Showing 12 changed files with 78 additions and 25 deletions.
4 changes: 2 additions & 2 deletions deps.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -121,8 +121,8 @@ def gazelle_dependencies(
go_repository,
name = "com_github_bmatcuk_doublestar_v4",
importpath = "github.com/bmatcuk/doublestar/v4",
sum = "h1:FH9SifrbvJhnlQpztAx++wlkk70QBf0iBWDwNy7PA4I=",
version = "v4.6.1",
sum = "h1:fdDeAqgT47acgwd9bd9HxJRDmc9UAmPpc+2m0CXv75Q=",
version = "v4.7.1",
)
_maybe(
go_repository,
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ toolchain go1.22.7
require (
github.com/bazelbuild/buildtools v0.0.0-20240827154017-dd10159baa91
github.com/bazelbuild/rules_go v0.50.1
github.com/bmatcuk/doublestar/v4 v4.6.1
github.com/bmatcuk/doublestar/v4 v4.7.1
github.com/fsnotify/fsnotify v1.7.0
github.com/google/go-cmp v0.6.0
github.com/pmezard/go-difflib v1.0.0
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ github.com/bazelbuild/buildtools v0.0.0-20240827154017-dd10159baa91 h1:/wpuwyWvp
github.com/bazelbuild/buildtools v0.0.0-20240827154017-dd10159baa91/go.mod h1:PLNUetjLa77TCCziPsz0EI8a6CUxgC+1jgmWv0H25tg=
github.com/bazelbuild/rules_go v0.50.1 h1:/BUvuaB8MEiUA2oLPPCGtuw5V+doAYyiGTFyoSWlkrw=
github.com/bazelbuild/rules_go v0.50.1/go.mod h1:Dhcz716Kqg1RHNWos+N6MlXNkjNP2EwZQ0LukRKJfMs=
github.com/bmatcuk/doublestar/v4 v4.6.1 h1:FH9SifrbvJhnlQpztAx++wlkk70QBf0iBWDwNy7PA4I=
github.com/bmatcuk/doublestar/v4 v4.6.1/go.mod h1:xBQ8jztBU6kakFMg+8WGxn0c6z1fTSPVIjEY1Wr7jzc=
github.com/bmatcuk/doublestar/v4 v4.7.1 h1:fdDeAqgT47acgwd9bd9HxJRDmc9UAmPpc+2m0CXv75Q=
github.com/bmatcuk/doublestar/v4 v4.7.1/go.mod h1:xBQ8jztBU6kakFMg+8WGxn0c6z1fTSPVIjEY1Wr7jzc=
github.com/fsnotify/fsnotify v1.7.0 h1:8JEhPFa5W2WU7YfeZzPNqzMP6Lwt7L2715Ggo0nosvA=
github.com/fsnotify/fsnotify v1.7.0/go.mod h1:40Bi/Hjc2AVfZrqy+aj+yEI+/bRxZnMJyTJwOpGvigM=
github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI=
Expand Down
3 changes: 2 additions & 1 deletion tests/bcr/go_mod/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,14 @@ module github.com/bazelbuild/bazel-gazelle/tests/bcr/go_mod
go 1.21.5

// Validate go.mod replace directives can be properly used:
replace github.com/bmatcuk/doublestar/v4 => github.com/bmatcuk/doublestar v1.3.4
replace github.com/bmatcuk/doublestar/v4 => github.com/bmatcuk/doublestar/v4 v4.7.1

require (
example.org/hello v1.0.0
github.com/DataDog/sketches-go v1.4.1
github.com/bazelbuild/buildtools v0.0.0-20230317132445-9c3c1fc0106e
github.com/bazelbuild/rules_go v0.39.1
// NOTE: keep <4.7.0 to test the 'replace'
github.com/bmatcuk/doublestar/v4 v4.6.0
github.com/cloudflare/circl v1.3.7
github.com/envoyproxy/protoc-gen-validate v1.0.1
Expand Down
7 changes: 3 additions & 4 deletions tests/bcr/go_mod/pkg/pkg_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,10 @@ import (
)

func TestReplace(t *testing.T) {
// doublestar.StandardOS does NOT exist in doublestar/v4
// See: https://pkg.go.dev/github.com/bmatcuk/doublestar#OS
// doublestar.MatchUnvalidated does NOT exist in doublestar <v4.7.0
// If we are able to initialize this variable, it validates that the dependency is properly
// being replaced with github.com/bmatcuk/doublestar@v1.3.4
_ = doublestar.StandardOS
// being replaced with github.com/bmatcuk/doublestar/[email protected]
_ = doublestar.MatchUnvalidated
}

func TestPatch(t *testing.T) {
Expand Down
3 changes: 2 additions & 1 deletion tests/bcr/go_work/pkg/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ require (
github.com/DataDog/sketches-go v1.4.4
github.com/bazelbuild/buildtools v0.0.0-20240207142252-03bf520394af
github.com/bazelbuild/rules_go v0.44.0
// NOTE: keep <4.7.0 to test the 'replace'
github.com/bmatcuk/doublestar/v4 v4.6.1
github.com/cloudflare/circl v1.3.7
github.com/envoyproxy/protoc-gen-validate v1.0.4
Expand All @@ -30,4 +31,4 @@ require (
)

// Validate go.mod replace directives can be properly used:
replace github.com/bmatcuk/doublestar/v4 => github.com/bmatcuk/doublestar v1.3.4
replace github.com/bmatcuk/doublestar/v4 => github.com/bmatcuk/doublestar/v4 v4.7.1
7 changes: 3 additions & 4 deletions tests/bcr/go_work/pkg/pkg_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,10 @@ import (
)

func TestReplace(t *testing.T) {
// doublestar.StandardOS does NOT exist in doublestar/v4
// See: https://pkg.go.dev/github.com/bmatcuk/doublestar#OS
// doublestar.MatchUnvalidated does NOT exist in doublestar <v4.7.0
// If we are able to initialize this variable, it validates that the dependency is properly
// being replaced with github.com/bmatcuk/doublestar@v1.3.4
_ = doublestar.StandardOS
// being replaced with github.com/bmatcuk/doublestar/[email protected]
_ = doublestar.MatchUnvalidated
}

func TestPatch(t *testing.T) {
Expand Down
29 changes: 29 additions & 0 deletions vendor/github.com/bmatcuk/doublestar/v4/README.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

22 changes: 22 additions & 0 deletions vendor/github.com/bmatcuk/doublestar/v4/match.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 10 additions & 0 deletions vendor/github.com/bmatcuk/doublestar/v4/utils.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion vendor/modules.txt
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ github.com/bazelbuild/rules_go/go/runfiles
github.com/bazelbuild/rules_go/go/tools/bazel
github.com/bazelbuild/rules_go/go/tools/bazel_testing
github.com/bazelbuild/rules_go/go/tools/internal/txtar
# github.com/bmatcuk/doublestar/v4 v4.6.1
# github.com/bmatcuk/doublestar/v4 v4.7.1
## explicit; go 1.16
github.com/bmatcuk/doublestar/v4
# github.com/fsnotify/fsnotify v1.7.0
Expand Down
10 changes: 1 addition & 9 deletions walk/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,15 +158,7 @@ func checkPathMatchPattern(pattern string) error {

func matchAnyGlob(patterns []string, path string) bool {
for _, x := range patterns {
matched, err := doublestar.Match(x, path)
if err != nil {
// doublestar.Match returns only one possible error, and only if the
// pattern is not valid. During the configuration of the walker (see
// Configure below), we discard any invalid pattern and thus an error
// here should not be possible.
log.Panicf("error during doublestar.Match. This should not happen, please file an issue https://github.com/bazelbuild/bazel-gazelle/issues/new: %s", err)
}
if matched {
if doublestar.MatchUnvalidated(x, path) {
return true
}
}
Expand Down

0 comments on commit 2a84bad

Please sign in to comment.