Skip to content
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

When a patch cannot be downloaded and only-new-issues is set, error instead of running #996

Open
3 tasks done
connorszczepaniak-wk opened this issue Mar 13, 2024 · 9 comments
Labels
enhancement New feature or request

Comments

@connorszczepaniak-wk
Copy link

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

I recently had a run of golangci-lint where the GitHub API responded with 422 downloading the patch for the PR. The PR was very small (6 lines, 1 file changed), but the API responded with the following:

failed to fetch pull request patch: RequestError [HttpError]: The diff could not be processed because too many files changed

This lint run has only-new-issues set, but without the patch it flagged a large number of issues in the entire repo because it ran without the patch.

It would be nice to have an option to error sooner and more obviously if the patch couldn't be downloaded instead of running the linter on the whole codebase. I understand that it may not be desirable for all use cases, so I propose making this a configuration option.

Version of golangci-lint

1.56.2

Version of the GitHub Action

4.0.0

Workflow file

     - name: Lint Main
        uses: golangci/golangci-lint-action@3cfe3a4abbb849e10058ce4af15d205b6da42804 # v4.0.0
        with:
          version: v1.56.2
          only-new-issues: true
          args: -v --timeout=10m

Go version

1.22.0

Code example or link to a public repository

N/A

@ldez ldez added the enhancement New feature or request label Mar 22, 2024
@nielskrijger
Copy link

Same issue; Go 1.21, golangci-lint 1.55.1, Github action v4.

Was this introduced by V4?

At the moment this will happen for any PR with --only-new-issues that changes >300 files.

@connorszczepaniak-wk
Copy link
Author

I just hit this even more explicitly:
failed to fetch pull request patch: RequestError [HttpError]: Sorry, the diff exceeded the maximum number of lines (20000): {"resource":"PullRequest","field":"diff","code":"too_large"}

I'm not sure what I'd want in this case. Failing would be bad, because then there'd be no way around it -- but I also don't want to run my linter on all code, because we have existing lints in our codebase that we haven't fixed yet (so it fails anyway).

@ldez
Copy link
Member

ldez commented Apr 29, 2024

Hello,

I'm not sure how to fix that 🤔

Solutions:

  1. reports an error
  2. creates a fake diff using the API to get files (with pagination)
  3. uses git to create the diff
  4. ?

Solution 1 is a problem because you will have an error but you will never be able to "fix" this error, so your PR will be blocked forever.

Solution 2 is very complex: manually creating a diff is not trivial and it's brittle.

Solution 3 can work, but by default the action actions/checkout doesn't check out the full git tree (it only checks out 1 commit), so it will require adding fetch-depth: 0 to check out the full tree to be able to use git to create the diff. And, this is not an option that the golangci-lint action can control because it's an option from another action.

For now, I see no other solutions inside the golangci-lint action.

@lcarva

This comment was marked as off-topic.

@ldez

This comment was marked as off-topic.

@lcarva

This comment was marked as off-topic.

@ldez

This comment was marked as off-topic.

@lcarva

This comment was marked as off-topic.

lcarva added a commit to lcarva/chains that referenced this issue Sep 17, 2024
Because this repository uses vendored dependencies, and grouping for
dependabot updates, the PRs created by dependabot are huge. This causes
issues for the linter:
    golangci/golangci-lint-action#996
The chances of dependabot creating new linting issues are minimal.

Signed-off-by: Luiz Carvalho <[email protected]>
tekton-robot pushed a commit to tektoncd/chains that referenced this issue Sep 17, 2024
Because this repository uses vendored dependencies, and grouping for
dependabot updates, the PRs created by dependabot are huge. This causes
issues for the linter:
    golangci/golangci-lint-action#996
The chances of dependabot creating new linting issues are minimal.

Signed-off-by: Luiz Carvalho <[email protected]>
holyspectral added a commit to holyspectral/neuvector that referenced this issue Oct 29, 2024
When there are more than 20,000 lines in GitHub diff, GitHub will return
406 error and prevent linter from running correctly.

This commit uses new-from-rev, so it can generate diff from git repo
instead of GitHub API.

Related issue:

golangci/golangci-lint-action#996
@paul-freeman
Copy link

I'd like to suggest a Solution 4.

If the diff fails, you can continue doing the full diff lint, like it currently does. But if that lint run fails, you could return an error saying that the diff failed instead of showing all the linter errors from the full fallback scan.

But, at the end of the day, I think the only issue here is that the diff failure isn't obvious to the user when the fallback linter prints a lot of unrelated linter errors. So finding a way to surface what the real error is would be helpful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants