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

Add meson build system #52311

Closed
wants to merge 17 commits into from
Closed

Add meson build system #52311

wants to merge 17 commits into from

Conversation

Faless
Copy link
Collaborator

@Faless Faless commented Sep 1, 2021

In this PR:

Missing modules:
raycast, webm, opus, webm, theora, gdnative, etcpak, freetype, mono.

text_server_adv fails to link.

Additionally, at least the X11 display driver does not work (fails to connect to display).
The windows build is broken because... well, I don't know, meson doesn't seem to get the path slashes right.

This is as far as I will go, I already spent 3 days fighting with this and I'm totally fed up.
If anyone want to keep up the work, I think this is a solid base which is also easy to rebase.

Note: some cleanup of the doc generation should fix the mac editor build. I'll try to extract some of the scons code and move it to editor_generators.py as it should be.
Note2: I don't think iphone is actually a valid iphone build.

@Faless Faless added this to the 4.0 milestone Sep 1, 2021
@Faless Faless changed the title Add meson a buildsystem Add meson build system Sep 1, 2021
@fire
Copy link
Member

fire commented Sep 1, 2021

My comment is if we can run meson side by side with scons, we can work on this incrementally.

Need to develop a plan with Akien though.

@Faless
Copy link
Collaborator Author

Faless commented Sep 1, 2021

@fire yes, that was my idea, and the reason hy I reworked the previous PR bringing the sources back in sync.

If anyone can help with the windows and osx builds it would be great.
I can also simplify the CI and have a single "meson" workflow that runs all the platforms.

version_cdata.set_quoted('VERSION_STATUS', 'dev')
version_cdata.set_quoted('VERSION_BUILD', 'custom_build')
version_cdata.set_quoted('VERSION_MODULE_CONFIG', '')
version_cdata.set('VERSION_YEAR', 2020)
Copy link
Contributor

Choose a reason for hiding this comment

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

*2021.

"patch": 0,
"status": "dev",
"module_config": "",
"year": 2020,
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here. 2021.

@@ -0,0 +1,30 @@
name: 'Setup python meson ninja.'
Copy link
Member

Choose a reason for hiding this comment

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

Should these be capitalized? Also, the quotes are inconsistent. Other places in the file use ', ", or no quotes.

@bruvzg
Copy link
Member

bruvzg commented Sep 2, 2021

text_server_adv fails to link.

It's linking for me (on macOS), but generated register_module_types.gen.cpp is empty, and it's not registered properly (I guess all other modules are broken for the same reason). Also, half of the both text server won't build without FreeType.

@Faless
Copy link
Collaborator Author

Faless commented Sep 2, 2021

@bruvzg nicely spot! Should be fixed now. Let's see what happens to CI

@bruvzg
Copy link
Member

bruvzg commented Sep 2, 2021

Another issue, disabling module doesn't disable its dependencies (at least true for denoiseoidn which is not supported on ARM64 and breaks M1 mac build).

@Listwon
Copy link
Contributor

Listwon commented Sep 3, 2021

The windows build is broken because... well, I don't know, meson doesn't seem to get the path slashes right.

@Faless I spent yesterday evening trying to figure out what's wrong and surprisingly it's not the problem with slashes but the Python shebang missing in affected Python scripts. I moved the files around and for unknown reason version.py worked while glsl_builders.py and some others didn't. Adding #!/usr/bin/python3 in glsl_builders.py (the same as in version.py) fixed find_program() issue and meson setup moved to the next script in order - default_theme_builders.py, also missing shebang.

@Faless
Copy link
Collaborator Author

Faless commented Sep 3, 2021

@Listwon thanks a lot for finding that out. I'm growing more and more disappointed with meson :/ .
But let's see how this goes.

@Faless
Copy link
Collaborator Author

Faless commented Sep 7, 2021

Added volk support, now the editor starts (on X11 at least).

Font rendering is totally broken, I realize that FreeType does actually get built (not the module though, because the module in scons only existed to let the library build).

So I think it might be some missing defines that causes the font rendering to be broken.

@Faless Faless force-pushed the meson-pr branch 2 times, most recently from 376cbb7 to dec2851 Compare September 8, 2021 13:36
@reduz
Copy link
Member

reduz commented Sep 13, 2021

