use_mingw="yes" not Honored in custom.py

[bcsmith2846]

Godot version

4.0.dev (0275d60)

System information

Windows 10, MinGW, LLVM Build

Issue description

If I run scons in the shell without use_mingw=yes even when it's in my custom.py file, the build crashes. This seems to be due to the fact that the if statement in the SConstruct file (line 106 in the referenced hash) that is supposed to assign custom_tools to ["mingw"] is reading from the command line only. The assignment of custom_tools actually happens before custom.py is even read.

It also doesn't seem to be honoring the platform variable from custom.py for this one if statement either (because custom.py hasn't been loaded), but that's not causing an issue because the detection is working as intended (i.e. I'm not building for Android or JavaScript).

custom.py:

use_mingw = "yes"
use_llvm = "yes"
use_lto = "yes"
platform = "windows"
tools = "yes"
target = "debug"
verbose = "yes"

Relevant lines in SConstruct:

custom_tools = ["default"]

platform_arg = ARGUMENTS.get("platform", ARGUMENTS.get("p", False))

if os.name == "nt" and (platform_arg == "android" or methods.get_cmdline_bool("use_mingw", False)):
    custom_tools = ["mingw"]
elif platform_arg == "javascript":
    # Use generic POSIX build toolchain for Emscripten.
    custom_tools = ["cc", "c++", "ar", "link", "textfile", "zip"]

#............ BREAK .....................

customs = ["custom.py"]

profile = ARGUMENTS.get("profile", "")
if profile:
    if os.path.isfile(profile):
        customs.append(profile)
    elif os.path.isfile(profile + ".py"):
        customs.append(profile + ".py")

opts = Variables(customs, ARGUMENTS)

This is fairly simple to correct, but I wanted to get some feedback first in the case that there's a reason it is done this way.

Steps to reproduce

1. Checkout 0275d60 on a Windows x64 host (targeting Windows x64)
2. Create custom.py with the following text

use_mingw = "yes"
use_llvm = "yes"
use_lto = "yes"
platform = "windows"
tools = "yes"
target = "debug"
verbose = "yes"

3. Ensure a mingw32/llvm environment is accessible to SCons
4. Run scons without arguments and attempt a build
5. Run scons use_mingw=yes to run a successful build

Minimal reproduction project

No response

[Lokno]

I ran into this same issue with the current version (#79851) helping someone try to compile for mingw. This might be part of a larger issue for any discrepancies between the command line arguments and what appears in custom.py

Specifically in the case of this flag, I don't think the build should fail. If anything, the use_mingw = "yes" in custom.py should be ignored after it has been read in SConstruct

To ignore the custom.py value we could do the same thing that is being done with selected_platform. Let's refer to this as the correction strategy.

platform_arg = ARGUMENTS.get("platform", ARGUMENTS.get("p", False))
use_mingw_arg = methods.get_cmdline_bool("use_mingw", False)

if platform_arg == "android":
    custom_tools = ["clang", "clang++", "as", "ar", "link"]
elif platform_arg == "web":
    # Use generic POSIX build toolchain for Emscripten.
    custom_tools = ["cc", "c++", "ar", "link", "textfile", "zip"]
elif os.name == "nt" and use_mingw_arg:
    custom_tools = ["mingw"]

...

# Update the environment to take platform-specific options into account.
opts.Update(env_base)
env_base["platform"] = selected_platform  # Must always be re-set after calling opts.Update().
env_base["use_mingw"] = use_mingw_arg     # Must always be re-set after calling opts.Update().

Note that opts.Update(env_base) is called multiple times within the file.

Another option would be to add the evaluated argument to ARGUMENTS

platform_arg = ARGUMENTS.get("platform", ARGUMENTS.get("p", False))
ARGUMENTS.update({"use_mingw": str(methods.get_cmdline_bool("use_mingw", False))})

However, the user should be informed that their setting in custom.py is being override because its not at the command line. Using the correction strategy, we can detect the discrepancy and report:

# Update the environment to take platform-specific options into account.
opts.Update(env_base)
env_base["platform"] = selected_platform  # Must always be re-set after calling opts.Update().
if use_mingw_arg != env_base["use_mingw"]:
    env_base["use_mingw"] = use_mingw_arg
    print(f'Warning: Value for "use_mingw" defined in custom.py ignored. Please set this flag via the commandline')

This is only an example of how it could be handled. It would be better to write something more general for any flags assumed to be command line only.

[shana]

#81700 should fix this issue (along with others), so for people running into this, if you could give that PR a try, that would be great!