vscode
vscode copied to clipboard
microsoft
shell: enable environment discovery on Windows
The current unix stuff works mostly unchanged for resolving shells on Windows. Our logic for shell discovery currently always returns the powershell install on Windows, but I left the generic logic in place in the event that is ever expanded for other shells.
Closes #249130
@connor4312
We still reference or check for macOS/Linux in these places:
https://github.com/microsoft/vscode/blob/08e2cba35a1c39b3d321b73e102f6a88b47907f5/src/vs/base/parts/sandbox/electron-sandbox/globals.ts#L80
https://github.com/microsoft/vscode/blob/08e2cba35a1c39b3d321b73e102f6a88b47907f5/src/vs/workbench/electron-sandbox/desktop.contribution.ts#L148
I am a little bit worried about the consequences this change has, in 2 areas:
- startup time (since we now launch a PowerShell process on startup)
- use of shell environment in terminal, its unclear to me how that impacts Windows
Thus I would like to also ask terminal to review this.
@connor4312 should we maybe be rolling this out behind a disabled setting for users that are in this setup, rather than forcing every Windows user (our largest population) to see a process being spawned on startup, slowing everything else down? Unfortunately this change will fly under the radar for our perf bots to discover because they launch from a terminal...
Unfortunately this change will fly under the radar for our perf bots to discover because they launch from a terminal...
Maybe they should launch with --force-user-env to emulate a real application?
Though I think the impact might not be that great, we make the first call to start resolving the shell environment when the window is created and only actually await that when spawning the Node.js extension host.
should we maybe be rolling this out behind a disabled setting for users that are in this setup
We can, but I think this should eventually be enabled globally because the default mode for installing Node.js on Windows is via fnm now, and this causes issues in other places too (e.g. the debugger.)
@connor4312 great idea, I have https://github.com/microsoft/vscode-perf/pull/46 open to add the flag, I am curious to even see the impact on Linux/macOS.
we make the first call to start resolving the shell environment when the window is created
Still, we cause a process to spawn while we try to restore the workbench. I would be surprised if this wouldnt impact startup times, but we will be able to measure this with the change above.
the default mode for installing Node.js on Windows is via fnm now
For me to understand: this requires use of powershell, or are profiles also supported for the new windows terminal?
For me to understand: this requires use of powershell, or are profiles also supported for the new windows terminal?
The new windows terminal is just a wrapper around cmd/powershell, it's not a new shell. I think the windows terminal by default will be using Powershell.
cmd.exe doesn't have profiles, but powershell does.
Still, we cause a process to spawn while we try to restore the workbench. I would be surprised if this wouldnt impact startup times, but we will be able to measure this with the change above.
Sounds good 👍
@connor4312 I think we could still probe via file service if a profile file exists to avoid the extra call by checking for common locations.
I've done so, and made it work on all platforms for the uncommon Linux/macOS users of powershell. I don't love adding profile path logic into VS Code but the efficiency win may be worth it 🙂
Fixed the handling for Powershell 5. I couldn't figure out how to get around the "The ampersand (&) character is not allowed" there. But, Powershell is actually quite capable of printing JSON itself, and so I made it print the envionrment natively, which should be a bit faster than using our Node.js process as well--no additional process launch. I verified it works on Powershell 5 and Powershell 7 on macOS and Windows.
@connor4312 should we maybe be rolling this out behind a disabled setting for users that are in this setup, rather than forcing every Windows user (our largest population) to see a process being spawned on startup, slowing everything else down? Unfortunately this change will fly under the radar for our perf bots to discover because they launch from a terminal...
This is a good idea that should have been acted upon.
We can, but I think this should eventually be enabled globally because the default mode for installing Node.js on Windows is via fnm now, and this causes issues in other places too (e.g. the debugger.)
This is a bad idea, and should have been clear from the get go.
-
You don't do global solutions to local problems. Especially when the solutions aren't solutions at all. That there exists a single program that behaves strangely is a poor justification for changing the entire way VSCode works.
-
This is now the "Windows way" of doing things. VSCode, like most software, should strive to act the way native applications act. That whoever wrote fnm has no clue how Windows works or simply doesn't care is their business. VSCode shouldn't attempt to patch up the problems arising from their choices by making more bad choices.
- It's incredibly uncommon on Windows to set global behavior required by the system in general (e.g. the
PATHenvironment variables) via scripts "that somehow always run". One reason is that it doesn't really work, as you have found out. - Truly global behavior goes into the system-defined store for environment variables. This store happens to be the Registry, but that's an implementation detail. There's an API for that.
- Ephemeral modifications of the
PATHgo into scripts the user runs when they need, e.g.vcvars*.batandVsDevCmd.bat.
In any case, no sane software on Windows injects itself into the users' CLI shell's profile script. Maybe someone besides fnm does that, but I wouldn't consider them sane. If a user wants to use
MSBuild.exe,cl.exe,link.exe,dumpbin.exe,signtool.exe,mc.exe,makecat.exe,tracelog.exe,tracefmt.exe,midl.exeor whatever they add that to thePATH(presumably using the.batscript or the PS module) and run whatever they need from that instance of the shell, and if needed that include running VSCode from there too. That should apply to other software as well. If it doesn't, you should break VSCode because somebody somewhere made a highly questionable design choice. You don't go and do that. - It's incredibly uncommon on Windows to set global behavior required by the system in general (e.g. the
@conioh sorry about the hassle, I see you've seen we reverted it https://github.com/microsoft/vscode/issues/252431#issuecomment-3016493724. Unfortunately it's a little too late in the month to do a recovery release but if we root caused it earlier we would have.
@Tyriar, I understand it may take some time for the fix for my issue to arrive. My comment here was forward-looking - trying to understand what went wrong so that something of that sort won't happen again.
@conioh rough post-mortem:
I was out at the time, maybe I would have caught it if I was around, maybe not. Ultimately we have very few people on the team that would have the type of Windows knowledge to say definitively whether this approach was good or bad, it raised some flags as being risky for me but I wasn't 100%. Luckily we're relatively close to the pwsh team who helped investigate and gave a recommendation.
I think I'm still getting the error. I think mine is patched because my log matches the new log messages in the diff above. If not, do I test it using insiders?
[main] doResolveShellEnv#shell C:\Program Files\PowerShell\7\pwsh.exe
[main] doResolveShellEnv#powershellProfile found in C:\Program Files\PowerShell\7\Profile.ps1
[main] doResolveShellEnv#spawn ["-Command"] Write-Output '86ada0258a4b'; [System.Environment]::GetEnvironmentVariables() | ConvertTo-Json -Compress; Write-Output '86ada0258a4b'
[main] resolveShellEnv(): running (--force-user-env)
Version: 1.101.2 (system setup)
Commit: 2901c5ac6db8a986a5666c3af51ff804d05af0d4
Date: 2025-06-24T20:27:15.391Z
Electron: 35.5.1
ElectronBuildId: 11727614
Chromium: 134.0.6998.205
Node.js: 22.15.1
V8: 13.4.114.21-electron.0
OS: Windows_NT x64 10.0.19045
If it is, it should probably use Powershell.exe -NoProfile else it runs extra code in modules, like CompletionPredictor
I'm finding that indirectly is causing defender to scan. That's pausing execution preventing the env test to complete before the 10 seconds. -NoProfile should fix that.
My logs from code --force-user-env
[main] doResolveShellEnv#shell C:\Program Files\PowerShell\7\pwsh.exe
[main] doResolveShellEnv#powershellProfile found in C:\Program Files\PowerShell\7\Profile.ps1
[main] doResolveShellEnv#spawn ["-Command"] Write-Output '86ada0258a4b'; [System.Environment]::GetEnvironmentVariables() | ConvertTo-Json -Compress; Write-Output '86ada0258a4b'
[main] resolveShellEnv(): running (--force-user-env)
[main] Unable to resolve your shell environment in a reasonable time. Please review your shell configuration and restart.
[main] resolving shell environment failed Unable to resolve your shell environment in a reasonable time. Please review your shell configuration and restart.
[main] #1: https://update.code.visualstudio.com/api/update/win32-x64/stable/2901c5ac6db8a986a5666c3af51ff804d05af0d4 - begin undefined N4 { b: {}, a: [Object: null prototype] {} }
[main] #2: https://update.code.visualstudio.com/api/update/win32-x64/stable/2901c5ac6db8a986a5666c3af51ff804d05af0d4 - begin undefined N4 { b: {}, a: [Object: null prototype] {} }
[main] update#checkForUpdates, state = idle
[main] update#setState checking for updates
[main] resolveShellEnv(): running (--force-user-env)
[main] #3: https://update.code.visualstudio.com/api/update/win32-x64/stable/2901c5ac6db8a986a5666c3af51ff804d05af0d4?bg=true - begin undefined N4 { b: {}, a: [Object: null prototype] {} }
[INFO:CONSOLE(3308)] "Unable to resolve your shell environment in a reasonable time. Please review your shell configuration and restart.", source: vscode-file://vscode-app/c:/.../workbench.desktop.main.js (3308)
[main] [UtilityProcess id: 1, type: extensionHost, pid: <none>]: creating new...
[main] [UtilityProcess id: 1, type: extensionHost, pid: 66868]: successfully created
[INFO:CONSOLE(35)] "%c INFO color: #33f Started local extension host with pid 66868.", source: vscode-file://vscode-app/c:/.../workbench.desktop.main.js (35)
␛[33;1mWARNING: The names of some imported commands from the module 'SomeModule' include unapproved verbs that might make them less discoverable. To find the commands with unapproved verbs, run the Import-Module command again with the Verbose parameter. For a list of approved verbs, type Get-Verb.␛[0m
