Skip to content

Commit

Permalink
Gazelle: use rule index to resolve dependencies
Browse files Browse the repository at this point in the history
  • Loading branch information
Jay Conrod committed Dec 5, 2017
1 parent fd30212 commit f30ee92
Show file tree
Hide file tree
Showing 6 changed files with 531 additions and 129 deletions.
174 changes: 165 additions & 9 deletions go/tools/gazelle/gazelle/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -391,15 +391,32 @@ go_library(
}
}

// TestMultipleDirectories checks that all directories in a repository are
// indexed but only directories listed on the command line are updated.
func TestMultipleDirectories(t *testing.T) {
files := []fileSpec{
{path: "WORKSPACE"},
{
path: "a/BUILD.bazel",
content: `# This file shouldn't be modified.
load("@io_bazel_rules_go//go:def.bzl", "go_library")
go_library(
name = "go_default_library",
srcs = ["a.go"],
importpath = "example.com/foo/x",
)
`,
}, {
path: "a/a.go",
content: "package a",
}, {
path: "b/b.go",
content: "package b",
path: "b/b.go",
content: `
package b
import _ "example.com/foo/x"
`,
},
}
dir, err := createFiles(files)
Expand All @@ -408,16 +425,27 @@ func TestMultipleDirectories(t *testing.T) {
}
defer os.RemoveAll(dir)

args := []string{"-go_prefix", "example.com/foo", "a", "b"}
args := []string{"-go_prefix", "example.com/foo", "b"}
if err := runGazelle(dir, args); err != nil {
t.Fatal(err)
}
for _, d := range []string{"a", "b"} {
path := filepath.Join(dir, d, config.DefaultValidBuildFileNames[0])
if _, err := os.Stat(path); err != nil {
t.Errorf("directory %s not visited: %v", d, err)
}
}

checkFiles(t, dir, []fileSpec{
files[1], // should not change
{
path: "b/BUILD.bazel",
content: `load("@io_bazel_rules_go//go:def.bzl", "go_library")
go_library(
name = "go_default_library",
srcs = ["b.go"],
importpath = "example.com/foo/b",
visibility = ["//visibility:public"],
deps = ["//a:go_default_library"],
)
`,
},
})
}

func TestErrorOutsideWorkspace(t *testing.T) {
Expand Down Expand Up @@ -845,6 +873,134 @@ go_library(
}})
}

// TestResolveKeptImportpath checks that Gazelle can resolve dependencies
// against a library with a '# keep' comment on its importpath attribute
// when the importpath doesn't match what Gazelle would infer.
func TestResolveKeptImportpath(t *testing.T) {
files := []fileSpec{
{path: "WORKSPACE"},
{
path: "foo/foo.go",
content: `
package foo
import _ "example.com/alt/baz"
`,
}, {
path: "bar/BUILD.bazel",
content: `load("@io_bazel_rules_go//go:def.bzl", "go_library")
go_library(
name = "go_default_library",
srcs = ["bar.go"],
importpath = "example.com/alt/baz", # keep
visibility = ["//visibility:public"],
)
`,
}, {
path: "bar/bar.go",
content: "package bar",
},
}

dir, err := createFiles(files)
if err != nil {
t.Fatal(err)
}
defer os.RemoveAll(dir)

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

checkFiles(t, dir, []fileSpec{
{
path: "foo/BUILD.bazel",
content: `
load("@io_bazel_rules_go//go:def.bzl", "go_library")
go_library(
name = "go_default_library",
srcs = ["foo.go"],
importpath = "example.com/repo/foo",
visibility = ["//visibility:public"],
deps = ["//bar:go_default_library"],
)
`,
}, {
path: "bar/BUILD.bazel",
content: `load("@io_bazel_rules_go//go:def.bzl", "go_library")
go_library(
name = "go_default_library",
srcs = ["bar.go"],
importpath = "example.com/alt/baz", # keep
visibility = ["//visibility:public"],
)
`,
},
})
}

// TestResolveVendorSubdirectory checks that Gazelle can resolve libraries
// in a vendor directory which is not at the repository root.
func TestResolveVendorSubdirectory(t *testing.T) {
files := []fileSpec{
{path: "WORKSPACE"},
{
path: "sub/vendor/example.com/foo/foo.go",
content: "package foo",
}, {
path: "sub/bar/bar.go",
content: `
package bar
import _ "example.com/foo"
`,
},
}
dir, err := createFiles(files)
if err != nil {
t.Fatal(err)
}
defer os.RemoveAll(dir)

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

checkFiles(t, dir, []fileSpec{
{
path: "sub/vendor/example.com/foo/BUILD.bazel",
content: `
load("@io_bazel_rules_go//go:def.bzl", "go_library")
go_library(
name = "go_default_library",
srcs = ["foo.go"],
importpath = "example.com/foo",
visibility = ["//visibility:public"],
)
`,
}, {
path: "sub/bar/BUILD.bazel",
content: `
load("@io_bazel_rules_go//go:def.bzl", "go_library")
go_library(
name = "go_default_library",
srcs = ["bar.go"],
importpath = "example.com/repo/sub/bar",
visibility = ["//visibility:public"],
deps = ["//sub/vendor/example.com/foo:go_default_library"],
)
`,
},
})
}

