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

Debug why a private linter was not added to the list #1276

Closed
alexisvisco opened this issue Jul 28, 2020 · 23 comments · Fixed by #4437
Closed

Debug why a private linter was not added to the list #1276

alexisvisco opened this issue Jul 28, 2020 · 23 comments · Fixed by #4437
Labels
enhancement New feature or improvement linter: custom About custom/private linters

Comments

@alexisvisco
Copy link

alexisvisco commented Jul 28, 2020

Hi, I am using a private linter but it doesn't appear in the list of linters.

linters:
 enable:
   - unparam
   - misspell
   - godox
   - gocritic
   - scopelint
   - golint
   - unconvert
   - gofmt
   - exportloopref
   - govet
   - bodyclose
   - gosimple
   - goconst
   - gosec
   - logwercase

linters-settings:
  custom:
    logwercase:
      path: /home/aviscogl/go/bin/logwercase.so
      description: Analyze case of log message and WithField keys
      original-url: github.com/alexisvisco/logwercase

There is an option to known why a private linter is not added? https://golangci-lint.run/contributing/debug/ Does not help me ...

@boring-cyborg
Copy link

boring-cyborg bot commented Jul 28, 2020

Hey, thank you for opening your first Issue ! 🙂 If you would like to contribute we have a guide for contributors.

@sayboras
Copy link
Member

@alexisvisco thanks for your issue. I just quickly tried, seems like it's showing for me. Can you share your steps ?

$ ./golangci-lint linters
Enabled by your configuration linters:
...
gomnd: An analyzer to detect magic numbers. [fast: true, auto-fix: false]
goprintffuncname: Checks that printf-like functions are named with `f` at the end [fast: true, auto-fix: false]
gosec (gas): Inspects source code for security problems [fast: true, auto-fix: false]
gosimple (megacheck): Linter for Go source code that specializes in simplifying a code [fast: true, auto-fix: false]
govet (vet, vetshadow): Vet examines Go source code and reports suspicious constructs, such as Printf calls whose arguments do not align with the format string [fast: true, auto-fix: false]
ineffassign: Detects when assignments to existing variables are not used [fast: true, auto-fix: false]
interfacer: Linter that suggests narrower interface types [fast: true, auto-fix: false]
lll: Reports long lines [fast: true, auto-fix: false]
logwercase: Analyze case of log message and WithField keys [fast: true, auto-fix: false]
misspell: Finds commonly misspelled English words in comments [fast: true, auto-fix: true]
nakedret: Finds naked returns in functions greater than a specified function length [fast: true, auto-fix: false]

$ ./golangci-lint linters | grep -is logwercase
logwercase: Analyze case of log message and WithField keys [fast: true, auto-fix: false]

@sayboras sayboras added the question Further information is requested label Jul 28, 2020
@sayboras sayboras self-assigned this Jul 28, 2020
@alexisvisco
Copy link
Author

Strange, I was not using the right config, unfortunately, it does not work as you because I get an output from golangci:
ERRO Unable to load custom analyzer logwercase:/home/aviscogl/go/bin/logwercase.so, plugin: not implemented

I don't know why it's outputting that because you were able to add it 🤔

@sayboras
Copy link
Member

Might be related to version. I used master branches with one small changes to make sure same x/tools version in your tool and golangci-lint. With your above config, I just ran the check.

Can you share version details as well?

@alexisvisco
Copy link
Author

I was pretty sure the exact same versions are present...

golangci-lint has version 1.29.0 built from 6a68907 on 2020-07-20T14:54:31Z

@sayboras
Copy link
Member

You are right, seems like there is some issue with v1.29.0. I didn't see any related commits, which could affect this one.

Can you try with master branches (in golangci-lint and in your logwercase) as well ?

@mattysweeps
Copy link

mattysweeps commented Jul 29, 2020

I am experiencing the same behavior.
When I build golangci-lint from the master branch (60613dc) my custom plugin works just fine.
When I use the docker images for versions 1.27, 1.28, and 1.29, I get the plugin: not implemented error.

