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

[CI] Tigron breakout: golangci v2 and cleanups #4041

Merged
merged 2 commits into from
Mar 27, 2025
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
95 changes: 52 additions & 43 deletions mod/tigron/.golangci.yml
Original file line number Diff line number Diff line change
@@ -1,54 +1,63 @@
---
output:
sort-results: true

issues:
max-issues-per-linter: 0
max-same-issues: 0
version: "2"

run:
concurrency: 0
timeout: 5m
issues-exit-code: 2
tests: true
modules-download-mode: readonly
allow-parallel-runners: true
allow-serial-runners: true

issues:
max-issues-per-linter: 0
max-same-issues: 0

linters:
disable-all: false
enable-all: true
default: all
disable:
# Opting-out
- nonamedreturns # named returns are occasionally useful
- exhaustruct # does not serve much of a purpose
- funlen # not interested
- cyclop # not interested much
- godox # having these are useful
# Duplicating
- gci # we use go-imports instead
# Deprecated
- tenv
# TODO: Temporarily out until we wrap up all of them
# - wrapcheck
- cyclop
- exhaustruct
- funlen
- godox
- nonamedreturns
settings:
depguard:
rules:
main:
files:
- $all
allow:

linters-settings:
staticcheck:
checks:
- "all"
- $gostd
- github.com/containerd/nerdctl/mod/tigron
- github.com/creack/pty
- golang.org/x/sync
- golang.org/x/term
- gotest.tools/v3
staticcheck:
checks:
- all
exclusions:
generated: disable

depguard:
rules:
main:
files:
- "$all"
allow:
- $gostd
- "github.com/containerd/nerdctl/mod/tigron"
# WATCHOUT! https://github.com/OpenPeeDeeP/depguard/issues/108
# Currently, depguard will fail detecting any dependency starting with a standard package name as third-party.
# Thus, the following three are allowed provisionally, though currently not "necessary".
- "golang.org/x/sync"
- "golang.org/x/term"
- "gotest.tools"
- "github.com/creack/pty"
formatters:
settings:
gci:
sections:
- standard
- default
- prefix(github.com/containerd)
- localmodule
no-inline-comments: true
no-prefix-comments: true
custom-order: true
gofumpt:
extra-rules: true
golines:
max-len: 100
tab-len: 4
shorten-comments: true
enable:
- gci
- gofumpt
- golines
exclusions:
generated: disable
57 changes: 24 additions & 33 deletions mod/tigron/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -57,16 +57,16 @@ endef
##########################
# High-level tasks definitions
##########################
lint: lint-go-all lint-imports lint-yaml lint-shell lint-commits lint-headers lint-mod lint-licenses-all
Copy link
Contributor Author

Choose a reason for hiding this comment

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

imports (lint and fix) are now provided by golangci-lint with gci.

Separate tasks and module are no longer necessary.

lint: lint-go-all lint-yaml lint-shell lint-commits lint-headers lint-mod lint-licenses-all
test: test-unit test-unit-race test-unit-bench
unit: test-unit test-unit-race test-unit-bench
fix: fix-mod fix-imports fix-go-all
fix: fix-mod fix-go-all

##########################
# Linting tasks
##########################
lint-go:
$(call title, $@)
$(call title, $@: $(GOOS))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Clarifies output.

@cd $(MAKEFILE_DIR) \
&& golangci-lint run $(VERBOSE_FLAG_LONG) ./...
$(call footer, $@)
Expand All @@ -80,12 +80,6 @@ lint-go-all:
&& GOOS=windows make lint-go
$(call footer, $@)

lint-imports:
$(call title, $@)
@cd $(MAKEFILE_DIR) \
&& goimports-reviser -recursive -list-diff -set-exit-status -output stdout -company-prefixes "$(ORG_PREFIXES)" ./...
$(call footer, $@)