// TODO(jayconrod): more tests
// run in fix mode in testdata directories to create new files
// run in diff mode in testdata directories to update existing files (no change)
62 changes: 35 additions & 27 deletions go/tools/gazelle/gazelle/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,15 +83,15 @@ func (vs byPkgRel) Swap(i, j int) { vs[i], vs[j] = vs[j], vs[i] }
func run(c *config.Config, cmd command, emit emitFunc) {
shouldFix := c.ShouldFix
l := resolve.NewLabeler(c)
ruleIndex := resolve.NewRuleIndex()

var visits []visitRecord

// Visit directories to modify.
// TODO: visit all directories in the repository in order to index rules.
for _, dir := range c.Dirs {
packages.Walk(c, dir, func(rel string, c *config.Config, pkg *packages.Package, file *bf.File, isUpdateDir bool) {
// Fix existing files.
if file != nil {
// Visit all directories in the repository.
packages.Walk(c, c.RepoRoot, func(rel string, c *config.Config, pkg *packages.Package, file *bf.File, isUpdateDir bool) {
if file != nil {
// Fix files in update directories.
if isUpdateDir {
file = merger.FixFileMinor(c, file)
if shouldFix {
file = merger.FixFile(c, file)
Expand All @@ -103,30 +103,38 @@ func run(c *config.Config, cmd command, emit emitFunc) {
}
}

// TODO: Index rules in existing files.
// TODO: delete rules in directories where pkg == nil (no buildable
// Go code).

// Generate rules.
if pkg != nil {
g := rules.NewGenerator(c, l, file)
rules, empty := g.GenerateRules(pkg)
file, rules = merger.MergeFile(rules, empty, file, merger.MergeableGeneratedAttrs)
if file.Path == "" {
file.Path = filepath.Join(c.RepoRoot, filepath.FromSlash(rel), c.DefaultBuildFileName())
}
visits = append(visits, visitRecord{
pkgRel: rel,
rules: rules,
file: file,
})
// Index existing rules.
ruleIndex.AddRulesFromFile(c, file)
}

// TODO(#939): delete rules in directories where pkg == nil (no buildable
// Go code).
if !isUpdateDir {
return
}

// Generate rules.
if pkg != nil {
g := rules.NewGenerator(c, l, file)
rules, empty := g.GenerateRules(pkg)
file, rules = merger.MergeFile(rules, empty, file, merger.MergeableGeneratedAttrs)
if file.Path == "" {
file.Path = filepath.Join(c.RepoRoot, filepath.FromSlash(rel), c.DefaultBuildFileName())
}
})
}
ruleIndex.AddGeneratedRules(c, rel, rules)
visits = append(visits, visitRecord{
pkgRel: rel,
rules: rules,
file: file,
})
}
})

// Finish building the index for dependency resolution.
ruleIndex.Finish()

// Resolve dependencies.
// TODO: resolve dependencies using the index.
resolver := resolve.NewResolver(c, l)
resolver := resolve.NewResolver(c, l, ruleIndex)
for i := range visits {
for j := range visits[i].rules {
visits[i].rules[j] = resolver.ResolveRule(visits[i].rules[j], visits[i].pkgRel)
Expand Down
4 changes: 1 addition & 3 deletions go/tools/gazelle/packages/walk.go
Original file line number Diff line number Diff line change
Expand Up @@ -153,9 +153,7 @@ func Walk(c *config.Config, root string, f WalkFunc) {
for _, f := range files {
base := f.Name()
switch {
case base == "" || base[0] == '.' || base[0] == '_' ||
excluded[base] ||
base == "vendor" && f.IsDir() && c.DepMode == config.ExternalMode:
case base == "" || base[0] == '.' || base[0] == '_' || excluded[base]:
continue

case f.IsDir():
Expand Down
64 changes: 0 additions & 64 deletions go/tools/gazelle/packages/walk_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -605,70 +605,6 @@ package proto_only;`,
checkFiles(t, files, "", want)
}

func TestVendor(t *testing.T) {
files := []fileSpec{
{path: "vendor/foo/foo.go", content: "package foo"},
{path: "x/vendor/bar/bar.go", content: "package bar"},
}

for _, tc := range []struct {
desc string
mode config.DependencyMode
want []*packages.Package
}{
{
desc: "external mode",
mode: config.ExternalMode,
want: nil,
},
{
desc: "vendored mode",
mode: config.VendorMode,
want: []*packages.Package{
{
Name: "foo",
Rel: "vendor/foo",
Library: packages.GoTarget{
Sources: packages.PlatformStrings{
Generic: []string{"foo.go"},
},
},
},
{
Name: "bar",
Rel: "x/vendor/bar",
Library: packages.GoTarget{
Sources: packages.PlatformStrings{
Generic: []string{"bar.go"},
},
},
},
},
},
} {
t.Run(tc.desc, func(t *testing.T) {
dir, err := createFiles(files)
if err != nil {
os.RemoveAll(dir)
}

for _, p := range tc.want {
p.Dir = filepath.Join(dir, filepath.FromSlash(p.Rel))
}

c := &config.Config{
RepoRoot: dir,
Dirs: []string{dir},
GoPrefix: "",
ValidBuildFileNames: config.DefaultValidBuildFileNames,
DepMode: tc.mode,
}
got := walkPackages(c)
checkPackages(t, got, tc.want)
})
}
}

func TestMalformedBuildFile(t *testing.T) {
files := []fileSpec{
{path: "BUILD", content: "????"},
Expand Down
Loading

0 comments on commit f30ee92

Please sign in to comment.