Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

files.exclude not working for symlinks #4206

Closed
RichardFurness opened this issue Sep 6, 2019 · 8 comments
Closed

files.exclude not working for symlinks #4206

RichardFurness opened this issue Sep 6, 2019 · 8 comments
Labels
bug Feature: Find All References Find All References, Peek References, Rename fixed Check the Milestone for the release in which the fix is or will be available. Language Service regression A bug that didn't exist in a previous release symlinks
Milestone

Comments

@RichardFurness
Copy link

Type: LanguageService

Describe the bug

  • OS and Version: Windows 10 using Remote-SSH to Linux
  • VS Code Version: latest insiders
  • C/C++ Extension Version: latest (0.25.1)
  • Other extensions you installed (and if the issue persists after disabling them):
  • A clear and concise description of what the bug is.

My workspace has a symlink "bazel-foo" (created by bazel) to a folder which in turn contains symlinks to all the sub-directories of the original workspace. I don't want this symlink to appear in vscode, so I added it to files.exclude. Having done this, I find that some of the sub-directories of my project are not tagged by the C extension (and hence, for instance, find-all-references won't find any references in those sub-directories).

To Reproduce

  • Set up a symlink as described above
  • Set up an exclusion in files.exclude for that symlink
  • Reset intellisense DB
  • Test with find-all-references
    (Note: this won't necessarily always repro the issue, see notes below)

Expected behavior
The exclusion shouldn't impact the ability of C intellisense to tag files in non-excluded sub-directories of the workspace.

Screenshots

Additional context
Some investigation suggests some sort of explanation: I tried running "find -L" under the workspace, and the order of the output was as follows (for the relevant dirs/files):
good-subdir/
bazel-foo/good-subdir/
bazel-foo/bad-subdir/
bad-subdir/

This suggests to me that the extension is effectively doing the same as "find -L" and receiving the results in this (undefined) order. It is presmuably then deciding to drop a file based on whichever path that file happens to first appear with. So for a file under good-subdir it sees the non-symlinked version first and hence parses it ok. But for bad-subdir it seems the version under bazel-foo first and hence decides to filter out the file (even though the file appears again later on).

@sean-mcmanus sean-mcmanus added bug Feature: Find All References Find All References, Peek References, Rename Language Service labels Sep 6, 2019
@sean-mcmanus sean-mcmanus self-assigned this Sep 6, 2019
@sean-mcmanus sean-mcmanus added this to the On Deck milestone Sep 6, 2019
@sean-mcmanus sean-mcmanus removed their assignment Sep 6, 2019
@sean-mcmanus
Copy link
Collaborator

Can you give more details on how to setup the symlinks? Is the target of the bazel-foo symlink outside of the workspace?

@RichardFurness
Copy link
Author

Yes the target is outside the workspace. So for example:
Workspace: /outer/foo
Main symlink: /outer/foo/bazel-foo -> /outer/bazel-folder
Subdir symlinks:
/outer/bazel-folder/subdir1 -> /outer/foo/subdir1
/outer/bazel-folder/subdir2 -> /outer/foo/subdir2
etc

The more subdirs you have the more likely you will be to repro, as I think it's ordering the "find" output by something like hash table iteration.

@sean-mcmanus
Copy link
Collaborator

I can't repro this on Windows or Linux. Do you know if the repro requires Remote-SSH? When you enable debug logging (https://code.visualstudio.com/docs/cpp/enable-logging-cpp) do you see the files not being "tag parsed" in the logs? If you see that the files aren't tag parsed, can you set the logging (temporarily) to the hidden value of "9" and look at the output related to exclusions. We an exclusion checking iterator to check if the currently iterated folder is excluded or not. If you're able to see from the logs where we're handling things buggily we may be able to fix it without a repro.

@sean-mcmanus sean-mcmanus added more info needed The issue report is not actionable in its current state not reproing We're not able to reproduce the issue (it's unlikely to get fixed until we find one). labels Sep 10, 2019
@RichardFurness
Copy link
Author

I don't know if this issue requires Remote-SSH, all I can say is that I've only seen it with Remote-SSH to Linux.
Yes, the files in question don't appear as "tag parsed" in the logs.
I tried setting logging level 9, and it doesn't appear as though the issue is with your exclusion checking, as the only directories appearing in the logs are:
/outer/foo/good-subdir (which is tagged fine)
/outer/foo/bazel-foo/bad-subdir (which has a "found matching exclude entry" line explaining why it's not tagged)

In other words, for each of the directories that are symlinked, only 1 instance of that directory is appearing in the logs, and its the instance that comes first in the "find -L" output. So in the case where that's the "real" path it gets tagged fine but in the case where it's the symlinked path then it gets excluded.

@sean-mcmanus
Copy link
Collaborator

Okay -- I repro the bug now, sorry about the confusion -- I discovered my symlink creation was done incorrectly.

The root bug is #3123 -- our code that calls nftw to iterate over the directories does not check the files.exclude, which causes it iterate into the excluded folder and generate paths under the excluded path, but when it encounters the non-excluded path, the ntfw method skips that folder as a duplicate due to FTW_PHYS (intentionally) not being set (https://linux.die.net/man/3/nftw).

This is "On Deck" to fix, but not our schedule yet.

An alternative fix might be to expose some user setting to disable all symlink navigation (i.e. set FTW_PHYS)...not sure if that is a good idea or not.

@sean-mcmanus sean-mcmanus added duplicate and removed more info needed The issue report is not actionable in its current state not reproing We're not able to reproduce the issue (it's unlikely to get fixed until we find one). labels Sep 11, 2019
@RichardFurness
Copy link
Author

Great, this explanation makes sense! And it sounds like the fix will bring other (performance) benefits too :-)

@bobbrow bobbrow modified the milestones: Post-v1, On Deck Jul 16, 2020
@sean-mcmanus sean-mcmanus self-assigned this Jan 29, 2021
@sean-mcmanus sean-mcmanus modified the milestones: On Deck, 1.3.0 Jan 29, 2021
@sean-mcmanus sean-mcmanus added the fixed Check the Milestone for the release in which the fix is or will be available. label Apr 5, 2021
@sean-mcmanus sean-mcmanus removed their assignment Apr 5, 2021
@michelleangela
Copy link
Contributor

@michelleangela
Copy link
Contributor

Fix is released in the 1.3.0 release version https://github.com/microsoft/vscode-cpptools/releases/tag/1.3.0

@github-actions github-actions bot locked and limited conversation to collaborators May 30, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Feature: Find All References Find All References, Peek References, Rename fixed Check the Milestone for the release in which the fix is or will be available. Language Service regression A bug that didn't exist in a previous release symlinks
Projects
None yet
Development

No branches or pull requests

4 participants