include debug extension host env in shell env (#241078)#298276
include debug extension host env in shell env (#241078)#298276meganrogge merged 3 commits intomicrosoft:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes an issue where environment variables specified in an extension debug launch configuration (via env or envFile properties) are not inherited by terminals and tasks spawned within the Extension Development Host. The fix adds these debug environment variables to the shell environment used by terminals in the local terminal backend.
Changes:
- Modified
LocalTerminalBackend.getShellEnvironment()to merge debug extension host environment variables into the shell environment - Added
INativeWorkbenchEnvironmentServicedependency injection to access debug configuration environment
src/vs/workbench/contrib/terminal/electron-browser/localTerminalBackend.ts
Outdated
Show resolved
Hide resolved
meganrogge
left a comment
There was a problem hiding this comment.
Thanks for the contribution! The approach is directionally correct and consistent with how localProcessExtensionHost.ts already applies debugExtensionHost.env.
One issue that needs addressing:
Mutating a potentially shared object: mergeEnvironments mutates env in place. Since getShellEnv() delegates to process.shellEnv(), which may cache and return the same object reference, this mutation could leak debug env vars into other consumers of process.shellEnv(). Please clone first:
const env = { ...await this._shellEnvironmentService.getShellEnv() };Minor note: this only covers the local (Electron) terminal backend — remote extension host debugging won't benefit. Not a blocker for this PR but worth tracking separately if needed.
|
Thanks for the feedback!
Great catch, fixed.
I would like to address that as well as I often do extension development in remote mode. But indeed let's leave it out of this PR. |
…derations Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
659a128 to
8a0fb1a
Compare
* include debug extension host env in shell env (#241078) * Use terminalEnvironment.mergeEnvironments for platform-specific considerations Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Clone the shell env to avoid mutation of a shared object --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
This PR proposes a fix for #241078 .
With some investigation and some help from AI, I managed to produce this fix which includes adds the debug extension host environment to the terminal environment.
With that I was able to observe that environment variables added in the
envproperty of a launch configuration or through an env file and theenvFile, are correctly inherited into terminals spawned by the debugged instance.More specifically, tasks now inherit the environment provided by the debug launch configuration.
Given my little experience with the codebase, I'm not sure this works in all configurations (web, electron, remote, etc) so guidance would be appreciated to steer this in the right direction.
Thanks!