lint-yaml:
$(call title, $@)
@cd $(MAKEFILE_DIR) \
Expand Down Expand Up @@ -115,10 +109,15 @@ lint-mod:
&& go mod tidy --diff
$(call footer, $@)

# FIXME: go-licenses cannot find LICENSE from root of repo when submodule is imported:
# https://github.com/google/go-licenses/issues/186
# This is impacting gotest.tools
lint-licenses:
$(call title, $@)
$(call title, $@: $(GOOS))
@cd $(MAKEFILE_DIR) \
&& ./hack/make-lint-licenses.sh
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Separate hack is no longer necessary.

&& go-licenses check --include_tests --allowed_licenses=Apache-2.0,BSD-2-Clause,BSD-3-Clause,MIT,MPL-2.0 \
--ignore gotest.tools \
./...
$(call footer, $@)

lint-licenses-all:
Expand All @@ -134,7 +133,7 @@ lint-licenses-all:
# Automated fixing tasks
##########################
fix-go:
$(call title, $@)
$(call title, $@: $(GOOS))
@cd $(MAKEFILE_DIR) \
&& golangci-lint run --fix
$(call footer, $@)
Expand All @@ -148,12 +147,6 @@ fix-go-all:
&& GOOS=windows make fix-go
$(call footer, $@)

fix-imports:
$(call title, $@)
@cd $(MAKEFILE_DIR) \
&& goimports-reviser -company-prefixes $(ORG_PREFIXES) ./...
$(call footer, $@)

fix-mod:
$(call title, $@)
@cd $(MAKEFILE_DIR) \
Expand All @@ -171,18 +164,16 @@ up:
##########################
install-dev-tools:
$(call title, $@)
# golangci: v1.64.5
# git-validation: main from 2023/11
# ltag: v0.2.5
# go-licenses: v2.0.0-alpha.1
# goimports-reviser: v3.9.0
# golangci: v2.0.2 (2024-03-26)
# git-validation: main (2025-02-25)
# ltag: main (2025-03-04)
# go-licenses: v2.0.0-alpha.1 (2024-06-27)
@cd $(MAKEFILE_DIR) \
&& go install github.com/golangci/golangci-lint/cmd/golangci-lint@0a603e49e5e9870f5f9f2035bcbe42cd9620a9d5 \
&& go install github.com/vbatts/git-validation@679e5cad8c50f1605ab3d8a0a947aaf72fb24c07 \
&& go install github.com/kunalkushwaha/ltag@b0cfa33e4cc9383095dc584d3990b62c95096de0 \
&& go install github.com/google/go-licenses/v2@d01822334fba5896920a060f762ea7ecdbd086e8 \
&& go install github.com/incu6us/goimports-reviser/v3@698f92d226d50a01731ca8551993ebc1bb7fc788
@echo "Remember to add GOROOT/bin to your path"
&& go install github.com/golangci/golangci-lint/v2/cmd/golangci-lint@2b224c2cf4c9f261c22a16af7f8ca6408467f338 \
&& go install github.com/vbatts/git-validation@7b60e35b055dd2eab5844202ffffad51d9c93922 \
&& go install github.com/containerd/ltag@66e6a514664ee2d11a470735519fa22b1a9eaabd \
&& go install github.com/google/go-licenses/v2@d01822334fba5896920a060f762ea7ecdbd086e8
@echo "Remember to add \$$HOME/go/bin to your path"
$(call footer, $@)

##########################
Expand All @@ -200,7 +191,7 @@ test-unit-bench:

test-unit-race:
$(call title, $@)
@go test $(VERBOSE_FLAG) $(MAKEFILE_DIR)/... -race
@CGO_ENABLED=1 go test $(VERBOSE_FLAG) $(MAKEFILE_DIR)/... -race
$(call footer, $@)

