Skip to content

Commit

Permalink
Handle packages called 'all' and refactor some code to consolidate th…
Browse files Browse the repository at this point in the history
…is (#129)

* Handle packages called 'all' and refactor some code to consolodate this

* Use splitlist
  • Loading branch information
Tatskaari authored Jun 20, 2023
1 parent 5801c18 commit 81df1a5
Show file tree
Hide file tree
Showing 4 changed files with 247 additions and 203 deletions.
11 changes: 10 additions & 1 deletion tools/please_go/generate/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ subinclude("///go//build_defs:go")

filegroup(
name = "srcs",
srcs = glob(["*.go"]),
srcs = glob(["*.go"], exclude=["*_test.go"]),
visibility = ["//tools/please_go:bootstrap"],
)

Expand All @@ -15,3 +15,12 @@ go_library(
"//third_party/go:mod",
],
)

go_test(
name = "generate_test",
srcs = glob(["*_test.go"]),
deps = [
"//third_party/go:testify",
":generate",
],
)
130 changes: 77 additions & 53 deletions tools/please_go/generate/generate.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,7 @@ func (g *Generate) targetsInDir(dir string) []string {
// for that package. Currently, we don't generate BUILD files for any other reason so this assumption holds
// true. We may want to check that the BUILD file contains a go_library() target otherwise.
if g.isBuildFile(path) {
pleasePkgDir := strings.TrimPrefix(strings.TrimPrefix(filepath.Dir(path), g.srcRoot), "/")
ret = append(ret, g.libTargetForPleasePackage(pleasePkgDir))
ret = append(ret, g.libTargetForPleasePackage(trimPath(filepath.Dir(path), g.srcRoot)))
}
return nil
})
Expand All @@ -88,6 +87,16 @@ func (g *Generate) targetsInDir(dir string) []string {
return ret
}

func (g *Generate) isBuildFile(file string) bool {
base := filepath.Base(file)
for _, file := range g.buildFileNames {
if base == file {
return true
}
}
return false
}

func (g *Generate) writeInstallFilegroup() error {
buildFile, err := parseOrCreateBuildFile(g.srcRoot, g.buildFileNames)
if err != nil {
Expand Down Expand Up @@ -156,7 +165,8 @@ func (g *Generate) generateAll(dir string) error {
if path != dir && strings.HasPrefix(info.Name(), "_") {
return filepath.SkipDir
}
if err := g.generate(filepath.Clean(strings.TrimPrefix(path, g.srcRoot))); err != nil {

if err := g.generate(trimPath(path, g.srcRoot)); err != nil {
switch err.(type) {
case *build.NoGoError:
// We might walk into a dir that has no .go files for the current arch. This shouldn't
Expand Down Expand Up @@ -187,7 +197,7 @@ func (g *Generate) generate(dir string) error {
return err
}

lib := g.libRule(pkg)
lib := g.libRule(pkg, dir)
if lib == nil {
return nil
}
Expand Down Expand Up @@ -320,17 +330,13 @@ func (g *Generate) depTargets(imports []string) []string {
return deps
}

func (g *Generate) libRule(pkg *build.Package) *Rule {
// The name of the target should match the dir it's in, or the basename of the module if it's in the repo root.
name := filepath.Base(pkg.Dir)
if strings.HasSuffix(pkg.Dir, g.srcRoot) || name == "" {
name = filepath.Base(g.moduleName)
}

func (g *Generate) libRule(pkg *build.Package, dir string) *Rule {
if len(pkg.GoFiles) == 0 && len(pkg.CgoFiles) == 0 {
return nil
}

name := nameForLibInPkg(g.moduleName, trimPath(dir, g.srcRoot))

return &Rule{
name: name,
kind: packageKind(pkg),
Expand All @@ -346,31 +352,6 @@ func (g *Generate) libRule(pkg *build.Package) *Rule {
}
}

func (g *Generate) testRule(pkg *build.Package, prodRule *Rule) *Rule {
// The name of the target should match the dir it's in, or the basename of the module if it's in the repo root.
name := filepath.Base(pkg.Dir)
if strings.HasSuffix(pkg.Dir, g.srcRoot) || name == "" {
name = filepath.Base(g.moduleName)
}

if len(pkg.TestGoFiles) == 0 {
return nil
}
deps := g.depTargets(pkg.TestImports)

if prodRule != nil {
deps = append(deps, ":"+prodRule.name)
}
// TODO(jpoole): handle external tests
return &Rule{
name: name,
kind: "go_test",
srcs: pkg.TestGoFiles,
deps: deps,
embedPatterns: pkg.TestEmbedPatterns,
}
}

func (g *Generate) depTarget(importPath string) string {
if target, ok := g.knownImportTargets[importPath]; ok {
return target
Expand Down Expand Up @@ -398,25 +379,52 @@ func (g *Generate) depTarget(importPath string) string {
}

subrepoName := g.subrepoName(module)
packageName := strings.TrimPrefix(importPath, module)
packageName = strings.TrimPrefix(packageName, "/")
name := filepath.Base(packageName)
if packageName == "" {
name = filepath.Base(module)
}
packageName := trimPath(importPath, module)
name := nameForLibInPkg(module, packageName)

target := buildTarget(name, packageName, subrepoName)
g.knownImportTargets[importPath] = target
return target
}

// nameForLibInPkg returns the lib target name for a target in pkg. The pkg should be the relative pkg part excluding
// the module, e.g. pkg would be asset, and module would be github.com/stretchr/testify for
// github.com/stretchr/testify/assert,
func nameForLibInPkg(module, pkg string) string {
name := filepath.Base(pkg)
if pkg == "" || pkg == "." {
name = filepath.Base(module)
}

if name == "all" {
return "lib"
}

return name
}

// trimPath is like strings.TrimPrefix but is path aware. It removes base from target if target starts with base,
// otherwise returns target unmodified.
func trimPath(target, base string) string {
baseParts := filepath.SplitList(base)
targetParts := filepath.SplitList(target)

if len(targetParts) < len(baseParts) {
return target
}

for i := range baseParts {
if baseParts[i] != targetParts[i] {
return target
}
}
return strings.Join(targetParts[len(baseParts):], "/")
}

// libTargetForPleasePackage returns the build label for the go_library() target that would be generated for a package
// at this path within the generated Please repo.
func (g *Generate) libTargetForPleasePackage(pkg string) string {
if pkg == "" || pkg == "." {
return buildTarget(filepath.Base(g.moduleName), "", "")
}
return buildTarget(filepath.Base(pkg), pkg, "")
return buildTarget(nameForLibInPkg(g.moduleName, pkg), pkg, "")
}

func (g *Generate) subrepoName(module string) string {
Expand All @@ -426,13 +434,29 @@ func (g *Generate) subrepoName(module string) string {
return filepath.Join(g.thirdPartyFolder, strings.ReplaceAll(module, "/", "_"))
}

func buildTarget(name, pkg, subrepo string) string {
if name == "" {
name = filepath.Base(pkg)
func buildTarget(name, pkgDir, subrepo string) string {
bs := new(strings.Builder)
if subrepo != "" {
bs.WriteString("///")
bs.WriteString(subrepo)
}
target := fmt.Sprintf("%v:%v", pkg, name)
if subrepo == "" {
return fmt.Sprintf("//%v", target)

// Bit of a special case here where we assume all build targets are absolute which is fine for our use case.
bs.WriteString("//")

if pkgDir == "." {
pkgDir = ""
}

if pkgDir != "" {
bs.WriteString(pkgDir)
if filepath.Base(pkgDir) != name {
bs.WriteString(":")
bs.WriteString(name)
}
} else {
bs.WriteString(":")
bs.WriteString(name)
}
return fmt.Sprintf("///%v//%v", subrepo, target)
return bs.String()
}
160 changes: 160 additions & 0 deletions tools/please_go/generate/generate_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,160 @@
package generate

import (
"github.com/stretchr/testify/assert"
"testing"
)

func TestTrimPath(t *testing.T) {
tests := []struct {
name string
base string
target string
expected string
}{
{
name: "trims base path",
base: "third_party/go/_foo#dl",
target: "third_party/go/_foo#dl/foo",
expected: "foo",
},
{
name: "returns target if base is shorter",
base: "foo/bar/baz",
target: "foo/bar",
expected: "foo/bar",
},
{
name: "returns target if not in base",
base: "foo/bar",
target: "bar/baz",
expected: "bar/baz",
},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
assert.Equal(t, test.expected, trimPath(test.target, test.base))
})
}
}

func TestBuildTarget(t *testing.T) {
tests := []struct {
test string
name, pkg, subrepo string
expected string
}{
{
test: "fully qualified",
subrepo: "subrepo",
pkg: "pkg",
name: "name",
expected: "///subrepo//pkg:name",
},
{
test: "fully qualified local package",
subrepo: "",
pkg: "pkg",
name: "name",
expected: "//pkg:name",
},
{
test: "root package",
subrepo: "",
pkg: "",
name: "name",
expected: "//:name",
},
{
test: "root package via .",
subrepo: "",
pkg: ".",
name: "name",
expected: "//:name",
},
{
test: "root package in subrepo",
subrepo: "subrepo",
pkg: "",
name: "name",
expected: "///subrepo//:name",
},
{
test: "pkg base matches name",
subrepo: "",
pkg: "foo",
name: "foo",
expected: "//foo",
},
}

for _, test := range tests {
t.Run(test.test, func(t *testing.T) {
assert.Equal(t, test.expected, buildTarget(test.name, test.pkg, test.subrepo))
})
}
}

func TestDepTarget(t *testing.T) {
tests := []struct {
name string
deps []string
importTarget string
expected string
}{
{
name: "resolves local import",
importTarget: "github.com/this/module/foo",
expected: "//foo",
},
{
name: "resolves local import in base",
importTarget: "github.com/this/module",
expected: "//:module",
},
{
name: "resolves import to another module",
importTarget: "github.com/some/module/foo",
expected: "///third_party/go/github.com_some_module//foo",
deps: []string{"github.com/some/module"},
},
{
name: "resolves import to longest match",
importTarget: "github.com/some/module/foo/bar",
expected: "///third_party/go/github.com_some_module_foo//bar",
deps: []string{"github.com/some/module", "github.com/some/module/foo"},
},
{
name: "root package matches module base",
importTarget: "github.com/some/module",
expected: "///third_party/go/github.com_some_module//:module",
deps: []string{"github.com/some/module"},
},
{
name: "replaces all with lib locally",
importTarget: "github.com/this/module/all",
expected: "//all:lib",
},
{
name: "replaces all with lib in another repo",
importTarget: "github.com/some/module/all",
expected: "///third_party/go/github.com_some_module//all:lib",
deps: []string{"github.com/some/module"},
},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
g := &Generate{
moduleName: "github.com/this/module",
thirdPartyFolder: "third_party/go",
replace: map[string]string{},
knownImportTargets: map[string]string{},
moduleDeps: test.deps,
}

assert.Equal(t, test.expected, g.depTarget(test.importTarget))
})
}
}
Loading

0 comments on commit 81df1a5

Please sign in to comment.