-
Notifications
You must be signed in to change notification settings - Fork 381
Description
The .NET dotnet.findPath API provided by the .NET Install Tool only uses machine wide SDKs at this time when searching under the sdk mode.
Resolved by #2354
To understand our API better, please see the original PR description if you're not familiar:
#1954
We could add a setting to VS Code described below to enable finding a custom SDK more easily.
However, we should close this in favor of parsing the paths in global json if we decide it's worse off to have 2 sources of truth. I lean in favor of closing this, but it will take a lot more time before #2377 is achieveable.
Explanation
This API is used by C# and C#DK, as well as others, to resolve the .NET SDK for the project system and other dotnet services. (It also resolves the runtime.) We can update this to better enable local SDK or custom SDK scenarios. In general, the PATH can be updated to a specific SDK, but users may get confused if they have multiple SDKs on the PATH, e.g. with multiple lines in their rc profile, which has happened internally (even with @jaredpar). In addition, DOTNET_ROOT and other variables impact the selection of DOTNET, and this may further complicate the issue, but DOTNET_ROOT is only applicable to the Runtime selection.
Historical Context
In the past, dotnetPath was supported by C#/OmniSharp but it was deprecated. I'm unsure of the name 'dotnetSDKPath' : unfortunately we can't change the old setting, existingDotnetPath.
This does not improve the scenario where global.json is not yet supported or parsed by our API. This setting would overrule the paths setting in global.json. We would like to 'light up' that scenario in the future.
Reasoning
The .NET Install Tool is an appropriate place for the setting given the API is already used by many extensions in the ecosystem. It is important to unify the logic, so differences and bugs/edge cases not considered by other callers do not remain, which was a problem with the prior fragmented logic and why findPath was implemented in the first place, as there was no completely correct implementation, and no shared implementation. I believe the test explorer has their own logic at this time, due to their dependence on global.json.
Settings
Caveats:
existingDotnetPath: Is a path to the dotnet.exe that should have runtime beside it, and is used to run the code that is shipped within C#/C#DK and other extensions.
dotnetSDKPath: Is a path to the dotnet.exe that should have an SDK beside it (and therefore also a runtime), that should be used by the project system and CPS to run the users project. We may need to have further work from the debugger team or coordinate with them as they may have their own setting.
Concerns
This does allow a user to override the system PATH and use a local SDK which may not be secure or up to date in an enterprise scenario. However, VS Code generally does not run in an elevated context and suggests against doing so. We should ensure that elevation is not leveraged by the internal calling APIs, and I dont believe that is the case.
The existing path setting for the runtime validates the path exists on disk and is sufficient for C# / CDK or whoever else who calls our API to run under. I did not add that validation to the SDK setting. The validation can be turned off for the runtime setting. But I assume users who use this would strictly want to force using their specified SDK no matter what.