Symlinks cause problems with exclude_paths

[jdelStrother]

Heya - I ran into a problem with exclude_paths and symlinks that took me a while to figure out. It's not clear to me if it's a bug in codeclimate-eslint or the codeclimate base (or even if it's a bug at all), but :

I have a directory structure like:

- vendor
  \-  javascripts
  \-  stylesheets

Both of javascripts & stylesheets are symlinked to another directory. My exclude_paths is set to vendor/**/*.

It appears that when figuring out which files to exclude, CC doesn't follow symlinks. However, when figuring out which files to analyse, CC-Eslint does follow symlinks, so it was analysing all my files under the symlinked directory.

It's fixable from the user's side by setting exclude_paths to :

- vendor/javascripts/**/*
- vendor/stylesheets/**/*

but I wonder if either the codeclimate repo should be updated to follow symlinks, or codeclimate-eslint should be updated to ignore them. WDYT?

[ABaldwinHunter]

@jdelStrother Thanks for the report and opening an issue. We're investigating this issue and will report back with our findings soon.

[dblandin]

Hi @jdelStrother,

Sorry for the delay on this. Are you still experiencing this issue? We made improvements to workspace calculation in qltysh/qlty#308 which may have had an impact on this issue.

[wfleming]

I was one of the engineers responsible for some of the recent changes we made to how we handle paths, so I can add some details here. I'm sorry for not noticing this issue & getting involved earlier.

We process exclude paths as "effective" paths. For example, if you have a file a/bad.js with issues, and that file is symlinked to to different/bad.js, and you've excluded a/, but not different/, then we would expect the engine to look at different/bad.js.

One important reason for this choice is that we generally want to respect the existing behavior of underlying analysis tools (like eslint) when we can. Running eslint directly exhibits the same behavior you're describing: if I exclude a sub-directory in my .eslintignore but the directory is symlinked elsewhere, running eslint against the whole directory will still report issues on the ignored files, because it still read the symlinked location (which was not ignored). I also found the fact that ESLint reports these issues using the original file's path surprising, though: if there's a bug here, I think that's it, and it would be a change to ESLint itself.

With all that said, we do think the best way of handling these cases is to just put both paths in your exclude_paths. Ultimately, we think that results in less surprising behavior in most cases, and less potential "breakage" for how many underlying analysis tools work. Like I said above, I did find it surprising that ESLint analyzes symlinks but reports the issues on the original paths: we'll continue to think about whether that's something that we should open up a PR with their team about.

Thanks for bringing this up, and please let me know if you have further questions.

[jdelStrother]

Yep, seems much better now.

I've found exclude_paths quite difficult to debug - I typically end up sshing into the docker VM, and repeatedly running lsof to get some idea of what it's busy scanning. Is there a better way?

[wfleming]

Is there a better way?

As of recently, yes, actually there is!

The CLI recognizes a CODECLIMATE_DEBUG environment variable, and when it is present & non-empty, runs in a very verbose mode that emits a lot of information you wouldn't normally see. e.g.:

$ CODECLIMATE_DEBUG=1 codeclimate analyze
Starting analysis
Running rubocop: |[DEBUG] engine config: "{\"enabled\":true,\"include_paths\":[\".bundle/\",\".codeclimate.yml\",\".dockerignore\",\".gitignore\",\".rubocop.yml\",\".ruby-version\",\"bin/\",\"circle.yml\",\"config/\",\"Dockerfile\",\"Gemfile\",\"Gemfile.lock\",\"lib/\",\"LICENSE\",\"Makefile\",\"Rakefile\",\"README.md\",\"spec/\",\"WERE_HIRING.md\"]}"
[DEBUG] docker run: ["docker", "run", "--name", "cc-engines-rubocop-b86740e9-f6cd-4631-80be-cf56a76ec639", "--cap-drop", "all", "--label", "com.codeclimate.label=cbfdf6d5-330a-4c86-9278-ae613a161b03", "--memory", "512000000", "--memory-swap", "-1", "--net", "none", "--rm", "--volume", "/Users/will/cc/codeclimate-rubocop:/code:ro", "--volume", "/tmp/cc/3b67536d-133c-40f7-9ce7-09f64c9a6a70:/config.json:ro", "--user", "9000:9000", "codeclimate/codeclimate-rubocop"]
...

It could certainly be easier to read, and the string escaping it does right now is excessive, but with that variable set, the CLI will emit the config that gets sent to the engine (which includes the include_paths, which we calculate based on the exclude_paths in your .codeclimate.yml).

I'm going to go ahead and close this issue, since I think we've addressed the original concerns. Feel free to ask more questions about the debug functionality here, though, and I'll respond.

[wfleming]

Oops, I lied a bit: that functionality is indeed in the CLI, but it's so recent that we haven't actually fully released it yet. I'll do that now: keep an eye out for a new v0.21.3 version soon that will include it.