SCons: Refactor profile option for custom.py-style overrides
#91794
Conversation
akien-mga
changed the title
custom.py-style overrides
akien-mga
changed the title
custom.py-style overridesprofile option for custom.py-style overrides
akien-mga
force-pushed
the
scons-refactor-profile-parameter-overrides
branch
from
06bb82b to
d84e4a4
Compare
| # Update the variables based on the profiles. | ||
| opts.Update(env, {**ARGUMENTS, **custom_args, **env}) |
There was a problem hiding this comment.
Might need to reason a bit on the order of merging overlapping arguments from command line, custom.py, and then potential further additions to the env. Will need to do more testing on this PR.
There was a problem hiding this comment.
I think that cli args should override profile args. Cli args + profile args = user args. User args should override env... Your current code does exactly opposite (prefers env over profile args over command line args)
And I would suggest to have separate variable for storing both cli and profile args (user_args or something like it). It will be handy in several places. And IIUC whole if (key not in ARGUMENTS or ARGUMENTS[key] == "auto") and key not in custom_args: could be then written shorter with user_args
Some of the logic in SCons depends on flags that get overridden in the platform-specific `detect.py`, so it needs to be processed first. For example the Android/iOS/Web platforms override the default `target` to `template_debug`, but this was processed too late so e.g. the logic that sets `env.editor_build` would set it to true due to the default `target` value in the environment being `editor`.
akien-mga
force-pushed
the
scons-refactor-profile-parameter-overrides
branch
from
d84e4a4 to
2621189
Compare
| @@ -209,6 +200,11 @@ opts.Add(BoolVariable("debug_paths_relative", "Make file paths in debug symbols | |||
| opts.Add(EnumVariable("lto", "Link-time optimization (production builds)", "none", ("none", "auto", "thin", "full"))) | |||
| opts.Add(BoolVariable("production", "Set defaults to build Godot for use in production", False)) | |||
| opts.Add(BoolVariable("threads", "Enable threading support", True)) | |||
| opts.Add( | |||
There was a problem hiding this comment.
I am not sure but before was only one file? What is the reason to have several profile files? You intend to create something like modularity? One common file and several platform specific?
Environmentvalues when updatingVariables#91792.template_debug#89597.