From 6112c416376c67ec33fc7b81406914692494f6b3 Mon Sep 17 00:00:00 2001 From: Tonis Tiigi Date: Wed, 24 Apr 2024 17:19:32 -0700 Subject: [PATCH 1/5] lint: nilness fixes Signed-off-by: Tonis Tiigi --- controller/remote/client.go | 2 +- controller/remote/io.go | 2 +- driver/docker-container/driver.go | 14 ++++++-------- tests/workers/docker.go | 7 ++----- tests/workers/remote.go | 5 +---- 5 files changed, 11 insertions(+), 19 deletions(-) diff --git a/controller/remote/client.go b/controller/remote/client.go index b3b6c12b..b60061f4 100644 --- a/controller/remote/client.go +++ b/controller/remote/client.go @@ -210,7 +210,7 @@ func (c *Client) build(ctx context.Context, ref string, options pb.BuildOptions, } return err } else if n > 0 { - if stream.Send(&pb.InputMessage{ + if err := stream.Send(&pb.InputMessage{ Input: &pb.InputMessage_Data{ Data: &pb.DataMessage{ Data: buf[:n], diff --git a/controller/remote/io.go b/controller/remote/io.go index 384585e6..6a8c59c3 100644 --- a/controller/remote/io.go +++ b/controller/remote/io.go @@ -358,7 +358,7 @@ func copyToStream(fd uint32, snd msgStream, r io.Reader) error { } return err } else if n > 0 { - if snd.Send(&pb.Message{ + if err := snd.Send(&pb.Message{ Input: &pb.Message_File{ File: &pb.FdMessage{ Fd: fd, diff --git a/driver/docker-container/driver.go b/driver/docker-container/driver.go index 9f77dfe6..d9b29a34 100644 --- a/driver/docker-container/driver.go +++ b/driver/docker-container/driver.go @@ -203,14 +203,12 @@ func (d *Driver) wait(ctx context.Context, l progress.SubLogger) error { bufStderr := &bytes.Buffer{} if err := d.run(ctx, []string{"buildctl", "debug", "workers"}, bufStdout, bufStderr); err != nil { if try > 15 { - if err != nil { - d.copyLogs(context.TODO(), l) - if bufStdout.Len() != 0 { - l.Log(1, bufStdout.Bytes()) - } - if bufStderr.Len() != 0 { - l.Log(2, bufStderr.Bytes()) - } + d.copyLogs(context.TODO(), l) + if bufStdout.Len() != 0 { + l.Log(1, bufStdout.Bytes()) + } + if bufStderr.Len() != 0 { + l.Log(2, bufStderr.Bytes()) } return err } diff --git a/tests/workers/docker.go b/tests/workers/docker.go index cd6d6084..b1d711f0 100644 --- a/tests/workers/docker.go +++ b/tests/workers/docker.go @@ -60,12 +60,9 @@ func (c dockerWorker) New(ctx context.Context, cfg *integration.BackendConfig) ( } cl = func() error { - var err error - if err1 := bkclose(); err == nil { - err = err1 - } + err := bkclose() cmd := exec.Command("docker", "context", "rm", "-f", name) - if err1 := cmd.Run(); err1 != nil { + if err1 := cmd.Run(); err == nil { err = errors.Wrapf(err1, "failed to remove buildx instance %s", name) } return err diff --git a/tests/workers/remote.go b/tests/workers/remote.go index 4ded704f..1e2038b8 100644 --- a/tests/workers/remote.go +++ b/tests/workers/remote.go @@ -54,10 +54,7 @@ func (w remoteWorker) New(ctx context.Context, cfg *integration.BackendConfig) ( } cl = func() error { - var err error - if err1 := bkclose(); err == nil { - err = err1 - } + err := bkclose() cmd := exec.Command("buildx", "rm", "-f", name) if err1 := cmd.Run(); err == nil { err = err1 From 9428447cd2670e999ec8b4dd45cf2e23a7e0dc8a Mon Sep 17 00:00:00 2001 From: Tonis Tiigi Date: Wed, 24 Apr 2024 17:19:52 -0700 Subject: [PATCH 2/5] lint: unusedwrite fixes Signed-off-by: Tonis Tiigi --- builder/node.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builder/node.go b/builder/node.go index 64a990fc..bb0ec2f3 100644 --- a/builder/node.go +++ b/builder/node.go @@ -186,7 +186,7 @@ func (b *Builder) LoadNodes(ctx context.Context, opts ...LoadNodesOption) (_ []N if pl := di.DriverInfo.DynamicNodes[i].Platforms; len(pl) > 0 { diClone.Platforms = pl } - nodes = append(nodes, di) + nodes = append(nodes, diClone) } dynamicNodes = append(dynamicNodes, di.DriverInfo.DynamicNodes...) } From ec98985b4e842a8f2d0e3013504301b32d615f33 Mon Sep 17 00:00:00 2001 From: Tonis Tiigi Date: Wed, 24 Apr 2024 17:20:27 -0700 Subject: [PATCH 3/5] hack: linter updates Signed-off-by: Tonis Tiigi --- .golangci.yml | 8 ++++++++ build/build.go | 2 -- hack/dockerfiles/lint.Dockerfile | 3 ++- 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index b0231cd5..040f34a6 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -25,6 +25,14 @@ linters: disable-all: true linters-settings: + govet: + enable: + - nilness + - unusedwrite + # enable-all: true + # disable: + # - fieldalignment + # - shadow depguard: rules: main: diff --git a/build/build.go b/build/build.go index 9854f899..9745bda9 100644 --- a/build/build.go +++ b/build/build.go @@ -52,10 +52,8 @@ var ( ) const ( - //nolint:gosec // G101: false-positive printFallbackImage = "docker/dockerfile:1.5@sha256:dbbd5e059e8a07ff7ea6233b213b36aa516b4c53c645f1817a4dd18b83cbea56" // https://github.com/moby/buildkit/commit/71f99c52a669dc0322b5ea57bc28a09c20427227 - //nolint:gosec // G101: false-positive printLintFallbackImage = "docker.io/docker/dockerfile-upstream@sha256:47663570b6cc49ed90dc6e3215090a366989ab934d12dc93856a8ae0d27a95e7" ) diff --git a/hack/dockerfiles/lint.Dockerfile b/hack/dockerfiles/lint.Dockerfile index ae5d4389..cf92430a 100644 --- a/hack/dockerfiles/lint.Dockerfile +++ b/hack/dockerfiles/lint.Dockerfile @@ -2,9 +2,10 @@ ARG GO_VERSION=1.21 ARG XX_VERSION=1.3.0 -ARG GOLANGCI_LINT_VERSION=1.54.2 +ARG GOLANGCI_LINT_VERSION=1.57.2 FROM --platform=$BUILDPLATFORM tonistiigi/xx:${XX_VERSION} AS xx + FROM --platform=$BUILDPLATFORM golang:${GO_VERSION}-alpine RUN apk add --no-cache git gcc musl-dev ENV GOFLAGS="-buildvcs=false" From b30566438bc878de0ae681ecba7a87e63639554f Mon Sep 17 00:00:00 2001 From: Tonis Tiigi Date: Wed, 24 Apr 2024 17:58:17 -0700 Subject: [PATCH 4/5] lint: gopls fixes Signed-off-by: Tonis Tiigi --- build/invoke.go | 2 +- build/result.go | 6 +++--- commands/build.go | 4 ++-- commands/install.go | 2 +- commands/uninstall.go | 2 +- commands/version.go | 2 +- controller/build/build.go | 4 ++-- driver/docker-container/driver.go | 6 +++--- util/gitutil/path_windows.go | 2 +- util/ioset/mux_test.go | 10 +++++----- 10 files changed, 20 insertions(+), 20 deletions(-) diff --git a/build/invoke.go b/build/invoke.go index eb17e0b2..8e917ee0 100644 --- a/build/invoke.go +++ b/build/invoke.go @@ -37,7 +37,7 @@ func NewContainer(ctx context.Context, resultCtx *ResultHandle, cfg *controllera cancel() }() - containerCfg, err := resultCtx.getContainerConfig(ctx, c, cfg) + containerCfg, err := resultCtx.getContainerConfig(cfg) if err != nil { return nil, err } diff --git a/build/result.go b/build/result.go index b571cb66..d7e63efa 100644 --- a/build/result.go +++ b/build/result.go @@ -292,10 +292,10 @@ func (r *ResultHandle) build(buildFunc gateway.BuildFunc) (err error) { return err } -func (r *ResultHandle) getContainerConfig(ctx context.Context, c gateway.Client, cfg *controllerapi.InvokeConfig) (containerCfg gateway.NewContainerRequest, _ error) { +func (r *ResultHandle) getContainerConfig(cfg *controllerapi.InvokeConfig) (containerCfg gateway.NewContainerRequest, _ error) { if r.res != nil && r.solveErr == nil { logrus.Debugf("creating container from successful build") - ccfg, err := containerConfigFromResult(ctx, r.res, c, *cfg) + ccfg, err := containerConfigFromResult(r.res, *cfg) if err != nil { return containerCfg, err } @@ -327,7 +327,7 @@ func (r *ResultHandle) getProcessConfig(cfg *controllerapi.InvokeConfig, stdin i return processCfg, nil } -func containerConfigFromResult(ctx context.Context, res *gateway.Result, c gateway.Client, cfg controllerapi.InvokeConfig) (*gateway.NewContainerRequest, error) { +func containerConfigFromResult(res *gateway.Result, cfg controllerapi.InvokeConfig) (*gateway.NewContainerRequest, error) { if cfg.Initial { return nil, errors.Errorf("starting from the container from the initial state of the step is supported only on the failed steps") } diff --git a/commands/build.go b/commands/build.go index 6f151d49..f51d8551 100644 --- a/commands/build.go +++ b/commands/build.go @@ -340,7 +340,7 @@ func runBuild(ctx context.Context, dockerCli command.Cli, options buildOptions) if confutil.IsExperimental() { resp, retErr = runControllerBuild(ctx, dockerCli, opts, options, printer) } else { - resp, retErr = runBasicBuild(ctx, dockerCli, opts, options, printer) + resp, retErr = runBasicBuild(ctx, dockerCli, opts, printer) } if err := printer.Wait(); retErr == nil { @@ -387,7 +387,7 @@ func getImageID(resp map[string]string) string { return dgst } -func runBasicBuild(ctx context.Context, dockerCli command.Cli, opts *controllerapi.BuildOptions, options buildOptions, printer *progress.Printer) (*client.SolveResponse, error) { +func runBasicBuild(ctx context.Context, dockerCli command.Cli, opts *controllerapi.BuildOptions, printer *progress.Printer) (*client.SolveResponse, error) { resp, res, err := cbuild.RunBuild(ctx, dockerCli, *opts, dockerCli.In(), printer, false) if res != nil { res.Done() diff --git a/commands/install.go b/commands/install.go index 9c400dfe..0014a52a 100644 --- a/commands/install.go +++ b/commands/install.go @@ -15,7 +15,7 @@ import ( type installOptions struct { } -func runInstall(dockerCli command.Cli, in installOptions) error { +func runInstall(_ command.Cli, _ installOptions) error { dir := config.Dir() if err := os.MkdirAll(dir, 0755); err != nil { return errors.Wrap(err, "could not create docker config") diff --git a/commands/uninstall.go b/commands/uninstall.go index 6bc6bf33..373a822e 100644 --- a/commands/uninstall.go +++ b/commands/uninstall.go @@ -15,7 +15,7 @@ import ( type uninstallOptions struct { } -func runUninstall(dockerCli command.Cli, in uninstallOptions) error { +func runUninstall(_ command.Cli, _ uninstallOptions) error { dir := config.Dir() cfg, err := config.Load(dir) if err != nil { diff --git a/commands/version.go b/commands/version.go index b65cc4db..f98b19dc 100644 --- a/commands/version.go +++ b/commands/version.go @@ -11,7 +11,7 @@ import ( "github.com/spf13/cobra" ) -func runVersion(dockerCli command.Cli) error { +func runVersion(_ command.Cli) error { fmt.Println(version.Package, version.Version, version.Revision) return nil } diff --git a/controller/build/build.go b/controller/build/build.go index 06096cf1..1cae9b6c 100644 --- a/controller/build/build.go +++ b/controller/build/build.go @@ -189,7 +189,7 @@ func RunBuild(ctx context.Context, dockerCli command.Cli, in controllerapi.Build return nil, nil, err } - resp, res, err := buildTargets(ctx, dockerCli, b.NodeGroup, nodes, map[string]build.Options{defaultTargetName: opts}, progress, generateResult) + resp, res, err := buildTargets(ctx, dockerCli, nodes, map[string]build.Options{defaultTargetName: opts}, progress, generateResult) err = wrapBuildError(err, false) if err != nil { // NOTE: buildTargets can return *build.ResultHandle even on error. @@ -203,7 +203,7 @@ func RunBuild(ctx context.Context, dockerCli command.Cli, in controllerapi.Build // NOTE: When an error happens during the build and this function acquires the debuggable *build.ResultHandle, // this function returns it in addition to the error (i.e. it does "return nil, res, err"). The caller can // inspect the result and debug the cause of that error. -func buildTargets(ctx context.Context, dockerCli command.Cli, ng *store.NodeGroup, nodes []builder.Node, opts map[string]build.Options, progress progress.Writer, generateResult bool) (*client.SolveResponse, *build.ResultHandle, error) { +func buildTargets(ctx context.Context, dockerCli command.Cli, nodes []builder.Node, opts map[string]build.Options, progress progress.Writer, generateResult bool) (*client.SolveResponse, *build.ResultHandle, error) { var res *build.ResultHandle var resp map[string]*client.SolveResponse var err error diff --git a/driver/docker-container/driver.go b/driver/docker-container/driver.go index d9b29a34..14248817 100644 --- a/driver/docker-container/driver.go +++ b/driver/docker-container/driver.go @@ -77,7 +77,7 @@ func (d *Driver) Bootstrap(ctx context.Context, l progress.Logger) error { return err } return sub.Wrap("starting container "+d.Name, func() error { - if err := d.start(ctx, sub); err != nil { + if err := d.start(ctx); err != nil { return err } return d.wait(ctx, sub) @@ -188,7 +188,7 @@ func (d *Driver) create(ctx context.Context, l progress.SubLogger) error { if err := d.copyToContainer(ctx, d.InitConfig.Files); err != nil { return err } - if err := d.start(ctx, l); err != nil { + if err := d.start(ctx); err != nil { return err } } @@ -302,7 +302,7 @@ func (d *Driver) run(ctx context.Context, cmd []string, stdout, stderr io.Writer return nil } -func (d *Driver) start(ctx context.Context, l progress.SubLogger) error { +func (d *Driver) start(ctx context.Context) error { return d.DockerAPI.ContainerStart(ctx, d.Name, container.StartOptions{}) } diff --git a/util/gitutil/path_windows.go b/util/gitutil/path_windows.go index a1a91538..d0cae36b 100644 --- a/util/gitutil/path_windows.go +++ b/util/gitutil/path_windows.go @@ -4,6 +4,6 @@ import ( "os/exec" ) -func gitPath(wd string) (string, error) { +func gitPath(_ string) (string, error) { return exec.LookPath("git.exe") } diff --git a/util/ioset/mux_test.go b/util/ioset/mux_test.go index 7aaef570..af318be1 100644 --- a/util/ioset/mux_test.go +++ b/util/ioset/mux_test.go @@ -120,14 +120,14 @@ func TestMuxIO(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - inBuf, end, in := newTestIn(t) + inBuf, end, in := newTestIn() var outBufs []*outBuf var outs []MuxOut if tt.outputsNum != len(tt.wants) { t.Fatalf("wants != outputsNum") } for i := 0; i < tt.outputsNum; i++ { - outBuf, out := newTestOut(t, i) + outBuf, out := newTestOut(i) outBufs = append(outBufs, outBuf) outs = append(outs, MuxOut{out, nil, nil}) } @@ -223,7 +223,7 @@ type inBuf struct { doneCh chan struct{} } -func newTestIn(t *testing.T) (*inBuf, Out, In) { +func newTestIn() (*inBuf, Out, In) { ti := &inBuf{ doneCh: make(chan struct{}), } @@ -262,7 +262,7 @@ type outBuf struct { doneCh chan struct{} } -func newTestOut(t *testing.T, idx int) (*outBuf, Out) { +func newTestOut(idx int) (*outBuf, Out) { to := &outBuf{ idx: idx, doneCh: make(chan struct{}), @@ -285,7 +285,7 @@ func newTestOut(t *testing.T, idx int) (*outBuf, Out) { errW.CloseWithError(err) return } - to.stdin = string(buf.Bytes()) + to.stdin = buf.String() outW.Close() errW.Close() close(to.doneCh) From d0cc9ed0cb14ecd39582f3c2bff2d39b6d21943c Mon Sep 17 00:00:00 2001 From: Tonis Tiigi Date: Wed, 24 Apr 2024 17:58:39 -0700 Subject: [PATCH 5/5] hack: add gopls based linters Signed-off-by: Tonis Tiigi --- .github/workflows/validate.yml | 1 + Makefile | 4 +++ docker-bake.hcl | 7 ++++- hack/dockerfiles/lint.Dockerfile | 54 +++++++++++++++++++++++++++++++- 4 files changed, 64 insertions(+), 2 deletions(-) diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index 78a02c27..da453906 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -24,6 +24,7 @@ jobs: matrix: target: - lint + - lint-gopls - validate-vendor - validate-docs - validate-generated-files diff --git a/Makefile b/Makefile index 916421e8..e95c854f 100644 --- a/Makefile +++ b/Makefile @@ -43,6 +43,10 @@ validate-all: lint test validate-vendor validate-docs validate-generated-files lint: $(BUILDX_CMD) bake lint +.PHONY: lint-gopls +lint-gopls: + $(BUILDX_CMD) bake lint-gopls + .PHONY: test test: ./hack/test diff --git a/docker-bake.hcl b/docker-bake.hcl index 084fdd64..bff95e59 100644 --- a/docker-bake.hcl +++ b/docker-bake.hcl @@ -28,7 +28,7 @@ group "default" { } group "validate" { - targets = ["lint", "validate-vendor", "validate-docs"] + targets = ["lint", "lint-gopls", "validate-vendor", "validate-docs"] } target "lint" { @@ -48,6 +48,11 @@ target "lint" { ] : [] } +target "lint-gopls" { + inherits = ["lint"] + target = "gopls-analyze" +} + target "validate-vendor" { inherits = ["_common"] dockerfile = "./hack/dockerfiles/vendor.Dockerfile" diff --git a/hack/dockerfiles/lint.Dockerfile b/hack/dockerfiles/lint.Dockerfile index cf92430a..3a6ce1fb 100644 --- a/hack/dockerfiles/lint.Dockerfile +++ b/hack/dockerfiles/lint.Dockerfile @@ -3,11 +3,17 @@ ARG GO_VERSION=1.21 ARG XX_VERSION=1.3.0 ARG GOLANGCI_LINT_VERSION=1.57.2 +ARG GOPLS_VERSION=v0.20.0 +# disabled: deprecated unusedvariable simplifyrange +ARG GOPLS_ANALYZERS="embeddirective fillreturns infertypeargs nonewvars noresultvalues simplifycompositelit simplifyslice stubmethods undeclaredname unusedparams useany" + FROM --platform=$BUILDPLATFORM tonistiigi/xx:${XX_VERSION} AS xx -FROM --platform=$BUILDPLATFORM golang:${GO_VERSION}-alpine +FROM --platform=$BUILDPLATFORM golang:${GO_VERSION}-alpine AS golang-base RUN apk add --no-cache git gcc musl-dev + +FROM golang-base AS lint ENV GOFLAGS="-buildvcs=false" ARG GOLANGCI_LINT_VERSION RUN wget -O- -nv https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s v${GOLANGCI_LINT_VERSION} @@ -18,3 +24,49 @@ RUN --mount=target=/go/src/github.com/docker/buildx \ --mount=target=/root/.cache,type=cache,id=lint-cache-$TARGETPLATFORM \ xx-go --wrap && \ golangci-lint run + +FROM golang-base AS gopls +RUN apk add --no-cache git +ARG GOPLS_VERSION +WORKDIR /src +RUN git clone https://github.com/golang/tools.git && \ + cd tools && git checkout ${GOPLS_VERSION} +WORKDIR tools/gopls +ARG GOPLS_ANALYZERS +RUN <<'EOF' + set -ex + mkdir -p /out + for analyzer in ${GOPLS_ANALYZERS}; do + mkdir -p internal/cmd/$analyzer + cat < internal/cmd/$analyzer/main.go +package main + +import ( + "golang.org/x/tools/go/analysis/singlechecker" + analyzer "golang.org/x/tools/gopls/internal/analysis/$analyzer" +) + +func main() { singlechecker.Main(analyzer.Analyzer) } +eot + echo "Analyzing with ${analyzer}..." + go build -o /out/$analyzer ./internal/cmd/$analyzer + done +EOF + +FROM golang-base AS gopls-analyze +COPY --link --from=xx / / +ARG GOPLS_ANALYZERS +ARG TARGETNAME +ARG TARGETPLATFORM +WORKDIR /go/src/github.com/docker/buildx +RUN --mount=target=. \ + --mount=target=/root/.cache,type=cache,id=lint-cache-${TARGETNAME}-${TARGETPLATFORM} \ + --mount=target=/gopls-analyzers,from=gopls,source=/out <