From 4f524f20aff5ae9b00ecaabbf43c3e8c9804bd0b Mon Sep 17 00:00:00 2001 From: Brandon Lico Date: Wed, 17 Apr 2019 08:21:37 -0700 Subject: [PATCH] repository_macro directive for referencing macros from WORKSPACE (#493) Added the repository_macro directive with the following format: # gazelle:repository_macro repos.bzl%go_repositories Gazelle only recognizes this directive when reading from the WORKSPACE file. fix-update uses the directive to know about rules defined in a macro file, so they can be used during dependency resolution. update-repos can now update repository rules in the corresponding macros without additional command line arguments instead of writing copies to WORKSPACE. The to-macro flag functionality has been modified to only write new repository rules to the corresponding macro, and to update existing rules in macros and the WORKSPACE file. However, duplicates will still be written to the macro if the repository rule is not a go_repository rule. Fixes #483 --- README.rst | 20 ++- cmd/gazelle/fix-update.go | 37 +++-- cmd/gazelle/integration_test.go | 240 ++++++++++++++++++++++++++++++-- cmd/gazelle/update-repos.go | 84 ++++++----- merger/merger.go | 8 +- merger/merger_test.go | 9 +- repo/BUILD.bazel | 1 + repo/repo.go | 105 ++++++++++++-- repo/repo_test.go | 100 +++++++++++-- rule/rule.go | 21 +++ 10 files changed, 533 insertions(+), 92 deletions(-) diff --git a/README.rst b/README.rst index 85eabf985..01070fc3c 100644 --- a/README.rst +++ b/README.rst @@ -415,9 +415,9 @@ The following flags are accepted: +----------------------------------------------------------------------------------------------------------+----------------------------------------------+ | :flag:`-to_macro macroFile%defName` | | +----------------------------------------------------------------------------------------------------------+----------------------------------------------+ -| Tells Gazelle to write repository rules into a .bzl macro function rather than the WORKSPACE file. | -| Currently, Gazelle won't see repositories declared in the macro when updating build files. This means that it won't use custom repository names, and it | -| will go out to the network when trying to determine repository roots. | +| Tells Gazelle to write new repository rules into a .bzl macro function rather than the WORKSPACE file. | +| | +| The ``repository_macro`` directive should be added to the WORKSPACE in order for future Gazelle calls to recognize the repos defined in the macro file. | +----------------------------------------------------------------------------------------------------------+----------------------------------------------+ | :flag:`-build_file_names file1,file2,...` | | +----------------------------------------------------------------------------------------------------------+----------------------------------------------+ @@ -669,6 +669,20 @@ The following directives are recognized: | | +---------------------------------------------------+----------------------------------------+ +Gazelle also reads directives from the WORKSPACE file. They may be used to +discover custom repository names and known prefixes. The ``fix`` and ``update`` +commands use these directives for dependency resolution. ``update-repos`` uses +them to learn about repository rules defined in alternate locations. + ++-------------------------------------------------------+----------------------------------------+ +| **WORKSPACE Directive** | **Default value** | ++=======================================================+========================================+ +| :direc:`# gazelle:repository_macro macroFile%defName` | n/a | ++-------------------------------------------------------+----------------------------------------+ +| Tells Gazelle to look for repository rules in a macro in a .bzl file. The directive can be | +| repeated multiple times. | +| The macro can be generated by calling ``update-repos`` with the ``to_macro`` flag. | ++-------------------------------------------------------+----------------------------------------+ Keep comments ~~~~~~~~~~~~~ diff --git a/cmd/gazelle/fix-update.go b/cmd/gazelle/fix-update.go index 3cdb21b7c..40e40be0b 100644 --- a/cmd/gazelle/fix-update.go +++ b/cmd/gazelle/fix-update.go @@ -360,11 +360,23 @@ func newFixUpdateConfiguration(cmd command, args []string, cexts []config.Config return nil, err } } else { - if err := fixWorkspace(c, workspace, loads); err != nil { + c.RepoName = findWorkspaceName(workspace) + var reposFiles map[*rule.File][]string + uc.repos, reposFiles, err = repo.ListRepositories(workspace) + if err != nil { + return nil, err + } + files := make([]*rule.File, 0, len(reposFiles)) + seen := make(map[string]bool) + for f := range reposFiles { + if !seen[f.Path] { + files = append(files, f) + seen[f.Path] = true + } + } + if err := fixRepoFiles(c, files, loads); err != nil { return nil, err } - c.RepoName = findWorkspaceName(workspace) - uc.repos = repo.ListRepositories(workspace) } repoPrefixes := make(map[string]bool) for _, r := range uc.repos { @@ -413,7 +425,7 @@ FLAGS: fs.PrintDefaults() } -func fixWorkspace(c *config.Config, workspace *rule.File, loads []rule.LoadInfo) error { +func fixRepoFiles(c *config.Config, files []*rule.File, loads []rule.LoadInfo) error { uc := getUpdateConfig(c) if !c.ShouldFix { return nil @@ -428,12 +440,19 @@ func fixWorkspace(c *config.Config, workspace *rule.File, loads []rule.LoadInfo) return nil } - merger.FixWorkspace(workspace) - merger.FixLoads(workspace, loads) - if err := merger.CheckGazelleLoaded(workspace); err != nil { - return err + for _, f := range files { + merger.FixLoads(f, loads) + if f.Path == filepath.Join(c.RepoRoot, "WORKSPACE") { + merger.FixWorkspace(f) + if err := merger.CheckGazelleLoaded(f); err != nil { + return err + } + } + if err := uc.emit(c, f); err != nil { + return err + } } - return uc.emit(c, workspace) + return nil } func findWorkspaceName(f *rule.File) string { diff --git a/cmd/gazelle/integration_test.go b/cmd/gazelle/integration_test.go index f92ccc579..276397eec 100644 --- a/cmd/gazelle/integration_test.go +++ b/cmd/gazelle/integration_test.go @@ -1253,10 +1253,37 @@ go_repository( }}) } -func TestImportReposFromDepToMacro(t *testing.T) { +func TestImportReposFromDepToWorkspaceWithMacro(t *testing.T) { files := []testtools.FileSpec{ - {Path: "WORKSPACE"}, { + Path: "WORKSPACE", + Content: ` +http_archive( + name = "io_bazel_rules_go", + url = "https://github.com/bazelbuild/rules_go/releases/download/0.10.1/rules_go-0.10.1.tar.gz", + sha256 = "4b14d8dd31c6dbaf3ff871adcd03f28c3274e42abc855cb8fb4d01233c0154dc", +) +http_archive( + name = "bazel_gazelle", + url = "https://github.com/bazelbuild/bazel-gazelle/releases/download/0.10.0/bazel-gazelle-0.10.0.tar.gz", + sha256 = "6228d9618ab9536892aa69082c063207c91e777e51bd3c5544c9c060cafe1bd8", +) +load("@io_bazel_rules_go//go:def.bzl", "go_rules_dependencies", "go_register_toolchains", "go_repository") +go_rules_dependencies() +go_register_toolchains() + +load("@bazel_gazelle//:deps.bzl", "gazelle_dependencies") +gazelle_dependencies() + +http_archive( + name = "com_github_go_yaml_yaml", + urls = ["https://example.com/yaml.tar.gz"], + sha256 = "1234", +) + +# gazelle:repository_macro repositories.bzl%go_repositories +`, + }, { Path: "repositories.bzl", Content: ` load("@bazel_gazelle//:deps.bzl", "go_repository") @@ -1274,12 +1301,6 @@ def go_repositories(): importpath = "golang.org/x/sys", remote = "https://github.com/golang/sys", ) - - http_archive( - name = "com_github_go_yaml_yaml", - urls = ["https://example.com/yaml.tar.gz"], - sha256 = "1234", - ) `, }, { Path: "Gopkg.lock", @@ -1322,13 +1343,53 @@ def go_repositories(): dir, cleanup := testtools.CreateFiles(t, files) defer cleanup() - args := []string{"update-repos", "-build_file_generation", "off", "-from_file", "Gopkg.lock", "-to_macro", "repositories.bzl%go_repositories"} + args := []string{"update-repos", "-build_file_generation", "off", "-from_file", "Gopkg.lock"} if err := runGazelle(dir, args); err != nil { t.Fatal(err) } testtools.CheckFiles(t, dir, []testtools.FileSpec{ { + Path: "WORKSPACE", + Content: ` +http_archive( + name = "io_bazel_rules_go", + url = "https://github.com/bazelbuild/rules_go/releases/download/0.10.1/rules_go-0.10.1.tar.gz", + sha256 = "4b14d8dd31c6dbaf3ff871adcd03f28c3274e42abc855cb8fb4d01233c0154dc", +) + +http_archive( + name = "bazel_gazelle", + url = "https://github.com/bazelbuild/bazel-gazelle/releases/download/0.10.0/bazel-gazelle-0.10.0.tar.gz", + sha256 = "6228d9618ab9536892aa69082c063207c91e777e51bd3c5544c9c060cafe1bd8", +) + +load("@io_bazel_rules_go//go:def.bzl", "go_register_toolchains", "go_rules_dependencies") + +go_rules_dependencies() + +go_register_toolchains() + +load("@bazel_gazelle//:deps.bzl", "gazelle_dependencies", "go_repository") + +gazelle_dependencies() + +http_archive( + name = "com_github_go_yaml_yaml", + urls = ["https://example.com/yaml.tar.gz"], + sha256 = "1234", +) + +# gazelle:repository_macro repositories.bzl%go_repositories + +go_repository( + name = "com_github_pkg_errors", + build_file_generation = "off", + commit = "645ef00459ed84a119197bfb8d8205042c6df63d", + importpath = "github.com/pkg/errors", +) +`, + }, { Path: "repositories.bzl", Content: ` load("@bazel_gazelle//:deps.bzl", "go_repository") @@ -1348,10 +1409,160 @@ def go_repositories(): remote = "https://github.com/golang/sys", ) - http_archive( - name = "com_github_go_yaml_yaml", - urls = ["https://example.com/yaml.tar.gz"], - sha256 = "1234", +`, + }, + }) +} + +func TestImportReposFromDepToMacroWithWorkspace(t *testing.T) { + files := []testtools.FileSpec{ + { + Path: "WORKSPACE", + Content: ` +http_archive( + name = "io_bazel_rules_go", + url = "https://github.com/bazelbuild/rules_go/releases/download/0.10.1/rules_go-0.10.1.tar.gz", + sha256 = "4b14d8dd31c6dbaf3ff871adcd03f28c3274e42abc855cb8fb4d01233c0154dc", +) + +http_archive( + name = "bazel_gazelle", + url = "https://github.com/bazelbuild/bazel-gazelle/releases/download/0.10.0/bazel-gazelle-0.10.0.tar.gz", + sha256 = "6228d9618ab9536892aa69082c063207c91e777e51bd3c5544c9c060cafe1bd8", +) + +load("@io_bazel_rules_go//go:def.bzl", "go_register_toolchains", "go_rules_dependencies") + +go_rules_dependencies() + +go_register_toolchains() + +load("@bazel_gazelle//:deps.bzl", "gazelle_dependencies", "go_repository") + +gazelle_dependencies() + +http_archive( + name = "com_github_go_yaml_yaml", + urls = ["https://example.com/yaml.tar.gz"], + sha256 = "1234", +) + +# gazelle:repository_macro repositories.bzl%go_repositories +# gazelle:repository_macro repositories.bzl%foo_repositories +`, + }, { + Path: "repositories.bzl", + Content: ` +load("@bazel_gazelle//:deps.bzl", "go_repository") + +def go_repositories(): + go_repository( + name = "org_golang_x_sys", + importpath = "golang.org/x/sys", + remote = "https://github.com/golang/sys", + ) + +def foo_repositories(): + go_repository( + name = "org_golang_x_net", + importpath = "golang.org/x/net", + tag = "1.2", + ) +`, + }, { + Path: "Gopkg.lock", + Content: `# This file is autogenerated, do not edit; changes may be undone by the next 'dep ensure'. + + +[[projects]] + name = "github.com/pkg/errors" + packages = ["."] + revision = "645ef00459ed84a119197bfb8d8205042c6df63d" + version = "v0.8.0" + +[[projects]] + branch = "master" + name = "golang.org/x/net" + packages = ["context"] + revision = "66aacef3dd8a676686c7ae3716979581e8b03c47" + +[[projects]] + branch = "master" + name = "golang.org/x/sys" + packages = ["unix"] + revision = "bb24a47a89eac6c1227fbcb2ae37a8b9ed323366" + +[solve-meta] + analyzer-name = "dep" + analyzer-version = 1 + inputs-digest = "05c1cd69be2c917c0cc4b32942830c2acfa044d8200fdc94716aae48a8083702" + solver-name = "gps-cdcl" + solver-version = 1 +`, + }, + } + dir, cleanup := testtools.CreateFiles(t, files) + defer cleanup() + + args := []string{"update-repos", "-build_file_generation", "off", "-from_file", "Gopkg.lock", "-to_macro", "repositories.bzl%foo_repositories"} + if err := runGazelle(dir, args); err != nil { + t.Fatal(err) + } + + testtools.CheckFiles(t, dir, []testtools.FileSpec{ + { + Path: "WORKSPACE", + Content: ` +http_archive( + name = "io_bazel_rules_go", + url = "https://github.com/bazelbuild/rules_go/releases/download/0.10.1/rules_go-0.10.1.tar.gz", + sha256 = "4b14d8dd31c6dbaf3ff871adcd03f28c3274e42abc855cb8fb4d01233c0154dc", +) + +http_archive( + name = "bazel_gazelle", + url = "https://github.com/bazelbuild/bazel-gazelle/releases/download/0.10.0/bazel-gazelle-0.10.0.tar.gz", + sha256 = "6228d9618ab9536892aa69082c063207c91e777e51bd3c5544c9c060cafe1bd8", +) + +load("@io_bazel_rules_go//go:def.bzl", "go_register_toolchains", "go_rules_dependencies") + +go_rules_dependencies() + +go_register_toolchains() + +load("@bazel_gazelle//:deps.bzl", "gazelle_dependencies", "go_repository") + +gazelle_dependencies() + +http_archive( + name = "com_github_go_yaml_yaml", + urls = ["https://example.com/yaml.tar.gz"], + sha256 = "1234", +) + +# gazelle:repository_macro repositories.bzl%go_repositories +# gazelle:repository_macro repositories.bzl%foo_repositories +`, + }, { + Path: "repositories.bzl", + Content: ` +load("@bazel_gazelle//:deps.bzl", "go_repository") + +def go_repositories(): + go_repository( + name = "org_golang_x_sys", + build_file_generation = "off", + commit = "bb24a47a89eac6c1227fbcb2ae37a8b9ed323366", + importpath = "golang.org/x/sys", + ) + +def foo_repositories(): + go_repository( + name = "org_golang_x_net", + build_file_generation = "off", + commit = "66aacef3dd8a676686c7ae3716979581e8b03c47", + importpath = "golang.org/x/net", ) go_repository( name = "com_github_pkg_errors", @@ -1360,7 +1571,8 @@ def go_repositories(): importpath = "github.com/pkg/errors", ) `, - }}) + }, + }) } func TestImportReposFromDepToEmptyMacro(t *testing.T) { diff --git a/cmd/gazelle/update-repos.go b/cmd/gazelle/update-repos.go index b484458c4..fe5195be4 100644 --- a/cmd/gazelle/update-repos.go +++ b/cmd/gazelle/update-repos.go @@ -31,7 +31,7 @@ import ( "github.com/bazelbuild/bazel-gazelle/rule" ) -type updateReposFn func(c *updateReposConfig, oldFile *rule.File, kinds map[string]rule.KindInfo) error +type updateReposFn func(c *updateReposConfig, workspace *rule.File, oldFile *rule.File, kinds map[string]rule.KindInfo) ([]*rule.File, error) type updateReposConfig struct { fn updateReposFn @@ -69,6 +69,9 @@ func (f macroFlag) Set(value string) error { if len(args) != 2 { return fmt.Errorf("Failure parsing to_macro: %s, expected format is macroFile%%defName", value) } + if strings.HasPrefix(args[0], "..") { + return fmt.Errorf("Failure parsing to_macro: %s, macro file path %s should not start with \"..\"", value, args[0]) + } *f.macroFileName = args[0] *f.macroDefName = args[1] return nil @@ -131,41 +134,43 @@ func updateRepos(args []string) error { } uc := getUpdateReposConfig(c) - var f *rule.File - var path string + path := filepath.Join(c.RepoRoot, "WORKSPACE") + workspace, err := rule.LoadWorkspaceFile(path, "") + if err != nil { + return fmt.Errorf("error loading %q: %v", path, err) + } + var destFile *rule.File if uc.macroFileName == "" { - path = filepath.Join(c.RepoRoot, "WORKSPACE") - f, err = rule.LoadWorkspaceFile(path, "") + destFile = workspace } else { - path = uc.macroFileName - if _, err := os.Stat(path); os.IsNotExist(err) { - f, err = rule.EmptyMacroFile(path, "", uc.macroDefName) + macroPath := filepath.Join(c.RepoRoot, filepath.Clean(uc.macroFileName)) + if _, err = os.Stat(macroPath); os.IsNotExist(err) { + destFile, err = rule.EmptyMacroFile(macroPath, "", uc.macroDefName) } else { - f, err = rule.LoadMacroFile(path, "", uc.macroDefName) + destFile, err = rule.LoadMacroFile(macroPath, "", uc.macroDefName) + } + if err != nil { + return fmt.Errorf("error loading %q: %v", macroPath, err) } - } - if err != nil { - return fmt.Errorf("error loading %q: %v", path, err) } - if uc.macroFileName == "" { - merger.FixWorkspace(f) - } + merger.FixWorkspace(workspace) - if err := uc.fn(uc, f, kinds); err != nil { + files, err := uc.fn(uc, workspace, destFile, kinds) + if err != nil { return err } - - merger.FixLoads(f, loads) - if uc.macroFileName == "" { - if err := merger.CheckGazelleLoaded(f); err != nil { + for _, f := range files { + merger.FixLoads(f, loads) + if f.Path == workspace.Path { + if err := merger.CheckGazelleLoaded(workspace); err != nil { + return err + } + } + if err := f.Save(f.Path); err != nil { return err } } - - if err := f.Save(f.Path); err != nil { - return fmt.Errorf("error writing %q: %v", f.Path, err) - } return nil } @@ -214,9 +219,12 @@ FLAGS: fs.PrintDefaults() } -func updateImportPaths(c *updateReposConfig, f *rule.File, kinds map[string]rule.KindInfo) error { - rs := repo.ListRepositories(f) - rc, cleanupRc := repo.NewRemoteCache(rs) +func updateImportPaths(c *updateReposConfig, workspace *rule.File, destFile *rule.File, kinds map[string]rule.KindInfo) ([]*rule.File, error) { + repos, reposByFile, err := repo.ListRepositories(workspace) + if err != nil { + return nil, err + } + rc, cleanupRc := repo.NewRemoteCache(repos) defer cleanupRc() genRules := make([]*rule.Rule, len(c.importPaths)) @@ -242,27 +250,29 @@ func updateImportPaths(c *updateReposConfig, f *rule.File, kinds map[string]rule for _, err := range errs { if err != nil { - return err + return nil, err } } - merger.MergeFile(f, nil, genRules, merger.PreResolve, kinds) - return nil + files := repo.MergeRules(genRules, reposByFile, destFile, kinds) + return files, nil } -func importFromLockFile(c *updateReposConfig, f *rule.File, kinds map[string]rule.KindInfo) error { - rs := repo.ListRepositories(f) - rc, cleanupRc := repo.NewRemoteCache(rs) +func importFromLockFile(c *updateReposConfig, workspace *rule.File, destFile *rule.File, kinds map[string]rule.KindInfo) ([]*rule.File, error) { + repos, reposByFile, err := repo.ListRepositories(workspace) + if err != nil { + return nil, err + } + rc, cleanupRc := repo.NewRemoteCache(repos) defer cleanupRc() genRules, err := repo.ImportRepoRules(c.lockFilename, rc) if err != nil { - return err + return nil, err } for i := range genRules { applyBuildAttributes(c, genRules[i]) } - - merger.MergeFile(f, nil, genRules, merger.PreResolve, kinds) - return nil + files := repo.MergeRules(genRules, reposByFile, destFile, kinds) + return files, nil } func applyBuildAttributes(c *updateReposConfig, r *rule.Rule) { diff --git a/merger/merger.go b/merger/merger.go index e658f46f3..7993eba07 100644 --- a/merger/merger.go +++ b/merger/merger.go @@ -106,7 +106,7 @@ func MergeFile(oldFile *rule.File, emptyRules, genRules []*rule.Rule, phase Phas // Merge empty rules into the file and delete any rules which become empty. for _, emptyRule := range emptyRules { - if oldRule, _ := match(oldFile.Rules, emptyRule, kinds[emptyRule.Kind()]); oldRule != nil { + if oldRule, _ := Match(oldFile.Rules, emptyRule, kinds[emptyRule.Kind()]); oldRule != nil { if oldRule.ShouldKeep() { continue } @@ -124,7 +124,7 @@ func MergeFile(oldFile *rule.File, emptyRules, genRules []*rule.Rule, phase Phas matchErrors := make([]error, len(genRules)) substitutions := make(map[string]string) for i, genRule := range genRules { - oldRule, err := match(oldFile.Rules, genRule, kinds[genRule.Kind()]) + oldRule, err := Match(oldFile.Rules, genRule, kinds[genRule.Kind()]) if err != nil { // TODO(jayconrod): add a verbose mode and log errors. They are too chatty // to print by default. @@ -179,7 +179,7 @@ func substituteRule(r *rule.Rule, substitutions map[string]string, info rule.Kin } } -// match searches for a rule that can be merged with x in rules. +// Match searches for a rule that can be merged with x in rules. // // A rule is considered a match if its kind is equal to x's kind AND either its // name is equal OR at least one of the attributes in matchAttrs is equal. @@ -195,7 +195,7 @@ func substituteRule(r *rule.Rule, substitutions map[string]string, info rule.Kin // the quality of the match (name match is best, then attribute match in the // order that attributes are listed). If disambiguation is successful, // the rule and nil are returned. Otherwise, nil and an error are returned. -func match(rules []*rule.Rule, x *rule.Rule, info rule.KindInfo) (*rule.Rule, error) { +func Match(rules []*rule.Rule, x *rule.Rule, info rule.KindInfo) (*rule.Rule, error) { xname := x.Name() xkind := x.Kind() var nameMatches []*rule.Rule diff --git a/merger/merger_test.go b/merger/merger_test.go index c2f06cc94..658b8d2dc 100644 --- a/merger/merger_test.go +++ b/merger/merger_test.go @@ -13,7 +13,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package merger +package merger_test import ( "path/filepath" @@ -22,6 +22,7 @@ import ( "github.com/bazelbuild/bazel-gazelle/language" "github.com/bazelbuild/bazel-gazelle/language/go" "github.com/bazelbuild/bazel-gazelle/language/proto" + "github.com/bazelbuild/bazel-gazelle/merger" "github.com/bazelbuild/bazel-gazelle/rule" ) @@ -883,8 +884,8 @@ func TestMergeFile(t *testing.T) { if err != nil { t.Fatalf("%s: %v", tc.desc, err) } - MergeFile(f, emptyFile.Rules, genFile.Rules, PreResolve, testKinds) - FixLoads(f, testLoads) + merger.MergeFile(f, emptyFile.Rules, genFile.Rules, merger.PreResolve, testKinds) + merger.FixLoads(f, testLoads) want := tc.expected if len(want) > 0 && want[0] == '\n' { @@ -981,7 +982,7 @@ go_binary(name = "z") } r := genFile.Rules[0] info := testKinds[r.Kind()] - if got, gotErr := match(oldFile.Rules, r, info); gotErr != nil { + if got, gotErr := merger.Match(oldFile.Rules, r, info); gotErr != nil { if !tc.wantError { t.Errorf("unexpected error: %v", gotErr) } diff --git a/repo/BUILD.bazel b/repo/BUILD.bazel index ac9a3f62a..a429d8e48 100644 --- a/repo/BUILD.bazel +++ b/repo/BUILD.bazel @@ -13,6 +13,7 @@ go_library( visibility = ["//visibility:public"], deps = [ "//label:go_default_library", + "//merger:go_default_library", "//pathtools:go_default_library", "//rule:go_default_library", "//vendor/github.com/pelletier/go-toml:go_default_library", diff --git a/repo/repo.go b/repo/repo.go index 52acb0528..058bedda9 100644 --- a/repo/repo.go +++ b/repo/repo.go @@ -22,11 +22,12 @@ import ( "sort" "strings" + "github.com/bazelbuild/bazel-gazelle/merger" "github.com/bazelbuild/bazel-gazelle/rule" ) // Repo describes an external repository rule declared in a Bazel -// WORKSPACE file. +// WORKSPACE file or macro file. type Repo struct { // Name is the value of the "name" attribute of the repository rule. Name string @@ -63,6 +64,12 @@ func (s byName) Len() int { return len(s) } func (s byName) Less(i, j int) bool { return s[i].Name < s[j].Name } func (s byName) Swap(i, j int) { s[i], s[j] = s[j], s[i] } +type byRuleName []*rule.Rule + +func (s byRuleName) Len() int { return len(s) } +func (s byRuleName) Less(i, j int) bool { return s[i].Name() < s[j].Name() } +func (s byRuleName) Swap(i, j int) { s[i], s[j] = s[j], s[i] } + type lockFileFormat int const ( @@ -100,6 +107,49 @@ func ImportRepoRules(filename string, repoCache *RemoteCache) ([]*rule.Rule, err return rules, nil } +// MergeRules merges a list of generated repo rules with the already defined repo rules, +// and then updates each rule's underlying file. If the generated rule matches an existing +// one, then it inherits the file where the existing rule was defined. If the rule is new then +// its file is set as the destFile parameter. A list of the updated files is returned. +func MergeRules(genRules []*rule.Rule, existingRules map[*rule.File][]string, destFile *rule.File, kinds map[string]rule.KindInfo) []*rule.File { + sort.Stable(byRuleName(genRules)) + + repoMap := make(map[string]*rule.File) + for file, repoNames := range existingRules { + if file.Path == destFile.Path && file.MacroName() != "" && file.MacroName() == destFile.MacroName() { + file = destFile + } + for _, name := range repoNames { + repoMap[name] = file + } + } + + rulesByFile := make(map[*rule.File][]*rule.Rule) + for _, rule := range genRules { + dest := destFile + if file, ok := repoMap[rule.Name()]; ok { + dest = file + } + rulesByFile[dest] = append(rulesByFile[dest], rule) + } + + updatedFiles := make(map[string]*rule.File) + for f, rules := range rulesByFile { + merger.MergeFile(f, nil, rules, merger.PreResolve, kinds) + f.Sync() + if uf, ok := updatedFiles[f.Path]; ok { + uf.SyncMacroFile(f) + } else { + updatedFiles[f.Path] = f + } + } + files := make([]*rule.File, 0, len(updatedFiles)) + for _, f := range updatedFiles { + files = append(files, f) + } + return files +} + func getLockFileFormat(filename string) lockFileFormat { switch filepath.Base(filename) { case "Gopkg.lock": @@ -167,13 +217,45 @@ func FindExternalRepo(repoRoot, name string) (string, error) { } // ListRepositories extracts metadata about repositories declared in a -// WORKSPACE file. -// -// The set of repositories returned is necessarily incomplete, since we don't -// evaluate the file, and repositories may be declared in macros in other files. -func ListRepositories(workspace *rule.File) []Repo { - var repos []Repo - for _, r := range workspace.Rules { +// file. +func ListRepositories(workspace *rule.File) (repos []Repo, repoNamesByFile map[*rule.File][]string, err error) { + repoNamesByFile = make(map[*rule.File][]string) + repos, repoNamesByFile[workspace] = getRepos(workspace.Rules) + for _, d := range workspace.Directives { + switch d.Key { + case "repository_macro": + f, defName, err := parseRepositoryMacroDirective(d.Value) + if err != nil { + return nil, nil, err + } + f = filepath.Join(filepath.Dir(workspace.Path), filepath.Clean(f)) + macroFile, err := rule.LoadMacroFile(f, "", defName) + if err != nil { + return nil, nil, err + } + currRepos, names := getRepos(macroFile.Rules) + repoNamesByFile[macroFile] = names + repos = append(repos, currRepos...) + } + } + + return repos, repoNamesByFile, nil +} + +func parseRepositoryMacroDirective(directive string) (string, string, error) { + vals := strings.Split(directive, "%") + if len(vals) != 2 { + return "", "", fmt.Errorf("Failure parsing repository_macro: %s, expected format is macroFile%%defName", directive) + } + f := vals[0] + if strings.HasPrefix(f, "..") { + return "", "", fmt.Errorf("Failure parsing repository_macro: %s, macro file path %s should not start with \"..\"", directive, f) + } + return f, vals[1], nil +} + +func getRepos(rules []*rule.Rule) (repos []Repo, names []string) { + for _, r := range rules { name := r.Name() if name == "" { continue @@ -206,10 +288,7 @@ func ListRepositories(workspace *rule.File) []Repo { continue } repos = append(repos, repo) + names = append(names, repo.Name) } - - // TODO(jayconrod): look for directives that describe repositories that - // aren't declared in the top-level of WORKSPACE (e.g., behind a macro). - - return repos + return repos, names } diff --git a/repo/repo_test.go b/repo/repo_test.go index 63c678e0c..375365514 100644 --- a/repo/repo_test.go +++ b/repo/repo_test.go @@ -13,7 +13,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package repo +package repo_test import ( "io/ioutil" @@ -23,16 +23,18 @@ import ( "strings" "testing" + "github.com/bazelbuild/bazel-gazelle/repo" "github.com/bazelbuild/bazel-gazelle/rule" + "github.com/bazelbuild/bazel-gazelle/testtools" ) func TestGenerateRepoRules(t *testing.T) { - repo := Repo{ + newRepo := repo.Repo{ Name: "org_golang_x_tools", GoPrefix: "golang.org/x/tools", Commit: "123456", } - r := GenerateRule(repo) + r := repo.GenerateRule(newRepo) f := rule.EmptyFile("test", "") r.Insert(f) got := strings.TrimSpace(string(f.Format())) @@ -76,7 +78,7 @@ func TestFindExternalRepo(t *testing.T) { t.Fatal(err) } - if got, err := FindExternalRepo(workspacePath, name); err != nil { + if got, err := repo.FindExternalRepo(workspacePath, name); err != nil { t.Fatal(err) } else if got != externalPath { t.Errorf("got %q ; want %q", got, externalPath) @@ -86,7 +88,7 @@ func TestFindExternalRepo(t *testing.T) { func TestListRepositories(t *testing.T) { for _, tc := range []struct { desc, workspace string - want []Repo + want []repo.Repo }{ { desc: "empty", @@ -101,7 +103,7 @@ go_repository( importpath = "example.com/repo", ) `, - want: []Repo{{ + want: []repo.Repo{{ Name: "custom_repo", GoPrefix: "example.com/repo", Remote: "https://example.com/repo", @@ -114,9 +116,91 @@ go_repository( if err != nil { t.Fatal(err) } - if got := ListRepositories(workspace); !reflect.DeepEqual(got, tc.want) { - t.Errorf("got %#v ; want %#v", got, tc.want) + got, _, err := repo.ListRepositories(workspace) + if err != nil { + t.Fatal(err) + } + for i := range tc.want { + if !reflect.DeepEqual(got[i], tc.want[i]) { + t.Errorf("got %#v ; want %#v", got, tc.want) + } } }) } } + +func TestListRepositoriesWithRepositoryMacroDirective(t *testing.T) { + files := []testtools.FileSpec{{ + Path: "repos1.bzl", + Content: ` +def go_repositories(): + go_repository( + name = "go_repo", + commit = "123456", + remote = "https://example.com/go", + importpath = "example.com/go", + ) + +def foo_repositories(): + go_repository( + name = "foo_repo", + commit = "123456", + remote = "https://example.com/foo", + importpath = "example.com/foo", + ) +`}, { + Path: "repos2.bzl", + Content: ` +def bar_repositories(): + go_repository( + name = "bar_repo", + commit = "123456", + remote = "https://example.com/bar", + importpath = "example.com/bar", + ) + +def baz_repositories(): + go_repository( + name = "ignored_repo", + commit = "123456", + remote = "https://example.com/ignored", + importpath = "example.com/ignored", + ) +`}} + dir, cleanup := testtools.CreateFiles(t, files) + defer cleanup() + workspaceString := ` +# gazelle:repository_macro repos1.bzl%go_repositories +# gazelle:repository_macro repos1.bzl%foo_repositories +# gazelle:repository_macro repos2.bzl%bar_repositories +` + workspace, err := rule.LoadData(dir+"/WORKSPACE", "", []byte(workspaceString)) + if err != nil { + t.Fatal(err) + } + got, _, err := repo.ListRepositories(workspace) + if err != nil { + t.Fatal(err) + } + want := []repo.Repo{{ + Name: "go_repo", + GoPrefix: "example.com/go", + Remote: "https://example.com/go", + Commit: "123456", + }, { + Name: "foo_repo", + GoPrefix: "example.com/foo", + Remote: "https://example.com/foo", + Commit: "123456", + }, { + Name: "bar_repo", + GoPrefix: "example.com/bar", + Remote: "https://example.com/bar", + Commit: "123456", + }} + for i := range want { + if !reflect.DeepEqual(got[i], want[i]) { + t.Errorf("got %#v ; want %#v", got, want) + } + } +} diff --git a/rule/rule.go b/rule/rule.go index 4114075ce..01c07fdbe 100644 --- a/rule/rule.go +++ b/rule/rule.go @@ -238,6 +238,27 @@ func MatchBuildFileName(dir string, names []string, files []os.FileInfo) string return "" } +// SyncMacroFile syncs the file's syntax tree with another file's. This is +// useful for keeping multiple macro definitions from the same .bzl file in sync. +func (f *File) SyncMacroFile(from *File) { + fromFunc := *from.function.stmt + _, _, toFunc := scanExprs(from.function.stmt.Name, f.File.Stmt) + if toFunc != nil { + *toFunc = fromFunc + } else { + f.File.Stmt = append(f.File.Stmt, &fromFunc) + } +} + +// MacroName returns the name of the macro function that this file is editing, +// or an empty string if a macro function is not being edited. +func (f *File) MacroName() string { + if f.function != nil && f.function.stmt != nil { + return f.function.stmt.Name + } + return "" +} + // Sync writes all changes back to the wrapped syntax tree. This should be // called after editing operations, before reading the syntax tree again. func (f *File) Sync() {