-
Notifications
You must be signed in to change notification settings - Fork 38.9k
Unexpected ${env:...} environment variable resolution for terminals #299520
Description
Does this issue occur when all extensions are disabled?: Yes
- VS Code Version: Version: 1.111.0-insider
- OS Version: irrelevant
This is a follow up to #241078 where we fixed the passing of environment variables specified in a launch configuration into the extension host environment and the terminal environment, both in local and remote configurations.
Now the issue I observe is that specifying terminal.integrated.env.[osx|linux|windows] yields an unexpected (IMO) result in the terminal environment.
Case 1
Here are two screenshots that demonstrates the problem. This is an extension development host started with a launch configuration that defines SOME_VAR in the env property. We can see in the notification emitted by a dummy command that the env var is correctly set in the extension host environment.
In the first screenshot terminal.integrated.env.osx is not contributing anything. We can see that the terminal inherits the env var correctly.
In the second screenshot terminal.integrated.env.osx is set by reading ${env:SOME_VAR}. This time the value in the terminal is unexpected because ${env:SOME_VAR} was evaluated over an environment that doesn't contain SOME_VAR.
Case 2
This also manifests in remote mode with a more detrimental effect. In this next screenshot, terminal.integrated.env.linux.PATH is not set, so the terminal environment has the code command (or code-insiders in this case) which is super handy for opening files from the terminal in the current window or server.
But if we set terminal.integrated.env.linux.PATH to ${env:PATH} which one would expect to introduce zero change, we no longer have the code-insiders command which is too bad.
In other words, whenever I use terminal.integrated.env.* to add tools to my PATH, I lose the code command which is very useful to my workflows.
Moreover, in a scenario very common in my work, when I debug my extension with a launch configuration where PATH is provided by a .env, that PATH is lost if I mistakenly set terminal.integrated.env.*.PATH in the workspace/remote/user settings, which happens often as I switch between projects with different needs.
Analysis
The bottom line seems to be that ${env:...} statements are evaluated over an environment that is different than the extension host environment.
With some debugging I found that the variable resolver is created at this line in terminalProcessManager.ts, based on the environment from the ITerminalProfileResolverService, which in the case of Electron defers to this line in terminalProfileResolverService.ts.
So the variable resolver uses ITerminalBackend.getEnvironment which doesn't have variables coming from the extension host.
Meanwhile ITerminalBackend.getShellEnvironment does have these variables and would yield a behavior more consistent with my expectations.
Changing the getEnvironment call to getShellEnvironment solves "Case 1" locally. I don't know if it also solves "Case 2" as I didn't take the time to reproduce a remote setup with my locally-built VS Code Server. But I suspect it might.
Conceptually what is the difference between getEnvironment and getShellEnvironment ?
Are there big consequences to making this change?
Thanks!