diff --git a/cmd/gazelle/fix-update.go b/cmd/gazelle/fix-update.go index 355145819..fb0172dec 100644 --- a/cmd/gazelle/fix-update.go +++ b/cmd/gazelle/fix-update.go @@ -23,6 +23,7 @@ import ( "log" "os" "path/filepath" + "sort" "strings" "github.com/bazelbuild/bazel-gazelle/config" @@ -40,12 +41,13 @@ 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 - walkMode walk.Mode - patchPath string - patchBuffer bytes.Buffer + dirs []string + emit emitFunc + repos []repo.Repo + workspaceFiles []*rule.File + walkMode walk.Mode + patchPath string + patchBuffer bytes.Buffer } type emitFunc func(c *config.Config, f *rule.File) error @@ -122,15 +124,21 @@ func (ucr *updateConfigurer) CheckFlags(fs *flag.FlagSet, c *config.Config) erro uc.walkMode = walk.UpdateDirsMode } + // Load the repo configuration file (WORKSPACE by default) to find out + // names and prefixes of other go_repositories. This affects external + // dependency resolution for Go. + // TODO(jayconrod): this should be moved to language/go. + var repoFileMap map[*rule.File][]string if ucr.repoConfigPath == "" { ucr.repoConfigPath = filepath.Join(c.RepoRoot, "WORKSPACE") } - if repoConfigFile, err := rule.LoadWorkspaceFile(ucr.repoConfigPath, ""); err != nil { + repoConfigFile, err := rule.LoadWorkspaceFile(ucr.repoConfigPath, "") + if err != nil { if !os.IsNotExist(err) { return err } } else { - uc.repos, _, err = repo.ListRepositories(repoConfigFile) + uc.repos, repoFileMap, err = repo.ListRepositories(repoConfigFile) if err != nil { return err } @@ -150,6 +158,39 @@ func (ucr *updateConfigurer) CheckFlags(fs *flag.FlagSet, c *config.Config) erro uc.repos = append(uc.repos, repo) } + // If the repo configuration file is not WORKSPACE, also load WORKSPACE + // so we can apply any necessary fixes. + workspacePath := filepath.Join(c.RepoRoot, "WORKSPACE") + var workspace *rule.File + if ucr.repoConfigPath == workspacePath { + workspace = repoConfigFile + } else { + workspace, err = rule.LoadWorkspaceFile(workspacePath, "") + if err != nil && !os.IsNotExist(err) { + return err + } + if workspace != nil { + _, repoFileMap, err = repo.ListRepositories(workspace) + if err != nil { + return err + } + } + } + if workspace != nil { + c.RepoName = findWorkspaceName(workspace) + uc.workspaceFiles = make([]*rule.File, 0, len(repoFileMap)) + seen := make(map[string]bool) + for f := range repoFileMap { + if !seen[f.Path] { + uc.workspaceFiles = append(uc.workspaceFiles, f) + seen[f.Path] = true + } + } + sort.Slice(uc.workspaceFiles, func(i, j int) bool { + return uc.workspaceFiles[i].Path < uc.workspaceFiles[j].Path + }) + } + return nil } @@ -382,29 +423,9 @@ func newFixUpdateConfiguration(cmd command, args []string, cexts []config.Config } } - workspacePath := filepath.Join(c.RepoRoot, "WORKSPACE") - if workspace, err := rule.LoadWorkspaceFile(workspacePath, ""); err != nil { - if !os.IsNotExist(err) { - return nil, err - } - } else { - c.RepoName = findWorkspaceName(workspace) - var reposFiles map[*rule.File][]string - _, 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 - } + uc := getUpdateConfig(c) + if err := fixRepoFiles(c, uc.workspaceFiles, loads); err != nil { + return nil, err } return c, nil diff --git a/cmd/gazelle/integration_test.go b/cmd/gazelle/integration_test.go index 287235235..25fffc70b 100644 --- a/cmd/gazelle/integration_test.go +++ b/cmd/gazelle/integration_test.go @@ -2643,6 +2643,62 @@ go_binary( }) } +// TestMinimalModuleCompatibilityAliases checks that importpath_aliases +// are emitted for go_libraries when needed. This can't easily be checked +// in language/go because the generator tests don't support running at +// the repository root or with additional flags, both of which are required. +func TestMinimalModuleCompatibilityAliases(t *testing.T) { + files := []testtools.FileSpec{ + { + Path: "go.mod", + Content: "module example.com/foo/v2", + }, { + Path: "foo.go", + Content: "package foo", + }, { + Path: "bar/bar.go", + Content: "package bar", + }, + } + dir, cleanup := testtools.CreateFiles(t, files) + defer cleanup() + + args := []string{"update", "-repo_root", dir, "-go_prefix", "example.com/foo/v2", "-go_repository_mode", "-go_repository_module_mode"} + if err := runGazelle(dir, args); err != nil { + t.Fatal(err) + } + + testtools.CheckFiles(t, dir, []testtools.FileSpec{ + { + Path: "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/v2", + importpath_aliases = ["example.com/foo"], + visibility = ["//visibility:public"], +) +`, + }, { + 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/foo/v2/bar", + importpath_aliases = ["example.com/foo/bar"], + visibility = ["//visibility:public"], +) +`, + }, + }) +} + // 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) diff --git a/cmd/gazelle/version.go b/cmd/gazelle/version.go index 0a7da2056..a281dfb77 100644 --- a/cmd/gazelle/version.go +++ b/cmd/gazelle/version.go @@ -26,7 +26,7 @@ import ( "github.com/bazelbuild/bazel-gazelle/repo" ) -var minimumRulesGoVersion = version.Version{0, 13, 0} +var minimumRulesGoVersion = version.Version{0, 19, 0} // checkRulesGoVersion checks whether a compatible version of rules_go is // being used in the workspace. A message will be logged if an incompatible diff --git a/internal/go_repository.bzl b/internal/go_repository.bzl index b29594ffe..3108db88b 100644 --- a/internal/go_repository.bzl +++ b/internal/go_repository.bzl @@ -132,6 +132,7 @@ def _go_repository_impl(ctx): gazelle = ctx.path(Label(_gazelle)) cmd = [ gazelle, + "-go_repository_mode", "-go_prefix", ctx.attr.importpath, "-mode", @@ -142,7 +143,7 @@ def _go_repository_impl(ctx): if config_path: cmd.extend(["-repo_config", str(config_path)]) if ctx.attr.version: - cmd.append("-go_experimental_module_mode") + cmd.append("-go_repository_module_mode") if ctx.attr.build_file_name: cmd.extend(["-build_file_name", ctx.attr.build_file_name]) if ctx.attr.build_tags: diff --git a/language/go/config.go b/language/go/config.go index b3b90569c..0e5e36a47 100644 --- a/language/go/config.go +++ b/language/go/config.go @@ -74,8 +74,14 @@ type goConfig struct { // goGrpcCompilersSet indicates whether goGrpcCompiler was set explicitly. goGrpcCompilersSet bool - // moduleMode is true if external dependencies should be resolved as modules. - // TODO(jayconrod): this should be the only mode in the future. + // goRepositoryMode is true if Gazelle was invoked by a go_repository rule. + // In this mode, we won't go out to the network to resolve external deps. + goRepositoryMode bool + + // moduleMode is true if the current directory is intended to be built + // as part of a module. Minimal module compatibility won't be supported + // if this is true in the root directory. External dependencies may be + // resolved differently (also depending on goRepositoryMode). moduleMode bool } @@ -228,16 +234,16 @@ func (_ *goLang) RegisterFlags(fs *flag.FlagSet, cmd string, c *config.Config) { &gzflag.MultiFlag{Values: &gc.goGrpcCompilers, IsSet: &gc.goGrpcCompilersSet}, "go_grpc_compiler", "go_proto_library compiler to use for gRPC (may be repeated)") - // TODO(jayconrod): remove this flag when gazelle within go_repository - // automatically has a list of repositories from the main WORKSPACE. - // Enabling module mode without this will make go_repository slower, - // since we'd be going out to the network much more often. + fs.BoolVar( + &gc.goRepositoryMode, + "go_repository_mode", + false, + "set when gazelle is invoked by go_repository") fs.BoolVar( &gc.moduleMode, - "go_experimental_module_mode", + "go_repository_module_mode", false, - "when enabled, external Go imports will be resolved as modules", - ) + "set when gazelle is invoked by go_repository in module mode") } c.Exts[goName] = gc } diff --git a/language/go/generate.go b/language/go/generate.go index a0e0c4bbb..5b45aa173 100644 --- a/language/go/generate.go +++ b/language/go/generate.go @@ -495,11 +495,22 @@ func (g *generator) setCommonAttrs(r *rule.Rule, pkgRel, visibility string, targ } func (g *generator) setImportAttrs(r *rule.Rule, importPath string) { + gc := getGoConfig(g.c) r.SetAttr("importpath", importPath) - goConf := getGoConfig(g.c) - if goConf.importMapPrefix != "" { - fromPrefixRel := pathtools.TrimPrefix(g.rel, goConf.importMapPrefixRel) - importMap := path.Join(goConf.importMapPrefix, fromPrefixRel) + + // Set importpath_aliases if we need minimal module compatibility. + // If a package is part of a module with a v2+ semantic import version + // suffix, packages that are not part of modules may import it without + // the suffix. + if gc.goRepositoryMode && gc.moduleMode && pathtools.HasPrefix(importPath, gc.prefix) && gc.prefixRel == "" { + if mmcImportPath := pathWithoutSemver(importPath); mmcImportPath != "" { + r.SetAttr("importpath_aliases", []string{mmcImportPath}) + } + } + + if gc.importMapPrefix != "" { + fromPrefixRel := pathtools.TrimPrefix(g.rel, gc.importMapPrefixRel) + importMap := path.Join(gc.importMapPrefix, fromPrefixRel) if importMap != importPath { r.SetAttr("importmap", importMap) } diff --git a/language/go/package.go b/language/go/package.go index 69317ac19..4de0c2fdb 100644 --- a/language/go/package.go +++ b/language/go/package.go @@ -19,6 +19,7 @@ import ( "fmt" "log" "path" + "regexp" "sort" "strings" @@ -486,3 +487,21 @@ func (si *platformStringInfo) convertToPlatforms() { si.archs = nil } } + +var semverRex = regexp.MustCompile(`^.*?(/v\d+)(?:/.*)?$`) + +// pathWithoutSemver removes a semantic version suffix from path. +// For example, if path is "example.com/foo/v2/bar", pathWithoutSemver +// will return "example.com/foo/bar". If there is no semantic version suffix, +// "" will be returned. +func pathWithoutSemver(path string) string { + m := semverRex.FindStringSubmatchIndex(path) + if m == nil { + return "" + } + v := path[m[2]+2 : m[3]] + if v[0] == '0' || v == "1" { + return "" + } + return path[:m[2]] + path[m[3]:] +} diff --git a/language/go/resolve.go b/language/go/resolve.go index 8dfabaf92..fb76df319 100644 --- a/language/go/resolve.go +++ b/language/go/resolve.go @@ -256,7 +256,7 @@ func resolveExternal(moduleMode bool, rc *repo.RemoteCache, imp string) (label.L // and not the common case, especially when known repositories aren't // listed in WORKSPACE (which is currently the case within go_repository). if !moduleMode { - moduleMode = modMajorRex.FindStringIndex(imp) != nil + moduleMode = pathWithoutSemver(imp) != "" } var prefix, repo string @@ -271,8 +271,12 @@ func resolveExternal(moduleMode bool, rc *repo.RemoteCache, imp string) (label.L } var pkg string - if imp != prefix { + if pathtools.HasPrefix(imp, prefix) { pkg = pathtools.TrimPrefix(imp, prefix) + } else if impWithoutSemver := pathWithoutSemver(imp); pathtools.HasPrefix(impWithoutSemver, prefix) { + // We may have used minimal module compatibility to resolve a path + // without a semantic import version suffix to a repository that has one. + pkg = pathtools.TrimPrefix(impWithoutSemver, prefix) } return label.New(repo, pkg, defaultLibName), nil diff --git a/language/go/resolve_test.go b/language/go/resolve_test.go index d3c5aa15f..5cf57ee60 100644 --- a/language/go/resolve_test.go +++ b/language/go/resolve_test.go @@ -1117,6 +1117,29 @@ func TestResolveExternal(t *testing.T) { }}, moduleMode: true, want: "@custom_repo//v2/foo:go_default_library", + }, { + desc: "min_module_compat", + importpath: "example.com/foo", + repos: []repo.Repo{{ + Name: "com_example_foo_v2", + GoPrefix: "example.com/foo/v2", + }}, + moduleMode: true, + want: "@com_example_foo_v2//:go_default_library", + }, { + desc: "min_module_compat_both", + importpath: "example.com/foo", + repos: []repo.Repo{ + { + Name: "com_example_foo", + GoPrefix: "example.com/foo", + }, { + Name: "com_example_foo_v2", + GoPrefix: "example.com/foo/v2", + }, + }, + moduleMode: true, + want: "@com_example_foo//:go_default_library", }, } { t.Run(tc.desc, func(t *testing.T) { diff --git a/repo/remote.go b/repo/remote.go index 8c19a08de..c19b4d87a 100644 --- a/repo/remote.go +++ b/repo/remote.go @@ -171,6 +171,27 @@ func NewRemoteCache(knownRepos []Repo) (r *RemoteCache, cleanup func() error) { }, } } + + // Augment knownRepos with additional prefixes for + // minimal module compatibility. For example, if repo "com_example_foo_v2" + // has prefix "example.com/foo/v2", map "example.com/foo" to the same + // entry. + // TODO(jayconrod): there should probably be some control over whether + // callers can use these mappings: packages within modules should not be + // allowed to use them. However, we'll return the same result nearly all + // the time, and simpler is better. + for _, repo := range knownRepos { + path := pathWithoutSemver(repo.GoPrefix) + if path == "" || r.root.cache[path] != nil { + continue + } + r.root.cache[path] = r.root.cache[repo.GoPrefix] + if e := r.remote.cache[repo.GoPrefix]; e != nil { + r.remote.cache[path] = e + } + r.mod.cache[path] = r.mod.cache[repo.GoPrefix] + } + return r, r.cleanup } @@ -455,3 +476,23 @@ func (m *remoteCacheMap) ensure(key string, load func() (interface{}, error)) (i } return e.value, e.err } + +var semverRex = regexp.MustCompile(`^.*?(/v\d+)(?:/.*)?$`) + +// pathWithoutSemver removes a semantic version suffix from path. +// For example, if path is "example.com/foo/v2/bar", pathWithoutSemver +// will return "example.com/foo/bar". If there is no semantic version suffix, +// "" will be returned. +// TODO(jayconrod): copied from language/go. This whole type should be +// migrated there. +func pathWithoutSemver(path string) string { + m := semverRex.FindStringSubmatchIndex(path) + if m == nil { + return "" + } + v := path[m[2]+2 : m[3]] + if v == "0" || v == "1" { + return "" + } + return path[:m[2]] + path[m[3]:] +}