From 2e4bf3a6ef3421d44e44d69c7fc8b544004fcb0e Mon Sep 17 00:00:00 2001 From: Radu Berinde Date: Thu, 1 Aug 2024 07:47:42 -0700 Subject: [PATCH] devtools: use separate module for dev tool dependencies We currently use `go run` directly to run tools like `crlfmt`, `golint`, etc. This is convenient but there are a few downsides: - there is no centralized place which has the tool versions; - `go run` requires an internet connection every time it is run; - it's more tedious to update the versions (compared to `go get`). This change adds an `internal/devtools` module with a bogus `.go` file with import dependencies on the tools. We use `go install` in this repository to install the correct versions of the tools, and then we run them directly. This scheme is documented here: https://play-with-go.dev/tools-as-dependencies_go119_en/ --- Makefile | 4 +-- internal/devtools/go.mod | 19 ++++++++++++ internal/devtools/go.sum | 41 ++++++++++++++++++++++++++ internal/devtools/tools.go | 10 +++++++ internal/lint/lint_test.go | 59 ++++++++++++++++++++++---------------- 5 files changed, 105 insertions(+), 28 deletions(-) create mode 100644 internal/devtools/go.mod create mode 100644 internal/devtools/go.sum create mode 100644 internal/devtools/tools.go diff --git a/Makefile b/Makefile index c3eeb906ee..5649fa70aa 100644 --- a/Makefile +++ b/Makefile @@ -130,11 +130,9 @@ endif @${GO} mod tidy $(MAKE) git-clean-check -# TODO(radu): switch back to @latest once bogus doc changes are -# addressed; see https://github.com/cockroachdb/crlfmt/pull/44 .PHONY: format format: - go install github.com/cockroachdb/crlfmt@44a36ec7 && crlfmt -w -tab 2 . + go install -C internal/devtools github.com/cockroachdb/crlfmt && crlfmt -w -tab 2 ../.. .PHONY: format-check format-check: diff --git a/internal/devtools/go.mod b/internal/devtools/go.mod new file mode 100644 index 0000000000..4c2cdb915d --- /dev/null +++ b/internal/devtools/go.mod @@ -0,0 +1,19 @@ +module github.com/cockroachdb/pebble/internal/devtools + +go 1.22 + +require ( + github.com/cockroachdb/crlfmt v0.0.0-20230505164321-461e8663b4b4 + golang.org/x/lint v0.0.0-20210508222113-6edffad5e616 + honnef.co/go/tools v0.4.7 +) + +require ( + github.com/BurntSushi/toml v1.2.1 // indirect + github.com/cockroachdb/gostdlib v1.19.0 // indirect + github.com/cockroachdb/ttycolor v0.0.0-20180709150743-a1d5aaeb377d // indirect + golang.org/x/exp/typeparams v0.0.0-20221208152030-732eee02a75a // indirect + golang.org/x/mod v0.12.0 // indirect + golang.org/x/sys v0.11.0 // indirect + golang.org/x/tools v0.12.1-0.20230825192346-2191a27a6dc5 // indirect +) diff --git a/internal/devtools/go.sum b/internal/devtools/go.sum new file mode 100644 index 0000000000..3783f855c1 --- /dev/null +++ b/internal/devtools/go.sum @@ -0,0 +1,41 @@ +github.com/BurntSushi/toml v1.2.1 h1:9F2/+DoOYIOksmaJFPw1tGFy1eDnIJXg+UHjuD8lTak= +github.com/BurntSushi/toml v1.2.1/go.mod h1:CxXYINrC8qIiEnFrOxCa7Jy5BFHlXnUU2pbicEuybxQ= +github.com/cockroachdb/crlfmt v0.0.0-20230505164321-461e8663b4b4 h1:bP9hmRTzF2bNvM+uFJT4YF2OmT3ddfo4YR+WZqlrk50= +github.com/cockroachdb/crlfmt v0.0.0-20230505164321-461e8663b4b4/go.mod h1:qtkxNlt5i3rrdirfJE/bQeW/IeLajKexErv7jEIV+Uc= +github.com/cockroachdb/gostdlib v1.19.0 h1:cSISxkVnTlWhTkyple/T6NXzOi5659FkhxvUgZv+Eb0= +github.com/cockroachdb/gostdlib v1.19.0/go.mod h1:+dqqpARXbE/gRDEhCak6dm0l14AaTymPZUKMfURjBtY= +github.com/cockroachdb/ttycolor v0.0.0-20180709150743-a1d5aaeb377d h1:TNsiMS1Ij2aleP/05UNSPzsOu9eJm9mfUGLm7Ylt7Dg= +github.com/cockroachdb/ttycolor v0.0.0-20180709150743-a1d5aaeb377d/go.mod h1:NltwFG0VBANi1jHKpn5KL9AbsHTE+8fPaAHT0TzL20k= +github.com/davecgh/go-spew v1.1.0 h1:ZDRjVQ15GmhC3fiQ8ni8+OwkZQO4DARzQgrnXU1Liz8= +github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= +github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= +github.com/stretchr/testify v1.6.1 h1:hDPOHmpOpP40lSULcqw7IrRb/u7w6RpDC9399XyoNd0= +github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= +golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= +golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= +golang.org/x/exp/typeparams v0.0.0-20221208152030-732eee02a75a h1:Jw5wfR+h9mnIYH+OtGT2im5wV1YGGDora5vTv/aa5bE= +golang.org/x/exp/typeparams v0.0.0-20221208152030-732eee02a75a/go.mod h1:AbB0pIl9nAr9wVwH+Z2ZpaocVmF5I4GyWCDIsVjR0bk= +golang.org/x/lint v0.0.0-20210508222113-6edffad5e616 h1:VLliZ0d+/avPrXXH+OakdXhpJuEoBZuwh1m2j7U6Iug= +golang.org/x/lint v0.0.0-20210508222113-6edffad5e616/go.mod h1:3xt1FjdF8hUf6vQPIChWIBhFzV8gjjsPE/fR3IyQdNY= +golang.org/x/mod v0.1.1-0.20191105210325-c90efee705ee/go.mod h1:QqPTAvyqsEbceGzBzNggFXnrqF1CaUcvgkdR5Ot7KZg= +golang.org/x/mod v0.12.0 h1:rmsUpXtvNzj340zd98LZ4KntptpfRHwpFOHG188oHXc= +golang.org/x/mod v0.12.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs= +golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= +golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= +golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= +golang.org/x/sync v0.3.0 h1:ftCYgMx6zT/asHUrPw8BLLscYtGznsLAnjq5RH9P66E= +golang.org/x/sync v0.3.0/go.mod h1:FU7BRWz2tNW+3quACPkgCx/L+uEAv1htQ0V83Z9Rj+Y= +golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= +golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.11.0 h1:eG7RXZHdqOJ1i+0lgLgCpSXAp6M3LYlAo6osgSi0xOM= +golang.org/x/sys v0.11.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= +golang.org/x/tools v0.0.0-20200130002326-2f3ba24bd6e7/go.mod h1:TB2adYChydJhpapKDTa4BR/hXlZSLoq2Wpct/0txZ28= +golang.org/x/tools v0.12.1-0.20230825192346-2191a27a6dc5 h1:Vk4mysSz+GqQK2eqgWbo4zEO89wkeAjJiFIr9bpqa8k= +golang.org/x/tools v0.12.1-0.20230825192346-2191a27a6dc5/go.mod h1:Sc0INKfu04TlqNoRA1hgpFZbhYXHPr4V5DzpSBTPqQM= +golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= +gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c h1:dUUwHk2QECo/6vqA44rthZ8ie2QXMNeKRTHCNY2nXvo= +gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= +honnef.co/go/tools v0.4.7 h1:9MDAWxMoSnB6QoSqiVr7P5mtkT9pOc1kSxchzPCnqJs= +honnef.co/go/tools v0.4.7/go.mod h1:+rnGS1THNh8zMwnd2oVOTL9QF6vmfyG6ZXBULae2uc0= diff --git a/internal/devtools/tools.go b/internal/devtools/tools.go new file mode 100644 index 0000000000..a47a0b25f3 --- /dev/null +++ b/internal/devtools/tools.go @@ -0,0 +1,10 @@ +//go:build tools +// +build tools + +package tools + +import ( + _ "github.com/cockroachdb/crlfmt" + _ "golang.org/x/lint/golint" + _ "honnef.co/go/tools/cmd/staticcheck" +) diff --git a/internal/lint/lint_test.go b/internal/lint/lint_test.go index 3386f788ce..a7b8785eaf 100644 --- a/internal/lint/lint_test.go +++ b/internal/lint/lint_test.go @@ -22,9 +22,9 @@ import ( const ( cmdGo = "go" - golint = "golang.org/x/lint/golint@6edffad5e6160f5949cdefc81710b2706fbcd4f6" - staticcheck = "honnef.co/go/tools/cmd/staticcheck@2023.1.7" - crlfmt = "github.com/cockroachdb/crlfmt@461e8663" + golint = "golang.org/x/lint/golint" + staticcheck = "honnef.co/go/tools/cmd/staticcheck" + crlfmt = "github.com/cockroachdb/crlfmt" ) func dirCmd(t *testing.T, dir string, name string, args ...string) stream.Filter { @@ -45,10 +45,22 @@ func ignoreGoMod() stream.Filter { return stream.GrepNot(`^go: (finding|extracting|downloading)`) } +func installTool(t *testing.T, path string) { + cmd := exec.Command("go", "install", "-C", "../devtools", path) + out, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("cannot install %q: %v\n%s\n", path, err, out) + } +} + func TestLint(t *testing.T) { if runtime.GOOS == "windows" { t.Skip("lint checks skipped on Windows") } + if runtime.GOARCH == "386" { + // GOARCH=386 messes with the installation of devtools. + t.Skip("lint checks skipped on GOARCH=386") + } if invariants.RaceEnabled { // We are not interested in race-testing the linters themselves. t.Skip("lint checks skipped on race builds") @@ -70,18 +82,14 @@ func TestLint(t *testing.T) { require.NoError(t, err) } - t.Run("TestGolint", func(t *testing.T) { - t.Parallel() - - args := []string{"run", golint} - args = append(args, pkgs...) - - // This is overkill right now, but provides a structure for filtering out - // lint errors we don't care about. + // TestGoVet is the fastest check that verifies that all files build, so we + // want to run it first (and not in parallel). + t.Run("TestGoVet", func(t *testing.T) { if err := stream.ForEach( stream.Sequence( - dirCmd(t, pkg.Dir, cmdGo, args...), - stream.GrepNot("go: downloading"), + dirCmd(t, pkg.Dir, "go", "vet", "-all", "./..."), + stream.GrepNot(`^#`), // ignore comment lines + ignoreGoMod(), ), func(s string) { t.Errorf("\n%s", s) }); err != nil { @@ -89,15 +97,15 @@ func TestLint(t *testing.T) { } }) - t.Run("TestStaticcheck", func(t *testing.T) { + t.Run("TestGolint", func(t *testing.T) { + installTool(t, golint) t.Parallel() - args := []string{"run", staticcheck} - args = append(args, pkgs...) - + // This is overkill right now, but provides a structure for filtering out + // lint errors we don't care about. if err := stream.ForEach( stream.Sequence( - dirCmd(t, pkg.Dir, cmdGo, args...), + dirCmd(t, pkg.Dir, "golint", pkgs...), stream.GrepNot("go: downloading"), ), func(s string) { t.Errorf("\n%s", s) @@ -106,14 +114,14 @@ func TestLint(t *testing.T) { } }) - t.Run("TestGoVet", func(t *testing.T) { + t.Run("TestStaticcheck", func(t *testing.T) { + installTool(t, staticcheck) t.Parallel() if err := stream.ForEach( stream.Sequence( - dirCmd(t, pkg.Dir, "go", "vet", "-all", "./..."), - stream.GrepNot(`^#`), // ignore comment lines - ignoreGoMod(), + dirCmd(t, pkg.Dir, "staticcheck", pkgs...), + stream.GrepNot("go: downloading"), ), func(s string) { t.Errorf("\n%s", s) }); err != nil { @@ -241,13 +249,14 @@ func TestLint(t *testing.T) { }) t.Run("TestCrlfmt", func(t *testing.T) { + installTool(t, crlfmt) t.Parallel() - args := []string{"run", crlfmt, "-fast", "-tab", "2", "."} + args := []string{"-fast", "-tab", "2", "."} var buf bytes.Buffer if err := stream.ForEach( stream.Sequence( - dirCmd(t, pkg.Dir, cmdGo, args...), + dirCmd(t, pkg.Dir, "crlfmt", args...), stream.GrepNot("go: downloading"), ), func(s string) { @@ -261,7 +270,7 @@ func TestLint(t *testing.T) { } if t.Failed() { - reWriteCmd := []string{crlfmt, "-w"} + reWriteCmd := []string{"crlfmt", "-w"} reWriteCmd = append(reWriteCmd, args...) t.Logf("run the following to fix your formatting:\n"+ "\n%s\n\n"+