I suggest the path forward with Meson would be as follows:

  • Merge this into a "meson" branch in godotengine/godot, so it becomes easier for other contributors to PR into it.
  • Try to get it into a more stable state, something we could call "beta". This means all platforms working and all modules working, minor issues maybe remaining. All build options should be working too.
  • Merge the branch into master and ask contributors to test it while the remaining minor issues are resolved. At this stage, any change to the build system via PR should be made for SCons and Meson, so the idea is that this time of co-existence is as minimal as possible.
  • Deprecate SCons

Does this make sense?

@jpakkane
Copy link

With this diff I can build this branch on Linux and run the result successfully:

diff --git a/modules/text_server_adv/text_server_adv.h b/modules/text_server_adv/text_server_adv.h
index 5989035800..a9a29fd35c 100644
--- a/modules/text_server_adv/text_server_adv.h
+++ b/modules/text_server_adv/text_server_adv.h
@@ -52,6 +52,7 @@
 #include <unicode/uloc.h>
 #include <unicode/uscript.h>
 #include <unicode/ustring.h>
+#include <unicode/utf16.h>
 #include <unicode/utypes.h>
 
 #include "modules/modules_enabled.gen.h"
diff --git a/thirdparty/harfbuzz/meson.build b/thirdparty/harfbuzz/meson.build
index caca75438e..e9b282854d 100644
--- a/thirdparty/harfbuzz/meson.build
+++ b/thirdparty/harfbuzz/meson.build
@@ -73,7 +73,7 @@ if get_option('builtin_harfbuzz')
         '-DHAVE_ICU',
         '-DHAVE_FREETYPE',
         '-DHAVE_GRAPHITE2',
-        '-DHB_DISABLE_DEPRECATED',
+        #'-DHB_DISABLE_DEPRECATED',
         '-DGRAPHITE2_STATIC',
     ]

Further, if you add this commit the ordering issues on macOS should get fixed.

@jpakkane
Copy link

With this change it builds and runs on Windows, too:

diff --git a/modules/camera/meson.build b/modules/camera/meson.build
index b5f4457b67..26c15871ed 100644
diff --git a/thirdparty/etcpak/meson.build b/thirdparty/etcpak/meson.build
index 6fc6f41d65..ccb6fb959c 100644
--- a/thirdparty/etcpak/meson.build
+++ b/thirdparty/etcpak/meson.build
@@ -6,5 +6,8 @@ _etcpak_srcs = files([
     'Tables.cpp',
 ])

-_lib_etcpak = static_library('etcpak', _etcpak_srcs, build_by_default: false)
+_lib_etcpak = static_library('etcpak',
+                             _etcpak_srcs,
+                             cpp_args: '-DNOMINMAX',
+                             build_by_default: false)
 DEP_ETCPAK = declare_dependency(link_with: _lib_etcpak)

diff --git a/thirdparty/oidn/common/platform.h b/thirdparty/oidn/common/platform.h
index 9373b617b5..a08cdf39e4 100644
--- a/thirdparty/oidn/common/platform.h
+++ b/thirdparty/oidn/common/platform.h
@@ -18,7 +18,9 @@

 #if defined(_WIN32)
   #define WIN32_LEAN_AND_MEAN
-  #define NOMINMAX
+  #ifndef NOMINMAX
+    #define NOMINMAX
+  #endif
   #include <windows.h>
 #elif defined(__APPLE__)
   #include <sys/sysctl.h>

@Faless
Copy link
Collaborator Author

Faless commented Sep 25, 2021

@jpakkane the whole point of this branch is not changing the C/C++ source.
If it builds with scons, it should build with meson without having to change the source files. Especially third party libraries code.

I'll try to pick the meson.build commits for now.

marstaik and others added 8 commits September 25, 2021 12:00
Finished all modules on except for mono and webm.
Windows compilation with MSVC (LLVM-mingw kind of)
Linux compilation with GCC
Linux cross compile to windows with gcc-mingw-64
Fixes desktop platforms (tested only X11) renderer.
Font rendering it still broken.
@jpakkane
Copy link

With the following patch this builds on Windows for me:

diff --git a/thirdparty/etcpak/meson.build b/thirdparty/etcpak/meson.build
index 6fc6f41d65..7c6cd63c35 100644
--- a/thirdparty/etcpak/meson.build
+++ b/thirdparty/etcpak/meson.build
@@ -1,10 +1,12 @@
 # the include directory
