Merge pull request #4041 from apostasie/ci-tigron-break-1

[CI] Tigron breakout: golangci v2 and cleanups

Author: AkihiroSuda

mod/tigron/.golangci.yml

@@ -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

mod/tigron/Makefile

@@ -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
+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))
 	@cd $(MAKEFILE_DIR) \
 		&& golangci-lint run $(VERBOSE_FLAG_LONG) ./...
 	$(call footer, $@)
@@ -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) \
@@ -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
+		&& 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:
@@ -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, $@)
@@ -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) \
@@ -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, $@)
 
 ##########################
@@ -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: \
@@ -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

mod/tigron/expect/comparators.go

@@ -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 {
@@ -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)
@@ -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)
 	}
@@ -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)
 	}

mod/tigron/expect/consts.go

@@ -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
 )

mod/tigron/expect/doc.go

@@ -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

mod/tigron/hack/make-lint-licenses.sh

@@ -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