.PHONY: \
Expand All @@ -210,6 +201,6 @@ test-unit-race:
up \
unit \
install-dev-tools \
lint-commits lint-go lint-go-all lint-headers lint-imports lint-licenses lint-licenses-all lint-mod lint-shell lint-yaml \
fix-go fix-go-all fix-imports fix-mod \
test-unit test-unit-race test-unit-bench
lint-commits lint-go lint-go-all lint-headers lint-licenses lint-licenses-all lint-mod lint-shell lint-yaml \
fix-go fix-go-all fix-mod \
test-unit test-unit-race test-unit-bench
13 changes: 7 additions & 6 deletions mod/tigron/expect/comparators.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ import (
// All can be used as a parameter for expected.Output to group a set of comparators.
func All(comparators ...test.Comparator) test.Comparator {
//nolint:thelper
return func(stdout string, info string, t *testing.T) {
return func(stdout, info string, t *testing.T) {
t.Helper()

for _, comparator := range comparators {
Expand All @@ -43,17 +43,18 @@ func All(comparators ...test.Comparator) test.Comparator {
// is found contained in the output.
func Contains(compare string) test.Comparator {
//nolint:thelper
return func(stdout string, info string, t *testing.T) {
return func(stdout, info string, t *testing.T) {
t.Helper()
assert.Check(t, strings.Contains(stdout, compare),
fmt.Sprintf("Output does not contain: %q", compare)+info)
}
}

// DoesNotContain is to be used for expected.Output to ensure a comparison string is NOT found in the output.
// DoesNotContain is to be used for expected.Output to ensure a comparison string is NOT found in
// the output.
func DoesNotContain(compare string) test.Comparator {
//nolint:thelper
return func(stdout string, info string, t *testing.T) {
return func(stdout, info string, t *testing.T) {
t.Helper()
assert.Check(t, !strings.Contains(stdout, compare),
fmt.Sprintf("Output does contain: %q", compare)+info)
Expand All @@ -63,7 +64,7 @@ func DoesNotContain(compare string) test.Comparator {
// Equals is to be used for expected.Output to ensure it is exactly the output.
func Equals(compare string) test.Comparator {
//nolint:thelper
return func(stdout string, info string, t *testing.T) {
return func(stdout, info string, t *testing.T) {
t.Helper()
assert.Equal(t, compare, stdout, info)
}
Expand All @@ -73,7 +74,7 @@ func Equals(compare string) test.Comparator {
// Provisional - expected use, but have not seen it so far.
func Match(reg *regexp.Regexp) test.Comparator {
//nolint:thelper
return func(stdout string, info string, t *testing.T) {
return func(stdout, info string, t *testing.T) {
t.Helper()
assert.Check(t, reg.MatchString(stdout), "Output does not match: "+reg.String(), info)
}
Expand Down
11 changes: 8 additions & 3 deletions mod/tigron/expect/consts.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,13 @@
package expect

const (
ExitCodeSuccess = 0
// ExitCodeSuccess will ensure that the command effectively ran returned with exit code zero.
ExitCodeSuccess = 0
// ExitCodeGenericFail will verify that the command ran and exited with a non-zero error code.
// This does NOT include timeouts, cancellation, or signals.
ExitCodeGenericFail = -1
ExitCodeNoCheck = -2
ExitCodeTimeout = -3
// ExitCodeNoCheck does not enforce any check at all on the function.
ExitCodeNoCheck = -2
// ExitCodeTimeout verifies that the command was cancelled on timeout.
ExitCodeTimeout = -3
)
19 changes: 19 additions & 0 deletions mod/tigron/expect/doc.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
/*
Copyright The containerd Authors.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

// Package expect provides a set of simple concrete test.Comparator implementations to use by tests
// on stdout, along with exit code expectations.
package expect
30 changes: 0 additions & 30 deletions mod/tigron/hack/make-lint-licenses.sh

This file was deleted.

19 changes: 19 additions & 0 deletions mod/tigron/require/doc.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
/*
Copyright The containerd Authors.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

// Package require provides a set of concrete test.Requirements to express the need for a specific
// architecture, OS, or binary, along with Not() and All() which allow Requirements composition.
package require
Loading