SCons: Preserve Environment values when updating Variables

[akien-mga]

Finally reading the docs for SCons.Variables.Update let me find this optional parameter, which solves the hacks and pain we've dealt with for years:

args (optional) – a dictionary of keys and values to update in env.
If omitted, uses the variables from the commandline.

By passing the environment itself, we preserve the values we've overridden in SConstruct or detect.py.

Might fix a number of reported issues.

CC @Repiteo

[AThousandShips]

Seems to ignore some options, it makes it ignore the sanitizer args somehow

[AThousandShips]

Tried a few different approaches but couldn't make it not ignore command line options, it seems unfortunately that this option doesn't allow updating from command line for some arguments, unsure how this could be fixed

[Repiteo]

It sounds like it's meant to update the command line arguments by default, but either that's excluded when passing env or the means by which they're parsed is somehow flawed on our end. Gonna do a few tests to see what makes this tick.

[AThousandShips]

Tested and this seems to work:

env_copy = env.Clone()
opts.Update(env_copy)
opts.Update(env, env_copy)

Not sure how to detect the broken stuff without the handling of platform, but it seems to work, unsure if anything is missing though

Edit: Doesn't seem to correctly catch if platform option is left empty, but that might be something that can be fixed with this

Edit 2:

env_copy = env.Clone()
opts.Update(env_copy)
env_copy["platform"] = env["platform"]
opts.Update(env, env_copy)

Seems to solve that part, but feels like we're circling around to hackiness again, but unsure exactly what's missing

[akien-mga]

How about something like this? Seems to work in a few tests, though I need to check with the follow-up PRs based on this one.

diff --git a/SConstruct b/SConstruct
index 5f1c50876e..8ce504ae2b 100644
--- a/SConstruct
+++ b/SConstruct
@@ -362,7 +362,7 @@ if env["platform"] in platform_opts:
         opts.Add(opt)
 
 # Update the environment to take platform-specific options into account.
-opts.Update(env, env)
+opts.Update(env, {**ARGUMENTS, **env})
 
 # Detect modules.
 modules_detected = OrderedDict()
@@ -422,7 +422,7 @@ for name, path in modules_detected.items():
 env.modules_detected = modules_detected
 
 # Update the environment again after all the module options are added.
-opts.Update(env, env)
+opts.Update(env, {**ARGUMENTS, **env})
 Help(opts.GenerateHelpText(env))
 
 # add default include paths

[AThousandShips]

That does seem to work, it handles platform being empty correctly, will see how it behaves for other cases

Edit: It seems to work and it now handles #91791 correctly

[Repiteo]

That does the trick!

[AThousandShips]

Works correctly in my testing