vscode-csharp icon indicating copy to clipboard operation
vscode-csharp copied to clipboard

Fix Visual Studio Code build task give 'The terminal shell path “dotnet” is a directory'

Open dauphine-dev opened this issue 6 years ago • 21 comments

Fix Visual Studio Code build task give 'The terminal shell path “dotnet” is a directory' See: https://stackoverflow.com/questions/58072505/visual-studio-code-build-task-give-the-terminal-shell-path-dotnet-is-a-direct/5

dauphine-dev avatar Oct 24 '19 12:10 dauphine-dev

CLA assistant check
All CLA requirements met.

dnfclas avatar Oct 24 '19 12:10 dnfclas

This seems reasonable to me, but I am not sure if there is any downside of running dotnet instead a shell instead of a process?

gregg-miskelly avatar Oct 24 '19 22:10 gregg-miskelly

I run dotnet as shell, and until now I haven't detected any downside. And also I run often directly in shell console too, and it works well.

dauphine-dev avatar Oct 25 '19 12:10 dauphine-dev

@dbaeumer this PR is proposing to change the tasks.json tasks generated by the C# extension to use shell instead of process because process does the "wrong" thing if there happens to be a directory with the same name as the process we are launching. Are you aware of anything we should be careful to test or any extra downside of using shell? I am assuming it is ever so slightly slower since now the shell has to spin up, but that perf penalty should be small?

gregg-miskelly avatar Oct 25 '19 17:10 gregg-miskelly

@gregg-miskelly besides spinning up the shell there is a different interpretation of the arguments. In process they go directly to the process and with shell they are subject to shell interpretation logic.

What is the underlying reasons of having process fail in this setup.

And in general you should use task providers now and not adding tasks to the tasks.json file.

dbaeumer avatar Oct 28 '19 08:10 dbaeumer

@dbaeumer The underlying problem with process is this -- we are trying to execute the dotnet executable. With process if PATH contains a directory that has a sub directory named dotnet then the process code will try to execute that subdirectory instead of ignoring it and continuing the PATH search to find the executable. The path searching code inside of (most?) shells seems to be a bit smarter than this and will ignore directories.

For using task providers instead -- you are suggesting this as a way of simplifying the tasks.json content as well as making it possible to change without requiring users to regenerate their tasks.json file? In our case, if I understand things correctly, I believe users would still need a tasks.json file since users need to specify the project file to build. But certainly we could use task providers to make this a bit simpler.

gregg-miskelly avatar Oct 28 '19 18:10 gregg-miskelly

@gregg-miskelly I assume this only happens under Windows :-) since there we have our own code to find the executable. The problem lies here:

https://github.com/Microsoft/vscode/blob/master/src/vs/base/node/processes.ts#L467

This should not do an exist. It should do a stat and then check if it is a file.

I opened https://github.com/microsoft/vscode/issues/83509 and we should try to fix this for October.

dbaeumer avatar Oct 29 '19 08:10 dbaeumer

@dbaeumer For information, I am running Visual Studio Code under Linux (Mint 19.2 Tina / Cinnamon)

dauphine-dev avatar Oct 29 '19 12:10 dauphine-dev

@alexr00 do we execute the code under Linux as well ?

dbaeumer avatar Oct 30 '19 11:10 dbaeumer

It looks like the terminal might be. @dauphine-dev could you install the latest Insiders build of VS Code and see if you're still seeing the issue?

alexr00 avatar Oct 30 '19 13:10 alexr00

@alexr00 With latest Insiders build (1.40.0-insider), I have still the issue, and I get that:

> Executing task: dotnet build /home/david/test/test.csproj /property:GenerateFullPaths=true /consoleloggerparameters:NoSummary <

The terminal shell path "dotnet" is a directory

Terminal will be reused by tasks, press any key to close it.

dauphine-dev avatar Oct 30 '19 17:10 dauphine-dev

