Repository: godotengine/godot (language: cpp)

Issue:
SCons detect configure removes LINKFLAGS added beforehand (eg. command line)
### Tested versions

Reproducible: Master
Not Reproducible: 4.3

### System information

Godot v4.4.dev (bb2225753) - Windows 11 (build 22631) - Multi-window, 3 monitors - Vulkan (Mobile) - dedicated NVIDIA GeForce RTX 4070 (NVIDIA; 32.0.15.6603) - AMD Ryzen 7 3700X 8-Core Processor (16 threads)

### Issue description

Hey,

building with SCons on Master does not respect linkflags in the command line anymore (4.3 is not a problem).
Thats the build command I use:
```scons dev_build=yes use_mingw=yes use_llvm=yes dev_mode=yes compiledb=yes linkflags="-Wl,-pdb=" ccflags="-g -gcodeview"```
And it does not add the linkflags anymore.

I could narrow it down to https://github.com/godotengine/godot/blob/1aaf20b1f1f3ab59f04db74b166540327a3f1f90/SConstruct#L616
Adding a linkflag parameter before is ignored and after not.
As this was already there in 4.3, I cannot exactly tell what brought up the behavior in the current master.




### Steps to reproduce

Add ```env.Append(LINKFLAGS=["-Whatever"])``` before: https://github.com/godotengine/godot/blob/1aaf20b1f1f3ab59f04db74b166540327a3f1f90/SConstruct#L616
It is removed.
Add ```env.Append(LINKFLAGS=["-Whatever"])``` after the line and it is added.

### Minimal reproduction project (MRP)

---


Discussion / hints:
Confirmed, that's a regression from #98105.

The `env.Tool("mingw")` added in this PR in the setup function resets flags, and then it just manually reparses 2 of the potential user-defined flags but not `linkflags`.

I'd question whether this change was necessary, this PR seems to add a number of hacks. (Not that it wasn't hacky before, but at least those were battle-tested hacks :P)

Fix patch:
diff --git a/SConstruct b/SConstruct
index 94bef048a072..8f8f329f233d 100644
--- a/SConstruct
+++ b/SConstruct
@@ -107,22 +107,11 @@ for x in sorted(glob.glob("platform/*")):
     sys.path.remove(tmppath)
     sys.modules.pop("detect")
 
