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

more flexible loading of custom plugins #3826

Open
pohly opened this issue May 8, 2023 · 4 comments
Open

more flexible loading of custom plugins #3826

pohly opened this issue May 8, 2023 · 4 comments
Labels
enhancement New feature or improvement linter: custom About custom/private linters no decision No decision to fix or not

Comments

@pohly
Copy link
Contributor

pohly commented May 8, 2023

Your feature request related to a problem? Please describe.

In Kubernetes, we have a hack/golangci-lint.yaml with a path for a logcheck.so which is relative to that config:
https://github.com/kubernetes/kubernetes/blob/b09b7e5bdb1c763c6d7e47e2c792d54f37f7d9a2/hack/golangci.yaml#L42

That works because normally, _output contains build artifacts. It fails when using containerized builds where build artifacts are located elsewhere:
kubernetes/kubernetes#117831

Describe the solution you'd like.

When loading a plugin, golangci-lint could first try the absolute or relative path and if that file doesn't exist, fall back to searching for the plugin in directories specified via env variables. This could be PATH (kind of convenient because the plugin gets compiled together with the golangci-lint binary to ensure that both use the same Go and compile options and already gets placed in the same directory) or a custom variable like GOLANGCI_LINT_CUSTOM_PLUGIN_PATH.

Describe alternatives you've considered.

  • An absolute path doesn't work, developers check out there source in different locations.
  • Patching the config file is ugly (dirty repository during the build).
  • Copying the config file elsewhere and modifying it there messes with other relative paths.

Additional context.

No response

@pohly pohly added the enhancement New feature or improvement label May 8, 2023
@boring-cyborg
Copy link

boring-cyborg bot commented May 8, 2023

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

@ldez ldez added the linter: custom About custom/private linters label May 8, 2023
@ldez
Copy link
Member

ldez commented May 8, 2023

Hello,

When loading a plugin, golangci-lint could first try the absolute or relative path and if that file doesn't exist, fall back to searching for the plugin in directories specified via env variables.

Do you mean: if the path is a relative path then use the env var as a root?
ex: ./foo/foo.so => filepath.Join("GOLANGCI_LINT_CUSTOM_PLUGIN_PATH", "./foo/foo.so")

Side note: maybe logcheck can be "merged" with loggercheck

@ldez ldez added the feedback required Requires additional feedback label May 8, 2023
@pohly
Copy link
Contributor Author

pohly commented May 8, 2023

I would:

  • Completely strip the file path: ./foo/foo.so -> foo.so. Otherwise re-creating the existing path elsewhere becomes awkward.
  • Treat GOLANGCI_LINT_CUSTOM_PLUGIN_PATH like PATH in a shell and support multiple directories. That way, multiple different custom plugins could get built in different directories. Might be overkill, though.

logcheck is heavily biased towards Kubernetes logging conventions. I don't think it can or should get merged with other linters, even if some aspects of it are similar.

@ldez ldez added no decision No decision to fix or not and removed feedback required Requires additional feedback labels May 31, 2023
@pohly
Copy link
Contributor Author

pohly commented Oct 24, 2023

I was pinged again about this because it's still unsolved for containerized Kubernetes builds.

Any preference on how to solve this? I'm leaning towards falling back to PATH when some relative path doesn't resolve to an actual file and could prepare a PR with that.

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 no decision No decision to fix or not
Projects
None yet
Development

No branches or pull requests

2 participants