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

Debug does not work for file in symlinked folder #57954

Closed
bpasero opened this Issue Sep 5, 2018 · 16 comments

Comments

Projects
None yet
6 participants
@bpasero
Member

bpasero commented Sep 5, 2018

  • VSCode Version: 1.28 (master)
  • OS Version: Linux Ubuntu 18

Steps to Reproduce:

  1. create a symbolic link to a folder (ln -s source target)
  2. open the folder (running out of master where our realpath logic is removed and you can open the link)
  3. add a app.js node.js file with just one console.log statement
  4. add a breakpoint
  5. create a launch config:
{
    "type": "node",
    "request": "launch",
    "name": "Launch Program",
    "program": "${workspaceFolder}/app.js",
    "runtimeArgs": [
        "--preserve-symlinks"
    ]
}
  1. Debug

=> 🐛 breakpoint is not hit

Works fine when opening the folder with its real path.

@roblourens

This comment has been minimized.

Show comment
Hide comment
@roblourens

roblourens Sep 5, 2018

Member

--preserve-symlinks is simply not working here...

Member

roblourens commented Sep 5, 2018

--preserve-symlinks is simply not working here...

@bpasero

This comment has been minimized.

Show comment
Hide comment
@bpasero

bpasero Sep 5, 2018

Member

@roblourens fyi with #18837 we no longer automatically open the resolved link as folder when someone opens a folder that is a symbolic link. This aligns with what we do in multi-root workspaces where we never resolved links.

Member

bpasero commented Sep 5, 2018

@roblourens fyi with #18837 we no longer automatically open the resolved link as folder when someone opens a folder that is a symbolic link. This aligns with what we do in multi-root workspaces where we never resolved links.

@roblourens

This comment has been minimized.

Show comment
Hide comment
@roblourens

roblourens Sep 5, 2018

Member

I see the same behavior with chrome devtools, this just isn't working the way I expect. Node always resolves the symlink for process.cwd() and the main script is resolved relative to that.

image

Maybe we can work around it for the debugger by always resolving the main script to an absolute path, if it's in a symlinked folder, and if the user has set --preserve-symlinks in their config?

Member

roblourens commented Sep 5, 2018

I see the same behavior with chrome devtools, this just isn't working the way I expect. Node always resolves the symlink for process.cwd() and the main script is resolved relative to that.

image

Maybe we can work around it for the debugger by always resolving the main script to an absolute path, if it's in a symlinked folder, and if the user has set --preserve-symlinks in their config?

@bpasero

This comment has been minimized.

Show comment
Hide comment
@bpasero

bpasero Sep 5, 2018

Member

If it helps, fs.realpath can be used to resolve a symlink to its actual path. That is what we used to do when opening any folder. We also use it for our file watchers to convert file paths from the watcher into the format we were given (in case it is a symlink).

Member

bpasero commented Sep 5, 2018

If it helps, fs.realpath can be used to resolve a symlink to its actual path. That is what we used to do when opening any folder. We also use it for our file watchers to convert file paths from the watcher into the format we were given (in case it is a symlink).

@roblourens

This comment has been minimized.

Show comment
Hide comment
@roblourens

roblourens Sep 5, 2018

Member

Yeah I guess we could also always resolve paths to their real path, and advise users not to use --preserve-symlinks, but that could be a breaking change. And users might have other reasons for using it - I think it actually affects where require() looks for files. Or they want to see the right paths in stack traces.

I said that resolving the script to an absolute path would work around it but that's actually not true. The main script is still excluded from --preserve-symlinks. If you use node 10 then you can also pass --preserve-symlinks-main which does what you'd expect for the main script.

So if you use both of those flags, and we pass the script as an absolute path, then it does work.

Member

roblourens commented Sep 5, 2018

Yeah I guess we could also always resolve paths to their real path, and advise users not to use --preserve-symlinks, but that could be a breaking change. And users might have other reasons for using it - I think it actually affects where require() looks for files. Or they want to see the right paths in stack traces.

I said that resolving the script to an absolute path would work around it but that's actually not true. The main script is still excluded from --preserve-symlinks. If you use node 10 then you can also pass --preserve-symlinks-main which does what you'd expect for the main script.

So if you use both of those flags, and we pass the script as an absolute path, then it does work.

@bpasero

This comment has been minimized.

Show comment
Hide comment
@bpasero

bpasero Sep 6, 2018

Member

I think its fine to support --preserve-symlinks again if possible. You guys should probably update your docs because you still say that as a workaround.

Member

bpasero commented Sep 6, 2018

I think its fine to support --preserve-symlinks again if possible. You guys should probably update your docs because you still say that as a workaround.

@roblourens

This comment has been minimized.

Show comment
Hide comment
@roblourens

roblourens Sep 6, 2018

Member

It should still work in the case of having a symlinked folder inside your workspace, which is what I've seen in the past, but not this scenario.

Member

roblourens commented Sep 6, 2018

It should still work in the case of having a symlinked folder inside your workspace, which is what I've seen in the past, but not this scenario.

@bpasero

This comment has been minimized.

Show comment
Hide comment
@bpasero

bpasero Sep 6, 2018

Member

Ah ok got it.

Member

bpasero commented Sep 6, 2018

Ah ok got it.

@roblourens

This comment has been minimized.

Show comment
Hide comment
@roblourens

roblourens Sep 6, 2018

Member

I wonder if it's ok to tell people that this debug scenario only works with node 10 which supports --preserve-symlinks-main or do we need a solution for < 10 also? Is there some common scenario that drove the change to not resolve symlink paths?

Member

roblourens commented Sep 6, 2018

I wonder if it's ok to tell people that this debug scenario only works with node 10 which supports --preserve-symlinks-main or do we need a solution for < 10 also? Is there some common scenario that drove the change to not resolve symlink paths?

@ivan-section-io

This comment has been minimized.

Show comment
Hide comment
@ivan-section-io

ivan-section-io Sep 7, 2018

I guess, damned if you do, damned if you don't.
VSCode resolving links had previously broken my golang editing+debugging experience, by resolving outside of the nicely constructed GOPATH those symlinks had placed everything into.

The question is: is VSCode the tool that just breaks the paths for everything (and hopes other tools will still work), or the tool that will have to cater specifically for the other "path breaking" tools?

I hope the later. But it sound like some things may have come to rely upon it... https://xkcd.com/1172/

ivan-section-io commented Sep 7, 2018

I guess, damned if you do, damned if you don't.
VSCode resolving links had previously broken my golang editing+debugging experience, by resolving outside of the nicely constructed GOPATH those symlinks had placed everything into.

The question is: is VSCode the tool that just breaks the paths for everything (and hopes other tools will still work), or the tool that will have to cater specifically for the other "path breaking" tools?

I hope the later. But it sound like some things may have come to rely upon it... https://xkcd.com/1172/

@roblourens roblourens added this to the September 2018 milestone Sep 10, 2018

@isidorn isidorn removed their assignment Sep 21, 2018

@weinand weinand removed their assignment Sep 21, 2018

@roblourens

This comment has been minimized.

Show comment
Hide comment
@roblourens

roblourens Sep 24, 2018

Member

Do we know how common this scenario is for node developers? My gut says not that common. I still don't have a < node 10 solution.

Member

roblourens commented Sep 24, 2018

Do we know how common this scenario is for node developers? My gut says not that common. I still don't have a < node 10 solution.

@weinand

This comment has been minimized.

Show comment
Hide comment
@weinand

weinand Sep 24, 2018

Member

one common scenario is "npm link": using an npm module under development in node.js project (e.g. the vscode-debugadapter).

Member

weinand commented Sep 24, 2018

one common scenario is "npm link": using an npm module under development in node.js project (e.g. the vscode-debugadapter).

@roblourens

This comment has been minimized.

Show comment
Hide comment
@roblourens

roblourens Sep 24, 2018

Member

Debugging with a linked npm module still works. What doesn't work is when the main script itself is inside a symlinked path. Node is treating the main script differently from other scripts.

Member

roblourens commented Sep 24, 2018

Debugging with a linked npm module still works. What doesn't work is when the main script itself is inside a symlinked path. Node is treating the main script differently from other scripts.

@roblourens

This comment has been minimized.

Show comment
Hide comment
@roblourens

roblourens Sep 25, 2018

Member

The best fix I have is to detect when the cwd is in a symlinked path, and if so, always use an absolute path instead of resolving it to a relative path. This is needed because --preserve-symlinks-main applies to whatever is passed in as the main script, but if a relative path is given and must be resolved with the cwd, the cwd is always resolved to its real path.

I'll update the docs to tell users when to use --preserve-symlinks-main

Member

roblourens commented Sep 25, 2018

The best fix I have is to detect when the cwd is in a symlinked path, and if so, always use an absolute path instead of resolving it to a relative path. This is needed because --preserve-symlinks-main applies to whatever is passed in as the main script, but if a relative path is given and must be resolved with the cwd, the cwd is always resolved to its real path.

I'll update the docs to tell users when to use --preserve-symlinks-main

roblourens added a commit to Microsoft/vscode-docs that referenced this issue Sep 25, 2018

@bpasero

This comment has been minimized.

Show comment
Hide comment
@bpasero

bpasero Sep 26, 2018

Member

@roblourens so what is the full fix for this? I need:

  • node.js 10
  • have the following launch config
{
    // Use IntelliSense to learn about possible attributes.
    // Hover to view descriptions of existing attributes.
    // For more information, visit: https://go.microsoft.com/fwlink/?linkid=830387
    "version": "0.2.0",
    "configurations": [
        {
            "type": "node",
            "request": "launch",
            "name": "Launch Program",
            "program": "${workspaceFolder}/index.js",
            "runtimeArgs": [
                "--preserve-symlinks",
                "--preserve-symlinks-main"
            ]
        }
    ]
}

?

Member

bpasero commented Sep 26, 2018

@roblourens so what is the full fix for this? I need:

  • node.js 10
  • have the following launch config
{
    // Use IntelliSense to learn about possible attributes.
    // Hover to view descriptions of existing attributes.
    // For more information, visit: https://go.microsoft.com/fwlink/?linkid=830387
    "version": "0.2.0",
    "configurations": [
        {
            "type": "node",
            "request": "launch",
            "name": "Launch Program",
            "program": "${workspaceFolder}/index.js",
            "runtimeArgs": [
                "--preserve-symlinks",
                "--preserve-symlinks-main"
            ]
        }
    ]
}

?

@roblourens

This comment has been minimized.

Show comment
Hide comment
@roblourens

roblourens Sep 26, 2018

Member

Correct

Member

roblourens commented Sep 26, 2018

Correct

@chrmarti chrmarti added the verified label Sep 28, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment