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

Quote launch paths when necessary #5101

Merged
merged 1 commit into from
Mar 10, 2022
Merged

Quote launch paths when necessary #5101

merged 1 commit into from
Mar 10, 2022

Conversation

333fred
Copy link
Member

@333fred 333fred commented Mar 9, 2022

Fixes #5099.

@333fred 333fred requested a review from JoeRobich March 9, 2022 22:42
@333fred 333fred merged commit a8df773 into master Mar 10, 2022
@333fred 333fred deleted the quote-path branch March 10, 2022 22:34
@vchirikov
Copy link

vchirikov commented Mar 21, 2022

Starting OmniSharp server at 21.03.2022, 13:00:03
    Target: c:\code\<some_path>\build\Build.sln

OmniSharp server started with .NET 6.0.300-preview.22154.4
.
    Path: c:\Users\<username>\.vscode\extensions\ms-dotnettools.csharp-1.24.2\.omnisharp\1.38.1-net6.0\OmniSharp.dll
    PID: 7684

Could not execute because the specified command or file was not found.
Possible reasons for this include:
  * You misspelled a built-in dotnet command.
  * You intended to execute a .NET program, but dotnet-"c:\Users\<username>\.vscode\extensions\ms-dotnettools.csharp-1.24.2\.omnisharp\1.38.1-net6.0\OmniSharp.dll" does not exist.
  * You intended to run a global tool, but a dotnet-prefixed executable with this name could not be found on the PATH.
  

ms-dotnettools.csharp-1.24.2\.omnisharp\1.38.1-net6.0\OmniSharp.dll exists.
"omnisharp.loggingLevel": "trace", doesn't give more information, but looks like it could be related
to this PR.
I will try to rollback to 1.24.1

--

Update:
with 1.24.1 works fine with v1.24.2-beta1 omnisharp doesn't start.

cc: @333fred ^^

@filipw
Copy link
Contributor

filipw commented Mar 21, 2022

yes, especially this looks very suspicious

dotnet-"c:\Users\<username>\.vscode\extensions\ms-dotnettools.csharp-1.24.2\.omnisharp\1.38.1-net6.0\OmniSharp.dll"

@333fred
Copy link
Member Author

333fred commented Mar 22, 2022

Hmm. I'm not sure where that - is coming from: this PR certainly doesn't add it in. I'm also not able to reproduce this on windows myself: 1.24.2-beta1 starts just fine for me. Can you share your omnisharp.path, and/or would you be willing to debug your installation a bit?

@vchirikov
Copy link

omnisharp.path isn't overridden (default value).
"omnisharp.useModernNet": true

dotnet --info:

.NET SDK (reflecting any global.json):
 Version:   6.0.300-preview.22154.4
 Commit:    1eb22793b6

Runtime Environment:
 OS Name:     Windows
 OS Version:  10.0.22000
 OS Platform: Windows
 RID:         win10-x64
 Base Path:   C:\Program Files\dotnet\sdk\6.0.300-preview.22154.4\

Host (useful for support):
  Version: 6.0.3
  Commit:  c24d9a9c91

.NET SDKs installed:
  6.0.300-preview.22154.4 [C:\Program Files\dotnet\sdk]

.NET runtimes installed:
  Microsoft.AspNetCore.App 6.0.2 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 6.0.3 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.NETCore.App 6.0.2 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 6.0.3 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.WindowsDesktop.App 6.0.2 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 6.0.3 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]

To install additional .NET runtimes or SDKs:
  https://aka.ms/dotnet-download

would you be willing to debug your installation a bit?

I didn't try to debug the omnisharp's vscode extension before. I can send any debug info that you need and maybe I will try to debug this by myself at the weekend.

@@ -408,7 +408,7 @@ function launchNix(launchPath: string, cwd: string, args: string[]): LaunchResul

function launchNixMono(launchPath: string, cwd: string, args: string[], environment: NodeJS.ProcessEnv, useDebugger: boolean): LaunchResult {
let argsCopy = args.slice(0); // create copy of details args
argsCopy.unshift(launchPath);
argsCopy.unshift(`$"{launchPath}"`);
Copy link

@vchirikov vchirikov Mar 22, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like a typo

Suggested change
argsCopy.unshift(`$"{launchPath}"`);
argsCopy.unshift(`"${launchPath}"`);

@333fred check this pls

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vchirikov no need to ping me so many times, I'm watching this issue :).

Yes, that does indeed look suspicious. However, this is launching mono on Linux or Unix, not anything to do with Windows.

@JoeRobich
Copy link
Member

JoeRobich commented Mar 24, 2022

yes, especially this looks very suspicious

dotnet-"c:\Users\<username>\.vscode\extensions\ms-dotnettools.csharp-1.24.2\.omnisharp\1.38.1-net6.0\OmniSharp.dll"

It is like System.CommandLine is parsing the omnisharp assembly path as a command name instead of a file argument. dotnet-* is the fallback for the dotnet CLI uses to invoke global tools as if they were subcommands of the CLI.

I cannot reproduce this behavior when trying to invoke with the same launch arguments from a command line.

That being said I am seeing this with the 1.24.2-beta2 build, so at the very least we should roll back quoting the launch path when starting the net6 build. Need to validate other launch types.

@JoeRobich
Copy link
Member

Even better. Passing shell: true in the spawn options causes the path to be parsed correctly. Opened #5125

@JoeRobich
Copy link
Member

Also, validated launching .NET Framework O# on Windows and system install of Mono work with the added quotes.

@vchirikov
Copy link

Confirmed, beta 3 with #5125 works fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Problem with OmniSharp...
4 participants