https://pantsbuild.org/ logo
Join Slack
Powered by
Hello all, I'm writing a linter plugin for pants ...
# general
i
Hello all, I'm writing a linter plugin for pants to run the linter golangci-lint (which is a popular meta-linter for go). I have pants running golangci-lint, however pants is splitting my project into two sandboxes. This causes golangci-lint to fail because neither sandbox is fully complete. I'm getting similar behavior with the go-vet linter, so I'm assuming its something about my project. When I run go-vet via pants, it fails with:
Copy code
config/config.go:26:2: module lookup disabled by GOPROXY=off
cmd/stentor/stentor.go:33:2: <http://github.com/davecgh/go-spew@v1.1.1|github.com/davecgh/go-spew@v1.1.1>: module lookup disabled by GOPROXY=off
cmd/stentor/stentor.go:34:2: <http://github.com/davecgh/go-spew@v1.1.1|github.com/davecgh/go-spew@v1.1.1>: module lookup disabled by GOPROXY=off
cmd/stentor/stentor.go:35:2: <http://github.com/davecgh/go-spew@v1.1.1|github.com/davecgh/go-spew@v1.1.1>: module lookup disabled by GOPROXY=off
However, if I run 
go vet ./...
in the sandbox, it fails with a clearer error:
Copy code
cmd/stentor/stentor.go:33:2: no required module provides package <http://github.com/wfscheper/stentor/internal/templates;|github.com/wfscheper/stentor/internal/templates;> to add it:
        go get <http://github.com/wfscheper/stentor/internal/templates|github.com/wfscheper/stentor/internal/templates>
cmd/stentor/stentor.go:34:2: no required module provides package <http://github.com/wfscheper/stentor/newsfile;|github.com/wfscheper/stentor/newsfile;> to add it:
        go get <http://github.com/wfscheper/stentor/newsfile|github.com/wfscheper/stentor/newsfile>
cmd/stentor/stentor.go:35:2: no required module provides package <http://github.com/wfscheper/stentor/release;|github.com/wfscheper/stentor/release;> to add it:
        go get <http://github.com/wfscheper/stentor/release|github.com/wfscheper/stentor/release>