-custom_tools = ["default"]
-
-platform_arg = ARGUMENTS.get("platform", ARGUMENTS.get("p", 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 methods.get_cmdline_bool("use_mingw", False):
-    custom_tools = ["mingw"]
-
 # We let SCons build its default ENV as it includes OS-specific things which we don't
-# want to have to pull in manually.
+# want to have to pull in manually. However we enforce no "tools", which we register
+# further down after parsing our platform-specific configuration.
 # Then we prepend PATH to make it take precedence, while preserving SCons' own entries.
-env = Environment(tools=custom_tools)
+env = Environment(tools=[])
 env.PrependENVPath("PATH", os.getenv("PATH"))
 env.PrependENVPath("PKG_CONFIG_PATH", os.getenv("PKG_CONFIG_PATH"))
 if "TERM" in os.environ:  # Used for colored output.
@@ -168,11 +157,7 @@ if profile:
 opts = Variables(customs, ARGUMENTS)
 
 # Target build options
-if env.scons_version >= (4, 3):
-    opts.Add(["platform", "p"], "Target platform (%s)" % "|".join(platform_list), "")
-else:
-    opts.Add("platform", "Target platform (%s)" % "|".join(platform_list), "")
-    opts.Add("p", "Alias for 'platform'", "")
+opts.Add((["platform", "p"], "Target platform (%s)" % "|".join(platform_list), ""))
 opts.Add(EnumVariable("target", "Compilation target", "editor", ("editor", "template_release", "template_debug")))
 opts.Add(EnumVariable("arch", "CPU architecture", "auto", ["auto"] + architectures, architecture_aliases))
 opts.Add(BoolVariable("dev_build", "Developer build with dev-only debugging code (DEV_ENABLED)", False))
@@ -312,10 +297,7 @@ if env["import_env_vars"]:
 
 # Platform selection: validate input, and add options.
 
-if env.scons_version < (4, 3) and not env["platform"]:
-    env["platform"] = env["p"]
-
-if env["platform"] == "":
+if not env["platform"]:
     # Missing `platform` argument, try to detect platform automatically
     if (
         sys.platform.startswith("linux")
@@ -330,7 +312,7 @@ if env["platform"] == "":
     elif sys.platform == "win32":
         env["platform"] = "windows"
 
-    if env["platform"] != "":
+    if env["platform"]:
         print(f'Automatically detected platform: {env["platform"]}')
 
 # Deprecated aliases kept for compatibility.
@@ -352,7 +334,7 @@ if env["platform"] not in platform_list:
 
     if env["platform"] == "list":
         print(text)
-    elif env["platform"] == "":
+    elif not env["platform"]:
         print_error("Could not detect platform automatically.\n" + text)
     else:
         print_error(f'Invalid target platform "{env["platform"]}".\n' + text)
@@ -434,6 +416,23 @@ env.modules_detected = modules_detected
 opts.Update(env, {**ARGUMENTS, **env.Dictionary()})
 Help(opts.GenerateHelpText(env))
 
+
+# FIXME: Tool assignment happening at this stage is a direct consequence of getting the platform logic AFTER the SCons
+# environment was already been constructed. Fixing this would require a broader refactor where all options are setup
+# ahead of time with native validator/converter functions.
+tmppath = "./platform/" + env["platform"]
+sys.path.insert(0, tmppath)
+import detect
+
+custom_tools = ["default"]
+try:  # Platform custom tools are optional
+    custom_tools = detect.get_tools(env)
+except AttributeError:
+    pass
+for tool in custom_tools:
+    env.Tool(tool)
+
+
 # add default include paths
 
 env.Prepend(CPPPATH=["#"])
@@ -515,10 +514,6 @@ if not env["deprecated"]:
 if env["precision"] == "double":
     env.Append(CPPDEFINES=["REAL_T_IS_DOUBLE"])
 
-tmppath = "./platform/" + env["platform"]
-sys.path.insert(0, tmppath)
-import detect
-
 # Default num_jobs to local cpu count if not user specified.
 # SCons has a peculiarity where user-specified options won't be overridden
 # by SetOption, so we can rely on this to know if we should use our default.
@@ -587,7 +582,7 @@ if env["dev_mode"]:
 if env["production"]:
     env["use_static_cpp"] = methods.get_cmdline_bool("use_static_cpp", True)
     env["debug_symbols"] = methods.get_cmdline_bool("debug_symbols", False)
-    if platform_arg == "android":
+    if env["platform"] == "android":
         env["swappy"] = methods.get_cmdline_bool("swappy", True)
     # LTO "auto" means we handle the preferred option in each platform detect.py.
     env["lto"] = ARGUMENTS.get("lto", "auto")
diff --git a/platform/android/detect.py b/platform/android/detect.py
index 03a208391cf3..571cfd9819c0 100644
--- a/platform/android/detect.py
+++ b/platform/android/detect.py
@@ -19,6 +19,10 @@ def can_build():
     return os.path.exists(get_env_android_sdk_root())
 
 
+def get_tools(env: "SConsEnvironment"):
+    return ["clang", "clang++", "as", "ar", "link"]
+
+
 def get_opts():
     from SCons.Variables import BoolVariable
 
diff --git a/platform/web/detect.py b/platform/web/detect.py
index 87315a405f27..9b9234480408 100644
--- a/platform/web/detect.py
+++ b/platform/web/detect.py
@@ -28,6 +28,11 @@ def can_build():
     return WhereIs("emcc") is not None
 
 
+def get_tools(env: "SConsEnvironment"):
+    # Use generic POSIX build toolchain for Emscripten.
+    return ["cc", "c++", "ar", "link", "textfile", "zip"]
+
+
 def get_opts():
     from SCons.Variables import BoolVariable
 
diff --git a/platform/windows/detect.py b/platform/windows/detect.py
index 943de74a3e9d..db65bb64a7b5 100644
--- a/platform/windows/detect.py
+++ b/platform/windows/detect.py
@@ -155,6 +155,13 @@ def detect_build_env_arch():
     return ""
 
 
+def get_tools(env: "SConsEnvironment"):
+    if os.name != "nt" or env["use_mingw"]:
+        return ["mingw"]
+    else:
+        return ["default"]
+
+
 def get_opts():
     from SCons.Variables import BoolVariable, EnumVariable
 
@@ -326,10 +333,6 @@ def setup_mingw(env: "SConsEnvironment"):
         print_error("No valid compilers found, use MINGW_PREFIX environment variable to set MinGW path.")
         sys.exit(255)
 
-    env.Tool("mingw")
-    env.AppendUnique(CCFLAGS=env.get("ccflags", "").split())
-    env.AppendUnique(RCFLAGS=env.get("rcflags", "").split())
-
     print("Using MinGW, arch %s" % (env["arch"]))
 
 


Test patch: