SCons: Fix handling of platform-specific tools, notably mingw
#101726
Conversation
SConstruct
Outdated
| # 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. | ||
| custom_tools = [] | ||
| if env["platform"] == "android": | ||
| custom_tools += ["clang", "clang++", "as", "ar", "link"] | ||
| elif env["platform"] == "web": | ||
| custom_tools += ["cc", "c++", "ar", "link", "textfile", "zip"] | ||
| elif env["platform"] == "windows" and methods.get_cmdline_bool("use_mingw", False): | ||
| custom_tools += ["mingw"] | ||
| for tool in custom_tools: | ||
| env.Tool(tool) |
There was a problem hiding this comment.
I moved this further down because there's more options overrides below, notably platform flags, and we likely want to respect them before calling detect.get_tools(env) so that this can make full use of the configuration so far.
akien-mga
force-pushed
the
scons-platform-custom-tools-v2
branch
from
832383d to
ed96da7
Compare
| tmppath = "./platform/" + env["platform"] | ||
| sys.path.insert(0, tmppath) | ||
| import detect |
There was a problem hiding this comment.
This had to move up, otherwise detect here was actually the one of the last platform we checked when parsing all of them... i.e. windows, so Linux and macOS were getting mingw registered and ending with .exe binary extensions :p
There was a problem hiding this comment.
Gonna do more thorough testing later, but everything appears to work-as-expected on Windows! It'll be pretty handy to have this specialized per-platform, and future PRs being able to trim that fat even further is an exciting prospect
EDIT: WSL validated & no longer causes overrides! Removing draft status
Add optional `detect.py` `get_tools` method to let platforms register SCons tools they need. This helps move this logic out of SConstruct, keeping platforms more self contained, and helping thirdparty platforms define their own custom tools. This logic was also unreliable (the `use_mingw` one would only work if passed manually on the command line, not in e.g. `get_flags`). Co-authored-by: Thaddeus Crews <[email protected]>
akien-mga
force-pushed
the
scons-platform-custom-tools-v2
branch
from
5bf354a to
90208f7
Compare
|
Rebased and squashed. |
|
Thanks! |
Redo of #99762 on top of #101715, aiming to fix various issues we have with the fact that we need to know the selected platform to define the right SCons "tools" to enable when creating the environment, but we need the environment to register command line options that help set said platform.
Using two environments with
env.Clone()seems to help solve the problems in #101715, and fixing the detection of whenmingwshould be set as tool via #99762 seems to make it work out of the box for Linux+mingw at least.