godot
godot copied to clipboard
SCons: Enable the experimental Ninja backend and minimize timestamp changes to generated code
This PR implements everything needed to have initial support for the experimental ninja build backend for SCons documented here: https://scons.org/doc/production/HTML/scons-user/ch27s02.html
Unfortunately, just enabling the backend (which was trivial) was not enough, as ninja is timestamp-based and the code generators kept rewriting (and thus retouching) everything for no good reason, tripping it up.
This PR is thus a patchset of 2 related but atomic commits:
- A commit that makes all generators I could find extremely conservative with timestamp changes, through a new method in
methods.pycalledwrite_file_if_needed, which compares the file's contents to the target string before writing to it. The SCU generators had to get a special treatment, where they keep track of every file considered and use that as a reference for what to not delete when globbing everything (thus only deleting any "stale" file). - The actual ninja-enabling commit, which is quite small as it adds just the new option and the few lines needed to turn the backend on.
To use it, you'll need to have ninja installed and a python module named ninja, which offers nothing more than the ninja_syntax.py logic bundled by ninja.
To get it, either install the ninja python package like the docs recommend:
# In a virtualenv, or "python" is the native executable:
python -m pip install ninja
# Windows using Python launcher:
py -m pip install ninja
# Anaconda:
conda install -c conda-forge ninja
or just put the two files needed in your local site-packages directory.
Here's a zip which makes SCons use the system version of ninja by bundling the upstream ninja_syntax.py and a stub __init__.py that does absolutely nothing (it just needs to be there to be recognized by SCons): ninja.zip
(yeah, I know, this is a bit unnecessary, but that's how it works. We can improve things upstream if this approach becomes more popular)
After that, just add the ninja=yes option when invoking scons. SCons will generate a build.ninja and quit. After that, just call ninja with whatever flags you might need and enjoy :D
Also, ideally you won't have to call scons never again, unless you want to change some build flag, as ninja will detect any (timestamp-based) change to any SCons build script and invoke everything needed to regenerate itself.
While experimenting sometimes it would trip itself in an infinite loop, probably due to some backwards-and-forwards that changed SConstruct but not its actual contents. In that case, start a clean build by deleting build.ninja and the .ninja folder and try again.
Be aware that not everything is transferred properly for some reason, such as ~whether to build compile_commands.json (it does so by default), the linker choice (it uses ld.bfd on my machine for some reason) and~ the number of jobs (you'll have to manually specify -j while invoking ninja or it will use whatever's the default, which on my machine is 10).
~Edit: more precisely, custom.py is not working completely for some reason. Passing manually the linker or other things will work, with the exception of compile_db and the jobs as it looks like they might be treated specially by SCons. I think that this workflow does not really allow it in the first place by design, so I would not rely on it.~
Edit2: I had an old version of scons (4.3.0). The latest at the time of writing, 4.6.0, imports custom.py fine. It also skips building compile_commands.json correctly now.
That said, since this is still marked as experimental, I would not be surprised if some of those were some upstream limitations, so I would consider this good enough for now and open to testing.
Ooops sorry I did some mistakes while cleaning up the generation method and messed some things up, one sec...
There, should be fixed :D
Edit: nope, not yet :/
Edit 2: ding ding ding! I had to add an handler for FileNotFoundError as I could not find a more clever way of doing it. I hoped that r+ would open a file for reading and creation but in the end this turned out to be the simplest solution I could find.
and the number of jobs (you'll have to manually specify
-jwhile invoking ninja).
The good news though is that Ninja uses all CPU threads by default (unlike make), so builds won't be very slow if you forget to specify it.
@Calinou yeah on my machine the default is 10, which is a bit more of my actual threads but still greater than 1, so that's something :D
@Calinou thanks a lot for testing!
A small issue I noticed is that if you run SCons and Ninja once, then run SCons with no changes and run ninja again, Ninja will check 100+ files instead of exiting early. I'm not sure if this can be fixed though.
Mh, that's interesting. I've run ninja with the -dexplain flag which, as the name implies, explains why it decides to rebuild some file and the only meaningful message that I get on a fresh scons->ninja chain is:
ninja explain: output compile_commands.json older than most recent input build.ninja (1710369857680153344 vs 1710370007912264671)
It doesn't look like the compile_commands.json thing is really a dependency on anything, but as you can notice from the message calling scons regenerates the whole build.ninja. Perhaps ninja invalidates a bunch of stuff when it notices an inconsistency like this. I'm not sure.
I'm not sure if this can be fixed though.
Yeah I don't think we can fix this easily if at all, although one has to call scons relatively rarely, as it's just there for setting the compilation options, see below for some more findings.
BTW I finally found out why in the world I couldn't get it to build with mold: apparently custom.py gets (partially?) ignored, relying entirely on the arguments passed from the CLI.
I don't think that the SCons folks can do much either; TIWAGOS, but I think that this is by design as ninja has to invoke SCons to (re)generate any "dynamic" source file (e.g. anything *.gen.* like the Wayland protocol wrappers) and it has to know the settings in advance or bad things can happen.
Also, it's an explicit non-goal to have configuration handling in ninja[^1]. They look to be extremely minimalist and opinionated, which is what gives them their speed and determinism.
So, I think that we might have to live with that and communicate to the users that this workflow is a bit different from what we're used to. It might not be that bad though, as after all one has to call SCons only once per configuration change (and any SConstruct/SCsub change is taken in account by ninja too)
I think that we can get this to an usable state as-is if we become aware of its quirks and limitations. The current speed boost sounds already very appetizing IMO.
Eventually, we could discuss this further with the SCons folks; I suppose that they would appreciate the feedback and perhaps we could help get a bunch of improvements upstream too, such as with ninja detection ~or perhaps a way to disable compile_commands.json for people who don't need it, as it looks hardcoded.~ Edit: I had an old SCons version, it's not hardcoded anymore.
Hopefully this shines some further light on this backend, as there's unfortunately not much documentation, despite this being surprisingly rock solid for being experimental (we're a huge SCons project and it built first try, requiring only a few tweaks to have perfect incremental rebuilds, that's impressive :D )
[^1]: From https://ninja-build.org/manual.html#_design_goals: "Some explicit non-goals: [...] * build-time customization of the build. Options belong in the program that generates the ninja files."
So folks I did some more testing and realized I had a 2 year old version of SCons...
The latest, 4.6.0 is faster, better, harder and stronger: custom.py works again (although it's a bit inconsistent as it exports the actual commands and the cli invocation but not the whole environment, so there's chance for some unreliability in the result) and compile_commands.json is not built anymore by default.
In other words, this is even more stable than I reported before :)
Can you do a small rebase against master? Thanks!
@fire rebased!
I also added a version check like with CompileDB, used the native setting for disabling the ninja autorun and renamed the new flaky file whitelist to paths_to_keep because that word is a bit... Controversial? And apparently native speakers feel way more uneasy with that word than non-native speakers like me[^1].
[^1]: Based from some previous informal discussions, I have no idea if that's really the case but in this case changing it was painless and, personally, a bit more consistent with how I like my variables named.
On windows 11, scons ninja=yes failed.
scons use_mingw=yes use_llvm=yes ninja=yes scu_build=yes
Found some warnings and a failure on Windows 11 with python 3.12.
In file included from platform/windows/rendering_context_driver_vulkan_windows.cpp:35:
platform/windows/rendering_context_driver_vulkan_windows.h:55:50: warning: class with destructor marked 'final' cannot be inherited from [-Wfinal-dtor-non-final-class]
41 | class RenderingContextDriverVulkanWindows : public RenderingContextDriverVulkan {
| final
42 | private:
43 | const char *_get_platform_surface_extension() const override final;
44 |
45 | protected:
46 | SurfaceID surface_create(const void *p_platform_data) override final;
47 |
48 | public:
49 | struct WindowPlatformData {
50 | HWND window;
51 | HINSTANCE instance;
52 | };
53 |
54 | RenderingContextDriverVulkanWindows();
55 | ~RenderingContextDriverVulkanWindows() override final;
| ^
platform/windows/rendering_context_driver_vulkan_windows.h:41:7: note: mark 'RenderingContextDriverVulkanWindows' as 'final' to silence this warning
41 | class RenderingContextDriverVulkanWindows : public RenderingContextDriverVulkan {
| ^
1 warning generated.
[583/1820] Compiling platform\windows\display_server_windows.windows.editor.x86_64.llvm.o
In file included from platform/windows/display_server_windows.cpp:43:
platform/windows/rendering_context_driver_vulkan_windows.h:55:50: warning: class with destructor marked 'final' cannot be inherited from [-Wfinal-dtor-non-final-class]
41 | class RenderingContextDriverVulkanWindows : public RenderingContextDriverVulkan {
| final
42 | private:
43 | const char *_get_platform_surface_extension() const override final;
44 |
45 | protected:
46 | SurfaceID surface_create(const void *p_platform_data) override final;
47 |
48 | public:
49 | struct WindowPlatformData {
50 | HWND window;
51 | HINSTANCE instance;
52 | };
53 |
54 | RenderingContextDriverVulkanWindows();
55 | ~RenderingContextDriverVulkanWindows() override final;
| ^
platform/windows/rendering_context_driver_vulkan_windows.h:41:7: note: mark 'RenderingContextDriverVulkanWindows' as 'final' to silence this warning
41 | class RenderingContextDriverVulkanWindows : public RenderingContextDriverVulkan {
| ^
1 warning generated.
FAILED: thirdparty/mbedtls/library/constant_time.windows.editor.x86_64.llvm.o
x86_64-w64-mingw32-clang "-o" "thirdparty/mbedtls/library/constant_time.windows.editor.x86_64.llvm.o" "-c" "-std=gnu11" "-O2" "-Wall" "-Wshadow-field-in-constructor" "-Wshadow-uncaptured-local" "-Wno-ordered-compare-function-pointers" "-MMD" "-MF" "thirdparty/mbedtls/library/constant_time.windows.editor.x86_64.llvm.o.d" "-w" "-isystem" "thirdparty/glad" "-DTOOLS_ENABLED" "-DDEBUG_ENABLED" "-DEIGEN_MPL2_ONLY" "-DNDEBUG" "-DNO_EDITOR_SPLASH" "-DWINDOWS_ENABLED" "-DWASAPI_ENABLED" "-DWINMIDI_ENABLED" "-DWINVER=0x0601" "-D_WIN32_WINNT=0x0601" "-DVULKAN_ENABLED" "-DRD_ENABLED" "-DGLES3_ENABLED" "-DMINGW_ENABLED" "-DMINGW_HAS_SECURE_API=1" "-DMINIZIP_ENABLED" "-DBROTLI_ENABLED" "-DTHREADS_ENABLED" "-DCLIPPER2_ENABLED" "-DZSTD_STATIC_LINKING_ONLY" "-DUSE_VOLK" "-DVK_USE_PLATFORM_WIN32_KHR" "-DGLAD_ENABLED" "-DEGL_ENABLED" "-DGODOT_MODULE" "-DMBEDTLS_CONFIG_FILE=/"thirdparty/mbedtls/include/godot_module_mbedtls_config.h/"" "-Ithirdparty/mbedtls/include" "-Ithirdparty/libpng" "-Ithirdparty/volk" "-Ithirdparty/vulkan" "-Ithirdparty/vulkan/include" "-Ithirdparty/zstd" "-Ithirdparty/zlib" "-Ithirdparty/clipper2/include" "-Ithirdparty/brotli/include" "-Ithirdparty/angle/include" "-Iplatform/windows" "-I." "thirdparty/mbedtls/library/constant_time.c"
In file included from thirdparty/mbedtls/library/constant_time.c:13:
thirdparty/mbedtls/library/common.h:15:10: error: expected "FILENAME" or <FILENAME>
15 | #include MBEDTLS_CONFIG_FILE
| ^
<command line>:26:29: note: expanded from macro 'MBEDTLS_CONFIG_FILE'
26 | #define MBEDTLS_CONFIG_FILE /thirdparty/mbedtls/include/godot_module_mbedtls_config.h/
| ^
In file included from thirdparty/mbedtls/library/constant_time.c:16:
thirdparty/mbedtls/include/mbedtls/error.h:16:10: error: expected "FILENAME" or <FILENAME>
16 | #include MBEDTLS_CONFIG_FILE
| ^
<command line>:26:29: note: expanded from macro 'MBEDTLS_CONFIG_FILE'
26 | #define MBEDTLS_CONFIG_FILE /thirdparty/mbedtls/include/godot_module_mbedtls_config.h/
| ^
In file included from thirdparty/mbedtls/library/constant_time.c:17:
thirdparty/mbedtls/include/mbedtls/platform_util.h:17:10: error: expected "FILENAME" or <FILENAME>
17 | #include MBEDTLS_CONFIG_FILE
| ^
<command line>:26:29: note: expanded from macro 'MBEDTLS_CONFIG_FILE'
26 | #define MBEDTLS_CONFIG_FILE /thirdparty/mbedtls/include/godot_module_mbedtls_config.h/
| ^
3 errors generated.
To be clear this isn't a blocker and we can work on it later as long as one platform works.
Can you check that there is no breakage if you don't use ninja=yes?
scons use_mingw=yes use_llvm=yes ninja=no scu_build=yes
Seems to error in different places.
[Initial build] Linking Program bin\godot.windows.editor.x86_64.llvm.exe ...
=====
ld.lld: error: undefined symbol: OS::get_exit_code() const
>>> referenced by platform\windows\godot_windows.windows.editor.x86_64.llvm.o:(widechar_main(int, wchar_t**))
>>> referenced by platform\windows\os_windows.windows.editor.x86_64.llvm.o:(vtable for OS_Windows)
ld.lld: error: undefined symbol: ScriptDebugger::set_depth(int)
>>> referenced by platform\windows\os_windows.windows.editor.x86_64.llvm.o:(HandlerRoutine(unsigned long))
ld.lld: error: undefined symbol: ScriptDebugger::set_lines_left(int)
>>> referenced by platform\windows\os_windows.windows.editor.x86_64.llvm.o:(HandlerRoutine(unsigned long))
ld.lld: error: undefined symbol: String::utf16() const
>>> referenced by platform\windows\os_windows.windows.editor.x86_64.llvm.o:(OS_Windows::alert(String const&, String const&))
>>> referenced by platform\windows\os_windows.windows.editor.x86_64.llvm.o:(OS_Windows::alert(String const&, String const&))
>>> referenced by platform\windows\os_windows.windows.editor.x86_64.llvm.o:(_error_handler(void*, char const*, char const*, int, char const*, char const*, bool, ErrorHandlerType))
>>> referenced 111 more times
ld.lld: error: undefined symbol: Char16String::get_data() const
>>> referenced by platform\windows\os_windows.windows.editor.x86_64.llvm.o:(OS_Windows::alert(String const&, String const&))
>>> referenced by platform\windows\os_windows.windows.editor.x86_64.llvm.o:(OS_Windows::alert(String const&, String const&))
>>> referenced by platform\windows\os_windows.windows.editor.x86_64.llvm.o:(debug_dynamic_library_check_dependencies(String const&, String const&, HashSet<String, HashMapHasherDefault, HashMapComparatorDefault<String>>&, HashSet<String, HashMapHasherDefault, HashMapComparatorDefault<String>>&))
>>> referenced 122 more times
ld.lld: error: undefined symbol: Memory::free_static(void*, bool)
>>> referenced by platform\windows\os_windows.windows.editor.x86_64.llvm.o:(OS_Windows::alert(String const&, String const&))
>>> referenced by platform\windows\os_windows.windows.editor.x86_64.llvm.o:(OS_Windows::alert(String const&, String const&))
>>> referenced by platform\windows\os_windows.windows.editor.x86_64.llvm.o:(OS_Windows::initialize())
>>> referenced 58637 more times
ld.lld: error: undefined symbol: NetSocketPosix::make_default()
>>> referenced by platform\windows\os_windows.windows.editor.x86_64.llvm.o:(OS_Windows::initialize())
ld.lld: error: undefined symbol: operator new(unsigned long long, char const*)
>>> referenced by platform\windows\os_windows.windows.editor.x86_64.llvm.o:(OS_Windows::initialize())
>>> referenced by platform\windows\os_windows.windows.editor.x86_64.llvm.o:(OS_Windows::OS_Windows(HINSTANCE__*))
>>> referenced by platform\windows\os_windows.windows.editor.x86_64.llvm.o:(OS_Windows::OS_Windows(HINSTANCE__*))
>>> referenced 3684 more times
ld.lld: error: undefined symbol: IPUnix::make_default()
>>> referenced by platform\windows\os_windows.windows.editor.x86_64.llvm.o:(OS_Windows::initialize())
ld.lld: error: undefined symbol: is_print_verbose_enabled()
>>> referenced by platform\windows\os_windows.windows.editor.x86_64.llvm.o:(OS_Windows::initialize())
>>> referenced by platform\windows\os_windows.windows.editor.x86_64.llvm.o:(OS_Windows::initialize())
>>> referenced by platform\windows\os_windows.windows.editor.x86_64.llvm.o:(OS_Windows::OS_Windows(HINSTANCE__*))
>>> referenced 117 more times
ld.lld: error: undefined symbol: Variant::Variant(char const*)
>>> referenced by platform\windows\os_windows.windows.editor.x86_64.llvm.o:(OS_Windows::initialize())
>>> referenced by platform\windows\os_windows.windows.editor.x86_64.llvm.o:(OS_Windows::get_memory_info() const)
>>> referenced by platform\windows\os_windows.windows.editor.x86_64.llvm.o:(OS_Windows::get_memory_info() const)
>>> referenced 1421 more times
ld.lld: error: undefined symbol: stringify_variants(Variant const&)
>>> referenced by platform\windows\os_windows.windows.editor.x86_64.llvm.o:(OS_Windows::initialize())
>>> referenced by platform\windows\os_windows.windows.editor.x86_64.llvm.o:(OS_Windows::OS_Windows(HINSTANCE__*))
>>> referenced by platform\windows\display_server_windows.windows.editor.x86_64.llvm.o:(DisplayServerWindows::_update_tablet_ctx(String const&, String const&))
>>> referenced 137 more times
ld.lld: error: undefined symbol: __print_line(String const&)
>>> referenced by platform\windows\os_windows.windows.editor.x86_64.llvm.o:(OS_Windows::initialize())
>>> referenced by platform\windows\os_windows.windows.editor.x86_64.llvm.o:(OS_Windows::OS_Windows(HINSTANCE__*))
>>> referenced by platform\windows\display_server_windows.windows.editor.x86_64.llvm.o:(DisplayServerWindows::_update_tablet_ctx(String const&, String const&))
>>> referenced 135 more times
ld.lld: error: undefined symbol: Variant::_clear_internal()
>>> referenced by platform\windows\os_windows.windows.editor.x86_64.llvm.o:(OS_Windows::initialize())
>>> referenced by platform\windows\os_windows.windows.editor.x86_64.llvm.o:(OS_Windows::get_memory_info() const)
>>> referenced by platform\windows\os_windows.windows.editor.x86_64.llvm.o:(OS_Windows::get_memory_info() const)
>>> referenced 8522 more times
ld.lld: error: undefined symbol: String::utf8(char const*, int)
>>> referenced by platform\windows\os_windows.windows.editor.x86_64.llvm.o:(_error_handler(void*, char const*, char const*, int, char const*, char const*, bool, ErrorHandlerType))
>>> referenced by platform\windows\os_windows.windows.editor.x86_64.llvm.o:(_error_handler(void*, char const*, char const*, int, char const*, char const*, bool, ErrorHandlerType))
>>> referenced by platform\windows\os_windows.windows.editor.x86_64.llvm.o:(_error_handler(void*, char const*, char const*, int, char const*, char const*, bool, ErrorHandlerType))
>>> referenced 212 more times
ld.lld: error: undefined symbol: String::String(char const*)
>>> referenced by platform\windows\os_windows.windows.editor.x86_64.llvm.o:(_error_handler(void*, char const*, char const*, int, char const*, char const*, bool, ErrorHandlerType))
>>> referenced by platform\windows\os_windows.windows.editor.x86_64.llvm.o:(_error_handler(void*, char const*, char const*, int, char const*, char const*, bool, ErrorHandlerType))
>>> referenced by platform\windows\os_windows.windows.editor.x86_64.llvm.o:(debug_dynamic_library_check_dependencies(String const&, String const&, HashSet<String, HashMapHasherDefault, HashMapComparatorDefault<String>>&, HashSet<String, HashMapHasherDefault, HashMapComparatorDefault<String>>&))
>>> referenced 21628 more times
ld.lld: error: undefined symbol: String::operator+(String const&) const
>>> referenced by platform\windows\os_windows.windows.editor.x86_64.llvm.o:(_error_handler(void*, char const*, char const*, int, char const*, char const*, bool, ErrorHandlerType))
>>> referenced by platform\windows\os_windows.windows.editor.x86_64.llvm.o:(_error_handler(void*, char const*, char const*, int, char const*, char const*, bool, ErrorHandlerType))
>>> referenced by platform\windows\os_windows.windows.editor.x86_64.llvm.o:(_error_handler(void*, char const*, char const*, int, char const*, char const*, bool, ErrorHandlerType))
>>> referenced 2493 more times
ld.lld: error: undefined symbol: itos(long long)
>>> referenced by platform\windows\os_windows.windows.editor.x86_64.llvm.o:(_error_handler(void*, char const*, char const*, int, char const*, char const*, bool, ErrorHandlerType))
>>> referenced by platform\windows\os_windows.windows.editor.x86_64.llvm.o:(format_error_message(unsigned long))
>>> referenced by platform\windows\os_windows.windows.editor.x86_64.llvm.o:(OS_Windows::move_to_trash(String const&))
>>> referenced 344 more times
ld.lld: error: undefined symbol: String::operator+=(char const*)
>>> referenced by platform\windows\os_windows.windows.editor.x86_64.llvm.o:(_error_handler(void*, char const*, char const*, int, char const*, char const*, bool, ErrorHandlerType))
>>> referenced by platform\windows\os_windows.windows.editor.x86_64.llvm.o:(OS_Windows::open_dynamic_library(String const&, void*&, bool, String*))
>>> referenced by libmain.windows.editor.x86_64.llvm.a(main.windows.editor.x86_64.llvm.o):(Main::setup(char const*, int, char**, bool))
>>> referenced 58 more times
ld.lld: error: undefined symbol: Memory::alloc_static(unsigned long long, bool)
>>> referenced by platform\windows\os_windows.windows.editor.x86_64.llvm.o:(HashMap<long long, OS_Windows::ProcessInfo, HashMapHasherDefault, HashMapComparatorDefault<long long>, DefaultTypedAllocator<HashMapElement<long long, OS_Windows::ProcessInfo>>>::insert(long long const&, OS_Windows::ProcessInfo const&, bool))
>>> referenced by platform\windows\os_windows.windows.editor.x86_64.llvm.o:(HashMap<long long, OS_Windows::ProcessInfo, HashMapHasherDefault, HashMapComparatorDefault<long long>, DefaultTypedAllocator<HashMapElement<long long, OS_Windows::ProcessInfo>>>::insert(long long const&, OS_Windows::ProcessInfo const&, bool))
>>> referenced by platform\windows\os_windows.windows.editor.x86_64.llvm.o:(HashSet<String, HashMapHasherDefault, HashMapComparatorDefault<String>>::insert(String const&))
>>> referenced 1307 more times
ld.lld: error: too many errors emitted, stopping now (use --error-limit=0 to see all errors)
clang-18: error: linker command failed with exit code 1 (use -v to see invocation)
=====
scons: *** [bin\godot.windows.editor.x86_64.llvm.exe] Error 1
scons: building terminated because of errors.
[Time elapsed: 00:02:35.360]
PS D:\20240307_windows\other\VSK_godot> scons use_mingw=yes use_llvm=yes ninja=no scu_build=yes
Let me try harder to see if I can get a build on any platform.
Edited:
Build failure on windows using msvc.
https://gist.github.com/fire/69086e702fc02bca20c515a50b518cdc
I wasn't able to install python package ninja easily, but I'll try mac.
I have to do some errands so I won't be able test currently.
I've tested this PR on Windows 11 23H2 with MSVC 2022, but I get a build failure when calling ninja:
C:\Users\Hugo\Documents\Git\godotengine\godot>ninja
[2/2403] Compiling core\config\project_settings.windows.editor.x86_64.obj
FAILED: core/config/project_settings.windows.editor.x86_64.obj
cl "/Focore/config/project_settings.windows.editor.x86_64.obj" "/c" "core/config/project_settings.cpp" "/TP" "/std:c++17" "/nologo" "/MT" "/Gd" "/GR" "/utf-8" "/bigobj" "/Zi" "/FS" "/Od" "/W3" "/w34458" "/wd4100" "/wd4127" "/wd4201" "/wd4244" "/wd4245" "/wd4267" "/wd4305" "/wd4514" "/wd4714" "/wd4820" "/showIncludes" "/DTOOLS_ENABLED" "/DDEBUG_ENABLED" "/DNDEBUG" "/DNO_EDITOR_SPLASH" "/DWINDOWS_ENABLED" "/DWASAPI_ENABLED" "/DWINMIDI_ENABLED" "/DTYPED_METHOD_BIND" "/DWIN32" "/DWINVER=0x0601" "/D_WIN32_WINNT=0x0601" "/DNOMINMAX" "/D_WIN64" "/DVULKAN_ENABLED" "/DRD_ENABLED" "/DGLES3_ENABLED" "/D_HAS_EXCEPTIONS=0" "/DMINIZIP_ENABLED" "/DBROTLI_ENABLED" "/DTHREADS_ENABLED" "/DCLIPPER2_ENABLED" "/DZSTD_STATIC_LINKING_ONLY" "/Ithirdparty/zstd" "/Ithirdparty/zlib" "/Ithirdparty/clipper2/include" "/Ithirdparty/brotli/include" "/Ithirdparty/angle/include" "/Iplatform/windows" "/I."
CreateProcess failed: The system cannot find the file specified.
[3/2403] Compiling core\extension\extension_api_dump.windows.editor.x86_64.obj
FAILED: core/extension/extension_api_dump.windows.editor.x86_64.obj
cl "/Focore/extension/extension_api_dump.windows.editor.x86_64.obj" "/c" "core/extension/extension_api_dump.cpp" "/TP" "/std:c++17" "/nologo" "/MT" "/Gd" "/GR" "/utf-8" "/bigobj" "/Zi" "/FS" "/Od" "/W3" "/w34458" "/wd4100" "/wd4127" "/wd4201" "/wd4244" "/wd4245" "/wd4267" "/wd4305" "/wd4514" "/wd4714" "/wd4820" "/showIncludes" "/DTOOLS_ENABLED" "/DDEBUG_ENABLED" "/DNDEBUG" "/DNO_EDITOR_SPLASH" "/DWINDOWS_ENABLED" "/DWASAPI_ENABLED" "/DWINMIDI_ENABLED" "/DTYPED_METHOD_BIND" "/DWIN32" "/DWINVER=0x0601" "/D_WIN32_WINNT=0x0601" "/DNOMINMAX" "/D_WIN64" "/DVULKAN_ENABLED" "/DRD_ENABLED" "/DGLES3_ENABLED" "/D_HAS_EXCEPTIONS=0" "/DMINIZIP_ENABLED" "/DBROTLI_ENABLED" "/DTHREADS_ENABLED" "/DCLIPPER2_ENABLED" "/DZSTD_STATIC_LINKING_ONLY" "/Ithirdparty/zstd" "/Ithirdparty/zlib" "/Ithirdparty/clipper2/include" "/Ithirdparty/brotli/include" "/Ithirdparty/angle/include" "/Iplatform/windows" "/I."
CreateProcess failed: The system cannot find the file specified.
[4/2403] Compiling editor\animation_track_editor.windows.editor.x86_64.obj
FAILED: editor/animation_track_editor.windows.editor.x86_64.obj
cl "/Foeditor/animation_track_editor.windows.editor.x86_64.obj" "/c" "editor/animation_track_editor.cpp" "/TP" "/std:c++17" "/nologo" "/MT" "/Gd" "/GR" "/utf-8" "/bigobj" "/Zi" "/FS" "/Od" "/W3" "/w34458" "/wd4100" "/wd4127" "/wd4201" "/wd4244" "/wd4245" "/wd4267" "/wd4305" "/wd4514" "/wd4714" "/wd4820" "/showIncludes" "/DTOOLS_ENABLED" "/DDEBUG_ENABLED" "/DNDEBUG" "/DNO_EDITOR_SPLASH" "/DWINDOWS_ENABLED" "/DWASAPI_ENABLED" "/DWINMIDI_ENABLED" "/DTYPED_METHOD_BIND" "/DWIN32" "/DWINVER=0x0601" "/D_WIN32_WINNT=0x0601" "/DNOMINMAX" "/D_WIN64" "/DVULKAN_ENABLED" "/DRD_ENABLED" "/DGLES3_ENABLED" "/D_HAS_EXCEPTIONS=0" "/DMINIZIP_ENABLED" "/DBROTLI_ENABLED" "/DTHREADS_ENABLED" "/DCLIPPER2_ENABLED" "/DZSTD_STATIC_LINKING_ONLY" "/DUSE_VOLK" "/DVK_USE_PLATFORM_WIN32_KHR" "/DGLAD_ENABLED" "/DEGL_ENABLED" "/Ithirdparty/freetype/include" "/Ithirdparty/libpng" "/Ithirdparty/glad" "/Ithirdparty/volk" "/Ithirdparty/vulkan" "/Ithirdparty/vulkan/include" "/Ithirdparty/zstd" "/Ithirdparty/zlib" "/Ithirdparty/clipper2/include" "/Ithirdparty/brotli/include" "/Ithirdparty/angle/include" "/Iplatform/windows" "/I."
CreateProcess failed: The system cannot find the file specified.
...
It works when I start x64 Native Tools Command Prompt for VS 2022 in the start menu, so the automatic MSVC detection isn't working anymore. Is this something we could contribute to Ninja upstream? It helps a lot with MSVC usability, especially for people less familiar with compiling C++ code on Windows.
Afterwards, there's a build failure early on:
[5/1813] Compiling core\crypto\crypto_core.windows.editor.x86_64.obj
FAILED: core/crypto/crypto_core.windows.editor.x86_64.obj
cl "/Focore/crypto/crypto_core.windows.editor.x86_64.obj" "/c" "core/crypto/crypto_core.cpp" "/TP" "/std:c++17" "/nologo" "/MT" "/Gd" "/GR" "/utf-8" "/bigobj" "/Zi" "/FS" "/Od" "/W3" "/w34458" "/wd4100" "/wd4127" "/wd4201" "/wd4244" "/wd4245" "/wd4267" "/wd4305" "/wd4514" "/wd4714" "/wd4820" "/showIncludes" "/DTOOLS_ENABLED" "/DDEBUG_ENABLED" "/DNDEBUG" "/DNO_EDITOR_SPLASH" "/DWINDOWS_ENABLED" "/DWASAPI_ENABLED" "/DWINMIDI_ENABLED" "/DTYPED_METHOD_BIND" "/DWIN32" "/DWINVER=0x0601" "/D_WIN32_WINNT=0x0601" "/DNOMINMAX" "/D_WIN64" "/DVULKAN_ENABLED" "/DRD_ENABLED" "/DGLES3_ENABLED" "/D_HAS_EXCEPTIONS=0" "/DMINIZIP_ENABLED" "/DBROTLI_ENABLED" "/DTHREADS_ENABLED" "/DCLIPPER2_ENABLED" "/DZSTD_STATIC_LINKING_ONLY" "/DMBEDTLS_CONFIG_FILE=/"thirdparty/mbedtls/include/godot_module_mbedtls_config.h/"" "/Ithirdparty/mbedtls/include" "/Ithirdparty/zstd" "/Ithirdparty/zlib" "/Ithirdparty/clipper2/include" "/Ithirdparty/brotli/include" "/Ithirdparty/angle/include" "/Iplatform/windows" "/I."
C:\Users\Hugo\Documents\Git\godotengine\godot\thirdparty\mbedtls\include\mbedtls/aes.h(34): error C2006: '#include': expected "FILENAME" or <FILENAME>
C:\Users\Hugo\Documents\Git\godotengine\godot\thirdparty\mbedtls\include\mbedtls/aes.h(34): fatal error C1083: Cannot open include file: '': No such file or directory
This is the code in question: https://github.com/godotengine/godot/blob/99ff024f78f65ba0bc54fb409cfeca43ba2008fe/thirdparty/mbedtls/include/mbedtls/aes.h#L34
The generated paths in build.ninja look like this:
"/DMBEDTLS_CONFIG_FILE=/"thirdparty/mbedtls/include/godot_module_mbedtls_config.h/"" $
Maybe it's an issue with how quotes are escaped?
@Calinou FTR, on my linux machine the same line is escaped properly:
"-DMBEDTLS_CONFIG_FILE=\"thirdparty/mbedtls/include/godot_module_mbedtls_config.h\"" $
I suppose that some platform code is incorrectly swapping \ and / or something (Windows uses a different path separator after all, so that makes some sense). Not sure where it's doing that though.
I don't think that this is blocking, as @fire said. This looks like an upstream issue to me. Edit: no idea where I took that from. They didn't say that, they only said it wasn't blocking. Oops, I think I took that from calinou, sorry.
Might be worth an issue ticket.
Been testing locally on my Windows machine, and two things stood out:
- SCons automatically generates
run_ninja_env.batin the repo root, and running that runs ninja with the proper environment variables set. The SCons repo admits this isn't the prettiest solution^1, but it's a manageable workaround while ninja currently lacks the functionality; thoughrun_ninja_env.batwill need to be added to.gitignore. - The
DMBEDTLS_CONFIG_FILEquote-escape issue can be circumvented by substituting angle brackets instead. By changing the relevant SCsub defines from'\\"some_header.h\\"'to"<some_header.h>", everything compiles as expected. This obviously doesn't "fix" the core issue, but this at least allows for MSVC functionality out of the gate.
Diff
diff --git a/.gitignore b/.gitignore
index 46dcf84b43..3946cc96e5 100644
--- a/.gitignore
+++ b/.gitignore
@@ -38,6 +38,7 @@ platform/windows/godot_res.res
# Ninja build files
build.ninja
.ninja
+run_ninja_env.bat
# Generated by Godot binary
.import/
diff --git a/core/crypto/SCsub b/core/crypto/SCsub
index ac79e10d19..093cb5a2f4 100644
--- a/core/crypto/SCsub
+++ b/core/crypto/SCsub
@@ -21,9 +21,7 @@ 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\\"')]
- )
+ env_crypto.Append(CPPDEFINES=[("MBEDTLS_CONFIG_FILE", "<thirdparty/mbedtls/include/godot_core_mbedtls_config.h>")])
# Build minimal mbedTLS library (MD5/SHA/Base64/AES).
env_thirdparty = env_crypto.Clone()
env_thirdparty.disable_warnings()
@@ -47,7 +45,7 @@ if not has_module:
elif is_builtin:
# Module mbedTLS config file
env_crypto.Append(
- CPPDEFINES=[("MBEDTLS_CONFIG_FILE", '\\"thirdparty/mbedtls/include/godot_module_mbedtls_config.h\\"')]
+ CPPDEFINES=[("MBEDTLS_CONFIG_FILE", "<thirdparty/mbedtls/include/godot_module_mbedtls_config.h>")]
)
# Needed to force rebuilding the core files when the configuration file is updated.
thirdparty_obj = ["#thirdparty/mbedtls/include/godot_module_mbedtls_config.h"]
diff --git a/modules/mbedtls/SCsub b/modules/mbedtls/SCsub
index 4b8f65d8ff..dc6b214552 100644
--- a/modules/mbedtls/SCsub
+++ b/modules/mbedtls/SCsub
@@ -101,7 +101,7 @@ if env["builtin_mbedtls"]:
env_mbed_tls.Prepend(CPPPATH=["#thirdparty/mbedtls/include/"])
env_mbed_tls.Append(
- CPPDEFINES=[("MBEDTLS_CONFIG_FILE", '\\"thirdparty/mbedtls/include/godot_module_mbedtls_config.h\\"')]
+ CPPDEFINES=[("MBEDTLS_CONFIG_FILE", "<thirdparty/mbedtls/include/godot_module_mbedtls_config.h>")]
)
env_thirdparty = env_mbed_tls.Clone()
Thanks!