Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

language/go: implement minimal module compatibility #564

Merged
merged 1 commit into from
Jul 1, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
83 changes: 52 additions & 31 deletions cmd/gazelle/fix-update.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"log"
"os"
"path/filepath"
"sort"
"strings"

"github.com/bazelbuild/bazel-gazelle/config"
Expand All @@ -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
Expand Down Expand Up @@ -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
}
Expand All @@ -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
}

Expand Down Expand Up @@ -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
Expand Down
56 changes: 56 additions & 0 deletions cmd/gazelle/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
2 changes: 1 addition & 1 deletion cmd/gazelle/version.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion internal/go_repository.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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:
Expand Down
24 changes: 15 additions & 9 deletions language/go/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -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
}
Expand Down
19 changes: 15 additions & 4 deletions language/go/generate.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
19 changes: 19 additions & 0 deletions language/go/package.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"fmt"
"log"
"path"
"regexp"
"sort"
"strings"

Expand Down Expand Up @@ -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]:]
}
8 changes: 6 additions & 2 deletions language/go/resolve.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
23 changes: 23 additions & 0 deletions language/go/resolve_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Loading