Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

SCons: Fix Ninja compilation with MSVC #90208

Merged
merged 1 commit into from
Apr 5, 2024
Merged

SCons: Fix Ninja compilation with MSVC #90208

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ platform/windows/godot_res.res
# Ninja build files
build.ninja
.ninja
run_ninja_env.bat

# Generated by Godot binary
.import/
Expand Down
12 changes: 6 additions & 6 deletions core/crypto/SCsub
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,9 @@ if is_builtin or not has_module:
# to make a "light" build with only the necessary mbedtls files.
if not has_module:
# Minimal mbedTLS config file
env_crypto.Append(
CPPDEFINES=[("MBEDTLS_CONFIG_FILE", '\\"thirdparty/mbedtls/include/godot_core_mbedtls_config.h\\"')]
)
config_path = "thirdparty/mbedtls/include/godot_core_mbedtls_config.h"
config_path = f"<{config_path}>" if env_crypto["ninja"] and env_crypto.msvc else f'\\"{config_path}\\"'
env_crypto.Append(CPPDEFINES=[("MBEDTLS_CONFIG_FILE", config_path)])
Comment on lines +24 to +26
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's starting to be a bit convoluted...
I guess it's fine for now, but it would be worth figuring out why it's not being escaped properly by Ninja+MSVC, and how upstream mbedtls even expect this define to be passed in a way that won't break on some platforms.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it's not the prettiest solution & is very much a workaround. It should be reverted once the issue is ironed out in SCons, but that'll require some digging.

# Build minimal mbedTLS library (MD5/SHA/Base64/AES).
env_thirdparty = env_crypto.Clone()
env_thirdparty.disable_warnings()
Expand All @@ -46,9 +46,9 @@ if not has_module:
env.core_sources += thirdparty_obj
elif is_builtin:
# Module mbedTLS config file
env_crypto.Append(
CPPDEFINES=[("MBEDTLS_CONFIG_FILE", '\\"thirdparty/mbedtls/include/godot_module_mbedtls_config.h\\"')]
)
config_path = "thirdparty/mbedtls/include/godot_module_mbedtls_config.h"
config_path = f"<{config_path}>" if env_crypto["ninja"] and env_crypto.msvc else f'\\"{config_path}\\"'
env_crypto.Append(CPPDEFINES=[("MBEDTLS_CONFIG_FILE", config_path)])
# Needed to force rebuilding the core files when the configuration file is updated.
thirdparty_obj = ["#thirdparty/mbedtls/include/godot_module_mbedtls_config.h"]

Expand Down
6 changes: 3 additions & 3 deletions modules/mbedtls/SCsub
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,9 +100,9 @@ if env["builtin_mbedtls"]:
thirdparty_sources = [thirdparty_dir + file for file in thirdparty_sources]

env_mbed_tls.Prepend(CPPPATH=["#thirdparty/mbedtls/include/"])
env_mbed_tls.Append(
CPPDEFINES=[("MBEDTLS_CONFIG_FILE", '\\"thirdparty/mbedtls/include/godot_module_mbedtls_config.h\\"')]
)
config_path = "thirdparty/mbedtls/include/godot_module_mbedtls_config.h"
config_path = f"<{config_path}>" if env_mbed_tls["ninja"] and env_mbed_tls.msvc else f'\\"{config_path}\\"'
env_mbed_tls.Append(CPPDEFINES=[("MBEDTLS_CONFIG_FILE", config_path)])

env_thirdparty = env_mbed_tls.Clone()
env_thirdparty.disable_warnings()
Expand Down
Loading