-_etcpak_srcs = files([
+_etcpak_srcs = files(
     'Dither.cpp',
     'ProcessDxtc.cpp',
     'ProcessRGB.cpp',
     'Tables.cpp',
-])
+)

-_lib_etcpak = static_library('etcpak', _etcpak_srcs, build_by_default: false)
+_lib_etcpak = static_library('etcpak',
+    _etcpak_srcs,
+    cpp_args: ['-DNOMINMAX'])
 DEP_ETCPAK = declare_dependency(link_with: _lib_etcpak)

@jpakkane
Copy link

jpakkane commented Sep 27, 2021

This patch is needed to build with Visual Studio. It fails with the same Freetype error as the other ones.

diff --git a/tests/meson.build b/tests/meson.build
index 8bb59768c6..6f4481c7a1 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -3,7 +3,11 @@ if get_option('tests')

     _tests_cpp_args = []
     if PLATFORM == 'windows'
+      if cpp.get_id() == 'msvc'
+        _tests_cpp_args += '/bigobj'
+      else
         _tests_cpp_args += '-DDOCTEST_THREAD_LOCAL'
+      endif
     endif

     #_tests_incdirs = include_directories('.')

@jpakkane
Copy link

In order to set up the VS environment on Github Actions, you need to use this action. From what I can tell, adding one line in .github/workflows/meson.yml line 106 should do it:

  - uses: actions/checkout@v2
  - uses: ilammy/msvc-dev-cmd@v1

Scons seems to autoactivate the VS environment. Meson also has code for that, but it only does it if it detects that no other build toolchain is currently available. The assumption here being that if you have added a build toolchain in your 'PATH` then that is probably what you want to use.

@lightspot21
Copy link
Contributor

Any progress on this?

@jpakkane
Copy link

I have kept my mesoncitest2 branch up to date, but that's about as much as I can do. To get this going someone from the dev team must make a policy decision to get it upstream and from there to usage.

@underdoeg
Copy link
Contributor

underdoeg commented Feb 2, 2022

Is this going somwhere? The release of 4.0 would be a great moment to introduce a better build system.

@akien-mga
Copy link
Member

I'll share what I wrote on the Godot chat for context:

jpakkane | There was some talk that the Meson branch would receive proper review after the alfa is out. What's the status for that (the branch is up to date as of a few days ago).

Akien | @jpakkane That was my expectation but I see now that it will take longer. I really need to be able to dedicate several days to reviewing your work, learning Meson at the same time, and assess what's the status for the many build configurations that we currently support (e.g. building for macOS and iOS from Linux with OSXCross, MinGW support with GCC and Clang from Windows, Linux and macOS, Mono builds, etc.).
That's a ton of work that I still can't allocate right now while all my time is taken by maintaining three branches in parallel (master, 3.x, 3.4).
Godot 4.0 is a very complex release and we really need to narrow down on bugfixing now so that we can get the release out. I hope to find time to review and assess Meson as a potential replacement for our current buildsystem before 4.0 is released, but at this stage I can't make promises. Changing the buildsystem is not something that we need to do right now, and I can't allocate much time to "nice to have"s at this time (especially right now where reduz has been AWOL for a couple months due to moving countries so I've been leading development mostly alone).
Of course there's always the option discussed a while ago to merge the Meson port in a dedicated feature branch, and let interested contributors work on to bring it to complete feature parity with SCons. But I don't feel right encouraging this work and letting many contributors invest time in it if we haven't decided loud and clear to change the buildsystem - and to decide this I need to spend time with it and confirm that it can do everything we currently do, do it better, and do it with a nicer implementation.

jpakkane | Is there anything I can do to make it easier?

Akien | @jpakkane Probably not right now, though your periodical rebases are appreciated! I don't want to give you more work before I'm able to do my part - especially before I can confirm that we want to proceed with this and commit to merging it as a replacement for the current buildsystem. Once I do spend the needed time to get comfortable with Meson and review the current state, I'll surely have a number of things I'll ask of you and other interested contributors to bring it to full feature parity. (And do my fair share to also get familiar with the implementation so that I'm able to maintain it.)

Note that @jpakkane has been maintaining a branch against current master here for those interested in testing / giving feedback: https://github.com/jpakkane/godot/tree/mesoncitest2

@Faless
Copy link
Collaborator Author

Faless commented Jun 26, 2022

Closing, as development is continuing in #51153

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

Successfully merging this pull request may close these issues.