Skip to content

Commit

Permalink
Add experimental flags to save and load the rule index between runs
Browse files Browse the repository at this point in the history
This is intended to serve as a foundation for external tools,
or a future version of autogazelle, that detects what files have changed
and only re-runs gazelle in those directories, while still also
supporting the full power of the rule index.

See bazel-contrib#1181 for more details.
  • Loading branch information
eric-skydio committed Nov 1, 2023
1 parent d8369a6 commit 9257a30
Show file tree
Hide file tree
Showing 9 changed files with 608 additions and 136 deletions.
46 changes: 33 additions & 13 deletions cmd/gazelle/fix-update.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,14 +46,15 @@ import (
// update commands. This includes everything in config.Config, but it also
// includes some additional fields that aren't relevant to other packages.
type updateConfig struct {
dirs []string
emit emitFunc
repos []repo.Repo
workspaceFiles []*rule.File
walkMode walk.Mode
patchPath string
patchBuffer bytes.Buffer
print0 bool
dirs []string
emit emitFunc
repos []repo.Repo
workspaceFiles []*rule.File
walkMode walk.Mode
patchPath string
patchBuffer bytes.Buffer
print0 bool
readIndexFile, writeIndexFile string
}

type emitFunc func(c *config.Config, f *rule.File) error
Expand Down Expand Up @@ -89,6 +90,9 @@ func (ucr *updateConfigurer) RegisterFlags(fs *flag.FlagSet, cmd string, c *conf
fs.BoolVar(&uc.print0, "print0", false, "when set with -mode=fix, gazelle will print the names of rewritten files separated with \\0 (NULL)")
fs.Var(&gzflag.MultiFlag{Values: &ucr.knownImports}, "known_import", "import path for which external resolution is skipped (can specify multiple times)")
fs.StringVar(&ucr.repoConfigPath, "repo_config", "", "file where Gazelle should load repository configuration. Defaults to WORKSPACE.")
fs.StringVar(&uc.readIndexFile, "experimental_read_index_path", "", "path to a file that Gazelle will read the initial index from")
fs.StringVar(&uc.writeIndexFile, "experimental_write_index_path", "", "path to a file that Gazelle will populate with the dependency resolution index before exiting")

}

func (ucr *updateConfigurer) CheckFlags(fs *flag.FlagSet, c *config.Config) error {
Expand Down Expand Up @@ -126,9 +130,11 @@ func (ucr *updateConfigurer) CheckFlags(fs *flag.FlagSet, c *config.Config) erro
uc.dirs[i] = dir
}

if ucr.recursive && c.IndexLibraries {
needsIndex := c.IndexLibraries && uc.readIndexFile == ""

if ucr.recursive && needsIndex {
uc.walkMode = walk.VisitAllUpdateSubdirsMode
} else if c.IndexLibraries {
} else if needsIndex {
uc.walkMode = walk.VisitAllUpdateDirsMode
} else if ucr.recursive {
uc.walkMode = walk.UpdateSubdirsMode
Expand Down Expand Up @@ -288,7 +294,6 @@ func runFixUpdate(wd string, cmd command, args []string) (err error) {
}
exts = append(exts, lang)
}
ruleIndex := resolve.NewRuleIndex(mrslv.Resolver, exts...)

if err := fixRepoFiles(c, loads); err != nil {
return err
Expand All @@ -305,6 +310,16 @@ func runFixUpdate(wd string, cmd command, args []string) (err error) {
// Visit all directories in the repository.
var visits []visitRecord
uc := getUpdateConfig(c)

ruleIndex := resolve.NewRuleIndex(mrslv.Resolver, exts...)
if uc.readIndexFile != "" {
exclude := walk.NewUpdateFilter(c.RepoRoot, uc.dirs, uc.walkMode).ShouldIndex
err := ruleIndex.ReadFromFile(uc.readIndexFile, exclude)
if err != nil {
return err
}
}

var errorsFromWalk []error
walk.Walk(c, cexts, uc.dirs, uc.walkMode, func(dir, rel string, c *config.Config, update bool, f *rule.File, subdirs, regularFiles, genFiles []string) {
// If this file is ignored or if Gazelle was not asked to update this
Expand Down Expand Up @@ -432,8 +447,13 @@ func runFixUpdate(wd string, cmd command, args []string) (err error) {
return fmt.Errorf("encountered multiple errors: %w, %v", errorsFromWalk[0], strings.Join(additionalErrors, ", "))
}

// Finish building the index for dependency resolution.
ruleIndex.Finish()
// Indexing is done, write out the file if requested
if uc.writeIndexFile != "" {
err := ruleIndex.WriteToFile(uc.writeIndexFile)
if err != nil {
return err
}
}

// Resolve dependencies.
rc, cleanupRc := repo.NewRemoteCache(uc.repos)
Expand Down
132 changes: 129 additions & 3 deletions cmd/gazelle/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ package main
import (
"bytes"
"flag"
"fmt"
"io/ioutil"
"log"
"os"
Expand Down Expand Up @@ -2038,7 +2037,7 @@ my_go_binary(
`,
},
{
Path: "enabled/multiple_mappings/multiple_mappings.go",
Path: "enabled/multiple_mappings/multiple_mappings.go",
Content: `
package main
Expand Down Expand Up @@ -3046,7 +3045,7 @@ github.com/selvatico/go-mocket v1.0.7/go.mod h1:7bSWzuNieCdUlanCVu3w0ppS0LvDtPAZ
if err := runGazelle(dir, args); err == nil {
t.Fatal("expected error, got nil")
} else if err.Error() != errMsg {
t.Error(fmt.Sprintf("want %s, got %s", errMsg, err.Error()))
t.Errorf("want %s, got %s", errMsg, err.Error())
}
}

Expand Down Expand Up @@ -4515,3 +4514,130 @@ require (
t.Fatalf("got %s ; want %s; diff %s", string(got), want, cmp.Diff(string(got), want))
}
}

// TestExperimentalPersistentIndex tests that the experimental persistent index
// feature works as expected. It performs several steps:
// 1. Set up a workspace with two directories, where b depends on a.
// 2. Run Gazelle on the workspace, exporting the index. This should create
// BUILD files for both a and b, and write the index to disk.
// 3. Modify the label for a in its BUILD file, but rerun Gazelle on only b.
// This should not modify b's BUILD file, since Gazelle is using the (stale)
// index.
// 4. Rerun Gazelle on a, using the index. This should update the index, but not
// modify b's BUILD file.
// 5. Using the new index, rerun Gazelle on b. This should update b's BUILD file
// to use the new label for a.
func TestExperimentalPersistentIndex(t *testing.T) {
dir, cleanup := testtools.CreateFiles(t, []testtools.FileSpec{
{Path: "WORKSPACE"},
{
Path: "a/a.go",
Content: `package a`,
},
{
Path: "b/b.go",
Content: `
package b
import _ "example.com/foo/a"
`,
},
})

t.Cleanup(cleanup)

index_path := filepath.Join(dir, "index.json")

args := []string{
"update",
"-go_prefix=example.com/foo",
"-experimental_write_index_path=" + index_path,
}
if err := runGazelle(dir, args); err != nil {
t.Fatal(err)
}

// Confirm that the index file was written
// var indexData string
if got, err := os.ReadFile(index_path); err != nil {
t.Fatal(err)
} else {
if len(got) < 10 {
t.Errorf("index file seems to be empty: %s", string(got))
}
}

buildA := testtools.FileSpec{
Path: "a/BUILD.bazel", Content: `
load("@io_bazel_rules_go//go:def.bzl", "go_library")
go_library(
name = "a",
srcs = ["a.go"],
importpath = "example.com/foo/a",
visibility = ["//visibility:public"],
)
`}
buildB := testtools.FileSpec{
Path: "b/BUILD.bazel", Content: `
load("@io_bazel_rules_go//go:def.bzl", "go_library")
go_library(
name = "b",
srcs = ["b.go"],
importpath = "example.com/foo/b",
visibility = ["//visibility:public"],
deps = ["//a"],
)
`}

testtools.CheckFiles(t, dir, []testtools.FileSpec{buildA, buildB})

// Modify the BUILD file file for a to have a different label, but use the old index and don't notify Gazelle about this fact.
newBuildA := buildA
newBuildA.Content = strings.ReplaceAll(newBuildA.Content, `"a"`, `"a_new"`)

if err := os.WriteFile(filepath.Join(dir, "a/BUILD.bazel"), []byte(newBuildA.Content), 0666); err != nil {
t.Fatal(err)
}

updateBArgs := []string{
"update",
"-go_prefix=example.com/foo",
"-experimental_read_index_path=" + index_path,
"b/",
}
if err := runGazelle(dir, updateBArgs); err != nil {
t.Fatal(err)
}

// Check that Gazelle has not modified buildB (because it didn't scan a/).
testtools.CheckFiles(t, dir, []testtools.FileSpec{newBuildA, buildB})

// This time, tell Gazelle to scan a/. It should notice that the label has change, and update
// the index. Because we're not processing b/, that change won't be reflected in b/BUILD.bazel.
updateAArgs := []string{
"update",
"-go_prefix=example.com/foo",
"-experimental_read_index_path=" + index_path,
"-experimental_write_index_path=" + index_path,
"a/",
}
if err := runGazelle(dir, updateAArgs); err != nil {
t.Fatal(err)
}

testtools.CheckFiles(t, dir, []testtools.FileSpec{newBuildA, buildB})

// Now, run Gazelle on b/ with the index again. It should notice that the label has changed,
// and update b/BUILD.bazel

if err := runGazelle(dir, updateBArgs); err != nil {
t.Fatal(err)
}

newBuildB := buildB
newBuildB.Content = strings.ReplaceAll(newBuildB.Content, `"//a"`, `"//a:a_new"`)

testtools.CheckFiles(t, dir, []testtools.FileSpec{newBuildA, newBuildB})
}
1 change: 1 addition & 0 deletions config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,7 @@ type Configurer interface {
// i.e., those that apply to Config itself and not to Config.Exts.
type CommonConfigurer struct {
repoRoot, buildFileNames, readBuildFilesDir, writeBuildFilesDir string
readIndexFile, writeIndexFile string
indexLibraries, strict bool
langCsv string
bzlmod bool
Expand Down
3 changes: 0 additions & 3 deletions language/go/resolve_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -999,7 +999,6 @@ go_proto_library(
imports[i] = convertImportsAttr(r)
ix.AddRule(c, r, f)
}
ix.Finish()
for i, r := range f.Rules {
mrslv.Resolver(r, "").Resolve(c, ix, rc, r, imports[i], label.New("", tc.old.rel, r.Name()))
}
Expand All @@ -1023,7 +1022,6 @@ func TestResolveDisableGlobal(t *testing.T) {
exts = append(exts, lang)
}
ix := resolve.NewRuleIndex(nil, exts...)
ix.Finish()
rc := testRemoteCache([]repo.Repo{
{
Name: "com_github_golang_protobuf",
Expand Down Expand Up @@ -1107,7 +1105,6 @@ func TestResolveExternal(t *testing.T) {
"-go_prefix=example.com/local")
gc := getGoConfig(c)
ix := resolve.NewRuleIndex(nil)
ix.Finish()
gl := langs[1].(*goLang)
for _, tc := range []struct {
desc, importpath string
Expand Down
1 change: 0 additions & 1 deletion language/proto/resolve_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,6 @@ proto_library(
imports[i] = convertImportsAttr(r)
ix.AddRule(c, r, f)
}
ix.Finish()
for i, r := range f.Rules {
lang.Resolve(c, ix, rc, r, imports[i], label.New("", "test", r.Name()))
}
Expand Down
16 changes: 15 additions & 1 deletion resolve/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
load("@io_bazel_rules_go//go:def.bzl", "go_library")
load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")

go_library(
name = "resolve",
Expand All @@ -23,6 +23,7 @@ filegroup(
"BUILD.bazel",
"config.go",
"index.go",
"index_test.go",
],
visibility = ["//visibility:public"],
)
Expand All @@ -32,3 +33,16 @@ alias(
actual = ":resolve",
visibility = ["//visibility:public"],
)

go_test(
name = "resolve_test",
srcs = ["index_test.go"],
deps = [
":resolve",
"//config",
"//label",
"//repo",
"//rule",
"@com_github_google_go_cmp//cmp",
],
)
Loading

0 comments on commit 9257a30

Please sign in to comment.