Check DotnetRuntimeExtensionResolver for DOTNET_ROOT #6567

WardenGnaw · 2023-10-18T22:43:31Z

This PR adds in a check to obtain the DOTNET_ROOT from DotnetRuntimeExtensionResolver.
This should be using the same dotnet as the roslyn LSP.

The future work for this would be a global API from the runtime acquisition and centralize how C# Extension would obtain the dotnet cli path and we can remove the getDotNetInfo utils methods.

This PR adds in a check to obtain the DOTNET_ROOT from DotnetRuntimeExtensionResolver. The future work for this would be a global API from the runtime acquisition and centralize how C# Extension would obtain the dotnet cli path and we can remove the getDotNetInfo utils methods.

dibarbet · 2023-10-18T22:47:56Z

src/coreclrDebug/activate.ts

@@ -278,6 +281,14 @@ export class DebugAdapterExecutableFactory implements vscode.DebugAdapterDescrip
        // use the executable specified in the package.json if it exists or determine it based on some other information (e.g. the session)
        if (!executable) {
            const dotNetInfo = await getDotnetInfo(omnisharpOptions.dotNetCliPaths);
+            const hostExecutableResolver = new DotnetRuntimeExtensionResolver(


Note - this currently only cares about the installed runtimes. It does not confirm there is an SDK. It will also only install a 7.0 runtime. Is that what you expect?

I think this is the best we have for now until the .NET SDK installation API is avaliable to use.

This is to just workaround the scenario where dotnet exists in the PATH and we can hope there is an SDK.

Do you require a .net 7 SDK or higher? Or can you use a .net 6 SDK? Because if oyu only have a .net 6 SDK installed on the path, this will return a locally installed .net 7 runtime (with no SDK information, you won't be able to see the .net 6 SDK from it)

What Andrew is trying to do is find a version of .NET for the target app. The debugger itself will ignore DOTNET_ROOT and run on its own runtime.

It would be whatever the user's target framework would be.

The scenario is that the user was able to build their application with dotnet build and I was hoping this API would help me grab the dotnet install path to set as DOTNET_ROOT.

E.g. ~/.dotnet

So if the user's app targets .net6 and they only have .net6 SDK installed, this API will not return the path to the dotnet 6 installation. It will return a locally installed .net7 runtime path. Just want to make sure you know what to expect from this API.

See https://github.com/dotnet/vscode-csharp/pull/6567/files#r1364688699

Maybe instead what should happen is getDotnetInfo should return the real CLI path and the set of SDKs that are installed at that path (via dotnet --list-sdks) - and then you use that.

Because DotnetRuntimeExtensionResolver is really focused on making sure the runtime to host the server processes is available.

gregg-miskelly · 2023-10-18T23:11:49Z

src/coreclrDebug/activate.ts

+            if (!dotnetRoot) {
+                if (dotNetInfo.CliPath) {
+                    dotnetRoot = path.dirname(dotNetInfo.CliPath);
+                } else if (hostExecutableInfo.path) {


if (hostExecutableInfo.path) {

I think you need some more checks here:

If the architecture of the debugger != the architecture of this runtime, we don't want to use it

If there already is a global .NET runtime, we don't want to use it

If the architecture of the debugger != the architecture of this runtime, we don't want to use it

We might need to re-write getHostExecutableInfo to get the correct architecture. Is it worth it when we will be throwing away this code for the NET SDK installation API?

If there already is a global .NET runtime, we don't want to use it

Whats the best way to check for this? I thought this was covered with process.env.DOTNET_ROOT

We might need to re-write getHostExecutableInfo to get the correct architecture.

I think you just want to ignore it/not compute it if we are trying to get a different architecture. In other words, if you want to target x64, but you are on an ARM64 mac, you need to download the the .NET Runtime yourself

Whats the best way to check for this? I thought this was covered with process.env.DOTNET_ROOT

Normally we wouldn't use DOTNET_ROOT if there was a globally installed .NET SDK. I would think you would want to exec dotnet --version and if that succeeded: we use the global version.

WardenGnaw · 2023-10-18T23:12:08Z

src/coreclrDebug/activate.ts

@@ -278,6 +281,14 @@ export class DebugAdapterExecutableFactory implements vscode.DebugAdapterDescrip
        // use the executable specified in the package.json if it exists or determine it based on some other information (e.g. the session)
        if (!executable) {
            const dotNetInfo = await getDotnetInfo(omnisharpOptions.dotNetCliPaths);
+            const hostExecutableResolver = new DotnetRuntimeExtensionResolver(
+                this.platformInfo,
+                () => _session?.configuration?.executable,


Suggested change

() => _session?.configuration?.executable,

() => _session?.configuration?.program

gregg-miskelly · 2023-10-18T23:14:23Z

Andrew, do you know how this is supposed to work? Do you know if there is some documentation?

A few specific questions:

Do the various extensions acquire a single .NET SDK and runtime?
The expectation is that if there isn't a global .NET, the target app will run on the same runtime as the extension?
Assuming the answer to Q2 is 'yes', one side effect is that as soon as we move the language server to a new version of .NET, all older projects will be broken (since they will still be targeting .NET 7). Did the designers of this plan consider this?

dibarbet · 2023-10-18T23:27:31Z

src/coreclrDebug/activate.ts

+            // Look to see if DOTNET_ROOT is set, then use dotnet cli path for omnisharp, then hostExecutableInfo
+            let dotnetRoot: string | undefined = process.env.DOTNET_ROOT;
+            if (!dotnetRoot) {
+                if (dotNetInfo.CliPath) {


Be careful with the clipath here - I believe it comes from here - https://github.com/dotnet/vscode-csharp/blob/main/src/shared/utils/getDotnetInfo.ts#L22

which will return 'dotnet.exe' unless you pass in explicit CLI paths to getDotnetInfo - it looks like you pass in the clipaths option, so I think it will only return a real path if that option has a value.

IMHO getDotnetInfo should probably actually return the real dotnet installation exe path in all cases, but I've been afraid of making that change (not sure what else will break 😆 )

Yeah. This normally worked with omnisharp beacuse I think they set the clipaths but this changed when it was all moved to roslyn.

WardenGnaw · 2023-10-19T16:13:53Z

Andrew, do you know how this is supposed to work? Do you know if there is some documentation?

A few specific questions:

Do the various extensions acquire a single .NET SDK and runtime?

The expectation is that if there isn't a global .NET, the target app will run on the same runtime as the extension?

Assuming the answer to Q2 is 'yes', one side effect is that as soon as we move the language server to a new version of .NET, all older projects will be broken (since they will still be targeting .NET 7). Did the designers of this plan consider this?

For Runtime, Yes. Theres a .NET Runtime install tool that the extension use. For SDK, I think its in progress @nagilson should have more information on this.
I don't think so, I think users should have their own SDK / runtime? If its not gobally installed, should we reccomend users to just set omnisharpOptions.dotNetCliPaths?
Skipping because I think its a no.

gregg-miskelly · 2023-10-19T16:29:51Z

Broadly speaking, I think there are two ways that we could solve the problem of acquiring a .NET SDK and/or Runtime for the app to use, and I don't know what one was selected --

Option 1: VS Code extension(s) install the .NET SDK and/or Runtime(s) that we think users are most likely to target

This is obviously what VS does, though VS installs the SDK globally. I don't entirely understand the VS behavior, and how similar it is to the VS Code behavior -- if VS switches from installing say, .NET 6 to .NET 7, does it remove .NET 6? Will this work with the way VS Code is installing them?

In an ideal world, under this option, I think the debugger should really be doing nothing. Otherwise, one can F5/Ctrl-F5 from VS code, but if one just opens a terminal, the app will no longer run. At the very least, I think we need documentation somewhere for what a user should do.

Option 2: User should be guided to install the .NET Runtime and/or SDK the "normal way"

In this option, we add some basic detection for understanding if there is a .NET runtime installed, and if that fails, we point them at instructions on how to install the runtime or SDK themselves. The nice thing about this is that the user is in charge of picking the right SDK/runtime, as there are many options and the one we pick may not be right for the user.

dibarbet · 2023-10-19T17:49:21Z

Broadly speaking, I think there are two ways that we could solve the problem of acquiring a .NET SDK and/or Runtime for the app to use, and I don't know what one was selected --

I can explain the current expectations from the language server / project side. Note that there are two distinct parts -

Acquiring the dotnet runtime used to start the server process - this is the code in DotnetRuntimeExtensionResolver
1. We first check if there is an installed runtime with high enough version (.NET 7 or above) and the right architecture on the PATH, if so, we use the runtime from the dotnet on path to launch the server.
2. If there is no matching runtime, we download a local runtime using the .net acquisition extension. This is only the runtime, and has no ties to any globally installed SDKs or runtimes.
Acquiring the dotnet SDK needed to load the project
1. Currently we rely on the dotnet SDK being on the path. We do not do any acquisition of SDKs. We use MSBuildLocator on the server side to find it.

However this may be changing once the .net runtime acquisition extension supports acquiring SDKs. I'm not 100% sure what the plans are there (I haven't seen the latest details).

nagilson · 2023-10-19T17:53:57Z

We are releasing the API to acquire a Global .NET SDK using the '.NET Runtime Install Tool', which will become the '.NET Install Tool' by the end of next week. C#/CDK will be able to call into that API to have us acquire a .NET SDK (versioned like so: 'major' like '6', major.minor like '3.1', feature band, such as '7.0.3xx' or specified version, like '7.0.103'.

Work is starting on CDK side to consume this new API and add a UI layer.

WardenGnaw · 2023-10-19T20:36:25Z

I think it would be best to hold off on these changes and utilize omnisharpOptions.dotNetCliPath for folks who do not install dotnet globally.

WardenGnaw added the Debugger label Oct 18, 2023

WardenGnaw requested review from gregg-miskelly and nagilson October 18, 2023 22:43

WardenGnaw self-assigned this Oct 18, 2023

WardenGnaw requested a review from a team as a code owner October 18, 2023 22:43

dibarbet reviewed Oct 18, 2023

View reviewed changes

gregg-miskelly reviewed Oct 18, 2023

View reviewed changes

WardenGnaw commented Oct 18, 2023

View reviewed changes

dibarbet reviewed Oct 18, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Check DotnetRuntimeExtensionResolver for DOTNET_ROOT #6567

Check DotnetRuntimeExtensionResolver for DOTNET_ROOT #6567

WardenGnaw commented Oct 18, 2023

dibarbet Oct 18, 2023 •

edited

Loading

WardenGnaw Oct 18, 2023

dibarbet Oct 18, 2023

gregg-miskelly Oct 18, 2023

WardenGnaw Oct 18, 2023

dibarbet Oct 18, 2023

dibarbet Oct 18, 2023

gregg-miskelly Oct 18, 2023

WardenGnaw Oct 18, 2023 •

edited

Loading

gregg-miskelly Oct 18, 2023 •

edited

Loading

gregg-miskelly Oct 18, 2023

WardenGnaw Oct 18, 2023

gregg-miskelly commented Oct 18, 2023

dibarbet Oct 18, 2023 •

edited

Loading

WardenGnaw Oct 19, 2023

WardenGnaw commented Oct 19, 2023

gregg-miskelly commented Oct 19, 2023

dibarbet commented Oct 19, 2023 •

edited

Loading

nagilson commented Oct 19, 2023

WardenGnaw commented Oct 19, 2023

	() => _session?.configuration?.executable,
	() => _session?.configuration?.program

Check DotnetRuntimeExtensionResolver for DOTNET_ROOT #6567

Are you sure you want to change the base?

Check DotnetRuntimeExtensionResolver for DOTNET_ROOT #6567

Conversation

WardenGnaw commented Oct 18, 2023

dibarbet Oct 18, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

WardenGnaw Oct 18, 2023 • edited Loading

Choose a reason for hiding this comment

gregg-miskelly Oct 18, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gregg-miskelly commented Oct 18, 2023

dibarbet Oct 18, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

WardenGnaw commented Oct 19, 2023

gregg-miskelly commented Oct 19, 2023

dibarbet commented Oct 19, 2023 • edited Loading

nagilson commented Oct 19, 2023

WardenGnaw commented Oct 19, 2023

dibarbet Oct 18, 2023 •

edited

Loading

WardenGnaw Oct 18, 2023 •

edited

Loading

gregg-miskelly Oct 18, 2023 •

edited

Loading

dibarbet Oct 18, 2023 •

edited

Loading

dibarbet commented Oct 19, 2023 •

edited

Loading