@dauphine-dev what's the date or commit hash of the VS Code version you're using? I'm able to repro on stable, but not on today's Insiders build, using this task config (where my dotnet.exe is at C:\Program Files\dotnet\dotnet.exe):

        {
            "label": "build",
            "command": "dotnet",
            "options": {
                "env": {
                    "PATH": "C:\\Program Files;C:\\Program Files\\dotnet"
                }
            },
            "type": "process",
            "args": [
                "build",
                "${workspaceFolder}/my.csproj", // replace this
                "/property:GenerateFullPaths=true",
                "/consoleloggerparameters:NoSummary"
            ],
            "problemMatcher": "$msCompile"
        },

connor4312 avatar Oct 30 '19 21:10 connor4312

@connor4312

From the about menu:

Version: 1.40.0-insider
Commit: 31f577ec88dc96ad2028699fb597b19022224b46
Date: 2019-10-30T05:32:25.621Z
Electron: 6.1.2
Chrome: 76.0.3809.146
Node.js: 12.4.0
V8: 7.6.303.31-electron.0
OS: Linux x64 4.15.0-66-generic

I will try again the .deb installer from today.

Edit: I have just test with the last version, and I still have the issue.

Version:

Version: 1.40.0-insider
Commit: 93ee2fc3121b7f66ddf568c051f3bfff7db8d618
Date: 2019-10-31T06:28:32.160Z
Electron: 6.1.2
Chrome: 76.0.3809.146
Node.js: 12.4.0
V8: 7.6.303.31-electron.0
OS: Linux x64 4.15.0-66-generic

tasks.json

{
    "version": "2.0.0",
    "tasks": [
        {
            "label": "build",
            "command": "dotnet",
            "type": "process",
            "args": [
                "build",
                "${workspaceFolder}/test1.csproj",
                "/property:GenerateFullPaths=true",
                "/consoleloggerparameters:NoSummary"
            ],
            "problemMatcher": "$msCompile"
        },
        {
            "label": "publish",
            "command": "dotnet",
            "type": "process",
            "args": [
                "publish",
                "${workspaceFolder}/test1.csproj",
                "/property:GenerateFullPaths=true",
                "/consoleloggerparameters:NoSummary"
            ],
            "problemMatcher": "$msCompile"
        },
        {
            "label": "watch",
            "command": "dotnet",
            "type": "process",
            "args": [
                "watch",
                "run",
                "${workspaceFolder}/test1.csproj",
                "/property:GenerateFullPaths=true",
                "/consoleloggerparameters:NoSummary"
            ],
            "problemMatcher": "$msCompile"
        }
    ]
}

Issue:

> Executing task: dotnet build /home/david/test1/test1.csproj /property:GenerateFullPaths=true /consoleloggerparameters:NoSummary <

The terminal shell path "dotnet" is a directory

dauphine-dev avatar Oct 31 '19 08:10 dauphine-dev

I eventually managed to reproduce this, somehow, but it's quite frustrating. I got it to happen on my Insiders built, and it now happens consistently there. But then when opening the same workspace with the OSS build (where I can actually debug), even on the same commit that Insiders was built from, I don't run into the issue, and I can't seem to get it to repro again. Trace-level logs in vscode aren't very useful, and auditd doesn't provide much useful information here. It does appear to be an issue somewhere in vscode itself, rather than this extension.

[2019-10-31 09:00:12.334] [renderer1] [trace] terminalInstance#ctor (id: 1) {"name":"Task - build","executable":"dotnet","args":["build","/home/connor/Downloads/TestApp/TestApp.csproj","/property:GenerateFullPaths=true","/consoleloggerparameters:NoSummary"],"waitOnExit":"Terminal will be reused by tasks, press any key to close it.","initialText":"\u001b[1m> Executing task: dotnet build /home/connor/Downloads/TestApp/TestApp.csproj /property:GenerateFullPaths=true /consoleloggerparameters:NoSummary <\u001b[0m\n","cwd":{"$mid":1,"path":"/home/connor/Downloads/TestApp","scheme":"file"},"env":{"DOTNET_ROOT":"/home/connor/Downloads/TestApp","PATH":"/home/connor/Downloads/TestApp:/usr/bin"}}

