Fix compiler detection.

[bruvzg]

Fixes compiler detection broken by #82325

With shell=true, if two arguments are used, the second (--version) is not passed to clang (like: zsh -c clang --version), if only one is used it is passed (like: zsh -c "clang --version").

Bugsquad edit:

• Fixes Source fails to link on OSX 13.6 / Apple Silicon M1 #82350

[MarioLiebisch]

Ah, fair point! I absolutely didn't expect it to handle the arguments that way (maybe I misunderstood Scons' idea of launching it with a shell?).

However, this seems to be a Scons bug(?). I plugged this change into my local copy and it broke execution on Windows again, since Scons now tries to execute an executable with the name em++ --version (read: including the whitespaces and argument as part of the name, like em++ --version.exe)

Maybe as a question to @akien-mga: Should there probably be a CI test to ensure version checks like this actually work and return valid numbers on the CI? I'm not sure how long this has been broken for me, it just always went unter tons of other warnings while not causing a fatal issue (like with indexing None now). And thanks to the new fallback values, it didn't break yesterday's CI builds either, which might be problematic long term.

[MarioLiebisch]

Assuming the old code worked fine on other platforms, how about changing the line into this?

version = subprocess.check_output([env.subst(env["CXX"]), "--version"], shell=(env['PLATFORM'] == "win32")).strip().decode("utf-8")

[akien-mga]

For now I would prefer just removing shell=True, which is consistent with all other uses of subprocess.check_output. If that's failing on some systems, we should look into it separately but without breaking what's seemingly working on lots of configurations.

[bruvzg]

However, this seems to be a Scons bug(?).

Or it's something wrong with a specific version of python. It should work the same on Windows as well.

For now I would prefer just removing shell=True, which is consistent with all other uses of subprocess.check_output

I guess it's probably safer. Changed.

[akien-mga]

Thanks!

[bferguson3]

@bruvzg Do you mind clarifying for me what shell=True does in this case? It's unclear on my side why the binary header for the linking object files is "incorrect". If there is something I can do manually on my side to get the source to build as a workaround for now that would be great.

[MarioLiebisch]

@bruvzg Do you mind clarifying for me what shell=True does in this case?
Rather than launching the program (the compiler in this case) directly, it will launch a shell instance and tell it to launch the compiler inside.

In this particular case, I think my original issue might be related to having a Python version installed that differs to the one used by Emscripten. Still have to dig a bit deeper there, but using the shell in my case forces Emscripten's batch file to be executed instead of the Python script directly (I think), which might circumvent the version difference.

[bruvzg]

It's unclear on my side why the binary header for the linking object files is "incorrect".

In case of macOS, Xcode 15 introduced a new linker ld-prime which is buggy does not work for Godot, so we use ld-classic linker when Xcode 15 is detected. If detection is broken, a wrong one is used to link static libs and executable.

[bferguson3]

It's unclear on my side why the binary header for the linking object files is "incorrect".

In case of macOS, Xcode 15 introduced a new linker ld-prime which is buggy does not work for Godot, so we use ld-classic linker when Xcode 15 is detected. If detection is broken, a wrong one is used to link static libs and executable.

@bruvzg Thank you for your reply. Since this does not fix the linking error in my case (yes I have Xcode 15 installed), do you know where I can modify the build script to get it to link properly for now?

[YuriSizov]

Cherry-picked for 4.1.3.