[wix-devs] Burn support for WM_SETTINGSCHANGED to refresh environment variables

Mon Jun 27 10:45:22 PDT 2022

> For example, let's say `MYVAR` has a default value of `1` but in the current process `MYVAR` has a local value of `2`. After receiving the `WM_SETTINGCHANGE` message, the default value is now `3`. There are going to be some cases where the local value of `2` needs to continue to override the default, while in other cases the new default of `3` is supposed to overwrite the local value.

I was trying to think of situations where Burn should not accept a WM_SETTINGCHANGE update to its environment but wasn't sure what would be considered "bad". Do you have a concrete scenario where this is a problem?

-----Original Message-----
From: wix-devs <wix-devs-bounces at lists.wixtoolset.org> On Behalf Of Sean Hall via wix-devs
Sent: Friday, June 24, 2022 2:08 PM
To: WiX Toolset Developer Mailing List <wix-devs at lists.wixtoolset.org>
Cc: Sean Hall <r.sean.hall at gmail.com>
Subject: Re: [wix-devs] Burn support for WM_SETTINGSCHANGED to refresh environment variables

This came across my radar when figuring out my options for working around https://github.com/dotnet/runtime/issues/68180. There are various other projects that have looked into keeping the environment block of the current process up to date:
https://github.com/chocolatey/choco/blob/f924d47fb4177a9a34ff0c2bf995938b5c12800b/src/chocolatey.resources/redirects/RefreshEnv.cmd,
https://github.com/Maximus5/ConEmu/issues/468,
https://github.com/microsoft/terminal/issues/1125,
https://github.com/microsoft/terminal/issues/9741. There's basically two options - throw away the current environment block and get a fresh one or merge the changes that were made with the current one.

