From 74df756050fef10167d5934fe8f4a9328bfcd1c1 Mon Sep 17 00:00:00 2001 From: Jason Bedard Date: Thu, 12 Sep 2024 10:40:50 -0700 Subject: [PATCH] feat: support only updating existing BUILD files Close #1832 --- walk/config.go | 23 ++++++++++++++--- walk/walk.go | 29 ++++++++++++++++----- walk/walk_test.go | 65 +++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 106 insertions(+), 11 deletions(-) diff --git a/walk/config.go b/walk/config.go index 6cee8a371..e5937368d 100644 --- a/walk/config.go +++ b/walk/config.go @@ -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" +) + // 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" @@ -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) { @@ -82,6 +95,8 @@ 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": + wcCopy.updateOnly = generationModeType(strings.TrimSpace(d.Value)) == generationModeUpdate 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) diff --git a/walk/walk.go b/walk/walk.go index 3ff813daf..5ecf96038 100644 --- a/walk/walk.go +++ b/walk/walk.go @@ -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 @@ -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 @@ -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 diff --git a/walk/walk_test.go b/walk/walk_test.go index 7c3906c20..d29b22307 100644 --- a/walk/walk_test.go +++ b/walk/walk_test.go @@ -17,8 +17,10 @@ package walk import ( "flag" + "fmt" "path" "path/filepath" + "reflect" "testing" "github.com/bazelbuild/bazel-gazelle/config" @@ -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{ {