[Bug] Cmder not expanding variable in environment variable

Version Information

Cmder version: v1.3.21
Operating system: Windows 11

Cmder Edition

Cmder Full (with Git)

Description of the issue

If a Windows Environment Variable from system settings is defined using another environment variables it is not expanded when using CMD.exe inside Cmder.

Example:

ANDROID_HOME=%LOCALAPPDATA%\Android\Sdk

(not just ANDROID_HOME other similar variables do not get expanded as well)

When opening CMD.exe directly (without Cmder) the path is expanded correctly as follows:

> echo %ANDROID_HOME% C:\Users\user.name\AppData\Local\Android\Sdk

If I print the same variable in CMD.exe from within Cmder then the path is not correct:

> echo %ANDROID_HOME% %LOCALAPPDATA%\Android\Sdk

This means that when building an android application from the command line it only works in CMD.exe directly but fails from within Cmder

How to reproduce

Set the environment variable ANDROID_HOME=%LOCALAPPDATA%\Android\Sdk from windows settings
open Cmder with a cmd.exe console
type echo %ANDROID_HOME% and press ENTER
Observe the command output in the console, the path is not expanded properly

Additional context

Setting the environment variables from the Cmder settings: startup/environement the variables are expanded correctly.

A workaround for me is currently to define the variables twice: in the windows system settings and in Cmder settings.

It would be best if the variable expansion is working without the need to also duplicate it in Cmder

Checklist

[X] I have read the documentation.
[X] I have searched for similar issues and found none that describe my issue.
[X] I have reproduced the issue on the latest version of Cmder.
[ ] I am certain my issues are not related to ConEmu, Clink, or other third-party tools that Cmder uses.

I wasn't able to skim through a lot of issues (maybe wrong keywords in search) on ConEmu to check whether the issue lies with ConEmu or clink

This is a funny one:

At first, I was able to reproduce it (using Cmder v1.3.21), but eventually it seemed to stop happening. It only reproduced with Cmder.exe, never with only ConEmu.exe or cmd.exe by themselves.

I reviewed the Cmder launcher source code and found that these lines run after CreateProcess:

https://github.com/cmderdev/cmder/blob/4aefd0bb7163c2183ea7a39b32e0aa5726395075/launcher/src/CmderLauncher.cpp#L473-L474

That's a bit strange: Why should starting Cmder alter the environment in all processes on the computer? And why do that only after having spawned ConEmu.exe? For them to have any kind of reliable effect, the lines would need to happen before spawning ConEmu.exe.

So the Cmder.exe launches ConEmu.exe, and then it attempts to force all processes on the computer to reload and reset their environments. That's probably not a good idea to be doing that every time Cmder.exe launches anyway, but it's especially strange because it's forcing a race condition:

If ConEmu.exe hasn't managed to launch quickly enough, it won't be able to receive the messages anyway.
If ConEmu.exe has managed to launch quickly enough, and has also already created a new tab, then the new tab will not be affected by the message, and only subsequent tabs could be affected.
If ConEmu.exe has managed to launch quickly enough, but hasn't managed to create a new tab yet, then there is a chance that the message could affect the new tab.

I was curious if source history could shed any light on this, and indeed it does. I used Blame to look up the history for those lines:

7 years ago, in v1.3.2, commit a78186f50222e3bae7a38b78a3ac3d7f7352a1e8 rearranged these messages to happen after spawning ConEmu.exe instead of happening before spawning ConEmu.exe. The reason why was to fix slow startup.
8 years ago, in commit 73e1eb2a8bea93d4759f07cbb3a3145203c682fe, the lines were first introduced to "ensure environment variables are propagated".
- @MartiUK do you remember the details behind why that change was introduced?

Running those lines before spawning ConEmu.exe will make launching Cmder slow, because it synchronously waits for all processes on the computer to receive the message and reload their environments.

But running those lines after spawning ConEmu.exe removes their supposed benefits, while also creating a race condition where now sometimes the environment may briefly not have the right content while starting a tab. (Note that it risks also creating race conditions for all other processes on the computer every time those lines run.)

@DRSDavidSoft @daxgames @MartiUK I would recommend to remove those two lines. Under the best of conditions, they have no effect on ConEmu. But they always interfere with all other processes on the computer. And when the timing happens just right, they can negatively affect ConEmu while it's trying to start a new tab.

Because it's a race condition and isn't consistent, I can't prove that it's what's causing this issue reported by @t-ricci-digit. But the lines really do not belong, and for the past 7 years they haven't been having whatever effect was originally intended.

@chrisant996 Thank you for your insights. I agree that the broadcast message is unnecessary for ConEmu and potentially harmful for other running apps. It could introduce an unnecessary delay right after launching ConEmu and interfere with other applications. If the broadcast message is not doing anything noticeable or useful, then it would be a good idea to remove it from the launcher code and avoid sending an unnecessary message to all top-level windows in the system, which even includes the disabled or invisible unowned windows.

And of course, if we really need to send the WM_SETTINGCHANGE message for some reason, we should limit it to ConEmu only. However, I doubt that this would be the case, as ConEmu might have already handled this internally; or perhaps this was a workaround for some old versions of ConEmu, but now it is obsolete and redundant.

The only scenario that I can think of where this message would be useful is if the environment variables that are set by CmderLauncher.cpp are not inherited by the child processes it creates. However, I believe this is unlikely, as the documentation states that child processes inherit the environment block of their parent process by default.

There are several environment variables that are set by CmderLauncher.cpp, such as CMDER_ROOT, CMDER_START, CMDER_USER_CONFIG, and CMDER_USER_BIN:

It sets the CMDER_ROOT environment variable to the directory where Cmder is located.
It sets the CMDER_START environment variable to the directory where Cmder should start.
It sets the CMDER_USER_CONFIG environment variable to the path of the user config file if the /usercfg option is used.
It sets the CMDER_ICON environment variable to the path of the icon file if the /icon option is used.
It sets the CMDER_TITLE environment variable to the title of the Cmder window if the /title option is used.
It sets the CMDER_TASK environment variable to the name of the task to run if the /task option is used.

The environment variables that are set by CmderLauncher.cpp need to be propagated to ConEmu and its child processes, such as init.bat. I suspect that when @MartiUK added the broadcast message, it was because this propagation was not working properly.

@DRSDavidSoft environment variables set into the Cmder.exe process are guaranteed to be inherited by CreateProcess, because Cmder.exe passes NULL for the environment block parameter.

But the WM_SETTINGCHANGE message would be useless for that anyway. It tells processes "reload your environment from the registry". It has nothing to do with propagating environment variables from one process to another.

cmderdev / cmder