depguard: fix GOROOT detection#3866
Conversation
Once v1.29 golangci/golangci-lint#3866 lands, we can revert this.
bombsimon
left a comment
There was a problem hiding this comment.
LGTM!
Would an alternative be to roll back depguard instead to not have to keep a fork around forever? It doesn't really matter having it around so I'll approve, just curious.
Latest version came with problems with depguard: OpenPeeDeeP/depguard#46 Current solution for now is pinning golangci-lint version to do not use latest deps. Fix is currently ongoing here: golangci/golangci-lint#3866 But anyway it's good idea to have a pinned version and do not consume from latest.
Latest version came with problems with depguard: OpenPeeDeeP/depguard#46 Current solution for now is pinning golangci-lint version to do not use latest deps. Fix is currently ongoing here: golangci/golangci-lint#3866 But anyway it's good idea to have a pinned version and do not consume from latest.
|
I didn't roll back because the depguard v2 configuration is breaking and I think that we don't have to wait for more to break this, and because this new version of depguard fixes an annoying old bug with concurrency. |
|
@ldez looks like you have to convince the CI that this is okay... https://github.com/golangci/golangci-lint/actions/runs/5153483603/jobs/9280765338 |
|
It's just because we use the previous version inside the CI. |
|
@ldez Do you know how long it takes to get the fix populated in GHA? We are using the |
|
the GitHub action is currently generating the GHA files https://github.com/golangci/golangci-lint/actions/runs/5153980997/jobs/9281874877 It will be available quickly. |
Following the plan that I explained here:
replacedirectives, becausereplacedirectives are not transitive, so users that are using that use thetoolspattern will not have a problem.)Summary of issue #3862:
depguard uses
build.Default.GOROOTbut this value is filled correctly only if theGOROOTis defined through an env var.The "expander" cannot be overridden because it's a global variable inside an internal package, the
compilemethod that produces the error and the analyzer constructor are not exported.The fix I have done in our
depguardfork is based on what is done insidegolang.org/x/tools.golangci/depguard@v2.0.1...ed68d37
When the PR OpenPeeDeeP/depguard#48 will be merged and when depguard will create a new release, we will drop this dependency (but we will have to keep the fork for compatibility.)
Fixes #3862