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.

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

Can't cross-compile to Windows with module_mbedtls_enabled=no #90652

Closed
kus04e4ek opened this issue Apr 14, 2024 · 3 comments · Fixed by #90691
Closed

Can't cross-compile to Windows with module_mbedtls_enabled=no #90652

kus04e4ek opened this issue Apr 14, 2024 · 3 comments · Fixed by #90691

Comments

@kus04e4ek
Copy link
Contributor

kus04e4ek commented Apr 14, 2024

Tested versions

43b32f9 (latest)

System information

Linux

Issue description

When I cross-compile from Linux to Windows using module_mbedtls_enabled=no, I get the following errors:

In file included from thirdparty/mbedtls/include/mbedtls/build_info.h:174,
                 from thirdparty/mbedtls/library/common.h:14,
                 from thirdparty/mbedtls/library/aes.c:14:
thirdparty/mbedtls/include/mbedtls/check_config.h:28:2: error: #error "MBEDTLS_PLATFORM_C is required on Windows"
   28 | #error "MBEDTLS_PLATFORM_C is required on Windows"
      |  ^~~~~
In file included from thirdparty/mbedtls/include/mbedtls/build_info.h:174,
                 from thirdparty/mbedtls/library/common.h:14,
                 from thirdparty/mbedtls/library/aes.c:14:
thirdparty/mbedtls/include/mbedtls/check_config.h:578:2: error: #error "MBEDTLS_PLATFORM_SNPRINTF_ALT defined, but not all prerequisites"
  578 | #error "MBEDTLS_PLATFORM_SNPRINTF_ALT defined, but not all prerequisites"
      |  ^~~~~
thirdparty/mbedtls/include/mbedtls/check_config.h:592:2: error: #error "MBEDTLS_PLATFORM_VSNPRINTF_ALT defined, but not all prerequisites"
  592 | #error "MBEDTLS_PLATFORM_VSNPRINTF_ALT defined, but not all prerequisites"
      |  ^~~~~

Just compiling to Linux doesn't produce any errors.
This diff seems to fix it partially, though I get linker errors with undefined references:

diff --git a/thirdparty/mbedtls/include/godot_core_mbedtls_config.h b/thirdparty/mbedtls/include/godot_core_mbedtls_config.h
index d27bf608fbe..e65f332849f 100644
--- a/thirdparty/mbedtls/include/godot_core_mbedtls_config.h
+++ b/thirdparty/mbedtls/include/godot_core_mbedtls_config.h
@@ -33,6 +33,9 @@
 
 #include <limits.h>
 
+// Include default mbedTLS config.
+#include <mbedtls/mbedtls_config.h>
+
 // For AES
 #define MBEDTLS_CIPHER_MODE_CBC
 #define MBEDTLS_CIPHER_MODE_CFB

Copied from:

// Include default mbedTLS config.
#include <mbedtls/mbedtls_config.h>

Cross-compiling is probably unrelated to this, but I can't test it on Windows and thought it would be better to mention it.

Steps to reproduce

Compile to Windows using module_mbedtls_enabled=no

@Faless
Copy link
Collaborator

Faless commented Apr 15, 2024

Just tested building Windows with VS 2019 MSVC and module_mbedtls_enabled=no and I get no error.

So it might be a mingw specific path, I'll try cross compiling from linux now.

@Faless
Copy link
Collaborator

Faless commented Apr 15, 2024

I confirm I get the same error with MinGW/LLVM from Linux, will investigate, we might be missing some new define in our core config.

@Faless
Copy link
Collaborator

Faless commented Apr 15, 2024

And this is why it works on VS 2019 (_MSC_VER > 1900), so the issue also affects VS 2017 (which AFAIU is the minimal supported version to build Godot) (EDIT: _MSC_VER <= 1900 only for VS <= 2015, so it's not an issue with supported MSVC versions, only MinGW):

#if defined(__MINGW32__) || (defined(_MSC_VER) && _MSC_VER <= 1900)
#if !defined(MBEDTLS_PLATFORM_C)
#error "MBEDTLS_PLATFORM_C is required on Windows"
#endif
/* See auto-enabling SNPRINTF_ALT and VSNPRINTF_ALT
* in * config_adjust_legacy_crypto.h */
#endif /* _MINGW32__ || (_MSC_VER && (_MSC_VER <= 1900)) */

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants