Skip to content

Commit

Permalink
refactor(cli): align aspect configure gitignore feature with proposed…
Browse files Browse the repository at this point in the history
… gazelle feature implementation (#6873)

This aligns the implementation more with what I'm trying to merge into
gazelle: bazel-contrib/bazel-gazelle#1908

There are no user facing changes here but the implementation is simpler
and should improve performance for many reasons:
* the .gitignore files are only `os.Open`ed once per directory (not per
directory * per language)
* the .gitignore files are only parsed once instead of once per gazelle
language
* the .gitignore patterns are only executed once because it is done
within the (patched) gazelle walk
* .gitignore-d directories are not recursed into at all because they are
filtered in the gazelle fs walk
* ...

---

### Changes are visible to end-users: no

### Test plan

- Covered by existing test cases

GitOrigin-RevId: 27fe8377ca89de91d3c95105ab37adea9347e6ab
  • Loading branch information
jbedard committed Sep 28, 2024
1 parent b613f5b commit 9f78efe
Show file tree
Hide file tree
Showing 17 changed files with 207 additions and 164 deletions.
1 change: 1 addition & 0 deletions WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,7 @@ http_archive(
patches = [
"//:patches/bazelbuild_bazel-gazelle_aspect-cli.patch",
"//:patches/bazelbuild_bazel-gazelle_aspect-walk-subdir.patch",
"//:patches/bazelbuild_bazel-gazelle_aspect-gitignore.patch",
],
sha256 = "872f1532567cdc53dc8e9f4681cd45021cd6787e2bde8a022bcec24a5867ce4c",
# Ensure this version always matches the go.mod version.
Expand Down
1 change: 1 addition & 0 deletions gazelle/common/git/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,5 @@ go_test(
name = "git_test",
srcs = ["gitignore_test.go"],
embed = [":git"],
deps = ["@bazel_gazelle//config:go_default_library"],
)
114 changes: 65 additions & 49 deletions gazelle/common/git/gitignore.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,44 +12,74 @@ import (
gitignore "github.com/go-git/go-git/v5/plumbing/format/gitignore"
)

// Wrap the ignore files along with the relative path they were loaded from
// to enable quick-exit checks.
type ignoreEntry struct {
i gitignore.Matcher
base string
}
// Must align with patched bazel-gazelle
const ASPECT_GITIGNORE = "__aspect:gitignore"

type GitIgnore struct {
ignores []ignoreEntry
}
// Directive to enable/disable gitignore support
const Directive_GitIgnore = "gitignore"

// Internal
const enabledExt = Directive_GitIgnore
const lastConfiguredExt = "gitignore:dir"
const ignorePatternsExt = "gitignore:patterns"

func NewGitIgnore() *GitIgnore {
return &GitIgnore{
ignores: make([]ignoreEntry, 0),
func CollectIgnoreFiles(c *config.Config, rel string) {
// Only parse once per directory.
if lastCollected, hasCollected := c.Exts[lastConfiguredExt]; hasCollected && lastCollected == rel {
return
}
}
c.Exts[lastConfiguredExt] = rel

func (i *GitIgnore) CollectIgnoreFiles(c *config.Config, rel string) {
// Collect gitignore style ignore files in this directory.
// Find and add .gitignore files from this directory
ignoreFilePath := path.Join(c.RepoRoot, rel, ".gitignore")

if ignoreReader, ignoreErr := os.Open(ignoreFilePath); ignoreErr == nil {
ignoreReader, ignoreErr := os.Open(ignoreFilePath)
if ignoreErr == nil {
BazelLog.Tracef("Add ignore file %s/.gitignore", rel)
defer ignoreReader.Close()
addIgnore(c, rel, ignoreReader)
} else if !os.IsNotExist(ignoreErr) {
BazelLog.Errorf("Failed to open %s/.gitignore: %v", rel, ignoreErr)
}
}

i.addIgnore(rel, ignoreReader)
func EnableGitignore(c *config.Config, enabled bool) {
c.Exts[enabledExt] = enabled
if enabled {
c.Exts[ASPECT_GITIGNORE] = createMatcherFunc(c)
} else {
c.Exts[ASPECT_GITIGNORE] = nil
}
}

func (i *GitIgnore) addIgnore(rel string, ignoreReader io.Reader) {
// Persist a relative path to the ignore file to enable quick-exit checks.
base := path.Clean(rel)
if base == "." {
base = ""
func isEnabled(c *config.Config) bool {
enabled, hasEnabled := c.Exts[enabledExt]
return hasEnabled && enabled.(bool)
}

func addIgnore(c *config.Config, rel string, ignoreReader io.Reader) {
var ignorePatterns []gitignore.Pattern

// Load parent ignore patterns
if c.Exts[ignorePatternsExt] != nil {
ignorePatterns = c.Exts[ignorePatternsExt].([]gitignore.Pattern)
}

// Append new ignore patterns
ignorePatterns = append(ignorePatterns, parseIgnore(rel, ignoreReader)...)

// Persist appended ignore patterns
c.Exts[ignorePatternsExt] = ignorePatterns

// Persist a matcher function with the updated ignore patterns if enabled
if isEnabled(c) {
c.Exts[ASPECT_GITIGNORE] = createMatcherFunc(c)
}
}

domain := []string{}
if base != "" {
domain = strings.Split(base, "/")
func parseIgnore(rel string, ignoreReader io.Reader) []gitignore.Pattern {
var domain []string
if rel != "" {
domain = strings.Split(path.Clean(rel), "/")
}

matcherPatterns := make([]gitignore.Pattern, 0)
Expand All @@ -64,31 +94,17 @@ func (i *GitIgnore) addIgnore(rel string, ignoreReader io.Reader) {
matcherPatterns = append(matcherPatterns, gitignore.ParsePattern(p, domain))
}

ignore := gitignore.NewMatcher(matcherPatterns)

// Add a trailing slash to the base path to ensure the ignore file only
// processes paths within that directory.
if base != "" && !strings.HasSuffix(base, "/") {
base += "/"
}

i.ignores = append(i.ignores, ignoreEntry{
i: ignore,
base: base,
})
return matcherPatterns
}

func (i *GitIgnore) Matches(p string) bool {
for _, ignore := range i.ignores {
// Ensure the path is within the base path of the ignore file
// to avoid strings.Split unless necessary.
if !strings.HasPrefix(p, ignore.base) {
continue
}
if ignore.i.Match(strings.Split(p, "/"), false) {
return true
}
func createMatcherFunc(c *config.Config) func(string) bool {
patterns, patternsFound := c.Exts[ignorePatternsExt]
if !patternsFound {
return nil
}

return false
matcher := gitignore.NewMatcher(patterns.([]gitignore.Pattern))
return func(s string) bool {
return matcher.Match(strings.Split(s, "/"), false)
}
}
Loading

0 comments on commit 9f78efe

Please sign in to comment.