connor4312 avatar Oct 31 '19 16:10 connor4312

Does <termcwd>/dotnet happen to exist?

Relevant code: https://github.com/microsoft/vscode/blob/14e2178b7f204c99a860cd92425408be058f45a2/src/vs/workbench/contrib/terminal/node/terminalProcess.ts#L85-L99

Tyriar avatar Oct 31 '19 16:10 Tyriar

I think that's it. I had created ~/dotnet at one point while fiddling around, but I didn't reference it in the task.json at all so I assumed it was irrelevant a few minutes later when I got a reproduction. It looks like I can consistently repro when ~/dotnet is present, and cannot do so if I remove the folder. Launching OSS from VS Code, apparently, sets the termcwd differently.

connor4312 avatar Oct 31 '19 16:10 connor4312

~~The task system should really be resolving that before it hands over to the terminal. I'll try that repro.~~ Nope, it shouldn't we only find executable for windows. @Tyriar, is this something you want to change in the terminal, or is this expected behavior?

alexr00 avatar Oct 31 '19 16:10 alexr00

@alexr00 it's a bug in the terminal too though; if it's a dir it should still check the path not fail there. https://github.com/microsoft/vscode/issues/83772

Tyriar avatar Oct 31 '19 16:10 Tyriar

@Tyriar makes sense!

Summary: The issue occurred when the command of a process task was also the name of a folder. It is fixed in VS Code Insiders for Windows and that fix will also be in the 1.40 stable release. On non-Windows, the issue still occurs when you have a folder in your project with the same name as the task command. The fix for non-Windows will not be in 1.40, but will probably come soon.

alexr00 avatar Nov 01 '19 08:11 alexr00

On non-Windows, the issue still occurs when you have a folder in your project with the same name as the task command.

I am not sure about that. This is the tree of my folder:

.
├── bin
│   └── Debug
│       └── netcoreapp3.0
├── obj
│   ├── Debug
│   │   └── netcoreapp3.0
│   │       ├── test1.AssemblyInfo.cs
│   │       ├── test1.AssemblyInfoInputs.cache
│   │       ├── test1.assets.cache
│   │       └── test1.csprojAssemblyReference.cache
│   ├── project.assets.json
│   ├── test1.csproj.nuget.cache
│   ├── test1.csproj.nuget.dgspec.json
│   ├── test1.csproj.nuget.g.props
│   └── test1.csproj.nuget.g.targets
├── Program.cs
└── test1.csproj

And my task.json:

{
    "version": "2.0.0",
    "tasks": [
        {
            "label": "build",
            "command": "dotnet",
            "type": "process",
            "args": [
                "build",
                "${workspaceFolder}/test1.csproj",
                "/property:GenerateFullPaths=true",
                "/consoleloggerparameters:NoSummary"
            ],
            "problemMatcher": "$msCompile"
        },
        {
            "label": "publish",
            "command": "dotnet",
            "type": "process",
            "args": [
                "publish",
                "${workspaceFolder}/test1.csproj",
                "/property:GenerateFullPaths=true",
                "/consoleloggerparameters:NoSummary"
            ],
            "problemMatcher": "$msCompile"
        },
        {
            "label": "watch",
            "command": "dotnet",
            "type": "process",
            "args": [
                "watch",
                "run",
                "${workspaceFolder}/test1.csproj",
                "/property:GenerateFullPaths=true",
                "/consoleloggerparameters:NoSummary"
            ],
            "problemMatcher": "$msCompile"
        }
    ]
}

Commands are dotnet and I haven't any "dotnet" folder

dauphine-dev avatar Nov 01 '19 12:11 dauphine-dev

This should now be fixed for non-windows systems as well in the next insider build: https://github.com/microsoft/vscode/pull/158666.

The issue only occured when the process PWD contained the folder with the same name as the command, which isn't always necessarily the project directory, and might for example be the user's home directory instead.

tobil4sk avatar Sep 15 '22 23:09 tobil4sk

it's still an issue.

georgedimac avatar Dec 12 '22 09:12 georgedimac