search

microsoft / vscode-dotnettools

This is a feedback repository to capture issues logged for the C# Dev Kit and related extensions from Visual Studio Code
Other
216 stars 9 forks source link

PATH, dotnet.dotnetPath, dotnetAcquisition.existingDotnetPath Usage is not Standardized #1243

Open nagilson opened 2 months ago

nagilson commented 2 months ago

Describe the Issue

dotnet.dotnetPath is a setting in the C# extension. This is only for the language server. dotnetAcquisitionExtension.ExistingDotnetPath and dotnetAcquisitionExtension.sharedExistingDotnetPath are settings in the .NET Install Tool. This is returned to api callers as the dotnet to use when acquisition is called.

Both settings apply to the SDK and Runtime.

However, ATM they dont take effect in many parts of CDK or C#. For example, CDK does not check the existing Path setting if a PATH setting exists.

The PATH lookup is very different across the product, within C#, C# DevKit, and other places, the dotnet.dotnetPath setting, existingDotnetPathSetting, PATH, etc, can all take different priority and work with different levels of effectiveness in different This has led to a lot of customer confusion.

(Examples: [BUG] C# Dev Kit does not respect local .NET installation configurations · Issue #1094 · microsoft/vscode-dotnettools (github.com) and [BUG] C# dev kit should respect the dotnet.dotnetPath set by C# extension · Issue #792 · microsoft/vscode-dotnettools (github.com), [BUG] dotnet.dotnetpath does not support relative paths (unlike omnisharp.dotnetpath) · Issue #53 · microsoft/vscode-dotnettools (github.com)).

We documented the manual path setting (here and here and here).

This is a very confusing UX. I think we need to find all of the places in C# and C# DevKit where these lookups happen. I can think of a few places but I don't know them all. I know @peterwald added one for the test explorer and @AArnott did for CDK.

Some people think the PATH should take precedence over the setting so the terminal behavior is the same. Other people suggest the setting is more user friendly and if you set the setting, you probably want it to take over the PATH. Alternatively, we could make the setting edit the PATH to satisfy both constraints.

In addition, we need to decide if these settings should impact both sdk and runtime lookup. Some suggest the runtime should use the setting and the SDK should not, and use the PATH. Not many people understand what the difference between dotnet.exe and the runtime or sdk is, and how they get selected, so this may be a bit moot.

Steps To Reproduce

No response

Expected Behavior

No response

Environment Information

cc @arkalyanms No response

tmeschter commented 2 months ago

Tagging area-project-cps as I believe the project system server uses one or more of these settings.

AArnott commented 1 month ago

To summarize my own thoughts:

Runtime

TLDR: I believe the runtime selection should be influenced by VS Code settings to be consistent across Microsoft extensions, insofar as the user-selected runtime is compatible with what the extension requires. We should document the behavior when an incompatible runtime is found there.

Runtime for the extension

The runtime that C# Dev Kit uses is an implementation detail of the extension and the user shouldn't be setting it at all, IMO. While it used to be a private runtime installation, customers complained (time to install, offline usage, etc.) so we added the ability to use a machine-wide installation of the runtime if one were found that was compatible. We still fallback to the private runtime when necessary. If this setting provides a path to a compatible runtime, I guess I don't object to using it (wherever in the priority list makes the most sense to be consistent across all extensions). But we'd need to get clear on failover behavior when the runtime specified there isn't compatible.

Runtime to execute the customer's code

I believe this setting should have no impact on how we select the runtime that is used on F5 to run the customer's code. But I'm open to discussion on this if folks feel differently.

Runtime to execute customer tests

Because we run dotnet test which only ships with the SDK, I believe this is actually a choice we don't make here. See the SDK section.

SDK

TLDR: I believe SDK discovery and selection should be entirely independent of any VS Code setting.

I suspect these settings were supposed to point to a runtime -- not an SDK. If that's true, there's no guarantee there's even an SDK there. And if there were an SDK there, compatibility concerns remain, but it's more complicated.

C# Dev Kit strives to work with the user's chosen SDK version (as set by global.json, or by the latest version installed if not specified in global.json). We limit our supported SDK versions to just those that .NET itself supports. So today we support .NET 6 and 8 (not 7), and we presumably support .NET 9 prereleases.

The .NET team already provides a few means to find the .NET SDK to be used for command line builds, and they are working to introduce more to support repo-local SDK directories. I believe C# Dev Kit should honor and reuse these conventions rather than supplant or augment them with new ones based on VS Code settings. C# Dev Kit internally operates by spawning any of several dotnet CLI commands, making the PATH relevant, just as it would if the user typed it out. In fact our guidance has been to emit such commands to the output window so that users can learn from what the extension is doing and choose to do it themselves at the CLI if they want. This works best if in fact our behavior can be reproduced in the VS Code Terminal or a Developer Prompt. Anything we do to use a different SDK than what the user would get with a dotnet command at the CLI detracts from that. It would also confuse the story that the .NET team may tell about how to set/influence the SDK choice.

For reference, the .NET SDK selection at the CLI is comprised of a few environment variables including PATH, DOTNET_ROOT, and DOTNET_MULTILEVEL_LOOKUP. There are more. To influence how the dotnet SDK commands work in the VS Code terminal, these environment variables must be set before spawning VS Code, or they must be set directly in the same VS Code Terminal that the user interacts with. No VS Code setting can influence the behaviors here, so if we want the SDK used by VS Code to be the same as that in the terminal, we must embrace standard SDK lookup behaviors.