Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

linter updates and gopls linting #2430

Merged
merged 5 commits into from
Apr 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/workflows/validate.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ jobs:
matrix:
target:
- lint
- lint-gopls
- validate-vendor
- validate-docs
- validate-generated-files
Expand Down
8 changes: 8 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,14 @@ linters:
disable-all: true

linters-settings:
govet:
enable:
- nilness
- unusedwrite
# enable-all: true
# disable:
# - fieldalignment
# - shadow
depguard:
rules:
main:
Expand Down
4 changes: 4 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 0 additions & 2 deletions build/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down
2 changes: 1 addition & 1 deletion build/invoke.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
6 changes: 3 additions & 3 deletions build/result.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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")
}
Expand Down
2 changes: 1 addition & 1 deletion builder/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a behavior change 😬

@AkihiroSuda FYI

}
dynamicNodes = append(dynamicNodes, di.DriverInfo.DynamicNodes...)
}
Expand Down
4 changes: 2 additions & 2 deletions commands/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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()
Expand Down
2 changes: 1 addition & 1 deletion commands/install.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
2 changes: 1 addition & 1 deletion commands/uninstall.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion commands/version.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
4 changes: 2 additions & 2 deletions controller/build/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion controller/remote/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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],
Expand Down
2 changes: 1 addition & 1 deletion controller/remote/io.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
7 changes: 6 additions & 1 deletion docker-bake.hcl
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ group "default" {
}

group "validate" {
targets = ["lint", "validate-vendor", "validate-docs"]
targets = ["lint", "lint-gopls", "validate-vendor", "validate-docs"]
}

target "lint" {
Expand All @@ -48,6 +48,11 @@ target "lint" {
] : []
}

target "lint-gopls" {
inherits = ["lint"]
target = "gopls-analyze"
}

target "validate-vendor" {
inherits = ["_common"]
dockerfile = "./hack/dockerfiles/vendor.Dockerfile"
Expand Down
20 changes: 9 additions & 11 deletions driver/docker-container/driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
}
}
Expand All @@ -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
}
Expand Down Expand Up @@ -304,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{})
}

Expand Down
57 changes: 55 additions & 2 deletions hack/dockerfiles/lint.Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,18 @@

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
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}
Expand All @@ -17,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 <<eot > 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 <<EOF
set -ex
xx-go --wrap
for analyzer in ${GOPLS_ANALYZERS}; do
go vet -vettool=/gopls-analyzers/$analyzer ./...
done
EOF

FROM lint
7 changes: 2 additions & 5 deletions tests/workers/docker.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 1 addition & 4 deletions tests/workers/remote.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion util/gitutil/path_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,6 @@ import (
"os/exec"
)

func gitPath(wd string) (string, error) {
func gitPath(_ string) (string, error) {
return exec.LookPath("git.exe")
}
10 changes: 5 additions & 5 deletions util/ioset/mux_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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})
}
Expand Down Expand Up @@ -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{}),
}
Expand Down Expand Up @@ -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{}),
Expand All @@ -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)
Expand Down
Loading