dotnet / vscode-dotnet-runtime

VSCode Extension for Installing .NET via VS Code
MIT License
139 stars 263 forks source link

dotnetAcquisitionExtension.sharedExistingDotnetPath should be environment specific #1952

Open dozer75 opened 1 month ago

dozer75 commented 1 month ago

Describe the bug.

Due to the environment I'm running Visual Studio Code has PowerShell restricted to be executed in Constrained Language Mode the .NET Install Tool fails to install a VS Code only .NET environment, and a suggestion (with auto fix functionality) is to set the dotnetAcquisitionExtension.sharedExistingDotnetPath setting to a custom dotnet path (which in my case will be "C:\\Program Files\\dotnet\\dotnet.exe" which works fine in Windows.

However, this settings will also affect some extension in WSL that uses this path to get the path to the dotnet executable in WSL, causing those extensions to fail (e.g. the Bicep extension which I have reported this as well in Azure/bicep#15098).

This path (and maybe other paths) should be possible to have different in Windows/WSL environments.

Please attach log file(s) here if you have any.

No log file on this.

Reproduction Steps

Repro steps:

  1. Have a Windows environment with WSL
  2. In the Visual Studio Code settings, set the dotnetAcquisitionExtension.sharedExistingDotnetPath to the Windows path to the dotnet executable.
  3. In WSL, open a Bicep folder in Visual Studio Code (with Bicep extension enabled for WSL) and an error stating Failed to find dotnet executable at path '/mnt/c/Users/USER/AppData/Local/Programs/Microsoft VS Code/C:\Program Files\dotnet\dotnet.exe'. is displayed.

Expected Behavior

Environment specific settings done in Windows should not affect WSL (and the other way around).

Exceptions & Errors (if any)

No response

.NET Install Tool Version

2.1.5

Your operating system? Please provide more detail, such as the version and or distro above.

Windows

VS Code Version

1.93.1

nagilson commented 1 month ago

This might actually get fixed in the latest version we plan to release this week. We are going to check whether the PATH works or not before using it, so I'd imagine this would be fixed via that.

dozer75 commented 1 month ago

This might actually get fixed in the latest version we plan to release this week. We are going to check whether the PATH works or not before using it, so I'd imagine this would be fixed via that.

@nagilson I am a bit unsure how checking if the path works are going to fix this? And what happens if it doesn't work, what's the fallback? Not knowing exactly what you mean, my worries is that if there is only one path configuration for both Windows and WSL based Code environments, it will be an issue if you need to have custom paths in both? 🤔

But, let's see if that fix you fixes this as well!

nagilson commented 1 month ago

It will cause it not to fail, because it will not use the setting, and instead it will download a runtime, which should succeed. However, youre right in that it does not give you two options for the different environments. Is it possible for you to open a different vscode workspace file with a different settings file for ubuntu development? This may be the best option and I dont see why it wouldnt work.

Adding @baronfel for a PM opinion on having an option for 2 custom paths per environment.

dozer75 commented 1 month ago

@nagilson Ok, as long as it downloads if it fails, it's ok for my scenario since it is in Windows/PowersShell we have restrictions, WSL is not protected by any such rules, so we can set custom path in Windows and then let default logic happen in WSL... However since Microsoft has added (or are about to at least) AppLocker support for WSL that may change...

As for your other question, I'm not quite sure what you are saying here.. But yes, if we have a workspace based settings file with the path it maybe work, but then you will need to have this in multiple locations if you have multiple projects/workspaces... I'm not sure if that is a good workaround, but it is probably a workaround.

nagilson commented 1 month ago

The new version is out, let us know if it was helpful or not. Youre right that the powershell restriction is in place but -- I dont believe our extension logic would ever call into powershell on linux so that shouldnt be a problem on WSL.

dozer75 commented 1 month ago

@nagilson

Sorry for delayed answer, but I tested it today, but still get the issue in VS Code...

Failed to find dotnet executable at path '/mnt/c/Users/<USER>/AppData/Local/Programs/Microsoft VS Code/C:\Program Files\dotnet\dotnet.exe'.

The .NET install tool output just states Using configured .NET path: C:\Program Files\dotnet\dotnet.exe.

Here is my config:

{
    "git.enableSmartCommit": true,
    "diffEditor.ignoreTrimWhitespace": false,
    "dotnetAcquisitionExtension.enableTelemetry": false,
    "dotnetAcquisitionExtension.existingDotnetPath": [

    ],
    "dotnetAcquisitionExtension.sharedExistingDotnetPath": "C:\\Program Files\\dotnet\\dotnet.exe"
}

Version information:

Version: 1.94.0 (user setup)
Commit: d78a74bcdfad14d5d3b1b782f87255d802b57511
Date: 2024-10-02T13:08:12.626Z
Electron: 30.5.1
ElectronBuildId: 10262041
Chromium: 124.0.6367.243
Node.js: 20.16.0
V8: 12.4.254.20-electron.0
OS: Windows_NT x64 10.0.22631
dozer75 commented 1 month ago

@nagilson Sorry, ignore the comment above. I realized the minute I pressed comment that I had forgotten to reload the VS code after extension update. Now I got a warning instead when I started stating

The 'existingDotnetPath' setting was set, but it did not meet the requirements for this extension to run properly.
This setting has been ignored.
If you would like to continue to use the setting anyways, set dotnetAcquisitionExtension.allowInvalidPaths to true in the .NET Install Tool Extension Settings.
Extension: ms-azuretools.vscode-bicep

According to the output of .NET install tool it seems to work correct...

It is however annoying that the warning comes all the time 😕

nagilson commented 1 month ago

Yeah, I had made a checkbox to get rid of the warning but I merged it into the option to ignore the setting which was actually a poor idea. At the very least we could add that!