Skip to content

Commit

Permalink
feat: support only updating existing BUILD files (#1921)
Browse files Browse the repository at this point in the history
Close #1832

**What type of PR is this?**

Feature

> Other

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

> all

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

Add an option to enable only updating existing BUILDs and not creating
any new ones.

I think this is common among extension authors, normally implemented
with a quick-exit within `Configure` if the `*rule.File` is `nil`.
However all the subdirs then need to be manually traversed to find files
in subdirectories with no BUILD.

I've always maintained a patch similar to this PR to avoid plugins
having to do manual fs operations.

**Which issues(s) does this PR fix?**

Fixes #1832

**Other notes for review**
  • Loading branch information
jbedard authored Feb 18, 2025
1 parent b36a3ed commit 2b9ae2d
Show file tree
Hide file tree
Showing 4 changed files with 119 additions and 11 deletions.
5 changes: 5 additions & 0 deletions README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -760,6 +760,11 @@ The following directives are recognized:
| your project contains non-Bazel files named ``BUILD`` (or ``build`` on |
| case-insensitive file systems). |
+---------------------------------------------------+----------------------------------------+
| :direc:`# gazelle:generation_mode` | ``create_and_update`` |
+---------------------------------------------------+----------------------------------------+
| Declares if gazelle should create and update BUILD files per directory or only update |
| existing BUILD files. Valid values are: ``create_and_update`` and ``update_only``. |
+---------------------------------------------------+----------------------------------------+
| :direc:`# gazelle:build_tags foo,bar` | none |
+---------------------------------------------------+----------------------------------------+
| List of Go build tags Gazelle will defer to Bazel for evaluation. Gazelle applies |
Expand Down
31 changes: 27 additions & 4 deletions walk/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,27 @@ import (
gzflag "github.com/bazelbuild/bazel-gazelle/flag"
)

// generationModeType represents one of the generation modes.
type generationModeType string

// Generation modes
const (
// Update: update and maintain existing BUILD files
generationModeUpdate generationModeType = "update_only"

// Create: create new and update existing BUILD files
generationModeCreate generationModeType = "create_and_update"
)

// TODO(#472): store location information to validate each exclude. They
// may be set in one directory and used in another. Excludes work on
// declared generated files, so we can't just stat.

type walkConfig struct {
excludes []string
ignore bool
follow []string
updateOnly bool
excludes []string
ignore bool
follow []string
}

const walkName = "_walk"
Expand Down Expand Up @@ -70,7 +83,7 @@ func (*Configurer) RegisterFlags(fs *flag.FlagSet, cmd string, c *config.Config)
func (*Configurer) CheckFlags(fs *flag.FlagSet, c *config.Config) error { return nil }

func (*Configurer) KnownDirectives() []string {
return []string{"exclude", "follow", "ignore"}
return []string{"generation_mode", "exclude", "follow", "ignore"}
}

func (cr *Configurer) Configure(c *config.Config, rel string, f *rule.File) {
Expand All @@ -82,6 +95,16 @@ func (cr *Configurer) Configure(c *config.Config, rel string, f *rule.File) {
if f != nil {
for _, d := range f.Directives {
switch d.Key {
case "generation_mode":
switch generationModeType(strings.TrimSpace(d.Value)) {
case generationModeUpdate:
wcCopy.updateOnly = true
case generationModeCreate:
wcCopy.updateOnly = false
default:
log.Fatalf("unknown generation_mode %q in //%s", d.Value, f.Pkg)
continue
}
case "exclude":
if err := checkPathMatchPattern(path.Join(rel, d.Value)); err != nil {
log.Printf("the exclusion pattern is not valid %q: %s", path.Join(rel, d.Value), err)
Expand Down
29 changes: 22 additions & 7 deletions walk/walk.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ func Walk(c *config.Config, cexts []config.Configurer, dirs []string, mode Mode,
//
// Traversal may skip subtrees or files based on the config.Config exclude/ignore/follow options
// as well as the UpdateFilter callbacks.
func visit(c *config.Config, cexts []config.Configurer, knownDirectives map[string]bool, updateRels *UpdateFilter, trie *pathTrie, wf WalkFunc, rel string, updateParent bool) {
func visit(c *config.Config, cexts []config.Configurer, knownDirectives map[string]bool, updateRels *UpdateFilter, trie *pathTrie, wf WalkFunc, rel string, updateParent bool) ([]string, bool) {
haveError := false

// Absolute path to the directory being visited
Expand All @@ -157,11 +157,16 @@ func visit(c *config.Config, cexts []config.Configurer, knownDirectives map[stri
haveError = true
}

configure(cexts, knownDirectives, c, rel, f)
wc := getWalkConfig(c)
collectionOnly := f == nil && getWalkConfig(c).updateOnly

// Configure the current directory if not only collecting files
if !collectionOnly {
configure(cexts, knownDirectives, c, rel, f)
}

wc := getWalkConfig(c)
if wc.isExcluded(rel) {
return
return nil, false
}

// Filter and collect files
Expand All @@ -188,19 +193,29 @@ func visit(c *config.Config, cexts []config.Configurer, knownDirectives map[stri
continue
}
if ent := resolveFileInfo(wc, dir, entRel, t.entry); ent != nil {
subdirs = append(subdirs, base)

if updateRels.shouldVisit(entRel, shouldUpdate) {
visit(c.Clone(), cexts, knownDirectives, updateRels, t, wf, entRel, shouldUpdate)
subFiles, shouldMerge := visit(c.Clone(), cexts, knownDirectives, updateRels, t, wf, entRel, shouldUpdate)
if shouldMerge {
for _, f := range subFiles {
regularFiles = append(regularFiles, path.Join(base, f))
}
} else {
subdirs = append(subdirs, base)
}
}
}
}

if collectionOnly {
return regularFiles, true
}

update := !haveError && !wc.ignore && shouldUpdate
if updateRels.shouldCall(rel, updateParent) {
genFiles := findGenFiles(wc, f)
wf(dir, rel, c, update, f, subdirs, regularFiles, genFiles)
}
return nil, false
}

// An UpdateFilter tracks which directories need to be updated
Expand Down
65 changes: 65 additions & 0 deletions walk/walk_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,10 @@ package walk

import (
"flag"
"fmt"
"path"
"path/filepath"
"reflect"
"testing"

"github.com/bazelbuild/bazel-gazelle/config"
Expand Down Expand Up @@ -141,6 +143,69 @@ func TestUpdateDirs(t *testing.T) {
}
}

func TestGenMode(t *testing.T) {
dir, cleanup := testtools.CreateFiles(t, []testtools.FileSpec{
{Path: "mode-create/"},
{Path: "mode-create/a.go"},
{Path: "mode-create/sub/"},
{Path: "mode-create/sub/b.go"},
{Path: "mode-create/sub/sub2/"},
{Path: "mode-create/sub/sub2/sub3/c.go"},
{Path: "mode-update/"},
{
Path: "mode-update/BUILD.bazel",
Content: "# gazelle:generation_mode update_only",
},
{Path: "mode-update/a.go"},
{Path: "mode-update/sub/"},
{Path: "mode-update/sub/b.go"},
{Path: "mode-update/sub/sub2/"},
{Path: "mode-update/sub/sub2/sub3/c.go"},
{Path: "mode-update/sub/sub3/"},
{Path: "mode-update/sub/sub3/BUILD.bazel"},
{Path: "mode-update/sub/sub3/d.go"},
{Path: "mode-update/sub/sub3/sub4/"},
{Path: "mode-update/sub/sub3/sub4/e.go"},
})
defer cleanup()

type visitSpec struct {
subdirs, files []string
}

t.Run("generation_mode create vs update", func(t *testing.T) {
c, cexts := testConfig(t, dir)
var visits []visitSpec
Walk(c, cexts, []string{"."}, VisitAllUpdateSubdirsMode, func(_ string, rel string, _ *config.Config, update bool, _ *rule.File, subdirs, regularFiles, _ []string) {
visits = append(visits, visitSpec{
subdirs: subdirs,
files: regularFiles,
})
})

if len(visits) != 7 {
t.Error(fmt.Sprintf("Expected 7 visits, got %v", len(visits)))
}

if !reflect.DeepEqual(visits[len(visits)-1].subdirs, []string{"mode-create", "mode-update"}) {
t.Errorf("Last visit should be root dir with 2 subdirs")
}

if len(visits[0].subdirs) != 0 || len(visits[0].files) != 1 || visits[0].files[0] != "c.go" {
t.Errorf("Leaf visit should be only files: %v", visits[0])
}
modeUpdateFiles1 := []string{"BUILD.bazel", "d.go", "sub4/e.go"}
if !reflect.DeepEqual(visits[4].files, modeUpdateFiles1) {
t.Errorf("update mode should contain files in subdirs. Want %v, got: %v", modeUpdateFiles1, visits[5].files)
}

modeUpdateFiles2 := []string{"BUILD.bazel", "a.go", "sub/b.go", "sub/sub2/sub3/c.go"}
if !reflect.DeepEqual(visits[5].files, modeUpdateFiles2) {
t.Errorf("update mode should contain files in subdirs. Want %v, got: %v", modeUpdateFiles2, visits[5].files)
}
})
}

func TestCustomBuildName(t *testing.T) {
dir, cleanup := testtools.CreateFiles(t, []testtools.FileSpec{
{
Expand Down

0 comments on commit 2b9ae2d

Please sign in to comment.