Update:
I just built from the v1.29.0 tag and everything worked fine. I wonder if the problem might be in the way the release binary is being created? Something that disables plugin use?

@mattysweeps
Copy link

This might be related: golang/go#19569

@mattysweeps
Copy link

Looks like CGO_ENABLED is the culprit.

golangci-lint my plugin result
CGO_ENABLED=1 CGO_ENABLED=1 Success
CGO_ENABLED=0 CGO_ENABLED=1 Failure plugin: not implemented
CGO_ENABLED=1 CGO_ENABLED=0 Cannot build plugin loadinternal: cannot find runtime/cgo
CGO_ENABLED=0 CGO_ENABLED=0 Cannot build plugin loadinternal: cannot find runtime/cgo

So custom plugins will never work with release versions because the release binary is compiled with CGO_ENABLED=0
Same goes for the docker images, since they also use a CGO_ENABLED=0 binary.

However, the docker images could work if the golangci-lint binary in them was compiled with CGO_ENABLED=1
Then users could create their own docker images which extend the golang-ci image and add the custom plugins as well.
Note that the custom plugins should probably be compiled as part of the build process for the customized docker image, to avoid using plugins which were created for different architectures (darwin)

@sayboras
Copy link
Member

@mattysweeps thanks for your detailed explaination. You are right, the release binaries are built with CGO_ENABLED=0 only. This might be something we can improve.

@sayboras sayboras added the enhancement New feature or improvement label Jul 30, 2020
@alexisvisco
Copy link
Author

Should the binary produced during the installation of golanci-lint not by default support plugins since it is a feature of the linter?

@sayboras
Copy link
Member

Should the binary produced during the installation of golanci-lint not by default support plugins since it is a feature of the linter?

Yeah, I am completely agreed with you. Just found related issue here #1182 though.

@sayboras
Copy link
Member

sayboras commented Aug 8, 2020

@alexisvisco seems like there is nothing we can do here, except update docs :). Please let me know if it's fine.

@draveness
Copy link

@alexisvisco seems like there is nothing we can do here, except update docs :). Please let me know if it's fine.

I encountered the same problem when dealing with golangci-lint custom plugin feature.

so for private linter, we have to build the golangci-lint with CGO_ENABLED=1 flag from source code to leverage this feature. But I think the extra step breaks the feature since the purpose of go plugin is to load packages without rebuilding the main binary. If we have to rebuild it, why not create a intree plugin instead of the custom one. Please correct me if there's something wrong. :)

@ldez ldez added linter: custom About custom/private linters and removed question Further information is requested labels Feb 13, 2021
@AlwxSin
Copy link

AlwxSin commented Mar 24, 2021

@sayboras correct me, if I wrong, please. If I want to use a private plugin I need to compile golangci with CGO_ENABLED=1 by myself? And I need to compile it on the docker image I use in my CI and for all platforms, my team uses for development?

@stale
Copy link

stale bot commented Mar 30, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale No recent correspondence or work activity label Mar 30, 2022
@ldez ldez removed the stale No recent correspondence or work activity label Mar 30, 2022
mwieser added a commit to allaboutapps/go-starter that referenced this issue May 31, 2022
current solution requires golangci-lint install per go get because cgo needs to be enabled for plugings
golangci/golangci-lint#1276
@ian-h-chamberlain

This comment was marked as off-topic.

@ldez

This comment was marked as off-topic.

@ian-h-chamberlain

This comment was marked as off-topic.

@stevenh
Copy link

stevenh commented Nov 6, 2022

This just caught me out after following guide for new-linters which doesn't mention that the release binaries are incompatible with this, making support for plugins a little mute as consumers will always need to create a custom build of golangci-lint.

Has there been any thoughts on creating release builds with CGO_ENABLED=1?

@mibo-fdc

This comment was marked as off-topic.

@ian-h-chamberlain
Copy link

@stevenh I've opened #3361 to discuss this since it seems like this issue is not the recommended place to discuss it. Posting this comment here just to let people know that it exists and direct them there.

@sayboras
Copy link
Member

Unassigned myself due to lack of cycles.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or improvement linter: custom About custom/private linters
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants