diff --git a/test/go_repo_transitive_deps/BUILD b/test/go_repo_transitive_deps/BUILD new file mode 100644 index 00000000..4d123bd2 --- /dev/null +++ b/test/go_repo_transitive_deps/BUILD @@ -0,0 +1,7 @@ +subinclude("///shell//build_defs:shell", "//test/build_defs:e2e") + +please_repo_e2e_test( + name = "go_repo_transitive_deps_test", + plz_command = "plz test", + repo = "test_repo", +) diff --git a/test/go_repo_transitive_deps/test_repo/.plzconfig b/test/go_repo_transitive_deps/test_repo/.plzconfig new file mode 100644 index 00000000..b1a9564d --- /dev/null +++ b/test/go_repo_transitive_deps/test_repo/.plzconfig @@ -0,0 +1,14 @@ +[Parse] +BuildFileName = BUILD_FILE +BuildFileName = BUILD + +[Plugin "go"] +Target = //plugins:go +PleaseGoTool = +GoTool = +FeatureFlags = go_get +Stdlib = //third_party/go:std +Modfile = //third_party/go:modfile + +[FeatureFlags] +ExcludeGoRules = true diff --git a/test/go_repo_transitive_deps/test_repo/BUILD_FILE b/test/go_repo_transitive_deps/test_repo/BUILD_FILE new file mode 100644 index 00000000..6c77ceee --- /dev/null +++ b/test/go_repo_transitive_deps/test_repo/BUILD_FILE @@ -0,0 +1,7 @@ +subinclude("///go//build_defs:go") + +go_test( + name = "go_repo_transitive_deps_test", + srcs = ["go_repo_transitive_deps_test.go"], + deps = ["//third_party/go:testify"], +) \ No newline at end of file diff --git a/test/go_repo_transitive_deps/test_repo/go_repo_transitive_deps_test.go b/test/go_repo_transitive_deps/test_repo/go_repo_transitive_deps_test.go new file mode 100644 index 00000000..be07ac35 --- /dev/null +++ b/test/go_repo_transitive_deps/test_repo/go_repo_transitive_deps_test.go @@ -0,0 +1,16 @@ +package test_repo + +// Testify has a dependency on github.com/pmezard/go-difflib which we DON'T +// have in our go.mod file but testify does. +// The test ensures that go_please generate considers both the host and the +// module go.mod and correctly generates the dependencies for the subrepo. + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestAssertImportable(t *testing.T) { + assert.True(t, true) +} diff --git a/test/go_repo_transitive_deps/test_repo/plugins/BUILD_FILE b/test/go_repo_transitive_deps/test_repo/plugins/BUILD_FILE new file mode 100644 index 00000000..f197577d --- /dev/null +++ b/test/go_repo_transitive_deps/test_repo/plugins/BUILD_FILE @@ -0,0 +1,4 @@ +plugin_repo( + name = "go", + revision = "master", +) \ No newline at end of file diff --git a/test/go_repo_transitive_deps/test_repo/third_party/go/BUILD_FILE b/test/go_repo_transitive_deps/test_repo/third_party/go/BUILD_FILE new file mode 100644 index 00000000..c2c9fe7b --- /dev/null +++ b/test/go_repo_transitive_deps/test_repo/third_party/go/BUILD_FILE @@ -0,0 +1,42 @@ +subinclude("///go//build_defs:go") + +filegroup( + name = "modfile", + srcs = ["go.mod"], +) + +go_stdlib( + name = "std", +) + +go_repo( + name = "testify", + licences = ["MIT"], + module = "github.com/stretchr/testify", + install = ["..."], + version = "v1.7.0", +) + +go_repo( + licences = ["MIT"], + module = "github.com/stretchr/objx", + version = "v0.5.0", +) + +go_repo( + licences = ["BSD-3-Clause"], + module = "github.com/pmezard/go-difflib", + version = "v1.0.0", +) + +go_repo( + licences = ["ISC"], + module = "github.com/davecgh/go-spew", + version = "v1.1.1", +) + +go_repo( + licences = ["MIT"], + module = "gopkg.in/yaml.v3", + version = "v3.0.0-20210107192922-496545a6307b", +) diff --git a/test/go_repo_transitive_deps/test_repo/third_party/go/go.mod b/test/go_repo_transitive_deps/test_repo/third_party/go/go.mod new file mode 100644 index 00000000..7491f7ff --- /dev/null +++ b/test/go_repo_transitive_deps/test_repo/third_party/go/go.mod @@ -0,0 +1,10 @@ +module gorepotransitivedeps + +go 1.21 + +require ( + // For this test to work this go.mod should only ever list "testify" as a dependency. + // The point of the test is to ensure that the go.mod of "testify" (or any third-party) + // itself is used as part of dependency resolving. + github.com/stretchr/testify v1.8.4 +) diff --git a/tools/please_go/BUILD b/tools/please_go/BUILD index 0013204f..6f386ab8 100644 --- a/tools/please_go/BUILD +++ b/tools/please_go/BUILD @@ -42,6 +42,7 @@ genrule( "//tools/please_go/embed:srcs", "//tools/please_go/filter:srcs", "//tools/please_go/generate:srcs", + "//tools/please_go/generate/gomoddeps:srcs", "//tools/please_go/goget:srcs", "//tools/please_go/install:srcs", "//tools/please_go/install/exec:srcs", diff --git a/tools/please_go/ChangeLog b/tools/please_go/ChangeLog index b180d139..bcc376d1 100644 --- a/tools/please_go/ChangeLog +++ b/tools/please_go/ChangeLog @@ -1,3 +1,8 @@ +Version 1.10.0 +------------- + * Inspect both the host repo and the module's go.mod when resolving dependencies in the + `generate` command. + Version 1.9.2 ------------- * Generate correct paths for subdir imports within a module. diff --git a/tools/please_go/VERSION b/tools/please_go/VERSION index 8fdcf386..81c871de 100644 --- a/tools/please_go/VERSION +++ b/tools/please_go/VERSION @@ -1 +1 @@ -1.9.2 +1.10.0 diff --git a/tools/please_go/generate/BUILD b/tools/please_go/generate/BUILD index 03c31004..50f01152 100644 --- a/tools/please_go/generate/BUILD +++ b/tools/please_go/generate/BUILD @@ -12,7 +12,7 @@ go_library( visibility = ["//tools/..."], deps = [ "//third_party/go:buildtools", - "//third_party/go:mod", + "//tools/please_go/generate/gomoddeps", ], ) @@ -23,4 +23,4 @@ go_test( "//third_party/go:testify", ":generate", ], -) \ No newline at end of file +) diff --git a/tools/please_go/generate/generate.go b/tools/please_go/generate/generate.go index 1f952121..5a0d4481 100644 --- a/tools/please_go/generate/generate.go +++ b/tools/please_go/generate/generate.go @@ -7,13 +7,14 @@ import ( "io/fs" "log" "os" + "path" "path/filepath" "strings" - "golang.org/x/mod/modfile" - bazelbuild "github.com/bazelbuild/buildtools/build" bazeledit "github.com/bazelbuild/buildtools/edit" + + "github.com/please-build/go-rules/tools/please_go/generate/gomoddeps" ) type Generate struct { @@ -22,17 +23,16 @@ type Generate struct { srcRoot string subrepo string buildContext build.Context - modFile string + hostModFile string buildFileNames []string moduleDeps []string - pluginTarget string replace map[string]string knownImportTargets map[string]string // cache these so we don't end up looping over all the modules for every import thirdPartyFolder string install []string } -func New(srcRoot, thirdPartyFolder, modFile, module, version, subrepo string, buildFileNames, moduleDeps, install []string, buildTags []string) *Generate { +func New(srcRoot, thirdPartyFolder, hostModFile, module, version, subrepo string, buildFileNames, moduleDeps, install []string, buildTags []string) *Generate { moduleArg := module if version != "" { moduleArg += "@" + version @@ -46,7 +46,7 @@ func New(srcRoot, thirdPartyFolder, modFile, module, version, subrepo string, bu buildContext: ctxt, buildFileNames: buildFileNames, moduleDeps: moduleDeps, - modFile: modFile, + hostModFile: hostModFile, knownImportTargets: map[string]string{}, thirdPartyFolder: thirdPartyFolder, install: install, @@ -59,9 +59,13 @@ func New(srcRoot, thirdPartyFolder, modFile, module, version, subrepo string, bu // Generate generates a new Please project at the src root. It will walk through the directory tree generating new BUILD // files. This is primarily intended to generate a please subrepo for third party code. func (g *Generate) Generate() error { - if err := g.readGoMod(g.modFile); err != nil { - return fmt.Errorf("failed to read go.mod: %w", err) + deps, replacements, err := gomoddeps.GetCombinedDepsAndReplacements(g.hostModFile, path.Join(g.srcRoot, "go.mod")) + if err != nil { + return err } + g.moduleDeps = append(deps, g.moduleName) + g.replace = replacements + if err := g.writeConfig(); err != nil { return fmt.Errorf("failed to write config: %w", err) } @@ -195,34 +199,6 @@ func (g *Generate) writeInstallFilegroup() error { return saveBuildFile(buildFile) } -// readGoMod reads the module dependencies -func (g *Generate) readGoMod(path string) error { - if path == "" { - path = filepath.Join(g.srcRoot, "go.mod") - } - data, err := os.ReadFile(path) - if err != nil { - return err - } - modFile, err := modfile.ParseLax(path, data, nil) - if err != nil { - return err - } - - // TODO we could probably validate these are known modules - for _, req := range modFile.Require { - g.moduleDeps = append(g.moduleDeps, req.Mod.Path) - } - - g.moduleDeps = append(g.moduleDeps, g.moduleName) - - g.replace = make(map[string]string, len(modFile.Replace)) - for _, replace := range modFile.Replace { - g.replace[replace.Old.Path] = replace.New.Path - } - return nil -} - func (g *Generate) writeConfig() error { file, err := os.Create(filepath.Join(g.srcRoot, ".plzconfig")) if err != nil { @@ -464,7 +440,7 @@ func (g *Generate) depTarget(importPath string) string { return target } - if replacement, ok := g.replace[importPath]; ok { + if replacement, ok := g.replace[importPath]; ok && replacement != importPath { target := g.depTarget(replacement) g.knownImportTargets[importPath] = target return target diff --git a/tools/please_go/generate/gomoddeps/BUILD b/tools/please_go/generate/gomoddeps/BUILD new file mode 100644 index 00000000..4ed194c9 --- /dev/null +++ b/tools/please_go/generate/gomoddeps/BUILD @@ -0,0 +1,30 @@ +subinclude("///go//build_defs:go") + +filegroup( + name = "srcs", + srcs = glob(["*.go"], exclude=["*_test.go"]), + visibility = ["//tools/please_go:bootstrap"], +) + +go_library( + name = "gomoddeps", + srcs = [ + ":srcs", + ], + visibility = ["//tools/please_go/generate/..."], + deps = [ + "//third_party/go:mod", + ], +) + +go_test( + name = "gomoddeps_test", + srcs = glob(["*_test.go"]), + data = [ + "//tools/please_go/generate/gomoddeps/test_data:test_go_mod_files", + ], + deps = [ + "//third_party/go:testify", + ":gomoddeps", + ], +) \ No newline at end of file diff --git a/tools/please_go/generate/gomoddeps/gomoddeps.go b/tools/please_go/generate/gomoddeps/gomoddeps.go new file mode 100644 index 00000000..93ad45f0 --- /dev/null +++ b/tools/please_go/generate/gomoddeps/gomoddeps.go @@ -0,0 +1,84 @@ +// Package gomoddeps parses dependencies and replacements from the host and module go.mod files. +package gomoddeps + +import ( + "errors" + "fmt" + "io/fs" + "os" + + "golang.org/x/mod/modfile" +) + +// GetCombinedDepsAndReplacements returns dependencies and replacements after inspecting both +// the host and the module go.mod files. +// Module's replacement are only returned if there is no host go.mod file. +func GetCombinedDepsAndReplacements(hostGoModPath, moduleGoModPath string) ([]string, map[string]string, error) { + var err error + + hostDeps := []string{} + hostReplacements := map[string]string{} + if hostGoModPath != "" { + hostDeps, hostReplacements, err = getDepsAndReplacements(hostGoModPath, false) + if err != nil { + return nil, nil, fmt.Errorf("failed to read host repo go.mod %q: %w", hostGoModPath, err) + } + } + + var moduleDeps []string + var moduleReplacements = map[string]string{} + useLaxParsingForModule := true + if hostGoModPath == "" { + // If we're only considering the module then we want to extract the replacement's as well (lax mode + // doesn't parse them). + useLaxParsingForModule = false + } + moduleDeps, moduleReplacements, err = getDepsAndReplacements(moduleGoModPath, useLaxParsingForModule) + if err != nil { + if errors.Is(err, fs.ErrNotExist) { + return hostDeps, hostReplacements, nil + } + return nil, nil, fmt.Errorf("failed to read module go.mod %q: %w", moduleGoModPath, err) + } + + var replacements map[string]string + if hostGoModPath == "" { + replacements = moduleReplacements + } else { + replacements = hostReplacements + } + + return append(hostDeps, moduleDeps...), replacements, nil +} + +// getDepsAndReplacements parses the go.mod file and returns all the dependencies +// and replacement directives from it. +func getDepsAndReplacements(goModPath string, useLaxParsing bool) ([]string, map[string]string, error) { + data, err := os.ReadFile(goModPath) + if err != nil { + return nil, nil, err + } + + var modFile *modfile.File + if useLaxParsing { + modFile, err = modfile.ParseLax(goModPath, data, nil) + } else { + modFile, err = modfile.Parse(goModPath, data, nil) + } + if err != nil { + return nil, nil, err + } + + moduleDeps := make([]string, 0, len(modFile.Require)) + // TODO we could probably validate these are known modules + for _, req := range modFile.Require { + moduleDeps = append(moduleDeps, req.Mod.Path) + } + + replacements := make(map[string]string, len(modFile.Replace)) + for _, replace := range modFile.Replace { + replacements[replace.Old.Path] = replace.New.Path + } + + return moduleDeps, replacements, nil +} diff --git a/tools/please_go/generate/gomoddeps/gomoddeps_test.go b/tools/please_go/generate/gomoddeps/gomoddeps_test.go new file mode 100644 index 00000000..96707c1b --- /dev/null +++ b/tools/please_go/generate/gomoddeps/gomoddeps_test.go @@ -0,0 +1,95 @@ +package gomoddeps + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +var hostGoModPath = "tools/please_go/generate/gomoddeps/test_data/host_go_mod" +var moduleGoModPath = "tools/please_go/generate/gomoddeps/test_data/module_go_mod" +var invalidGoModPath = "tools/please_go/generate/gomoddeps/test_data/invalid_go_mod" + +func TestErrors(t *testing.T) { + t.Run("errors if host go.mod does not exist", func(t *testing.T) { + deps, replacements, err := GetCombinedDepsAndReplacements("/does/not/exist", "/does/not/matter") + assert.Error(t, err) + assert.Nil(t, deps) + assert.Nil(t, replacements) + }) + + t.Run("does not error if module go.mod does not exist", func(t *testing.T) { + deps, replacements, err := GetCombinedDepsAndReplacements(hostGoModPath, "/does/not/exist") + assert.NoError(t, err) + assert.NotNil(t, deps) + assert.NotNil(t, replacements) + }) + + t.Run("errors if host go.mod is invalid", func(t *testing.T) { + deps, replacements, err := GetCombinedDepsAndReplacements(invalidGoModPath, "/does/not/exist") + assert.Error(t, err) + assert.Nil(t, deps) + assert.Nil(t, replacements) + }) + + t.Run("errors if module go.mod is invalid", func(t *testing.T) { + deps, replacements, err := GetCombinedDepsAndReplacements(hostGoModPath, invalidGoModPath) + assert.Error(t, err) + assert.Nil(t, deps) + assert.Nil(t, replacements) + }) +} + +func TestHostOnlyDeps(t *testing.T) { + t.Run("returns all host dependencies", func(t *testing.T) { + deps, _, err := GetCombinedDepsAndReplacements(hostGoModPath, "/does/not/matter") + assert.NoError(t, err) + + assert.Len(t, deps, 3) + assert.Equal(t, []string{"example.com/foo", "example.com/bar", "example.com/bob"}, deps) + }) + + t.Run("returns all host replacements", func(t *testing.T) { + _, replacements, err := GetCombinedDepsAndReplacements(hostGoModPath, "/does/not/matter") + assert.NoError(t, err) + + assert.Len(t, replacements, 1) + assert.Equal(t, map[string]string{"example.com/bob": "example.com/new-bob"}, replacements) + }) +} + +func TestModuleOnlyDeps(t *testing.T) { + t.Run("returns all host dependencies", func(t *testing.T) { + deps, _, err := GetCombinedDepsAndReplacements("", moduleGoModPath) + assert.NoError(t, err) + + assert.Len(t, deps, 2) + assert.Equal(t, []string{"example.com/dob", "example.com/bab"}, deps) + }) + + t.Run("returns all host replacements", func(t *testing.T) { + _, replacements, err := GetCombinedDepsAndReplacements("", moduleGoModPath) + assert.NoError(t, err) + + assert.Len(t, replacements, 1) + assert.Equal(t, map[string]string{"example.com/bab": "example.com/new-bab"}, replacements) + }) +} + +func TestBothDeps(t *testing.T) { + t.Run("returns combined dependencies", func(t *testing.T) { + deps, _, err := GetCombinedDepsAndReplacements(hostGoModPath, moduleGoModPath) + assert.NoError(t, err) + + assert.Len(t, deps, 5) + assert.Equal(t, []string{"example.com/foo", "example.com/bar", "example.com/bob", "example.com/dob", "example.com/bab"}, deps) + }) + + t.Run("returns only host replacements", func(t *testing.T) { + _, replacements, err := GetCombinedDepsAndReplacements(hostGoModPath, moduleGoModPath) + assert.NoError(t, err) + + assert.Len(t, replacements, 1) + assert.Equal(t, map[string]string{"example.com/bob": "example.com/new-bob"}, replacements) + }) +} diff --git a/tools/please_go/generate/gomoddeps/test_data/BUILD b/tools/please_go/generate/gomoddeps/test_data/BUILD new file mode 100644 index 00000000..9b2e08e6 --- /dev/null +++ b/tools/please_go/generate/gomoddeps/test_data/BUILD @@ -0,0 +1,10 @@ +filegroup( + name = "test_go_mod_files", + srcs = [ + "host_go_mod", + "invalid_go_mod", + "module_go_mod", + ], + test_only = True, + visibility = ["//tools/please_go/generate/..."], +) diff --git a/tools/please_go/generate/gomoddeps/test_data/host_go_mod b/tools/please_go/generate/gomoddeps/test_data/host_go_mod new file mode 100644 index 00000000..5d3b7e3d --- /dev/null +++ b/tools/please_go/generate/gomoddeps/test_data/host_go_mod @@ -0,0 +1,11 @@ +module hostmod + +go 1.21 + +require ( + example.com/foo v0.0.0-1234567890 + example.com/bar v1.1.1 + example.com/bob v1.2.2 +) + +replace example.com/bob => example.com/new-bob v42.0.0 diff --git a/tools/please_go/generate/gomoddeps/test_data/invalid_go_mod b/tools/please_go/generate/gomoddeps/test_data/invalid_go_mod new file mode 100644 index 00000000..79a49745 --- /dev/null +++ b/tools/please_go/generate/gomoddeps/test_data/invalid_go_mod @@ -0,0 +1,7 @@ +module malformed + +go 1.21 + +require # uh oh no ( here ... + example.com/foo v0.0.0-1234567890 +) diff --git a/tools/please_go/generate/gomoddeps/test_data/module_go_mod b/tools/please_go/generate/gomoddeps/test_data/module_go_mod new file mode 100644 index 00000000..811a71a0 --- /dev/null +++ b/tools/please_go/generate/gomoddeps/test_data/module_go_mod @@ -0,0 +1,10 @@ +module modulemod + +go 1.21 + +require ( + example.com/dob v1.1.1 + example.com/bab v1.2.2 +) + +replace example.com/bab => example.com/new-bab v42.0.0 diff --git a/tools/please_go/please_go.go b/tools/please_go/please_go.go index 2b33eefb..ca8f03c8 100644 --- a/tools/please_go/please_go.go +++ b/tools/please_go/please_go.go @@ -96,7 +96,7 @@ var opts = struct { SrcRoot string `short:"r" long:"src_root" description:"The src root of the module to inspect"` ImportPath string `long:"import_path" description:"overrides the module's import path. If not set, the import path from the go.mod will be used.'"` ThirdPartyFolder string `short:"t" long:"third_part_folder" description:"The folder containing the third party subrepos" default:"third_party/go"` - ModFile string `long:"mod_file" description:"Path to the mod file to use to resolve dependencies against"` + ModFile string `long:"mod_file" description:"Path to the host repo mod file to use to resolve dependencies against (dependencies will be resolved against the module as well if it exists)"` Module string `long:"module" description:"The name of the current module"` Version string `long:"version" description:"The version of the current module"` Install []string `long:"install" description:"The packages to add to the :install alias"`