Skip to content

Commit

Permalink
devtools: use separate module for dev tool dependencies
Browse files Browse the repository at this point in the history 
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/
  • Loading branch information
@RaduBerinde
RaduBerinde committed Aug 1, 2024
1 parent 62cd2a2 commit 2e4bf3a
Show file tree
Hide file tree
Showing 5 changed files with 105 additions and 28 deletions.
4 changes: 1 addition & 3 deletions Makefile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
19 changes: 19 additions & 0 deletions internal/devtools/go.mod
View file
Original file line number Diff line number Diff line change
@@ -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
)
41 changes: 41 additions & 0 deletions internal/devtools/go.sum
View file
Original file line number Diff line number Diff line change
@@ -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=
10 changes: 10 additions & 0 deletions internal/devtools/tools.go
View file
Original file line number Diff line number Diff line change
@@ -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"
)
59 changes: 34 additions & 25 deletions internal/lint/lint_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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")
Expand All @@ -70,34 +82,30 @@ 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 {
t.Error(err)
}
})

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)
Expand All @@ -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 {
Expand Down Expand Up @@ -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) {
Expand All @@ -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"+
Expand Down

0 comments on commit 2e4bf3a

Please sign in to comment.