-
Notifications
You must be signed in to change notification settings - Fork 376
Add dotnetSDKPath Setting for User SDKs and Projects
#2354
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
base: main
Are you sure you want to change the base?
Conversation
The .NET `dotnet.findPath` API provided by the .NET Install Tool only uses machine wide SDKs at this time. This API is used by C# and C#DK, as well as others, to resolve the .NET SDK to use for the project system. We can update this to better enable local SDK or custom SDK scenarios. In general, the PATH can be updated to an admin 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. 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. 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`. 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 an SDK), 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. 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 code is very quickly written, and I haven't looked at the formatting yet to make sure it renders properly. Basically more validation needs to be done. That's OK.
|
|
IS it possible to set this using a env variable ? My case is we have a script that downloads the sdk and workloads to a particular location on my Mac, then I want to use a solution on VSCODE but pointing to the sdk location I provisioned. |
|
I dont believe that it would work via environment variables - instead it would need to be set at a launch.json or workspace level configuration file for Code. |
We decided to do this for the runtimes, may as well do it for the SDK as well.
|
Since the future of |
|
Seems that will be supported better going forward due to dotnet/runtime#118249, so we should consider whether we want this feature with the |
Resolves microsoft/vscode-dotnettools#792
The .NET
dotnet.findPathAPI provided by the .NET Install Tool only uses machine wide SDKs at this time when searching under thesdkmode.To understand our API better, please see the original PR description if you're not familiar:
#1954
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,
dotnetPathwas 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.jsonis not yet supported or parsed by our API. This setting would overrule thepathssetting inglobal.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
findPathwas 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 onglobal.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.