Support for module plugin system via .custom-gcl.yml

[Thiht]

Welcome

• Yes, I understand that the GitHub action repository is not the repository of golangci-lint itself.
• Yes, I've searched similar issues on GitHub and didn't find any.

Your feature request related to a problem? Please describe.

golangci-lint features a way to support plugins automatically with a configuration file.

Basically it boils down to:

• having a .custom-gcl.yml config file
• building a custom binary using golangci-lint custom
• declaring and using the custom plugin(s) in .golangci.yml
• and then using the custom binary ./custom-gcl instead of golangci-lint
• run custom-gcl instead of golangci-lint

Describe the solution you'd like.

I'd like for the GitHub action to:

• download and install golangci-lint as usual (no change to the normal flow)
• detect if a .custom-gcl.yml exists
• compute the hash of the file and check the cache
• if the cache is empty run golangci-lint custom (or use the cached version)
• cache the result binary custom-gcl using a hash sum of the .custom-gcl.yml

Describe alternatives you've considered.

I'm currently doing the above manually without using the GitHub action

Additional context.

I want to use nilaway as part of my CI, but it's only supported with golangci-lint via the plugin system. See: https://github.com/uber-go/nilaway?tab=readme-ov-file#golangci-lint--v1570

[ldez]

Hello,

We declined nilway because of its false positives, so I should suggest not using it inside your CI unless you want to fix non-real issues.

The suggestion of the nilway doc about using the hash of .custom-gcl.yml to skip the build is clearly a bad suggestion:

• if you are using latest as a nilway version, the hash will not be usable to trigger a rebuild when nilway is updated.
• if you update the version of golangci-lint of the GitHub Action or use latest as a version, the hash will not be usable to trigger a rebuild.

So this suggestion should not be followed if you don't understand the limitations of this appraoch.

[Thiht]

Hi! Thanks for the perspective.

We declined nilway because of its false positives, so I should suggest not using it inside your CI unless you want to fix non-real issues.

I completely understand why it was not accepted as a golangci-lint linter, but I'm actually ok with adding a few //nolints in my code. I'm more interested in true-positives than I'm annoyed with false-positives/negatives, for 1 specific codebase I'm working on.

Keeping nilaway apart, I'm thinking if the module plugin system exists in golangci-lint, it would be a good thing to integrate it with the GH action anyway. I'm currently looking in custom company linters so the plugin support would come handy (and I like the convenience of golangci-lint to run even custom linters)

if you are using latest as a nilway version, the hash will not be usable to trigger a rebuild when nilway is updated.

I'd say this can be fixed by encouraging people to use a tagged version in the documentation. Maybe a warning could be issued by the GH action if it detects a non-semantic versioning tag (or a non numeric tag) in the .custom-gcl.yml.

The custom-gcl cache could be made compatible with the existing cache-invalidation-interval and skip-cache too.

if you update the version of golangci-lint or use latest as a version, the hash will not be usable to trigger a rebuild.

I think adding the hash of the golangci-lint binary to the cache key would work here? And cache-invalidation-interval can do part of the lifting too

[ldez]

Most of the cost of a build can be skipped only with the Go cache (build and modules), so an extra cache (the binary) will not provide a major improvement.
Also, the cost (time) of the cache restoration can be important.

[ldez]

Another point, in this system there are 2 golangci-lint versions:

• the version defined by the GitHub Action
• the version defined inside the .custom-gcl.yml

This can (and will) create confusion.

[ldez]

Another element, the binary name is defined inside the .custom-gcl.yml, so it is required to know this name to call the binary.

This file can have several extensions: .yml, .yaml, .json.
Also, it requires parsing this file so to have a YAML parser.

[ldez]

To be clear, I understand the need, it's interesting but it's not as simple as you describe in your proposal.

[Thiht]

To be clear, I understand the need, it's interesting but it's not as simple as you describe in your proposal.

Don't worry, it's your job, I certainly don't want to rush a solution! These are all very valid points.

---

If we forget about the cache, do you think it would be acceptable to just:

• download golangci-lint as usual (no change at all to the current logic)
• if there'a .customgcl.yml file, run golangci-lint custom followed by ./custom-gcl, instead of golangci-lint run

This would result in relatively minor changes in the code of the action, and would "just work" for people using custom linters (I don't think anyone will randomly have a .custom-gcl.yml file in their repo)

[ldez]

This would result in relatively minor changes in the code of the action,

I think you didn't say that to minorize our work, but I recommend avoiding this type of estimation.

You are a dev, like me, and we know that a change is often related to other elements.

For me, your issue is related to #1049, this issue is about using a binary inside the PATH but this also requires thinking about local installation outside of the PATH (#631).

Otherwise, it requires to handle the working directory.

So even without caching the binary, the support of the custom build requires more important changes and more thinking than you can quickly evaluate.

My previous comments were a way to share my thoughts during my thinking about the topic.
These thoughts are not blocking, they just express my journey on the topic.

[Thiht]

This would result in relatively minor changes in the code of the action,

I think you didn't say that to minorize our work, but I recommend avoiding this type of estimation.

Apologies for the miscommunication, I meant "relatively" as in "relatively compared to doing with the cache" (since dealing with the cache comes with more complexity as you explained above) ; not an absolute estimation of how much work this is