Skip to content

Comments

ci: don't install private golangci-lint; bump to v2.4#195

Merged
thaJeztah merged 1 commit intomoby:mainfrom
kolyshkin:golangci
Aug 14, 2025
Merged

ci: don't install private golangci-lint; bump to v2.4#195
thaJeztah merged 1 commit intomoby:mainfrom
kolyshkin:golangci

Conversation

@kolyshkin
Copy link
Collaborator

@kolyshkin kolyshkin commented Aug 13, 2025

First, let's not install golangci-lint from Makefile, instead relying on whatever version a developer already has installed and available.

Second, we still need golangci-lint in GHA CI. For that, let's use golangci-lint-action. Unfortunately, there's no way to NOT run it from the action, or run it on multiple directories (see 1), so run it on a single module.

This is better for CI because we use all the good bits from the golangci-lint-action (caching and annotations).

While at it, bump golangci-lint from v2.0.3 to v2.4.x.

@kolyshkin kolyshkin force-pushed the golangci branch 2 times, most recently from f042b6f to 5d58379 Compare August 13, 2025 21:02
@kolyshkin kolyshkin marked this pull request as ready for review August 13, 2025 21:04
@kolyshkin
Copy link
Collaborator Author

OK, it works like expected:

  • cache is saved
  • annotations are shown (albeit there are multiple ones since our matrix is big)

@kolyshkin kolyshkin changed the title ci: don't install private golangci-lint ci: don't install private golangci-lint; bump to v2.4 Aug 14, 2025
@kolyshkin kolyshkin requested review from Copilot and thaJeztah and removed request for thaJeztah August 14, 2025 05:57
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR modernizes the golangci-lint setup by removing local installation from the Makefile and updating the CI configuration to use the official golangci-lint GitHub Action. The changes also bump the golangci-lint version from v2.0.2 to v2.4.

  • Removes private golangci-lint installation logic from Makefile, expecting developers to have it pre-installed
  • Adds golangci-lint-action to GitHub Actions workflow for better CI integration with caching and annotations
  • Updates golangci-lint version from v2.0.2 to v2.4

Reviewed Changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated no comments.

File Description
Makefile Removes BINDIR variable and golangci-lint installation targets, simplifies lint command to use system-installed golangci-lint
.github/workflows/test.yml Adds golangci-lint-action step with v2.4 version targeting reexec module

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines 49 to 55
.PHONY: lint
lint: $(BINDIR)/golangci-lint
lint: CMD=go mod download; ../$(BINDIR)/golangci-lint run
lint: CMD=go mod download; golangci-lint run
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps would be nice to have an early fail; currently this shows;

docker run -it --rm -v ./:/sys -w /sys golang:1.24 make lint
set -eu; \
for p in atomicwriter capability mountinfo mount reexec sequential signal symlink user userns ; do \
	(cd $p; go mod download; golangci-lint run;) \
done
/bin/sh: 3: golangci-lint: not found
make: *** [Makefile:18: foreach] Error 127

If we add a step before foreach, we can fail before that output;

.PHONY: check-golangci
check-golangci:
	@command -v golangci-lint >/dev/null 2>&1 || { echo "golangci-lint is not installed"; exit 1; }
	@golangci-lint version

.PHONY: lint
lint: check-golangci
lint: CMD=go mod download; golangci-lint run
lint: foreach
docker run -it --rm -v ./:/sys -w /sys golang:1.24 make lint
golangci-lint is not installed
make: *** [Makefile:51: check-golangci] Error 1

And after installing;

docker run -it --rm -v ./:/sys -w /sys golang:1.24
curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/HEAD/install.sh | sh -s -- -b $(go env GOPATH)/bin v2.4.0
make lint
golangci-lint has version 2.4.0 built with go1.25.0 from 43d03392 on 2025-08-13T23:36:29Z
set -eu; \
for p in atomicwriter capability mountinfo mount reexec sequential signal symlink user userns ; do \
	(cd $p; go mod download; golangci-lint run;) \
done

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Perhaps would be nice to have an early fail

Good idea; I've simplified this to:

--- a/Makefile
+++ b/Makefile
@@ -46,11 +46,14 @@ test-local:
        cd atomicwriter && go mod tidy $(MOD) && go test $(MOD) $(RUN_VIA_SUDO) -v .
        $(RM) atomicwriter/go-local.*
 
+.PHONY: golangci-lint-version
+golangci-lint-version:
+       golangci-lint version
+
 .PHONY: lint
+lint: golangci-lint-version
 lint: CMD=go mod download; golangci-lint run
 lint: foreach
-lint:
-       golangci-lint version
 
 .PHONY: cross
 cross:

which results in a similar effect (slightly different but clear enough error message):

[kir@kir-tp1 sys]$ podman run -it --rm -v ./:/sys -w /sys golang:1.24 make lint
golangci-lint version
make: golangci-lint: No such file or directory
make: *** [Makefile:51: golangci-lint-version] Error 127

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

PR updated; PTAL @thaJeztah

Copy link
Member

Choose a reason for hiding this comment

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

which results in a similar effect (slightly different but clear enough error message):

Works for me; good enough!

Copy link
Member

Choose a reason for hiding this comment

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

LOL, I initially had that, then went looking to make it nicer, but agreed; let's not complicate things. What works, works (and most users of this repo should probably be able to deduct what's happening!)

First, let's not install golangci-lint from Makefile, instead relying on
whatever version a developer already has installed and available.

Second, we still need golangci-lint in GHA CI. For that, let's use
golangci-lint-action. Unfortunately, there's no way to NOT run it
from the action, or run it on multiple directories (see [1]), so
run it on a single module.

This is better for CI because we use all the good bits from the
golangci-lint-action (caching and annotations).

While at it, bump golangci-lint from v2.0.3 to v2.4.x.

[1]: golangci/golangci-lint-action#1226
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM

@thaJeztah thaJeztah merged commit 37e0dac into moby:main Aug 14, 2025
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants