Don't load and analyze dirs from --skip-dirs #301

nicksnyder · 2018-11-21T16:50:58Z

I am trying to run golangci-lint on sourcegraph/sourcegraph but it is failing due to the presence of some Go code in one of our node_modules dependencies (weird but ¯\_(ツ)_/¯). I tried using --skip-dirs node_modules, but it seems to not have any effect.

Reproduce:

git clone https://github.com/sourcegraph/sourcegraph
cd sourcegraph
yarn
golangci-lint -v run --skip-dirs node_modules

Requested info:

Version of golangci-lint: golangci-lint --version (or git commit if you don't use binary distribution)
55a18ae
Config file: cat .golangci.yml
None
Go environment: go version && go env

go version go1.11.2 darwin/amd64
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/nick/Library/Caches/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/nick/dev/gopath"
GOPROXY=""
GORACE=""
GOROOT="/usr/local/go"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/l6/djg_zw1j5lgbwz4h5q92lnwr0000gn/T/go-build800779018=/tmp/go-build -gno-record-gcc-switches -fno-common"

Verbose output of running: golangci-lint run -v

$ golangci-lint -v run --skip-dirs node_modules
INFO [config_reader] Config search paths: [./ /Users/nick/dev/sourcegraph/node_modules /Users/nick/dev/sourcegraph /Users/nick/dev /Users/nick /Users /]
INFO Gocritic enabled checks: [appendAssign assignOp caseOrder dupArg dupBranchBody dupCase flagDeref ifElseChain regexpMust singleCaseSwitch sloppyLen switchTrue typeSwitchVar underef unlambda unslice defaultCaseOrder]
INFO [lintersdb] Active 8 linters: [deadcode errcheck govet ineffassign megacheck structcheck typecheck varcheck]
INFO [loader] Go packages loading at mode load deps types and syntax took 3.037896104s
ERRO Running error: context loading failed: failed to load program with go/packages: go [list -e -json -compiled -test=true -export=false -deps=true -- ./...]: exit status 1: build github.com/sourcegraph/sourcegraph/node_modules/snyk-go-plugin/gosrc: cannot find module for path _/Users/nick/dev/sourcegraph/node_modules/snyk-go-plugin/gosrc/resolver

INFO Memory: 32 samples, avg is 69.2MB, max is 69.2MB
INFO Execution took 3.13999768s

The text was updated successfully, but these errors were encountered:

jirfag · 2018-11-23T20:09:45Z

hi!
it's the restriction of go/packages (official library for loading packages for analysis): it uses go list and we just pass args (default are ./...) to it. We can't exclude any directory from go list ./.... Therefore skip-dirs analyzes set dirs and just skips issues from them.

You can use something like:

go list -f '{{.Dir}}' ./...  | fgrep -v node_modules/ | xargs realpath --relative-to=. | xargs golangci-lint run -v

Prior to this change the SkipDir runner was not skipping files such as `foo/bar/baz.go` with a `skip-dir` entry of `foo/bar` when the `run` command was invoked with an argument of `foo/...`. This is both a surprising and problematic behavior change. The pathology was: 1. `shouldPassIssue()` was receiving an input like `foo/bar/baz.go` 2. `shouldPassIssue()` would call `p.getLongestArgRelativeIssuePath()` which was returning `bar`, not `foo/bar` as expected. 3. The reason for this was because inside of `getLongestArgRelativeIssuePath()` it was trimming the prefix that matched the path prefix (e.g. `foo/`). If you have the file structure: - foo/bar/baz.go - bur/bar/baz.go There is no way to isolate `foo/bar` from `bur/baz` without strictly controlling both your `skip-dirs` configuration and the arguments to `run`. The rest of the logic to skip files that don't match `run`'s argument is valid, however the regexp should be evaluated based on the `filepath.Dir()` of the input (e.g. `foo/bar`) and not the truncated version of the issue's filepath. Fixes: golangci#301

lnshi · 2019-03-29T08:24:07Z

@jirfag

Looks this is still happening for v1.15.0, i totally cannot run the lining because our project depends on tensorflow project, and the --skip-dirs doesn't really skip it:

ERRO Running error: context loading failed: failed to load program with go/packages: go [list -e -json -compiled -test=true -export=false -deps=true -find=false -- ./...]: exit status 2: # ----------path/rmt_serving/tensorflow/tensorflow/go/genop/internal
ld: library not found for -ltensorflow
clang: error: linker command failed with exit code 1 (use -v to see invocation)
# github.com/tensorflow/tensorflow/tensorflow/go/genop/internal
ld: library not found for -ltensorflow
clang: error: linker command failed with exit code 1 (use -v to see invocation)

jirfag · 2019-04-20T19:32:54Z

@lnshi ld: library not found for -ltensorflow says that you need to install the tensorflow library, it's not installed.

arjantop · 2019-05-06T18:42:48Z

@jirfag When does the filtering currently happen, on a per-file basis? Could not golangci-lint run the filter on go list output and not even consider ignored directories? (like the example with grep above)

arjantop · 2019-05-07T07:39:00Z

One big problem if also if you have packages that do not compile (as detected by go list but are really not go packages). This prevents the whole megacheck from running, but linting still exits with exit code 0 (this is a separate issue but still related, #529).

stale · 2020-04-12T18:00:56Z

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.

After changing some linter settings, we began seeing that the linter was failing to run on v6 (when the "V7" build tag was not given). The errors were complaining about duplicate declarations in the "fixtures/plugins" directory (which was meant to be ignored by the linter, via the "skip-dirs" option). After investigating, I learned that Golangci-lint will still try to parse and analyze directories that are skipped in this way - it only uses the "skip-dirs" option to filter out any lint errors that are found after analysis is complete. Here's an open GitHub issue about this: golangci/golangci-lint#301 This means that if there are compilation errors in these skipped dirs, they will fail the linter, even though we want to skip them. I'm fixing this with a workaround suggested in that issue. Something like: ``` go list ./... | (grep for dirs you care to lint) | xargs golangci-lint run ``` A nice side effect is that, by truly skipping analysis of these dirs, the linter runs faster. [#172389465] Authored-by: Reid Mitchell <rmitchell@pivotal.io>

luhexcp · 2020-08-23T00:47:58Z

hi!
it's the restriction of go/packages (official library for loading packages for analysis): it uses go list and we just pass args (default are ./...) to it. We can't exclude any directory from go list ./.... Therefore skip-dirs analyzes set dirs and just skips issues from them.

You can use something like:
go list -f '{{.Dir}}' ./...  | fgrep -v node_modules/ | xargs realpath --relative-to=. | xargs golangci-lint run -v

hey @jirfag Thanks for that shell and I have skipped these dirs. But I have a question is that If I use the shell with fgrep -v, the skip-dirs in config will be invalid.

nicksnyder changed the title ~~skip_dirs doesn't skip directory~~ --skip-dirs doesn't skip directory Nov 21, 2018

sean- mentioned this issue Dec 6, 2018

Fix the skip-dirs option. #313

Closed

jirfag closed this as completed in 441bdb3 Dec 22, 2018

jirfag reopened this Dec 22, 2018

aLekSer mentioned this issue Sep 26, 2019

Add SDK server HTTP API test googleforgames/agones#1079

Merged

tpounds added bug Something isn't working area: config Related to .golangci.yml and/or cli options stale No recent correspondence or work activity labels Oct 3, 2019

jirfag changed the title ~~--skip-dirs doesn't skip directory~~ Don't load and analyze dirs from --skip-dirs Oct 15, 2019

stale bot removed the stale No recent correspondence or work activity label Oct 15, 2019

jirfag added enhancement New feature or improvement and removed bug Something isn't working area: config Related to .golangci.yml and/or cli options labels Oct 15, 2019

stale bot added the stale No recent correspondence or work activity label Apr 12, 2020

stale bot closed this as completed May 12, 2020

snyk-bot mentioned this issue Dec 19, 2020

[Snyk] Upgrade gatsby-plugin-robots-txt from 1.5.1 to 1.5.3 XirdigH/golangci-lint#6

Closed

This was referenced Feb 18, 2022

golangci-lint should ignore node_modules go-gitea/gitea#18803

Closed

golangci-lint fails if it can't read the contents of a directory, even if it's in --skip-dirs #1832

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Don't load and analyze dirs from --skip-dirs #301

Don't load and analyze dirs from --skip-dirs #301

nicksnyder commented Nov 21, 2018 •

edited

Loading

jirfag commented Nov 23, 2018

lnshi commented Mar 29, 2019

jirfag commented Apr 20, 2019

arjantop commented May 6, 2019

arjantop commented May 7, 2019 •

edited

Loading

stale bot commented Apr 12, 2020

luhexcp commented Aug 23, 2020 •

edited

Loading

Don't load and analyze dirs from --skip-dirs #301

Don't load and analyze dirs from --skip-dirs #301

Comments

nicksnyder commented Nov 21, 2018 • edited Loading

jirfag commented Nov 23, 2018

lnshi commented Mar 29, 2019

jirfag commented Apr 20, 2019

arjantop commented May 6, 2019

arjantop commented May 7, 2019 • edited Loading

stale bot commented Apr 12, 2020

luhexcp commented Aug 23, 2020 • edited Loading

nicksnyder commented Nov 21, 2018 •

edited

Loading

arjantop commented May 7, 2019 •

edited

Loading

luhexcp commented Aug 23, 2020 •

edited

Loading