Skip to content

only-new-issues: true option seems broken since v6.3.0 and v6.3.1 releases #1161

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

Closed
3 tasks done
qdm12 opened this issue Feb 10, 2025 · 10 comments · Fixed by #1162
Closed
3 tasks done

only-new-issues: true option seems broken since v6.3.0 and v6.3.1 releases #1161

qdm12 opened this issue Feb 10, 2025 · 10 comments · Fixed by #1162
Labels
bug Something isn't working

Comments

@qdm12
Copy link

qdm12 commented Feb 10, 2025

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.
  • Yes, I've included all information below (version, config, etc).

Description of the problem

We use the action pinned as @v6 and with the option only-new-issues: true (full configuration)
Since 3 days ago (2025-02-07) it seems the option only-new-issues: true has no effect only on pushes, given files not changed in the commit pushed (squashed and merged) are being linted.

This can be seen in our commits CI reports of 2025-02-07 in our default branch: commit history

In particular, the last commit only modifies/adds the following files:

  • core/rawdb/database.go
  • core/rawdb/database.libevm.go
  • core/rawdb/database.libevm_test.go

But the action lints for example file libevm/pseudo/rlp_test.go (logs line) which is NOT part of that commit.

And the commit before that one works as expected, however.

This might correlate with the v6.3.0 release.

Version of golangci-lint

v1.60.3

Version of the GitHub Action

v6

Workflow file

name: golangci-lint

on:
  push:
    branches: [main, "release/**"]
  pull_request:
    branches: [main, "release/**"]
  workflow_dispatch:

permissions:
  # Required: allow read access to the content for analysis.
  contents: read
  # Optional: allow read access to pull request. Use with `only-new-issues` option.
  pull-requests: read

concurrency:
  group: ${{ github.workflow }}-${{ github.ref }}
  cancel-in-progress: ${{ github.ref != 'refs/heads/main' }}

jobs:
  golangci:
    name: lint
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v4
      - uses: actions/setup-go@v5
        with:
          go-version-file: "go.mod"
      - name: goheader
        # The goheader linter is only enabled in the CI so that it runs only on modified or new files
        # (see only-new-issues: true). It is disabled in .golangci.yml because
        # golangci-lint running locally is not aware of new/modified files compared to the base
        # commit of a pull request, and we want to avoid reporting invalid goheader errors.
        uses: golangci/golangci-lint-action@v6
        with:
          version: v1.60
          only-new-issues: true
          args: --enable-only goheader
      - name: golangci-lint
        uses: golangci/golangci-lint-action@v6
        with:
          version: v1.60

Golangci-lint configuration

https://github.com/ava-labs/libevm/blob/main/.github/workflows/golangci-lint.yml
https://github.com/ava-labs/libevm/blob/main/.libevm-header

# This file configures github.com/golangci/golangci-lint.

run:
  timeout: 20m
  tests: true

linters:
  enable:
    # Every available linter at the time of writing was considered (quickly) and
    # inclusion was liberal. Linters are good at detecting code smells, but if
    # we find that a particular one causes too many false positives then we can
    # configure it better or, as a last resort, remove it.
    - containedctx
    - errcheck
    - forcetypeassert
    - gci
    - gocheckcompilerdirectives
    - gofmt
    # goheader is disabled but it is enabled in the CI with a flag.
    # Please see .github/workflows/golangci-lint.yml which explains why.
    # - goheader
    - goimports
    - gomodguard
    - gosec
    - govet
    - ineffassign
    # TODO(arr4n): investigate ireturn
    - misspell
    - nakedret
    - nestif
    - nilerr
    - nolintlint
    - reassign
    - revive
    - sloglint
    - staticcheck
    - tagliatelle
    - testableexamples
    - testifylint
    - thelper
    - tparallel
    - unconvert
    - usestdlibvars
    - unused
    - whitespace

linters-settings:
  gci:
    custom-order: true
    sections:
      - standard
      - default
      - localmodule
      # The rest of these break developer expections, in increasing order of
      # divergence, so are at the end to increase the chance of being seen.
      - alias
      - dot
      - blank
  goheader:
    template-path: .libevm-header

  gomodguard:
    blocked:
      modules:
        - github.com/ethereum/go-ethereum:
            reason: "Original, forked repo"
        - github.com/ava-labs/avalanchego:
            reason: "Avoid dependency loop"
        - github.com/ava-labs/coreth:
            reason: "Avoid dependency loop"
        - github.com/ava-labs/subnet-evm:
            reason: "Avoid dependency loop"
  revive:
    rules:
      - name: unused-parameter
        # Method parameters may be required by interfaces and forcing them to be
        # named _ is of questionable benefit.
        disabled: true

