[Bug]: Linter in CI is uncached

[ValarDragon]

Summary of Bug

CI linter runs are taking ~18 mins right now. This appears to be due to re-linting the entire repo, and not utilizing caches. https://github.com/cosmos/cosmos-sdk/actions/runs/5030991342/jobs/9023780698?pr=16227

Would love if we could get some investigation into whats the root cause and if there is a sensible way to employ caches here. (E.g. could we just read from a lint cache from main, but not write to it during PR's?)

[julienrbrt]

If saving and reusing caches does not work, we could as well use the diff of the PR and run linting only on the touched packages. This may as well be simpler to achieve.

[ValarDragon]

That seems like a better idea regardless tbh

[faddat]

@ValarDragon --

I disagree on running the linter on only touched files. I'll try and see if there are other ways to speed it up, because in the past the technique you've described has led to "grandfathering" in linter issues.

name: Lint
on:
  push:
    branches:
      - main
      - release/**
  pull_request:
  merge_group:
permissions:
  contents: read
jobs:
  golangci:
    name: golangci-lint
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v3
      - uses: actions/setup-go@v4
        with:
          go-version: "1.20"
          check-latest: true
      - uses: technote-space/get-diff-action@v6.1.2
        id: git_diff
        with:
          PATTERNS: |
            **/*.go
            go.mod
            go.sum
            **/go.mod
            **/go.sum
      - name: run linting
        if: env.GIT_DIFF
        run: |
          make lint

So, I think that this issue with the linter is more that we aren't using the official golangci-lint github runner, which has pretty extreme caching, because it seems like we are already filtering for only what has changed.

I think we aren't using it because of the multi-module situation, I'll try and see if we can get it to handle multiple paths before we try and migrate to using go workspaces.

Check out the performance section of the github action's docs:

• https://github.com/golangci/golangci-lint-action#performance

[julienrbrt]

because in the past the technique you've described has led to "grandfathering" in linter issues

The thing is, unless you bump golangci-lint, running on the touched files would work.
We just need a condition that checks the whole repo when the Makefile (where the golangci-lint version is) is edited and otherwise just the diff.

I'll try and see if we can get it to handle multiple paths before we try and migrate to using go workspaces.

Please do not revert using the golangci-lint action, and please do not try to integrate go workspaces again, those are unrelated.

[faddat]

So, per your request I won't make those changes, but I do think they are related. Golangci-lint will pick up everything in a workspace.

[julienrbrt]

So, per your request I won't make those changes, but I do think they are related. Golangci-lint will pick up everything in a workspace.

The issue is about not running golangci on the whole codebase when not necessary, so it isn't related. Thank you for picking this up! Happy to review.

[julienrbrt]

Looks like IBC does it like this: https://github.com/cosmos/ibc-go/blob/main/scripts/linting/lint-changed-go-files.sh. It would fit our use case as well, with the additional condition mentioned above.