Pants is aware of these dependencies:
Copy code
./pants dependencies cmd/stentor/stentor.go
22:19:24.75 [INFO] Initialization options changed: reinitializing scheduler...
22:19:25.18 [INFO] Scheduler initialized.
//:root#<http://github.com/stretchr/testify/assert|github.com/stretchr/testify/assert>
//:root#<http://github.com/stretchr/testify/require|github.com/stretchr/testify/require>
... # Trimmed out testdata files
config:config
fragment:fragment
internal/templates:templates
internal/test:test
newsfile:newsfile
release:release
b
Pants takes the input specs and splits then into sizeable chunks in fmt/lint so that if you change a file, you only invalidate the one chunk instead of the entire repo. That's likely what you're seeing. Is your rule getting the relevant dependencies and putting them in the sandbox? Sharing code/errors/etc is very helpful. Also feel free to open a PR to upstream your code. Then we can help on PR, you won't have to worry about API breaks, and everyone benefits
w
to expand on what @bitter-ability-32190 said: if a particular linter needs the transitive dependencies of code to be in the sandbox, it will need to request them. @fast-nail-55400 might be able to suggest what you need in this case
also: if you’re interested in contributing this upstream, we’d love to accept a patch for it. it will reduce the maintenance burden on you, and make us feel better about some expected API changes that are coming!
f
@incalculable-manchester-87310: can you post the source code for your plugin somewhere?
i
@fast-nail-55400, sure. I'll make a branch in my personal project with the pants + plugin setup I have, and then we can work on how to improve and extract it for a PR to pants.
f
At least for initial debugging, the branch on your personal project will be sufficient. I just need to see the code to get better context around what the plugin is actually doing.
i
I have pushed what I have to https://github.com/wfscheper/stentor/tree/pants-integration. The following should reproduce the issue I described:
Copy code
git clone <https://github.com/wfscheper/stentor.git> --branch pants-integration
cd stentor
./pants lint --only=golangci-lint ::
f
The plugin does not request transitive dependencies so they would be unavailable in the sandbox.
You could use 
TransitiveTargetsRequest
to obtain them.
Does the golangci tool need third-party dependencies available as well?
If so, you can use 
GoImportPathMappingRequest
to obtain a mapping from every import path in a module to the address of the target for each package. Then you can hydrate sources for first-party packages and use 
ThirdPartyPkgAnalysisRequest
to obtain a 
Digest
with the third-party module sources (which will be available on the same GOPATH used within the sandbox).
Use 
MergeDigests
to merge all the relevant digests together and use that as the input root.
note: 
GoImportPathMappingRequest
is only available on 
main
and the latest 2.14 RC release.
i
I don't think golangci-lint would need third-party deps, but it is wrapping a lot of linters so maybe... I'll start with TransitiveTargetRequest and see where that gets me.
I think I worked out the transitive dep issue (thanks for the pointer!). The issues now are: 1) I'm getting two sandboxes due to the chunking, but they're identical. Is there some way to disable chunking or dedupe them? 2) Apparently, golangci-lint either needs CGO completely disabled, or I need to set more of the CGO related go env variables (I'm assuming PATH is too restricted to find the necessary compiler?). Not sure which is the best option for pants. I pushed to my project's branch a commit adding TransitiveTargetsRequest, and for now the wrapper script is just disabling CGO. While I'm waiting for feedback, I'll start working on adding this as a PR to pants. Thanks all!
f
I'm assuming PATH is too restricted to find the necessary compiler?)
given the way I wrote the initial cgo support, it uses 
--golang-cgo-tool-search-paths
to search for C compiler. That defaults to 
<PATH>
though so it should be using the PATH.
are you enabling cgo? it should still be disabled by default.
I have a in-progress PR that would allow your rule to selectively disable cgo. https://github.com/pantsbuild/pants/pull/16843. However, it has rule graph errors related to my introduction of a new 
GoBuildContext
type, so I haven't been able to make progress on it.
separately, what Go packages were you seeing cgo errors in? probably good if I use those packages for testing the cgo support.
i
I'm not explicitly enabling CGO that I know of, but 
go env CGO_ENABLED
returns 1. If I don't set CGO_ENABLED=0 in the wrapper script, then I get some variation on:
Copy code
19:25:45.27 [ERROR] Completed: Lint with golangci-lint - golangci-lint failed (exit code 3).
level=warning msg="[runner] Can't run linter goanalysis_metalinter: buildir: failed to load package cgo: could not load export data: no export data for \"runtime/cgo\""
level=error msg="Running error: 1 error occurred:\n\t* can't run linter goanalysis_metalinter: buildir: failed to load package cgo: could not load export data: no export data for \"runtime/cgo\"\n\n"
I read through some other linters and I think I resolved the duplicate sandbox problem. I have opened a PR (https://github.com/pantsbuild/pants/pull/16994). Is it better to continue this conversation in the PR comments or on Slack? In addition to the existing CGO questions, I also have some UX questions about how pants should run golangci-lint by default.
f
That error message doesn't look like it is coming from any Pants rule. So it may be specific to golangci-lint and how it interacts with Go code.
i
It doesn't occur when I run the integration tests I wrote, but those are very simple go programs. Now I'm curious why CGO isn't disabled already by pants, though. If I add 
go env
to the wrapper script before explicitly disabling CGO, I see CGO_ENABLED=1.
f
the rules only pass CGO_ENABLED=0 to 
go
invocations that actually need it
e.g., when the rules invoke 
go tool compile
, it isn't set at all since not needed
it is set when the package analyzer is run which is where it actually matters
so if golangci-lint needs it, then I'd recommend passing it. you can base it off the 
GolangSubsystem.cgo_enabled
option
👍 1
20 Views
Open in Slack
PreviousNext
Powered by