The first option of getting a fresh one has a fatal flaw - there's no documented way to get a fresh one with all the variables that would normally be set when launching from explorer. ::CreateEnvironmentBlock is the documented way to create a fresh one but it doesn't include everything.
If you get this wrong then parts of Windows might not work correctly, for example [The case of the SHGetFolderPath(CSIDL_COMMON_DOCUMENTS) that returned ERROR_PATH_NOT_FOUND]( https://devblogs.microsoft.com/oldnewthing/20200520-00/?p=103775). There is an undocumented shell32 function `RegenerateUserEnvironment` that is explained at https://textslashplain.com/2018/10/11/shellexecute-doesnt/.

The other problem with the fresh option is that it's probably not the right thing to do. The user may have set an environment variable that it wanted to propagate for the entire lifetime of the process but this will undo it.

The second option of merging the changes has the same issue. For example, let's say `MYVAR` has a default value of `1` but in the current process `MYVAR` has a local value of `2`. After receiving the `WM_SETTINGCHANGE` message, the default value is now `3`. There are going to be some cases where the local value of `2` needs to continue to override the default, while in other cases the new default of `3` is supposed to overwrite the local value.

There is theoretically a third option here - add an attribute to executable packages to make Burn provide a fresh environment block to ::CreateProcessW. Of course, this has the same problem as the first option in that there's no documented way to get a fresh environment block. And it would mean that the executable would never get environment variables from the Burn process, though that could be mitigated if users could explicitly pass certain environment variables (by implementing https://github.com/wixtoolset/issues/issues/2697). Note that this only would help executable packages and not `MsiPackage` or `MspPackage`.

A more targeted approach might be to add the ability to specify a specific environment variable that gets overwritten in the current process by the default after a package has run.

On Sat, Apr 13, 2019 at 9:12 AM Blair Murri via wix-devs < wix-devs at lists.wixtoolset.org> wrote:

> *1
> ________________________________
> From: Blair Murri
> Sent: Saturday, April 13, 2019 8:12:06 AM
> To: WiX Toolset Developer Mailing List; Sean Farrow
> Cc: Rob Mensching
> Subject: Re: Burn support for WM_SETTINGSCHANGED to refresh 
> environment variables
>
> 2. user overrides system, except for PATH, where user prepends system.
> ________________________________
> From: wix-devs <wix-devs-bounces at lists.wixtoolset.org> on behalf of 
> Rob Mensching via wix-devs <wix-devs at lists.wixtoolset.org>
> Sent: Friday, April 12, 2019 2:37:55 PM
> To: Sean Farrow; WiX Toolset Developer Mailing List
> Cc: Rob Mensching
> Subject: Re: [wix-devs] Burn support for WM_SETTINGSCHANGED to refresh 
> environment variables
>
> 1. System and user variables should be merged into a single view. I 
> _think_ user overwrites system except for PATH that merges them.
>
> 2. The behavior should match what the system does when it creates the 
> process. My bet is that it does not expand values but would be good to 
> validate the behavior.
>
> Regards,
>
>   Rob Mensching
>   CEO
>   FireGiant
>
> -----Original Message-----
> From: Sean Farrow <sean.farrow at seanfarrow.co.uk>
> Sent: Friday, April 12, 2019 1:36 PM
> To: Rob Mensching <rob at firegiant.com>; WiX Toolset Developer Mailing 
> List <wix-devs at lists.wixtoolset.org>
> Subject: RE: Burn support for WM_SETTINGSCHANGED to refresh 
> environment variables
>
> Hi,
>
> Should we export separate functions to retrieve the user and system 
> variables as well as a function to refresh them for use in the 
> WM_SETTINGSCHANGED code itself?
> Would it also be useful to have a function to expand variables? I 
> noticed that pathutil.cpp expands variables so wondered whether it was 
> worth centralising this code in case any other code needed it.
>
> Cheers
> Sean.
> -----Original Message-----
> From: Rob Mensching <rob at firegiant.com>
> Sent: 12 April 2019 17:24
> To: WiX Toolset Developer Mailing List <wix-devs at lists.wixtoolset.org>
> Cc: Sean Farrow <sean.farrow at seanfarrow.co.uk>
> Subject: RE: Burn support for WM_SETTINGSCHANGED to refresh 
> environment variables
>
> 1. It has to be added to the engine (stub) since that is the process 
> starting the ExePackage processes. Fortunately, there is already a 
> message loop in the engine for handling shutdown messages.
>
> 2. It might be nice to add to the native BA but I'm not sure how 
> useful that would be in practice since the native BA wouldn't be able 
> to do anything with it.
>
> 3. Managed BA doesn't own the message loop so it isn't "in the loop" 
> (ha,
> ha) to handle WM_SETTINGSCHANGED.
>
> To get started, add an "EnvRefresh()" function (or better name) to a 
> new envutil.cpp in https://github.com/wixtoolset/dutil. I expect that 
> will be the hardest part. Then add the WM_SETTINGSCHANGED switch case 
> to the message handling in the stub and call the new function.
>
> That _should_ do it.
>
> Regards,
>
>   Rob Mensching
>   CEO
>   FireGiant

-----Original Message-----
> From: wix-devs <wix-devs-bounces at lists.wixtoolset.org> On Behalf Of 
> Sean Farrow via wix-devs
> Sent: Friday, April 12, 2019 5:16 AM
> To: WiX Toolset Developer Mailing List <wix-devs at lists.wixtoolset.org>
> Cc: Sean Farrow <sean.farrow at seanfarrow.co.uk>
> Subject: [wix-devs] Burn support for WM_SETTINGSCHANGED to refresh 
> environment variables
>
> Hi,
>
> Just following up to start the design discussion for this feature as 
> requested.
> Do we want to add this to both the managed and unmanaged bootstrappers?
> Or, can we get away with just adding it to the stub?
> Cheers
> Sean.
>
____________________________________________________________________
WiX Toolset Developer Mailing List provided by FireGiant http://www.firegiant.com/