issues:
  exclude-dirs-use-default: false
  exclude-rules:
    - path-except: libevm
      linters:
        # If any issue is flagged in a non-libevm file, add the linter here
        # because the problem isn't under our control.
        - containedctx
        - forcetypeassert
        - errcheck
        - gci
        - gofmt
        - goheader
        - goimports
        - gosec
        - gosimple
        - govet
        - nakedret
        - nestif
        - nilerr
        - nolintlint
        - revive
        - staticcheck
        - tagliatelle
        - testableexamples
        - testifylint
        - thelper
        - tparallel
        - typecheck
        - usestdlibvars
        - varnamelen
        - wastedassign
        - whitespace
  include:
    # Many of the default exclusions are because, verbatim "Annoying issue",
    # which defeats the point of a linter.
    - EXC0002
    - EXC0004
    - EXC0005
    - EXC0006
    - EXC0007
    - EXC0008
    - EXC0009
    - EXC0010
    - EXC0011
    - EXC0012
    - EXC0013
    - EXC0014
    - EXC0015

Go version

1.20

Code example or link to a public repository

https://github.com/ava-labs/libevm/

qdm12 referenced this issue in ava-labs/libevm Feb 10, 2025
## Why this should be merged

Allows for `ava-labs/coreth` equivalent modifications of
`rawdb.InspectDatabase()` through external logic injection.

## How this works

Variadic options to:

1. Record a database statistic;
2. Mark a database statistic as metadata;
3. Filter statistics for printing.

## How this was tested

Testable example acting as a unit test.

---------

Signed-off-by: Arran Schlosberg <[email protected]>
Co-authored-by: Quentin McGaw <[email protected]>
@ldez
Copy link
Member

ldez commented Feb 10, 2025

Hello,

No changes have been made to this option since v6.2.0.

The changes are about documentation, comments, and renaming.

v6.3.0...v6.3.1
v6.2.0...v6.3.0

@qdm12
Copy link
Author

qdm12 commented Feb 10, 2025

There are quite a few dependabot pull requests, could it be related?

We did not change anything in our CI on last Friday where this problem started happening, and the CI will just trigger on unmodified file paths on pushes.
This is most likely an issue with this action, given it handles the only-new-issues option. Maybe try to reproduce it on your side just to be sure? Otherwise, would you have any hint on why that would happen? Thank you!

@ldez
Copy link
Member

ldez commented Feb 10, 2025

🤔

https://github.com/ava-labs/libevm/actions/runs/13203150868/job/36859809588

only new issues on push: /tmp/tmp-1894-YVmNa7McCv3u/push.patch
Running [/home/runner/golangci-lint-1.60.3-linux-amd64/golangci-lint run --new-from-patch=/tmp/tmp-1894-YVmNa7McCv3u/push.patch --new=false --new-from-rev= --enable-only goheader] in [/home/runner/work/libevm/libevm] ...

https://github.com/ava-labs/libevm/actions/runs/13240000666/job/36958888508

only new issues on push:
Running [/home/runner/golangci-lint-1.60.3-linux-amd64/golangci-lint run --enable-only goheader] in [/home/runner/work/libevm/libevm] ...

@ldez
Copy link
Member

ldez commented Feb 10, 2025

Ok I think I found the problem.

@ldez ldez mentioned this issue Feb 10, 2025
@ldez ldez added the bug Something isn't working label Feb 10, 2025
@ldez
Copy link
Member

ldez commented Feb 10, 2025

@qdm12 I created a new tag, you can update your CI if you want.

@a-hilaly
Copy link

@ldez we're still running into the same issue (for PRs created today) - is there anything we're doing wrong in https://github.com/kro-run/kro/blob/main/.github/workflows/golangci-lint.yaml#L1-L28 ?

@ldez
Copy link
Member

ldez commented Feb 13, 2025

Today 🤔 in which timezone? (for me it's 4 am) Can you add a reference to the job?

There is nothing wrong with the configuration of the GitHub Action.

You are using an old version of golangci-lint (v1.60), the current is v1.64, but it's not a major problem.

@ldez
Copy link
Member

ldez commented Feb 13, 2025

I checked the logs of some jobs: your problem is not related to this issue.

You only have typecheck errors, so your problem is inside your code.

Maybe a problem with your go.mod.

typecheck errors are not linting reports but "compilation" errors.

@ldez
Copy link
Member

ldez commented Feb 13, 2025

I think the change that happened "today" is the update of Go version (go1.23 -> go1.24) inside your CI.

So you must update the golangci-lint version to v1.64

kro-run/kro#302

@a-hilaly
Copy link

Thank you very much for debugging this @ldez ! the golangci-lint version definitely needed an update. Really appreciate the PR to bump it to v1.64!

Will take a look at our Go version pinning too - bit weird, AFAIK we're on 1.23/1.22 there. We'll sort that out separately.

Thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants