terminal
terminal copied to clipboard
Feature Request: Terminal should listen for the WM_SETTINGCHANGE for environment variable updates
Summary of the new feature/enhancement
When I install an app that updates the path or I update the path manually, it isn't enough to kill an individual tab in the Terminal and start a new one. The new tab still inherits the old, unchanged environment variable values. This means I have to kill/restart the Terminal and lose all my tabs.
Proposed technical implementation details (optional)
Add a Windows message handler for WM_SETTINGCHANGE and update the Terminal process's environment block so that any new tabs get the updated environment variables.
Related discussion in ConEmu. https://github.com/Maximus5/ConEmu/issues/468
Opening a new tab should definitely inherit from the current system environment.
<This was a kind of unfocused train of thought, though I'm happy with the conclusion at the bottom. leaving my own notes so I can remember whenever I get back to this>
Huh, this might be a bit trickier than I thought. WM_SETTINGCHANGE doesn't tell you which variable changed, only that the "Environment" changed. If we call GetEnvironmentStrings, we're still going to just get our launch environment variables.
We could call CreateEnvironmentBlock with nullptr as the hToken to get a fresh system environment block, but that's probably wrong, since we want the user's environment block. That involves also calling LogonUser. That's presumably doable.
Though, if the user launched the Terminal from one process with some extra variables set, and expected it to inherit variables from the parent process, then we probably shouldn't blow away the env vars for the first tab that's created. I suppose we could only do this refreshing of the environment block when we do get a WM_SETTINGCHANGE. Subsequent tabs would all use the user's default env block, not the one inherited from the parent. That's a little funky - it sorta creates scenarios that aren't totally expected, because sometimes the parent's values would persist to subsequent tabs.
I guess the most unified solution would be to just use a fresh environment block for all connections created after startup. That would at least be consistent behavior. We'd probably want to wait for #4023 to merge first, and set some internal flag to use a fresh env block, only after all the startup actions are processed. Then we wouldn't even need to use WM_SETTINGCHANGE, we'd just assume that you wanted a fresh one.
Opening a new tab should definitely inherit from the current system environment.
nit: I would suggest the current user environment. explorer.exe does this correctly when launching new processes, so I hope it's not too complicated.
Make sure when we reload this, we also reload settings. That'll probably fix the keyboard issue in 4735.
We could call
CreateEnvironmentBlockwithnullptras thehTokento get a fresh system environment block, but that's probably wrong, since we want the user's environment block. That involves also callingLogonUser.
CreateEnvironmentBlock(&lpEnvironment, GetCurrentProcessToken(), FALSE) seems to work OK without LogonUser, although the documentation of CreateEnvironmentBlock does not quite allow this use.
If this one is solved by loading a new environment block on each new tab creation, then we'll still need separately to handle WM_SETTINGCHANGE to rectify the currently-closed-as-duplicate-of-this #4735.
It's not something I'm volunteering to implement myself, but if at startup the CreateEnvironmentBlock state was captured but not applied, then on WM_SETTINGCHANGE for Environment, we could CreateEnvironmentBlock again, and apply a 3-way diff (resolving conflicts in favour of the existing value: those're user-overrides) to the environment used for new tabs. That way existing user changes and overrides could be maintained, as we'd only change things that had that value in the default env block. Potentially, certain env-vars could be handled as lists, e.g., that'd make PATH do nice things if you append to the end of it as an override, and then install something that adds itself to the PATH. I assume there's some way an env-var works as a list, based on the Environment settings letting you treat some env-vars as a list.
Unless of course the user has overridden a value to be the same as the default env-block, and expects that override to survive if the Environment settings are changed. Then you get a less-nice outcome.
Conversely, this might have the nice effect that if the user had overridden a value, then changes the Environment to match it, and then changes the Environment again, their override is gone, and the second Environment value wins. Probably what they wanted, since they changed it after the override.
I am 100% open to not being informed that I have not used the word "nice" correctly here. ^_^
Perhaps wt could "just use a fresh environment block for all connections created after startup" as previously suggested, but let the command line or settings.json list some environment variables that need to be copied from the environment of the wt process instead. Deliberately running wt with environment variables that differ from the user's defaults is a niche scenario. I don't think a three-way diff should be implemented.
The environment variables bit of this was addressed in #7243; I'm leaving this issue open for the other environment settings.
The environment variables bit of this was addressed in #7243; I'm leaving this issue open for the other environment settings.
I think it makes more sense to have a new ticket for WM_SETTINGCHANGE handling, the vast majority of tickets duplicated to this, and discussion on this ticket, were about env-vars specifically. They were mostly $ENV:PATH in fact.
I think it's only #1230, and #4735 that were interested in other uses of WM_SETTINGCHANGE. And #6491, but that's a super-set of #1230.
Add a Windows message handler for
WM_SETTINGCHANGEand update the Terminal process's environment block so that any new tabs get the updated environment variables.
Windows API allows to create new process with custom env block. So there is no need to handle events. PowerShell does this in some scenarios.
That's the approach used in #7243, but it caused problems and had to be reverted; see #7418 and e.g., 616a71dd23e8fff717919c20c382edeaf59f433a
That's the approach used in #7243, but it caused problems and had to be reverted; see #7418 and e.g., 616a71d
@TBBle Thanks for pointing #7418. I leave a comment there. I guess there was a wrong implementation :-(
Windows API allows to create new process with custom env block. So there is no need to handle events. PowerShell does this in some scenarios.
The environment should only be reloaded when some process in the session requests a reload by broadcasting WM_SETTINGCHANGE for an "Environment" update, which is when Explorer reloads its environment. It should not be reloaded for every new tab. It should change in lock step with Explorer.
The major problem is getting access to a public API that updates the environment using the same sequence as the shell API's private RegenerateUserEnvironment() function. There's a need for a public version of RegenerateUserEnvironment(), or CreateEnvironmentBlock2().
For those of you who can follow along, I've filed MSFT-37424054 to track documenting and making available RegenerateUserEnvironment. Right now it's an unanswered ask on the team that owns shell32. :smile:
Please, just check for new environment variables when a new tab is created.
Please, just check for new environment variables when a new tab is created.
I thought the point was to behave like running a console application from Explorer, which inherits its environment from Explorer.
If a user or application modifies environment variables in the registry without broadcasting WM_SETTINGCHANGE "Environment", then it's not meant for the current session. Otherwise, for the past 25 years or so, the shell API would also have called RegenerateUserEnvironment() when spawning a new process.
The common ways for a user to update the environment in the registry all broadcast WM_SETTINGCHANGE, including the GUI environment variable editor, PowerShell [System.Environment]::SetEnvironmentVariable(), and setx.exe.
@ErykSun Pending us getting RegenerateUserEnvironment documented/publicly available, do you have any thoughts on whether Terminal should try to maintain environment variables it inherited that were different from the user environment?
There have been a couple reports that WT Preview no longer supports e.g. FOO_BAR=1 wt cmd ... echo %FOO_BAR%.
It seems like we'd be playing with fire, trying to reconstitute what the user wants in that case. I'm failing to think of a better thing for us to do, unless we just want to say "plz don't do that, just accept that you can't inherit environment variables."
After all, inheritance becomes ~ ~ strange ~ ~ when one Terminal window can act as the destination for any number of wt commands from any number of sources.
Maybe that's my answer.
@miniksa is looking at some of our options here and will report back.
Just so we have this all in one place
- original issue: #1125
- Original PR: #7243
- PowerShell Issue that made it apparent that
CreateEnvironmentBlockwas insufficient: #7418 - Internal tracking
RegenerateUserEnvironment: MSFT:37424054 - Implement env vars in settings (like in VScode) (original request: #2785, PR: #9287)
- Also, this thread: https://github.com/microsoft/terminal/issues/9741#issuecomment-816758495
- Slightly different, but still related: #11777
@zadjii-msft, note my comment in https://github.com/microsoft/terminal/issues/12157#issuecomment-1012606082. When Terminal is run from the start menu or "Open in Windows Terminal", a system service does the work of spawning the process. The service creates a new environment block for the user via CreateEnvironmentBlock(). So Terminal's initial environment may not be expanded the same as Explorer's. I think it should immediately call RegenerateUserEnvironment(), or something equivalent to that. Don't wait for a WM_SETTINGCHANGE message.
When using just CreateEnvironmentBlock(), a second expansion of the environment block using RtlExpandEnvironmentStrings() may help if a REG_EXPAND_SZ variable wasn't expanded completely when the environment was created. This is what the "appinfo" service does when Terminal is run from the start menu.
For example, a system REG_EXPAND_SZ variable can't depend on system variables that CreateEnvironmentBlock() hasn't set yet, such as "COMPUTERNAME", "ProgramFiles" and "CommonProgramFiles", but these variables should be defined the second time around. Similarly a user REG_EXPAND_SZ variable can't rely on volatile user variables that CreateEnvironmentBlock() hasn't set yet, such as "USERNAME", "HOMEDRIVE", and "HOMEPATH", but they should be defined when the environment is expanded the second time.
A second expansion does not guarantee, however, that REG_EXPAND_SZ variables can arbitrarily reference other REG_EXPAND_SZ variables that are defined in the same "Environment" key. For example, assume that REG_EXPAND_SZ variable "X" is defined as "%Y%"; REG_EXPAND_SZ variable "Y" is defined as "%Z"; and REG_SZ variable "Z" is defined as "spam". When the environment is created, if "X" is set before "Y" is set (based on the arbitrary enumeration order of the registry key), the value of "X" won't be expanded, but for sure the value of variable "Y" will be expanded to "spam" since REG_SZ "Z" is loaded first. In this case, a second expansion fixes the value of "X". On the other hand, say these are system variables, and the raw value of "Y" is changed to "%ProgramFiles%". In this case, "Y" will never be expanded when the environment is created since CreateEnvironmentBlock() doesn't set "ProgramFiles" in the environment block until after the system "Environment" key is loaded. Thus a second expansion sets the value of "X" to "%ProgramFiles%", which will require a third expansion.
@eryksun I'm working on this here if you want to peek: https://github.com/microsoft/terminal/blob/dev/miniksa/env/src/inc/til/env.h
I can't say I've addressed all your concerns yet, but I have tried to replicate the spirit of how the OS and Explorer work behind the RegenerateUserEnvironment() function. It should be pretty close.
@miniksa, Windows uses reserved environment variables with names that begin with "=", which effectively makes them hidden variables in most cases. This isn't a problem in practice, given an empty name is disallowed in "name=value" entries. CMD uses a hidden "=ExitCode" variable. The most common use is to store the working directory on a drive. For example, to resolve "Z:spam", the Windows API checks for an "=Z:" environment variable. The CMD shell's CHDIR command sets these variables, as does the C runtime's _[w]chdir().
You can set and check hidden variables directly via WinAPI SetEnvironmentVariableW() and GetEnvironmentVariableW(), or by examining the PEB with a debugger (e.g. the !peb command). CMD has a well-known bug when executing set " or set "" that displays them.
Defining hidden environment variables in an "Environment" key is not supported. They're documented as reserved names: "[equals] must not be used in the name". They can't be defined or modified conventionally as persistent variables -- not via the GUI editor, "setx.exe", or .NET SetEnvironmentVariable(variable, value, target). One would have to modify the registry directly.
That said, RegenerateUserEnvironment() loads any 'hidden' environment variables defined in the "Environment" keys. If you're looking to match the behavior of RegenerateUserEnvironment() as close as possible, then your code should retain them. At the least, I think an intentional choice should be made. If you want to support hidden environment variables that are defined in the registry, then the find_first_of() call in parse() should be modified to start at index 1. If you instead want to filter out all hidden variables, the loop should be modified to skip the entry when pos is 0. The current code keeps the first such entry that's found.
@miniksa In PT Run I have recreated the behavior of the os nearly exactly including removing and renaming variables.
You find the code here and many helpful informations here.
I you have questions don't hesitate to aske me. 😉
(Regarding hidden env vars I am not sure where the use case should be.)
(Regarding hidden env vars I am not sure where the use case should be.)
It's a matter of which behaviors of RegenerateUserEnvironment() should be intentionally preserved or rejected, instead of letting something slip through as undefined behavior. The current code stores the first 'hidden' variable to the mapping, and ignores the rest. I think ignoring them all is perfectly acceptable behavior. It's a small change to the code.
I just wanted to explain the details, in case people are unfamiliar with such variables. They are illegal environment variable names in the .NET and C runtime (though the C runtime itself sets them), and often they aren't displayed (e.g. Process Explorer doesn't show them), so I'd venture that some developers aren't even aware of their existence.
If someone happened across the existence of 'hidden' environment variables (e.g. set "" in CMD) and decided to define one in the registry, it's better for it to not be defined at all under Windows Terminal, instead of letting just that one variable slip through.
In PT Run I have recreated the behavior of the os nearly exactly including removing and renaming variables.
RegenerateUserEnvironment() and CreateEnvironmentBlock() have to load almost every environment variable from the registry or API calls. RegenerateUserEnvironment() starts with an already loaded environment for the current user, so it can selectively re-use more, but it's not much more. It still loads most variables from the registry and API. Refer to Michael's code.
@miniksa, Windows uses reserved environment variables with names that begin with "=", which effectively makes them hidden variables in most cases. This isn't a problem in practice, given an empty name is disallowed in "name=value" entries. CMD uses a hidden "=ExitCode" variable. The most common use is to store the working directory on a drive. For example, to resolve "Z:spam", the Windows API checks for an "=Z:" environment variable. The CMD shell's
CHDIRcommand sets these variables, as does the C runtime's_[w]chdir().You can set and check hidden variables directly via WinAPI
SetEnvironmentVariableW()andGetEnvironmentVariableW(), or by examining the PEB with a debugger (e.g. the!pebcommand). CMD has a well-known bug when executingset "orset ""that displays them.Defining hidden environment variables in an "Environment" key is not supported. They're documented as reserved names: "[equals] must not be used in the name". They can't be defined or modified conventionally as persistent variables -- not via the GUI editor, "setx.exe", or .NET
SetEnvironmentVariable(variable, value, target). One would have to modify the registry directly.That said,
RegenerateUserEnvironment()loads any 'hidden' environment variables defined in the "Environment" keys. If you're looking to match the behavior ofRegenerateUserEnvironment()as close as possible, then your code should retain them. At the least, I think an intentional choice should be made. If you want to support hidden environment variables that are defined in the registry, then thefind_first_of()call inparse()should be modified to start at index 1. If you instead want to filter out all hidden variables, the loop should be modified to skip the entry whenposis 0. The current code keeps the first such entry that's found.
I did notice the allowance for the leading = in the underlying code, but I wasn't really aware of the impacts or usages of them. I don't think I excluded them on purpose. It could be that I misread some of what is behind RegenerateUserEnvironment() and still need to work it out. My goal is this operates the same as that.
I do also know I still need to handle the recursive processing aspect of later variables being able to have things substituted in from earlier ones.
When I faced the issue yesterday and wondered whether it was already tracked here, I had no idea this was being discussed for more than 2 years...
I understand there are many corner cases, but couldn't the base use cases (user or app installer updates the user or system env vars) be addressed as a temporary solution?
Funny enough, we had a temporary solution! We only recently backed it out because it caused more problems than it fixed. :)
Being a "late joiner", I wonder if there is a solution for windows-terminal to reload updated PATH environment variable. I just noticed that after I updated the PATH environment variable, it was not updated in an existing command prompt in windows-terminal, nor was it updated it new command prompt tabs However, when I opened a new window of windows-terminal, the PATH was up to date
Is there a "reload environment" command in windows-terminal?
No, which is why this issue is still open. The closest workaround I have is to run
$env:Path = [System.Environment]::GetEnvironmentVariable("Path","Machine") + ";" + [System.Environment]::GetEnvironmentVariable("Path","User")
after you update the PATH.