depguard: fix GOROOT detection #3866
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ldez
added
bug
gandarez
approved these changes
Jun 2, 2023
4 tasks
jacobbednarz
added a commit
to cloudflare/cloudflare-go
that referenced
this pull request
Jun 2, 2023
Once v1.29 golangci/golangci-lint#3866 lands, we can revert this.
bombsimon
approved these changes
Jun 2, 2023
yrobla
added a commit
to stacklok/minder
that referenced
this pull request
Jun 2, 2023
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.
yrobla
added a commit
to stacklok/minder
that referenced
this pull request
Jun 2, 2023
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 |
This was referenced Jun 2, 2023
|
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. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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