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

Attempt to automatically de/reference symlinked files #776

Closed
connor4312 opened this issue Sep 29, 2020 · 5 comments
Closed

Attempt to automatically de/reference symlinked files #776

connor4312 opened this issue Sep 29, 2020 · 5 comments
Assignees
Labels
feature-request Request for new features or functionality verification-needed Verification of issue is requested verified Verification succeeded
Milestone

Comments

@connor4312
Copy link
Member

connor4312 commented Sep 29, 2020

Currently for Node we sometimes require people to pass --preseve-symlinks (and document this) since Node dereferences scripts it loads, which can cause file paths to not patch up.

However we could also use fs.realPath to dereference them ourselves. Breakpoints should be applied in both places (since we won't know which is the right one ahead of time). Still need to figure out how to deal with loaded sources, however, since when a source loads we need to tell VS code the path it knows about rather than the real path.

@restjohn
Copy link

Hi @connor4312. Is the VS Code team actively working or planning to work this issue? Do you have any insight on whether debugging works with NPM 7's workspaces? I would imagine projects that adopt workspaces might run into this same unbound breakpoint problem, but I have not yet had an opportunity to test it myself, so that's just my speculation for now. Hopefully I'll get some time to test that soon, though.

@connor4312
Copy link
Member Author

Improved some behavior here -- see the linked commit message for details.

I considered making --preserve-symlinks a default argument, which is still a decent option, but I would be concerned about breaking existing users' setups.

Regarding microsoft/vscode#113283 (comment), I learned something new that, at least on macOS, you can never be "CD"'d into a symlinked path. This is a chrome that the shell provides, but the working directory provided to any launched application is the physical path. We could sort of get around this by using a different cwd if we see that the launched directory is linked, but this also subtly changes some Node module resolution behavior, so this is not a safe change: we will just advise that users avoid having a symlinked cwd.

@connor4312
Copy link
Member Author

To verify, have a setup with symlinked files:

mkdir real
echo "require('./b');\nconsole.log('a');" > real/a.js
echo "console.log('b');" > real/b.js
ln -s real linked
  1. Set a breakpoint on line 2 of linked/a
  2. Create a vanilla Node.js launch config that runs linked/a
  3. Verify that the breakpoint gets hit, but you'll be in real/a instead
  4. Verify that you see a helpful warning message
  5. Verify that adding the suggested --preserve-symlinks and --preserve-symlinks-main flags cause you to break in linked/a and the warning no longer appears.

@TylerLeonhardt
Copy link
Member

So if I set a breakpoint in linked/b.js and line 2 of linked/a.js

if I just have "--preserve-symlinks-main" I go to real/b.js BUT linked/a.js (I expect this)
if I have both "--preserve-symlinks-main" and "--preserve-symlinks" I go to linked/b.js and linked/a.js (I expect this)
if I just have "--preserve-symlinks" I still go to the real/b.js and real/a.js (I don't expect this - it should go to linked/b.js and then real/a.js no?)

@TylerLeonhardt TylerLeonhardt added the verification-found Issue verification failed label Mar 25, 2021
@connor4312
Copy link
Member Author

@TylerLeonhardt this is expected since without preserve-symlinks-main, "real" is resolved to real/a.js, then it requires real/b.js without hitting the symlink at all.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request Request for new features or functionality verification-needed Verification of issue is requested verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

3 participants