-
Notifications
You must be signed in to change notification settings - Fork 28.4k
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
Prioritize PATH executables over local directories in terminals #158666
Prioritize PATH executables over local directories in terminals #158666
Conversation
I think there is still one potential issue with this solution, which is that findExecutable itself can sometimes return the path to a directory. EDIT: the error doesn't get caught on the main branch, but this PR actually prevents the error from not being caught. |
Actually, that is not an issue with this solution, and is an unrelated problem which is already present on the main branch. I'll try to handle that in a separate issue/PR. |
Right now, when validating an executable, the check to ensure the path is a file and not a directory comes before the check for executables in PATH. This means that a sub directory of the current process's PWD will take priority over an executable in PATH, and so an error will be given saying that the specified path "is not a file or a symlink", even if there is a valid executable in PATH which was intended to be executed. This fix reorders the checks to ensure that the executable in PATH takes priority over a sub directory of the local PWD.
I see this when I try open a terminal:
Pushed a fix to cover that exception only and throw for others (which we can fix if they're hit) |
@meganrogge second review please |
Great stuff, thanks for your time! |
Right now, when validating an executable, the check to ensure the path is a file and not a directory comes before the check for executables in PATH. This means that a sub directory of the current process's PWD will take priority over an executable in PATH, and so an error will be given saying that the specified path "is not a file or a symlink", even if there is a valid executable in PATH which was intended to be executed.
This fix reorders the checks to ensure that the executable in PATH takes priority over a sub directory of the local PWD.
Fixes #106661 (or rather the root issue behind it, which was misdiagnosed in the thread. The error was not happening because the OS couldn't find
dotnet
in PATH, but because thedotnet
folder in the home directory (which is usually set as the PWD when starting VSCode from an application menu) took priority over thedotnet
executable that would be found in PATH.To test that this issue is fixed:
python
python
)code .
in a directory which contains a sub directory with the same name as the executable (in this case,python
)test-task
in the sample extension I have uploaded)The terminal process failed to launch: Path to shell executable "python" is not a file or a symlink.
error.Also properly fixes #79346, which is the same issue.
Also fixes #83772
The steps to test this issue are the same as specified in the original issue (which can be replaced with any executable):
And then of course now verify that the terminal opens and there is no:
The terminal process failed to launch: Path to shell executable "dotnet" is not a file or a symlink.
error popup.