From 965e66ecc5697d7cb0e392f37f583fa562df2579 Mon Sep 17 00:00:00 2001 From: Marcus Caisey Date: Sat, 29 Jan 2022 03:18:15 +0000 Subject: [PATCH 01/13] pass in root and wd to suite.New --- domain/wollemi/service_format_test.go | 7 ++++++- domain/wollemi/service_rules_unused_test.go | 9 ++------- domain/wollemi/service_suite_test.go | 13 ++++++------- domain/wollemi/service_symlink_go_path_test.go | 5 +---- domain/wollemi/service_symlink_list_test.go | 15 +++++---------- 5 files changed, 20 insertions(+), 29 deletions(-) diff --git a/domain/wollemi/service_format_test.go b/domain/wollemi/service_format_test.go index 88d128d..a79f856 100644 --- a/domain/wollemi/service_format_test.go +++ b/domain/wollemi/service_format_test.go @@ -29,6 +29,11 @@ const ( gopkg = "github.com/example" ) +var ( + root = filepath.Join(gosrc, gopkg) + wd = root +) + func (t *ServiceSuite) TestService_GoFormat() { type T = ServiceSuite @@ -1554,7 +1559,7 @@ func (t *ServiceSuite) TestService_GoFormat() { t.MockGoFormat(tt.Data, write) - wollemi := t.New(tt.Data.Gosrc, tt.Data.Gopkg) + wollemi := t.New(root, wd, tt.Data.Gosrc, tt.Data.Gopkg) require.NoError(t, wollemi.GoFormat(tt.Config, tt.Data.Paths)) close(write) diff --git a/domain/wollemi/service_rules_unused_test.go b/domain/wollemi/service_rules_unused_test.go index 58a3363..574ddd0 100644 --- a/domain/wollemi/service_rules_unused_test.go +++ b/domain/wollemi/service_rules_unused_test.go @@ -18,15 +18,10 @@ func TestService_RulesUnused(t *testing.T) { func (t *ServiceSuite) TestService_RulesUnused() { type T = ServiceSuite - const ( - gopkg = "github.com/wollemi_test" - gosrc = "/go/src" - ) - t.It("can list unused build rules", func(t *T) { t.MockRulesUnused() - wollemi := t.New(gosrc, gopkg) + wollemi := t.New(root, wd, gosrc, gopkg) var ( prune bool @@ -106,7 +101,7 @@ func (t *ServiceSuite) TestService_RulesUnused() { expect.Equal(t, want, have) }) - wollemi := t.New(gosrc, gopkg) + wollemi := t.New(root, wd, gosrc, gopkg) var ( prune bool = true diff --git a/domain/wollemi/service_suite_test.go b/domain/wollemi/service_suite_test.go index fa7b1fe..9408ee3 100644 --- a/domain/wollemi/service_suite_test.go +++ b/domain/wollemi/service_suite_test.go @@ -2,7 +2,6 @@ package wollemi_test import ( "fmt" - "path/filepath" "strings" "sync" "testing" @@ -10,9 +9,9 @@ import ( "github.com/golang/mock/gomock" "github.com/tcncloud/wollemi/domain/wollemi" - "github.com/tcncloud/wollemi/ports/golang/mock" - "github.com/tcncloud/wollemi/ports/please/mock" - "github.com/tcncloud/wollemi/ports/wollemi/mock" + mock_golang "github.com/tcncloud/wollemi/ports/golang/mock" + mock_please "github.com/tcncloud/wollemi/ports/please/mock" + mock_wollemi "github.com/tcncloud/wollemi/ports/wollemi/mock" "github.com/tcncloud/wollemi/testdata/mem" ) @@ -61,14 +60,14 @@ func (suite *ServiceSuite) Run(name string, yield func(*ServiceSuite)) { }) } -func (suite *ServiceSuite) New(gosrc, gopkg string) *wollemi.Service { +func (suite *ServiceSuite) New(root, wd, gosrc, gopkg string) *wollemi.Service { return wollemi.New( suite.logger, suite.filesystem, suite.golang, suite.please, - filepath.Join(gosrc, gopkg), - filepath.Join(gosrc, gopkg), + root, + wd, gosrc, gopkg, ) diff --git a/domain/wollemi/service_symlink_go_path_test.go b/domain/wollemi/service_symlink_go_path_test.go index da420f0..02d2c9f 100644 --- a/domain/wollemi/service_symlink_go_path_test.go +++ b/domain/wollemi/service_symlink_go_path_test.go @@ -37,9 +37,6 @@ func (t *ServiceSuite) TestService_SymlinkGoPath() { "//ports/wollemi:wollemi", } - gopkg := "github.com/wollemi_test" - gosrc := "/go/src" - goSrcPath := func(elems ...string) string { return filepath.Join(gosrc, filepath.Join(elems...)) } @@ -155,7 +152,7 @@ func (t *ServiceSuite) TestService_SymlinkGoPath() { t.DefaultMocks() - wollemi := t.New(gosrc, gopkg) + wollemi := t.New(root, wd, gosrc, gopkg) var ( force = false diff --git a/domain/wollemi/service_symlink_list_test.go b/domain/wollemi/service_symlink_list_test.go index b0f272b..be620c3 100644 --- a/domain/wollemi/service_symlink_list_test.go +++ b/domain/wollemi/service_symlink_list_test.go @@ -19,11 +19,6 @@ func TestService_SymlinkList(t *testing.T) { func (t *ServiceSuite) TestService_SymlinkList() { type T = ServiceSuite - const ( - gopkg = "github.com/wollemi_test" - gosrc = "/go/src" - ) - t.It("can list all project symlinks", func(t *T) { data := t.GoSymlinkTestData() @@ -55,7 +50,7 @@ func (t *ServiceSuite) TestService_SymlinkList() { return filepath.Join(gosrc, gopkg, s), nil }) - wollemi := t.New(gosrc, gopkg) + wollemi := t.New(root, wd, gosrc, gopkg) var ( name = "*" @@ -147,7 +142,7 @@ func (t *ServiceSuite) TestService_SymlinkList() { return info, nil }) - wollemi := t.New(gosrc, gopkg) + wollemi := t.New(root, wd, gosrc, gopkg) var ( name = "*" @@ -212,7 +207,7 @@ func (t *ServiceSuite) TestService_SymlinkList() { return filepath.Join(gosrc, gopkg, s), nil }) - wollemi := t.New(gosrc, gopkg) + wollemi := t.New(root, wd, gosrc, gopkg) var ( name = "*.mg.go" @@ -292,7 +287,7 @@ func (t *ServiceSuite) TestService_SymlinkList() { t.filesystem.EXPECT().Remove("app/protos/service.pb.go") - wollemi := t.New(gosrc, gopkg) + wollemi := t.New(root, wd, gosrc, gopkg) var ( name = "*.pb.go" @@ -361,7 +356,7 @@ func (t *ServiceSuite) TestService_SymlinkList() { return filepath.Join(gosrc, gopkg, s), nil }) - wollemi := t.New(gosrc, gopkg) + wollemi := t.New(root, wd, gosrc, gopkg) var ( name = "*" From 76c74d2bd252df707c84e66dd128d097b454d369 Mon Sep 17 00:00:00 2001 From: Marcus Caisey Date: Sat, 29 Jan 2022 04:15:50 +0000 Subject: [PATCH 02/13] make all paths relative to root when normalizing --- domain/wollemi/service.go | 12 +-- domain/wollemi/service_format_test.go | 141 ++++++++++++++++++++++++++ 2 files changed, 146 insertions(+), 7 deletions(-) diff --git a/domain/wollemi/service.go b/domain/wollemi/service.go index c992f2a..df64429 100644 --- a/domain/wollemi/service.go +++ b/domain/wollemi/service.go @@ -59,14 +59,12 @@ func (this *Service) normalizePaths(paths []string) []string { paths = []string{"..."} } - var relpath string - - if this.wd != this.root && strings.HasPrefix(this.wd, this.root) { - relpath = strings.TrimPrefix(this.wd, this.root+"/") - - for i, path := range paths { - paths[i] = filepath.Join(relpath, strings.TrimSuffix(path, "/")) + for i, path := range paths { + path = filepath.Clean(path) + if strings.HasPrefix(path, this.root) { + path, _ = filepath.Rel(this.root, path) } + paths[i] = path } return paths diff --git a/domain/wollemi/service_format_test.go b/domain/wollemi/service_format_test.go index a79f856..4bdd5f0 100644 --- a/domain/wollemi/service_format_test.go +++ b/domain/wollemi/service_format_test.go @@ -1547,6 +1547,137 @@ func (t *ServiceSuite) TestService_GoFormat() { }, }, }, + }, { // TEST_CASE ------------------------------------------------------------- + Title: "accepts absolute paths when in the root", + Data: &GoFormatTestData{ + Gosrc: gosrc, + Gopkg: gopkg, + Root: "/root", + Wd: "/root", + Paths: []string{"/root/app"}, + Parse: t.WithThirdPartyGo(nil), + ImportDir: map[string]*golang.Package{ + "app": { + Name: "main", + GoFiles: []string{"main.go"}, + GoFileImports: map[string][]string{ + "main.go": { + "github.com/spf13/cobra", + }, + }, + }, + }, + Write: map[string]*please.BuildFile{ + "app/BUILD.plz": { + Stmt: []please.Expr{ + please.NewCallExpr("go_binary", []please.Expr{ + please.NewAssignExpr("=", "name", "app"), + please.NewAssignExpr("=", "srcs", please.NewGlob([]string{"*.go"}, "*_test.go")), + please.NewAssignExpr("=", "visibility", []string{"PUBLIC"}), + please.NewAssignExpr("=", "deps", []string{ + "//third_party/go/github.com/spf13:cobra", + }), + }), + }, + }, + }, + }, + }, { // TEST_CASE ------------------------------------------------------------- + Title: "accepts absolute paths when in a child of the root", + Data: &GoFormatTestData{ + Gosrc: gosrc, + Gopkg: gopkg, + Root: "/root", + Wd: "/root/wd", + Paths: []string{"/root/wd/app"}, + Parse: t.WithThirdPartyGo(nil), + ImportDir: map[string]*golang.Package{ + "wd/app": { + Name: "main", + GoFiles: []string{"main.go"}, + GoFileImports: map[string][]string{ + "main.go": { + "github.com/spf13/cobra", + }, + }, + }, + }, + Write: map[string]*please.BuildFile{ + "wd/app/BUILD.plz": { + Stmt: []please.Expr{ + please.NewCallExpr("go_binary", []please.Expr{ + please.NewAssignExpr("=", "name", "app"), + please.NewAssignExpr("=", "srcs", please.NewGlob([]string{"*.go"}, "*_test.go")), + please.NewAssignExpr("=", "visibility", []string{"PUBLIC"}), + please.NewAssignExpr("=", "deps", []string{ + "//third_party/go/github.com/spf13:cobra", + }), + }), + }, + }, + }, + }, + }, { // TEST_CASE ------------------------------------------------------------- + Title: "merges configs between wd and given path together", + Config: wollemi.Config{ + KnownDependency: map[string]string{ + "github.com/dep1": "//third_party/go/github.com/wont_be_used", + }, + }, + Data: &GoFormatTestData{ + Gosrc: gosrc, + Gopkg: gopkg, + Config: map[string]wollemi.Config{ + "wd": wollemi.Config{ + KnownDependency: map[string]string{ + "github.com/dep1": "//third_party/go/github.com/foo_override", + }, + }, + "wd/foo": wollemi.Config{ + KnownDependency: map[string]string{ + "github.com/dep2": "//third_party/go/github.com/foo_override", + }, + }, + "wd/foo/app": wollemi.Config{ + KnownDependency: map[string]string{ + "github.com/dep3": "//third_party/go/github.com/app_override", + }, + }, + }, + Root: "/root", + Wd: "/root/wd", + Paths: []string{"foo/app"}, + Parse: t.WithThirdPartyGo(nil), + ImportDir: map[string]*golang.Package{ + "wd/foo/app": { + Name: "main", + GoFiles: []string{"main.go"}, + GoFileImports: map[string][]string{ + "main.go": { + "github.com/dep1", + "github.com/dep2", + "github.com/dep3", + }, + }, + }, + }, + Write: map[string]*please.BuildFile{ + "wd/foo/app/BUILD.plz": { + Stmt: []please.Expr{ + please.NewCallExpr("go_binary", []please.Expr{ + please.NewAssignExpr("=", "name", "app"), + please.NewAssignExpr("=", "srcs", please.NewGlob([]string{"*.go"}, "*_test.go")), + please.NewAssignExpr("=", "visibility", []string{"PUBLIC"}), + please.NewAssignExpr("=", "deps", []string{ + "//third_party/go/github.com/wd_override", + "//third_party/go/github.com/foo_override", + "//third_party/go/github.com/app_override", + }), + }), + }, + }, + }, + }, }} { focus := "" @@ -1559,6 +1690,14 @@ func (t *ServiceSuite) TestService_GoFormat() { t.MockGoFormat(tt.Data, write) + root := root + if tt.Data.Root != "" { + root = tt.Data.Root + } + wd := wd + if tt.Data.Wd != "" { + wd = tt.Data.Wd + } wollemi := t.New(root, wd, tt.Data.Gosrc, tt.Data.Gopkg) require.NoError(t, wollemi.GoFormat(tt.Config, tt.Data.Paths)) @@ -1711,6 +1850,8 @@ func (t *ServiceSuite) MockGoFormat(data *GoFormatTestData, write chan please.Fi type GoFormatTestData struct { Gosrc string Gopkg string + Root string + Wd string Paths []string Config map[string]wollemi.Config ImportDir map[string]*golang.Package From adfe8b05fd01e73f4714e1af8ed6ccdc60a68386 Mon Sep 17 00:00:00 2001 From: Marcus Caisey Date: Sat, 29 Jan 2022 04:26:55 +0000 Subject: [PATCH 03/13] add back relative path in child dir normalising --- domain/wollemi/service.go | 6 +- domain/wollemi/service_format_test.go | 99 ++++++++++++++++++--------- 2 files changed, 72 insertions(+), 33 deletions(-) diff --git a/domain/wollemi/service.go b/domain/wollemi/service.go index df64429..93bc380 100644 --- a/domain/wollemi/service.go +++ b/domain/wollemi/service.go @@ -60,7 +60,11 @@ func (this *Service) normalizePaths(paths []string) []string { } for i, path := range paths { - path = filepath.Clean(path) + if !filepath.IsAbs(path) { + path = filepath.Join(this.wd, path) + } + // TODO: should we check that all absolute paths live under the root before we get to here? i don't think we can + // format them if not if strings.HasPrefix(path, this.root) { path, _ = filepath.Rel(this.root, path) } diff --git a/domain/wollemi/service_format_test.go b/domain/wollemi/service_format_test.go index 4bdd5f0..66602b7 100644 --- a/domain/wollemi/service_format_test.go +++ b/domain/wollemi/service_format_test.go @@ -1618,66 +1618,101 @@ func (t *ServiceSuite) TestService_GoFormat() { }, }, }, { // TEST_CASE ------------------------------------------------------------- - Title: "merges configs between wd and given path together", - Config: wollemi.Config{ - KnownDependency: map[string]string{ - "github.com/dep1": "//third_party/go/github.com/wont_be_used", - }, - }, + Title: "accepts relative paths when in a child of the root", Data: &GoFormatTestData{ Gosrc: gosrc, Gopkg: gopkg, - Config: map[string]wollemi.Config{ - "wd": wollemi.Config{ - KnownDependency: map[string]string{ - "github.com/dep1": "//third_party/go/github.com/foo_override", - }, - }, - "wd/foo": wollemi.Config{ - KnownDependency: map[string]string{ - "github.com/dep2": "//third_party/go/github.com/foo_override", - }, - }, - "wd/foo/app": wollemi.Config{ - KnownDependency: map[string]string{ - "github.com/dep3": "//third_party/go/github.com/app_override", - }, - }, - }, Root: "/root", Wd: "/root/wd", - Paths: []string{"foo/app"}, + Paths: []string{"app"}, Parse: t.WithThirdPartyGo(nil), ImportDir: map[string]*golang.Package{ - "wd/foo/app": { + "wd/app": { Name: "main", GoFiles: []string{"main.go"}, GoFileImports: map[string][]string{ "main.go": { - "github.com/dep1", - "github.com/dep2", - "github.com/dep3", + "github.com/spf13/cobra", }, }, }, }, Write: map[string]*please.BuildFile{ - "wd/foo/app/BUILD.plz": { + "wd/app/BUILD.plz": { Stmt: []please.Expr{ please.NewCallExpr("go_binary", []please.Expr{ please.NewAssignExpr("=", "name", "app"), please.NewAssignExpr("=", "srcs", please.NewGlob([]string{"*.go"}, "*_test.go")), please.NewAssignExpr("=", "visibility", []string{"PUBLIC"}), please.NewAssignExpr("=", "deps", []string{ - "//third_party/go/github.com/wd_override", - "//third_party/go/github.com/foo_override", - "//third_party/go/github.com/app_override", + "//third_party/go/github.com/spf13:cobra", }), }), }, }, }, }, + // }, { // TEST_CASE ------------------------------------------------------------- + // Title: "merges configs between wd and given path together", + // Config: wollemi.Config{ + // KnownDependency: map[string]string{ + // "github.com/dep1": "//third_party/go/github.com/wont_be_used", + // }, + //}, + // Data: &GoFormatTestData{ + // Gosrc: gosrc, + // Gopkg: gopkg, + // Config: map[string]wollemi.Config{ + // "wd": wollemi.Config{ + // KnownDependency: map[string]string{ + // "github.com/dep1": "//third_party/go/github.com/foo_override", + // }, + // }, + // "wd/foo": wollemi.Config{ + // KnownDependency: map[string]string{ + // "github.com/dep2": "//third_party/go/github.com/foo_override", + // }, + // }, + // "wd/foo/app": wollemi.Config{ + // KnownDependency: map[string]string{ + // "github.com/dep3": "//third_party/go/github.com/app_override", + // }, + // }, + // }, + // Root: "/root", + // Wd: "/root/wd", + // Paths: []string{"foo/app"}, + // Parse: t.WithThirdPartyGo(nil), + // ImportDir: map[string]*golang.Package{ + // "wd/foo/app": { + // Name: "main", + // GoFiles: []string{"main.go"}, + // GoFileImports: map[string][]string{ + // "main.go": { + // "github.com/dep1", + // "github.com/dep2", + // "github.com/dep3", + // }, + // }, + // }, + // }, + // Write: map[string]*please.BuildFile{ + // "wd/foo/app/BUILD.plz": { + // Stmt: []please.Expr{ + // please.NewCallExpr("go_binary", []please.Expr{ + // please.NewAssignExpr("=", "name", "app"), + // please.NewAssignExpr("=", "srcs", please.NewGlob([]string{"*.go"}, "*_test.go")), + // please.NewAssignExpr("=", "visibility", []string{"PUBLIC"}), + // please.NewAssignExpr("=", "deps", []string{ + // "//third_party/go/github.com/wd_override", + // "//third_party/go/github.com/foo_override", + // "//third_party/go/github.com/app_override", + // }), + // }), + // }, + // }, + // }, + // }, }} { focus := "" From 9fc8726cfe5e1ec30b51fdb3a0ad1b7af116fcee Mon Sep 17 00:00:00 2001 From: Marcus Caisey Date: Sat, 29 Jan 2022 17:03:00 +0000 Subject: [PATCH 04/13] remove unneeded config test --- domain/wollemi/service_format_test.go | 61 --------------------------- 1 file changed, 61 deletions(-) diff --git a/domain/wollemi/service_format_test.go b/domain/wollemi/service_format_test.go index 66602b7..f31a0e6 100644 --- a/domain/wollemi/service_format_test.go +++ b/domain/wollemi/service_format_test.go @@ -1652,67 +1652,6 @@ func (t *ServiceSuite) TestService_GoFormat() { }, }, }, - // }, { // TEST_CASE ------------------------------------------------------------- - // Title: "merges configs between wd and given path together", - // Config: wollemi.Config{ - // KnownDependency: map[string]string{ - // "github.com/dep1": "//third_party/go/github.com/wont_be_used", - // }, - //}, - // Data: &GoFormatTestData{ - // Gosrc: gosrc, - // Gopkg: gopkg, - // Config: map[string]wollemi.Config{ - // "wd": wollemi.Config{ - // KnownDependency: map[string]string{ - // "github.com/dep1": "//third_party/go/github.com/foo_override", - // }, - // }, - // "wd/foo": wollemi.Config{ - // KnownDependency: map[string]string{ - // "github.com/dep2": "//third_party/go/github.com/foo_override", - // }, - // }, - // "wd/foo/app": wollemi.Config{ - // KnownDependency: map[string]string{ - // "github.com/dep3": "//third_party/go/github.com/app_override", - // }, - // }, - // }, - // Root: "/root", - // Wd: "/root/wd", - // Paths: []string{"foo/app"}, - // Parse: t.WithThirdPartyGo(nil), - // ImportDir: map[string]*golang.Package{ - // "wd/foo/app": { - // Name: "main", - // GoFiles: []string{"main.go"}, - // GoFileImports: map[string][]string{ - // "main.go": { - // "github.com/dep1", - // "github.com/dep2", - // "github.com/dep3", - // }, - // }, - // }, - // }, - // Write: map[string]*please.BuildFile{ - // "wd/foo/app/BUILD.plz": { - // Stmt: []please.Expr{ - // please.NewCallExpr("go_binary", []please.Expr{ - // please.NewAssignExpr("=", "name", "app"), - // please.NewAssignExpr("=", "srcs", please.NewGlob([]string{"*.go"}, "*_test.go")), - // please.NewAssignExpr("=", "visibility", []string{"PUBLIC"}), - // please.NewAssignExpr("=", "deps", []string{ - // "//third_party/go/github.com/wd_override", - // "//third_party/go/github.com/foo_override", - // "//third_party/go/github.com/app_override", - // }), - // }), - // }, - // }, - // }, - // }, }} { focus := "" From 71cbcfd5fac3f3dde73e94ebfaec702d4092b464 Mon Sep 17 00:00:00 2001 From: Marcus Caisey Date: Sat, 29 Jan 2022 17:39:10 +0000 Subject: [PATCH 05/13] add test for reading config from absolute path --- domain/wollemi/service_format_test.go | 39 +++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/domain/wollemi/service_format_test.go b/domain/wollemi/service_format_test.go index f31a0e6..53d279a 100644 --- a/domain/wollemi/service_format_test.go +++ b/domain/wollemi/service_format_test.go @@ -1652,6 +1652,45 @@ func (t *ServiceSuite) TestService_GoFormat() { }, }, }, + }, { // TEST_CASE ------------------------------------------------------------- + Title: "reads config from directory when absolute path given", + Config: wollemi.Config{}, + Data: &GoFormatTestData{ + Gosrc: gosrc, + Gopkg: gopkg, + Root: "/root", + Wd: "/root", + Config: map[string]wollemi.Config{ + "parent/app": {KnownDependency: map[string]string{ + "github.com/example/dep": "//third_party/go/github.com/example/override"}}, + }, + Paths: []string{"/root/parent/app"}, + ImportDir: map[string]*golang.Package{ + "parent/app": { + Name: "main", + GoFiles: []string{"main.go"}, + GoFileImports: map[string][]string{ + "main.go": { + "github.com/example/dep", + }, + }, + }, + }, + Write: map[string]*please.BuildFile{ + "parent/app/BUILD.plz": { + Stmt: []please.Expr{ + please.NewCallExpr("go_binary", []please.Expr{ + please.NewAssignExpr("=", "name", "app"), + please.NewAssignExpr("=", "srcs", please.NewGlob([]string{"*.go"}, "*_test.go")), + please.NewAssignExpr("=", "visibility", []string{"PUBLIC"}), + please.NewAssignExpr("=", "deps", []string{ + "//third_party/go/github.com/example/override", + }), + }), + }, + }, + }, + }, }} { focus := "" From 96d53f05f9000e985ba9e7c39276581a320928ed Mon Sep 17 00:00:00 2001 From: Marcus Caisey Date: Sun, 10 Apr 2022 16:25:45 +0100 Subject: [PATCH 06/13] update gomock gen --- third_party/go/github.com/golang/mock/BUILD.plz | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/third_party/go/github.com/golang/mock/BUILD.plz b/third_party/go/github.com/golang/mock/BUILD.plz index 778524e..f5a10f9 100644 --- a/third_party/go/github.com/golang/mock/BUILD.plz +++ b/third_party/go/github.com/golang/mock/BUILD.plz @@ -1,7 +1,11 @@ +package(default_visibility = ["PUBLIC"]) + +GOMOCK_VERSION = "1.6.0" + mock_download = remote_file( name = "mockgen", _tag = "download", - url = "https://github.com/golang/mock/releases/download/v1.4.3/mock_1.4.3_" + CONFIG.HOSTOS + "_" + CONFIG.HOSTARCH + ".tar.gz", + url = f"https://github.com/golang/mock/releases/download/v{GOMOCK_VERSION}/mock_{GOMOCK_VERSION}_" + CONFIG.HOSTOS + "_" + CONFIG.HOSTARCH + ".tar.gz", ) build_rule( @@ -27,6 +31,5 @@ go_module( "ci", "sample", ], - version = "v1.6.0", - visibility = ["PUBLIC"], + version = f"v{GOMOCK_VERSION}", ) From dee2e88a8611ecdec0a3c480fa8b7afff389a3b3 Mon Sep 17 00:00:00 2001 From: Marcus Caisey Date: Thu, 14 Apr 2022 23:04:11 +0100 Subject: [PATCH 07/13] validate paths for gofmt --- domain/wollemi/service_format.go | 12 ++++++++++++ domain/wollemi/service_format_test.go | 9 +++++++++ 2 files changed, 21 insertions(+) diff --git a/domain/wollemi/service_format.go b/domain/wollemi/service_format.go index 3a358a1..f770f44 100644 --- a/domain/wollemi/service_format.go +++ b/domain/wollemi/service_format.go @@ -385,6 +385,9 @@ func (this *Service) isInternal(path string) bool { } func (this *Service) GoFormat(config wollemi.Config, paths []string) error { + if err := this.validateAbsolutePaths(paths); err != nil { + return err + } this.goFormat = newGoFormat(this.normalizePaths(paths)) defer this.goFormat.resolveLimiter.Close() @@ -406,6 +409,15 @@ func (this *Service) GoFormat(config wollemi.Config, paths []string) error { return nil } +func (this *Service) validateAbsolutePaths(paths []string) error { + for _, path := range paths { + if filepath.IsAbs(path) && !strings.HasPrefix(path, this.root) { + return fmt.Errorf("absolute paths must be under the repo root") + } + } + return nil +} + func (this *Service) getRuleDeps(files []string, config wollemi.Config, dir *Directory) ([]string, []string, error) { var imports, resolved, unresolved []string diff --git a/domain/wollemi/service_format_test.go b/domain/wollemi/service_format_test.go index 53d279a..2da27db 100644 --- a/domain/wollemi/service_format_test.go +++ b/domain/wollemi/service_format_test.go @@ -1729,6 +1729,15 @@ func (t *ServiceSuite) TestService_GoFormat() { } }) } + + t.It("returns an error if given an absolute path which is not under the repo root", func(t *T) { + paths := []string{filepath.Join(root, "subdir"), "/outside/of/root"} + w := t.New(root, wd, gosrc, gopkg) + + err := w.GoFormat(wollemi.Config{}, paths) + + assert.Error(t, err) + }) } func (t *ServiceSuite) MockGoFormat(data *GoFormatTestData, write chan please.File) { From 0d1b7103390c529be70b1e8011521825f1938eaa Mon Sep 17 00:00:00 2001 From: Marcus Caisey Date: Thu, 14 Apr 2022 23:10:08 +0100 Subject: [PATCH 08/13] validate paths in rules unused --- domain/wollemi/service_rules_unused.go | 4 ++++ domain/wollemi/service_rules_unused_test.go | 10 ++++++++++ 2 files changed, 14 insertions(+) diff --git a/domain/wollemi/service_rules_unused.go b/domain/wollemi/service_rules_unused.go index e379579..caa1926 100644 --- a/domain/wollemi/service_rules_unused.go +++ b/domain/wollemi/service_rules_unused.go @@ -11,6 +11,10 @@ import ( ) func (this *Service) RulesUnused(prune bool, kinds, paths, exclude []string) error { + if err := this.validateAbsolutePaths(paths); err != nil { + return err + } + graph, err := this.please.Graph() if err != nil { return err diff --git a/domain/wollemi/service_rules_unused_test.go b/domain/wollemi/service_rules_unused_test.go index 574ddd0..74c2d59 100644 --- a/domain/wollemi/service_rules_unused_test.go +++ b/domain/wollemi/service_rules_unused_test.go @@ -3,6 +3,7 @@ package wollemi_test import ( "bytes" "os" + "path/filepath" "testing" "github.com/stretchr/testify/assert" @@ -112,6 +113,15 @@ func (t *ServiceSuite) TestService_RulesUnused() { wollemi.RulesUnused(prune, kinds, paths, excludePaths) }) + + t.It("returns an error if given an absolute path which is not under the repo root", func(t *T) { + paths := []string{filepath.Join(root, "subdir"), "/outside/of/root"} + w := t.New(root, wd, gosrc, gopkg) + + err := w.RulesUnused(false, nil, paths, nil) + + assert.Error(t, err) + }) } func (t *ServiceSuite) MockRulesUnused() { From 270fd2230671192e3e03d7f770afd8e53b30f647 Mon Sep 17 00:00:00 2001 From: Marcus Caisey Date: Thu, 14 Apr 2022 23:11:22 +0100 Subject: [PATCH 09/13] move validateAbsolutePaths to service.go --- domain/wollemi/service.go | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/domain/wollemi/service.go b/domain/wollemi/service.go index 93bc380..6d1ea48 100644 --- a/domain/wollemi/service.go +++ b/domain/wollemi/service.go @@ -54,6 +54,15 @@ type Service struct { goFormat *goFormat } +func (this *Service) validateAbsolutePaths(paths []string) error { + for _, path := range paths { + if filepath.IsAbs(path) && !strings.HasPrefix(path, this.root) { + return fmt.Errorf("absolute paths must be under the repo root") + } + } + return nil +} + func (this *Service) normalizePaths(paths []string) []string { if len(paths) == 0 { paths = []string{"..."} @@ -63,8 +72,6 @@ func (this *Service) normalizePaths(paths []string) []string { if !filepath.IsAbs(path) { path = filepath.Join(this.wd, path) } - // TODO: should we check that all absolute paths live under the root before we get to here? i don't think we can - // format them if not if strings.HasPrefix(path, this.root) { path, _ = filepath.Rel(this.root, path) } From 71f94c04a5dd59edcc3ad8542637ac977e921d7a Mon Sep 17 00:00:00 2001 From: Marcus Caisey Date: Thu, 14 Apr 2022 23:26:11 +0100 Subject: [PATCH 10/13] validate absolute paths for symlink go-path --- domain/wollemi/service_format.go | 10 +--------- domain/wollemi/service_format_test.go | 11 +++++++++-- domain/wollemi/service_rules_unused_test.go | 13 +++++++++++-- domain/wollemi/service_symlink_go_path.go | 4 ++++ domain/wollemi/service_symlink_go_path_test.go | 16 ++++++++++++++++ 5 files changed, 41 insertions(+), 13 deletions(-) diff --git a/domain/wollemi/service_format.go b/domain/wollemi/service_format.go index f770f44..331b925 100644 --- a/domain/wollemi/service_format.go +++ b/domain/wollemi/service_format.go @@ -388,6 +388,7 @@ func (this *Service) GoFormat(config wollemi.Config, paths []string) error { if err := this.validateAbsolutePaths(paths); err != nil { return err } + this.goFormat = newGoFormat(this.normalizePaths(paths)) defer this.goFormat.resolveLimiter.Close() @@ -409,15 +410,6 @@ func (this *Service) GoFormat(config wollemi.Config, paths []string) error { return nil } -func (this *Service) validateAbsolutePaths(paths []string) error { - for _, path := range paths { - if filepath.IsAbs(path) && !strings.HasPrefix(path, this.root) { - return fmt.Errorf("absolute paths must be under the repo root") - } - } - return nil -} - func (this *Service) getRuleDeps(files []string, config wollemi.Config, dir *Directory) ([]string, []string, error) { var imports, resolved, unresolved []string diff --git a/domain/wollemi/service_format_test.go b/domain/wollemi/service_format_test.go index 2da27db..a77c16f 100644 --- a/domain/wollemi/service_format_test.go +++ b/domain/wollemi/service_format_test.go @@ -1731,10 +1731,17 @@ func (t *ServiceSuite) TestService_GoFormat() { } t.It("returns an error if given an absolute path which is not under the repo root", func(t *T) { - paths := []string{filepath.Join(root, "subdir"), "/outside/of/root"} w := t.New(root, wd, gosrc, gopkg) - err := w.GoFormat(wollemi.Config{}, paths) + var ( + config = wollemi.Config{} + paths = []string{ + filepath.Join(root, "subdir"), + "/outside/of/root", + } + ) + + err := w.GoFormat(config, paths) assert.Error(t, err) }) diff --git a/domain/wollemi/service_rules_unused_test.go b/domain/wollemi/service_rules_unused_test.go index 74c2d59..b3fc557 100644 --- a/domain/wollemi/service_rules_unused_test.go +++ b/domain/wollemi/service_rules_unused_test.go @@ -115,10 +115,19 @@ func (t *ServiceSuite) TestService_RulesUnused() { }) t.It("returns an error if given an absolute path which is not under the repo root", func(t *T) { - paths := []string{filepath.Join(root, "subdir"), "/outside/of/root"} w := t.New(root, wd, gosrc, gopkg) - err := w.RulesUnused(false, nil, paths, nil) + var ( + prune bool = true + kinds []string + paths = []string{ + filepath.Join(root, "subdir"), + "/outside/of/root", + } + excludePaths []string + ) + + err := w.RulesUnused(prune, kinds, paths, excludePaths) assert.Error(t, err) }) diff --git a/domain/wollemi/service_symlink_go_path.go b/domain/wollemi/service_symlink_go_path.go index f0f8162..e0f6d62 100644 --- a/domain/wollemi/service_symlink_go_path.go +++ b/domain/wollemi/service_symlink_go_path.go @@ -12,6 +12,10 @@ import ( ) func (this *Service) SymlinkGoPath(force bool, paths []string) error { + if err := this.validateAbsolutePaths(paths); err != nil { + return err + } + paths = this.normalizePaths(paths) deps, err := this.please.QueryDeps(paths...) diff --git a/domain/wollemi/service_symlink_go_path_test.go b/domain/wollemi/service_symlink_go_path_test.go index 02d2c9f..e47f179 100644 --- a/domain/wollemi/service_symlink_go_path_test.go +++ b/domain/wollemi/service_symlink_go_path_test.go @@ -164,6 +164,22 @@ func (t *ServiceSuite) TestService_SymlinkGoPath() { wollemi.SymlinkGoPath(force, paths) }) + + t.It("returns an error if given an absolute path which is not under the repo root", func(t *T) { + w := t.New(root, wd, gosrc, gopkg) + + var ( + force = false + paths = []string{ + filepath.Join(root, "subdir"), + "/outside/of/root", + } + ) + + err := w.SymlinkGoPath(force, paths) + + assert.Error(t, err) + }) } func ignoreSkipDir(err error) error { From a0168947671acb359221e889f0c04ffc2e36cefc Mon Sep 17 00:00:00 2001 From: Marcus Caisey Date: Fri, 15 Apr 2022 16:47:24 +0100 Subject: [PATCH 11/13] validate absolute paths in symlink list --- adapters/cobra/symlink_list.go | 4 ++- domain/wollemi/service_symlink_list.go | 6 +++- domain/wollemi/service_symlink_list_test.go | 37 ++++++++++++++++++--- ports/ctl/wollemi.go | 2 +- 4 files changed, 41 insertions(+), 8 deletions(-) diff --git a/adapters/cobra/symlink_list.go b/adapters/cobra/symlink_list.go index d601543..ccc6b08 100644 --- a/adapters/cobra/symlink_list.go +++ b/adapters/cobra/symlink_list.go @@ -64,7 +64,9 @@ func SymlinkListCmd(app ctl.Application) *cobra.Command { args = []string{filepath.Join(wollemi.GoSrcPath(), "...")} } - wollemi.SymlinkList(name, broken, prune, exclude, args) + if err := wollemi.SymlinkList(name, broken, prune, exclude, args); err != nil { + return err + } return nil }, diff --git a/domain/wollemi/service_symlink_list.go b/domain/wollemi/service_symlink_list.go index 434a80f..6bddeeb 100644 --- a/domain/wollemi/service_symlink_list.go +++ b/domain/wollemi/service_symlink_list.go @@ -8,7 +8,10 @@ import ( "github.com/tcncloud/wollemi/ports/please" ) -func (this *Service) SymlinkList(name string, broken, prune bool, exclude, include []string) { +func (this *Service) SymlinkList(name string, broken, prune bool, exclude, include []string) error { + if err := this.validateAbsolutePaths(include); err != nil { + return err + } include = this.normalizePaths(include) for _, targetPath := range include { @@ -101,4 +104,5 @@ func (this *Service) SymlinkList(name string, broken, prune bool, exclude, inclu Warn("could not walk") } } + return nil } diff --git a/domain/wollemi/service_symlink_list_test.go b/domain/wollemi/service_symlink_list_test.go index be620c3..a89e5c2 100644 --- a/domain/wollemi/service_symlink_list_test.go +++ b/domain/wollemi/service_symlink_list_test.go @@ -7,6 +7,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/tcncloud/wollemi/ports/golang" "github.com/tcncloud/wollemi/testdata/please" @@ -60,7 +61,8 @@ func (t *ServiceSuite) TestService_SymlinkList() { include []string ) - wollemi.SymlinkList(name, broken, prune, exclude, include) + err := wollemi.SymlinkList(name, broken, prune, exclude, include) + require.NoError(t, err) want := []map[string]interface{}{ map[string]interface{}{ @@ -152,7 +154,8 @@ func (t *ServiceSuite) TestService_SymlinkList() { include []string ) - wollemi.SymlinkList(name, broken, prune, exclude, include) + err := wollemi.SymlinkList(name, broken, prune, exclude, include) + require.NoError(t, err) want := []map[string]interface{}{ map[string]interface{}{ @@ -217,7 +220,8 @@ func (t *ServiceSuite) TestService_SymlinkList() { include []string ) - wollemi.SymlinkList(name, broken, prune, exclude, include) + err := wollemi.SymlinkList(name, broken, prune, exclude, include) + require.NoError(t, err) want := []map[string]interface{}{ map[string]interface{}{ @@ -297,7 +301,8 @@ func (t *ServiceSuite) TestService_SymlinkList() { include []string ) - wollemi.SymlinkList(name, broken, prune, exclude, include) + err := wollemi.SymlinkList(name, broken, prune, exclude, include) + require.NoError(t, err) want := []map[string]interface{}{ map[string]interface{}{ @@ -368,7 +373,8 @@ func (t *ServiceSuite) TestService_SymlinkList() { include []string ) - wollemi.SymlinkList(name, broken, prune, exclude, include) + err := wollemi.SymlinkList(name, broken, prune, exclude, include) + require.NoError(t, err) want := []map[string]interface{}{ map[string]interface{}{ @@ -391,6 +397,27 @@ func (t *ServiceSuite) TestService_SymlinkList() { assert.ElementsMatch(t, want, t.logger.Lines()) }) + + t.It("returns an error if given an absolute path which is not under the repo root", func(t *T) { + wollemi := t.New(root, wd, gosrc, gopkg) + + var ( + name = "*" + broken bool + prune bool + exclude []string = []string{ + "app/protos/mock", + } + include = []string{ + filepath.Join(root, "subdir"), + "/outside/of/root", + } + ) + + err := wollemi.SymlinkList(name, broken, prune, exclude, include) + + assert.Error(t, err) + }) } func (t *ServiceSuite) GoSymlinkTestData() *GoFormatTestData { diff --git a/ports/ctl/wollemi.go b/ports/ctl/wollemi.go index 85560a6..d235459 100644 --- a/ports/ctl/wollemi.go +++ b/ports/ctl/wollemi.go @@ -15,7 +15,7 @@ type Wollemi interface { GoFormat(wollemi.Config, []string) error GoPkgPath(...string) string GoSrcPath(...string) string - SymlinkList(string, bool, bool, []string, []string) + SymlinkList(string, bool, bool, []string, []string) error SymlinkGoPath(bool, []string) error RulesUnused(bool, []string, []string, []string) error } From b5a644e9ec622142c2fdd8e503d7bf65765d1a04 Mon Sep 17 00:00:00 2001 From: Marcus Caisey Date: Fri, 15 Apr 2022 16:48:56 +0100 Subject: [PATCH 12/13] w -> wollemi --- domain/wollemi/service_rules_unused_test.go | 4 ++-- domain/wollemi/service_symlink_go_path_test.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/domain/wollemi/service_rules_unused_test.go b/domain/wollemi/service_rules_unused_test.go index b3fc557..7dd2204 100644 --- a/domain/wollemi/service_rules_unused_test.go +++ b/domain/wollemi/service_rules_unused_test.go @@ -115,7 +115,7 @@ func (t *ServiceSuite) TestService_RulesUnused() { }) t.It("returns an error if given an absolute path which is not under the repo root", func(t *T) { - w := t.New(root, wd, gosrc, gopkg) + wollemi := t.New(root, wd, gosrc, gopkg) var ( prune bool = true @@ -127,7 +127,7 @@ func (t *ServiceSuite) TestService_RulesUnused() { excludePaths []string ) - err := w.RulesUnused(prune, kinds, paths, excludePaths) + err := wollemi.RulesUnused(prune, kinds, paths, excludePaths) assert.Error(t, err) }) diff --git a/domain/wollemi/service_symlink_go_path_test.go b/domain/wollemi/service_symlink_go_path_test.go index e47f179..19f5263 100644 --- a/domain/wollemi/service_symlink_go_path_test.go +++ b/domain/wollemi/service_symlink_go_path_test.go @@ -166,7 +166,7 @@ func (t *ServiceSuite) TestService_SymlinkGoPath() { }) t.It("returns an error if given an absolute path which is not under the repo root", func(t *T) { - w := t.New(root, wd, gosrc, gopkg) + wollemi := t.New(root, wd, gosrc, gopkg) var ( force = false @@ -176,7 +176,7 @@ func (t *ServiceSuite) TestService_SymlinkGoPath() { } ) - err := w.SymlinkGoPath(force, paths) + err := wollemi.SymlinkGoPath(force, paths) assert.Error(t, err) }) From 164be99a23d55961b1880fdb17c235ece486e4d0 Mon Sep 17 00:00:00 2001 From: Marcus Caisey Date: Fri, 15 Apr 2022 17:44:12 +0100 Subject: [PATCH 13/13] make error message clearer --- domain/wollemi/service.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/domain/wollemi/service.go b/domain/wollemi/service.go index 6d1ea48..d9bce4d 100644 --- a/domain/wollemi/service.go +++ b/domain/wollemi/service.go @@ -55,11 +55,15 @@ type Service struct { } func (this *Service) validateAbsolutePaths(paths []string) error { + invalidPaths := make([]string, 0, len(paths)) for _, path := range paths { if filepath.IsAbs(path) && !strings.HasPrefix(path, this.root) { - return fmt.Errorf("absolute paths must be under the repo root") + invalidPaths = append(invalidPaths, path) } } + if len(invalidPaths) > 0 { + return fmt.Errorf("absolute paths %q are not under the plz repo root %q", invalidPaths, this.root) + } return nil }