Skip to content

Commit

Permalink
refactor(cli): move gazelle gitignore and walk logic to common (#7016)
Browse files Browse the repository at this point in the history
Gitignore and generation-mode are both generic gazelle config and should require no code in plugins.

Note that both of these are currently patched into gazelle while waiting for PRs which
will allow the removal of all aspect code:
bazel-contrib/bazel-gazelle#1921
bazel-contrib/bazel-gazelle#1908

---

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

### Test plan

- Covered by existing test cases

GitOrigin-RevId: cdfe6690ff0ab2584e429419612cc32e94c6b379
  • Loading branch information
jbedard committed Oct 23, 2024
1 parent baeb9b6 commit ffd8941
Show file tree
Hide file tree
Showing 45 changed files with 79 additions and 266 deletions.
2 changes: 2 additions & 0 deletions gazelle/common/git/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,10 @@ go_library(
importpath = "aspect.build/cli/gazelle/common/git",
visibility = ["//visibility:public"],
deps = [
"//gazelle/common",
"//pkg/logger",
"@bazel_gazelle//config:go_default_library",
"@bazel_gazelle//rule:go_default_library",
"@com_github_go_git_go_git_v5//plumbing/format/gitignore",
],
)
Expand Down
39 changes: 32 additions & 7 deletions gazelle/common/git/gitignore.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,15 @@ import (
"path"
"strings"

common "aspect.build/cli/gazelle/common"
BazelLog "aspect.build/cli/pkg/logger"
"github.com/bazelbuild/bazel-gazelle/config"
"github.com/bazelbuild/bazel-gazelle/rule"
gitignore "github.com/go-git/go-git/v5/plumbing/format/gitignore"
)

// TODO: remove and align with gazelle after https://github.com/aspect-build/aspect-cli/issues/755

// Must align with patched bazel-gazelle
const ASPECT_GITIGNORE = "__aspect:gitignore"

Expand All @@ -25,7 +29,7 @@ const enabledExt = Directive_GitIgnore
const lastConfiguredExt = "gitignore:dir"
const ignorePatternsExt = "gitignore:patterns"

func CollectIgnoreFiles(c *config.Config, rel string) {
func collectIgnoreFiles(c *config.Config, rel string) {
// Only parse once per directory.
if lastCollected, hasCollected := c.Exts[lastConfiguredExt]; hasCollected && lastCollected == rel {
return
Expand All @@ -44,12 +48,33 @@ func CollectIgnoreFiles(c *config.Config, rel string) {
}
}

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 ReadGitConfig(c *config.Config, rel string, f *rule.File) {
// Enable .gitignore support by default in Aspect gazelle languages.
// TODO: default to false and encourage use of .bazelignore instead
if rel == "" {
_, exists := c.Exts[enabledExt]
if !exists {
c.Exts[enabledExt] = true
}
}

// Collect ignore files within this config directory.
collectIgnoreFiles(c, rel)

// Collect config from directives within this BUILD.
if f != nil {
for _, d := range f.Directives {
switch d.Key {
case Directive_GitIgnore:
enabled := common.ReadEnabled(d)
c.Exts[enabledExt] = enabled
if enabled {
c.Exts[ASPECT_GITIGNORE] = createMatcherFunc(c)
} else {
c.Exts[ASPECT_GITIGNORE] = nil
}
}
}
}
}

Expand Down
31 changes: 29 additions & 2 deletions gazelle/common/walk.go
Original file line number Diff line number Diff line change
@@ -1,17 +1,44 @@
package gazelle

import (
"log"
"strings"

BazelLog "aspect.build/cli/pkg/logger"
"github.com/bazelbuild/bazel-gazelle/config"
"github.com/bazelbuild/bazel-gazelle/language"
"github.com/bazelbuild/bazel-gazelle/rule"
)

type GazelleWalkFunc func(path string) error

// Must align with patched bazel-gazelle
const ASPECT_WALKSUBDIR = "__aspect:walksubdir"

// Walk the directory of the language.GenerateArgs, optionally recursing into
// subdirectories unlike the files provided in GenerateArgs.RegularFiles.
// Read any configuration regarding walk options.
func ReadWalkConfig(c *config.Config, rel string, f *rule.File) bool {
if f == nil {
v, ok := c.Exts[ASPECT_WALKSUBDIR]
return !ok || !v.(bool)
}

for _, d := range f.Directives {
switch d.Key {
case Directive_GenerationMode:
switch GenerationModeType(strings.TrimSpace(d.Value)) {
case GenerationModeCreate:
c.Exts[ASPECT_WALKSUBDIR] = false
case GenerationModeUpdate:
c.Exts[ASPECT_WALKSUBDIR] = true
default:
log.Fatalf("invalid value for directive %q: %s", Directive_GenerationMode, d.Value)
}
}
}
return true
}

// Walk the directory being generated, respecting any walk generation config.
func GazelleWalkDir(args language.GenerateArgs, walkFunc GazelleWalkFunc) error {
BazelLog.Tracef("GazelleWalkDir: %s", args.Rel)

Expand Down
15 changes: 0 additions & 15 deletions gazelle/js/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
"path"
"strings"

common "aspect.build/cli/gazelle/common"
"github.com/bazelbuild/bazel-gazelle/label"
"github.com/bmatcuk/doublestar/v4"
"github.com/emirpasic/gods/maps/linkedhashmap"
Expand Down Expand Up @@ -158,7 +157,6 @@ type JsGazelleConfig struct {
parent *JsGazelleConfig

generationEnabled bool
generationMode common.GenerationModeType

packageTargetKind PackageTargetKind

Expand Down Expand Up @@ -191,7 +189,6 @@ func newRootConfig() *JsGazelleConfig {
protoGenerationEnabled: true,
tsconfigGenerationEnabled: false,
packageGenerationEnabled: NpmPackageReferencedMode,
generationMode: common.GenerationModeCreate,
packageTargetKind: PackageTargetKind_Package,
pnpmLockPath: "pnpm-lock.yaml",
ignoreDependencies: make([]string, 0),
Expand Down Expand Up @@ -384,18 +381,6 @@ func (c *JsGazelleConfig) ValidateImportStatements() ValidationMode {
return c.validateImportStatements
}

// SetGenerationMode sets whether coarse-grained targets should be
// generated or not.
func (c *JsGazelleConfig) SetGenerationMode(generationMode common.GenerationModeType) {
c.generationMode = generationMode
}

// GenerationMode returns whether coarse-grained targets should be
// generated or not.
func (c *JsGazelleConfig) GenerationMode() common.GenerationModeType {
return c.generationMode
}

// SetLibraryNamingConvention sets the ts_project target naming convention.
func (c *JsGazelleConfig) SetLibraryNamingConvention(libraryNamingConvention string) {
c.targetNamingOverrides[DefaultLibraryName] = libraryNamingConvention
Expand Down
69 changes: 11 additions & 58 deletions gazelle/js/configure.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,6 @@ var _ config.Configurer = (*typeScriptLang)(nil)
// starts. RegisterFlags may set an initial values in Config.Exts. When flags
// are set, they should modify these values.
func (ts *typeScriptLang) RegisterFlags(fs *flag.FlagSet, cmd string, c *config.Config) {
// Enable .gitignore support by default
// TODO: change to false and encourage .bazelignore
git.EnableGitignore(c, true)
}

// CheckFlags validates the configuration after command line flags are parsed.
Expand Down Expand Up @@ -59,15 +56,13 @@ func (ts *typeScriptLang) KnownDirectives() []string {
Directive_LibraryFiles,
Directive_TestFiles,

// Common directives supported by this language
common.Directive_GenerationMode,

// TODO(deprecated): remove
Directive_CustomTargetFiles,
Directive_CustomTargetTestFiles,

// TODO: move to common
git.Directive_GitIgnore,
common.Directive_GenerationMode,
}
}

Expand Down Expand Up @@ -99,11 +94,9 @@ func (ts *typeScriptLang) Configure(c *config.Config, rel string, f *rule.File)
ts.readConfigurations(c, rel)
}

// TODO: move to common global config.Configurer
// Enable the WALKSUBDIR gazelle patch, setting the flag depending on js the GenerationMode.
c.Exts[common.ASPECT_WALKSUBDIR] = c.Exts[LanguageName].(*JsGazelleConfig).generationMode == common.GenerationModeUpdate
common.ReadWalkConfig(c, rel, f)

git.CollectIgnoreFiles(c, rel)
git.ReadGitConfig(c, rel, f)
}

func (ts *typeScriptLang) readConfigurations(c *config.Config, rel string) {
Expand Down Expand Up @@ -230,57 +223,17 @@ func (ts *typeScriptLang) readDirectives(c *config.Config, rel string, f *rule.F
}

config.addTargetGlob(group, groupGlob, true)
case Directive_CustomTargetFiles:
groupGlob := strings.Split(value, " ")
if len(groupGlob) != 2 {
err := fmt.Errorf("invalid value for directive %q: %s: value must be group + glob",
Directive_CustomTargetFiles, d.Value)
log.Fatal(err)
}

fmt.Printf("DEPRECATED: %s is deprecated, use %s %s instead\n", Directive_CustomTargetFiles, Directive_LibraryFiles, groupGlob[0])

config.addTargetGlob(groupGlob[0], groupGlob[1], false)
// TODO: remove, deprecated
case Directive_CustomTargetFiles:
fmt.Fprintf(os.Stderr, "DEPRECATED: %s is deprecated, use %s\n", Directive_CustomTargetFiles, Directive_LibraryFiles)
os.Exit(1)
case Directive_CustomTargetTestFiles:
groupGlob := strings.Split(value, " ")
if len(groupGlob) != 2 {
err := fmt.Errorf("invalid value for directive %q: %s: value must be group + glob",
Directive_CustomTargetTestFiles, d.Value)
log.Fatal(err)
}

fmt.Printf("DEPRECATED: %s is deprecated, use %s %s instead\n", Directive_CustomTargetTestFiles, Directive_TestFiles, groupGlob[0])

config.addTargetGlob(groupGlob[0], groupGlob[1], true)

fmt.Fprintf(os.Stderr, "DEPRECATED: %s is deprecated, use %s\n", Directive_CustomTargetTestFiles, Directive_TestFiles)
os.Exit(1)
case Directive_GenerationMode:
mode := strings.TrimSpace(d.Value)
switch mode {
case "directory":
config.SetGenerationMode(common.GenerationModeCreate)
case "none":
config.SetGenerationMode(common.GenerationModeUpdate)
default:
log.Fatalf("invalid value for directive %q: %s", Directive_GenerationMode, d.Value)
}

fmt.Printf("DEPRECATED: %s is deprecated, use %s %s|%s\n", Directive_GenerationMode, common.Directive_GenerationMode, common.GenerationModeUpdate, common.GenerationModeCreate)

// Inherited aspect-cli common+pro values
// TODO: move to common location
case common.Directive_GenerationMode:
mode := common.GenerationModeType(strings.TrimSpace(d.Value))
switch mode {
case common.GenerationModeCreate:
config.SetGenerationMode(common.GenerationModeCreate)
case common.GenerationModeUpdate:
config.SetGenerationMode(common.GenerationModeUpdate)
default:
log.Fatalf("invalid value for directive %q: %s", common.Directive_GenerationMode, d.Value)
}
// TODO: move to common
case git.Directive_GitIgnore:
git.EnableGitignore(c, common.ReadEnabled(d))
fmt.Fprintf(os.Stderr, "DEPRECATED: %s is deprecated, use %s %s|%s\n", Directive_GenerationMode, common.Directive_GenerationMode, common.GenerationModeUpdate, common.GenerationModeCreate)
os.Exit(1)
}
}
}
7 changes: 0 additions & 7 deletions gazelle/js/generate.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,13 +55,6 @@ func (ts *typeScriptLang) GenerateRules(args language.GenerateArgs) language.Gen
return language.GenerateResult{}
}

// If this directory has not been declared as a bazel package only continue
// if generating new BUILD files is enabled.
if cfg.GenerationMode() == gazelle.GenerationModeUpdate && args.File == nil {
BazelLog.Tracef("GenerateRules(%s) BUILD creation disabled: %s", LanguageName, args.Rel)
return language.GenerateResult{}
}

BazelLog.Tracef("GenerateRules(%s): %s", LanguageName, args.Rel)

var result language.GenerateResult
Expand Down
1 change: 1 addition & 0 deletions gazelle/js/tests/gazelle_generation_mode_legacy/BUILD.in
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
# gazelle:js_generation_mode directory
7 changes: 1 addition & 6 deletions gazelle/js/tests/gazelle_generation_mode_legacy/BUILD.out
Original file line number Diff line number Diff line change
@@ -1,6 +1 @@
load("@aspect_rules_ts//ts:defs.bzl", "ts_project")

ts_project(
name = "gazelle_generation_mode",
srcs = ["main.ts"],
)
# gazelle:js_generation_mode directory

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

Loading

0 comments on commit ffd8941

Please sign in to comment.