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

[Sean Hall]

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 SHGet­Folder­Path(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.
>