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

workspace symlink causes error #3691

Closed
aentity opened this issue Mar 23, 2020 · 8 comments · Fixed by #6246
Closed

workspace symlink causes error #3691

aentity opened this issue Mar 23, 2020 · 8 comments · Fixed by #6246
Labels
E-hard S-unactionable Issue requires feedback, design decisions or is blocked on other work

Comments

@aentity
Copy link

aentity commented Mar 23, 2020

If the root workspace (maybe even regular single crate) is symlink, rust-analyzer reports error "Rust file outside current workspace is not supported yet".

E.g. if we have project foo which is symlink:

/home/me/foo -> /home/me/projects/foo

User opening up /home/me/foo/src/lib.rs will cause error.

Workaround is open real path, but rust-analyzer should just follow symlink if possible (or change error message, as it is misleading).

@rinon
Copy link

rinon commented Jun 15, 2020

I hit this issue as well. It looks like rust-analyzer chokes if a crate file is opened via a symlinked path anywhere in its parent directories (i.e. the symlink can be to a parent, not just the crate folder itself).

@Ten0
Copy link

Ten0 commented Aug 17, 2020

Similarly, I most of the time (though not always) get unresolved module errors when running RA on a crate that has one of its .rs files be a symlink.

@dfoxfranke
Copy link
Contributor

I think the offending function is here: https://github.com/rust-analyzer/rust-analyzer/blob/3a38554f86e5e1b41b111ed8ccc688e84a9d5ae4/crates/vfs-notify/src/lib.rs#L148-L202

WalkDir is constructed without calling follow_links(true), which means that entry.file_type().is_dir() and entry.file_type().is_file() will both return false when the entry is a symlink, and it will resultingly get skipped over. The fix might be as simple as adding a call to follow_links(true).

@matklad matklad added E-hard S-unactionable Issue requires feedback, design decisions or is blocked on other work labels Oct 15, 2020
@matklad
Copy link
Member

matklad commented Oct 15, 2020

The fix might be as simple as adding a call to follow_links(true).

I doubt that, proper handling of symlinks I think needs to be pretty invovled. Though, it definitelly is worth to try this! If this gets us 80% soln, then great!

@dfoxfranke
Copy link
Contributor

I don't think it needs to be particularly involved. AFAICT, for all rust-analyzer's purposes, the Right Thing is to treat symlinks transparently:

  1. When distinguishing among file types, never treat a symlink as something distinct; always ascribe it the file type of its target.

  2. Never call readlink(2). If /a is the root of our project tree and /a/b is a symlink to /d, then /a/b/c is always just /a/b/c, never /d/c.

  3. Never assign special interpretation to ... In particular, never assume that /a/b/../c is the same path as /a/c.

I audited a lot of code, including all of the project_model crate, and the above function is the first incorrect code I found (it breaks rule 1). I'm not certain yet that it's the only incorrect code, but I don't think this is going to be a big patch. There are only a handful of system calls that are capable of distinguishing a symlink from its target — readlink, lstat or statx with AF_SYMLINK_NOFOLLOW, and getdents.
An strace of a rust-analyzer session suggests that tracking down everything that makes any of those calls should be pretty manageable.

@lnicola
Copy link
Member

lnicola commented Oct 15, 2020

We use inotify to monitor file changes, how does it interact with symlinks?

@dfoxfranke
Copy link
Contributor

We use inotify to monitor file changes, how does it interact with symlinks?

It doesn't, much. Inotify is inode-based, not path-based. You don't have to worry about getting confused by a notification about a path outside your tree because those notifications will never mention any path at all, they'll only mention the watch descriptor. Just avoid providing the IN_DONT_FOLLOW flag when calling inotify_add_watch(2), so that the inode you're monitoring is that of the target and not that of the symlink.

@dfoxfranke
Copy link
Contributor

The one-liner fix seems at least to resolve this bug for my personal use case which is mozilla/nixpkgs-mozilla#238. I'll open a PR marked as WIP pending further testing to make sure inotify events get handled properly.

bors bot added a commit that referenced this issue Oct 16, 2020
6246: Follow symlinks when walking project trees r=lnicola a=dfoxfranke

Fixes #3691.

~~WIP pending further testing~~:

- [X] Verify that symlinked files get indexed.
- [x] Verify that files in symlinked directories get indexed.
- [x] Verify that inotify events are properly received and handled when the target of a symlink resides outside the project tree.

Co-authored-by: Daniel Fox Franke <dfoxfranke@gmail.com>
@bors bors bot closed this as completed in e821aa8 Oct 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
E-hard S-unactionable Issue requires feedback, design decisions or is blocked on other work
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants