Daemon icon indicating copy to clipboard operation
Daemon copied to clipboard

external_deps: revamp lib build, add cmake and configure wrapper, update lib feature disablement, update lib versions

Open illwieckz opened this issue 1 year ago • 48 comments

This is WIP, current external_deps build status:

system deps build static build test
linux-amd64-default ✅️ ✅️ ✅️
linux-arm64-default ✅️
linux-i686-default ✅️ ✅️ ✅️
linux-armhf-default ✅️
windows-amd64-mingw ✅️ N/A ✅️
windows-i686-mingw ✅️ Buster ❌️ GLEW Noble N/A ✅️
windows-amd64-msvc ✅️
windows-i686-msvc ✅️
macos-amd64-default ✅️ N/A ✅️

~~I haven't tested if the engine builds and runs properly with those.~~ WIP

What this PR does:

  • Use CMake when packages provide a CMakeLists.txt file
  • Factorize much code
  • Make a nice list of package base url, so it's easy to click all of them and check for new versions
  • Update package versions
  • Update package build options

illwieckz avatar Nov 11 '24 16:11 illwieckz

For windows-i686-mingw I get this with GLEW:

i686-w64-mingw32-ld -nostdlib -shared -soname libglew32.dll --out-implib lib/libglew32.dll.a
     -o lib/glew32.dll tmp/linux-mingw32/default/shared/glew.o
     -Lexternal_deps/build-windows-i686-mingw_10/prefix/lib -L/usr/i686-w64-mingw32/lib
     -lopengl32 -lgdi32 -luser32 -lkernel32 
i686-w64-mingw32-ld: tmp/linux-mingw32/default/shared/glew.o:glew.c:(.text+0x141ab):
 undefined reference to `strlen'

Everything else build.

illwieckz avatar Nov 11 '24 17:11 illwieckz

For windows-amd64-msvc I get this with Vorbis:

[ 84%] Linking C shared library libvorbis.dll
/usr/bin/x86_64-w64-mingw32-ld:
 external_deps/build-windows-amd64-msvc_10/vorbis/libvorbis-1.3.7/win32/vorbis.def:3: syntax error
/usr/bin/x86_64-w64-mingw32-ld:
 external_deps/build-windows-amd64-msvc_10/vorbis/libvorbis-1.3.7/win32/vorbis.def: file format not recognized; treating as linker script
/usr/bin/x86_64-w64-mingw32-ld:
 external_deps/build-windows-amd64-msvc_10/vorbis/libvorbis-1.3.7/win32/vorbis.def:2: syntax error

Everything else build.

illwieckz avatar Nov 11 '24 17:11 illwieckz

  • OGG has a CMakeLists.txt file but no install target, so I returned back to configure for this one.
  • Opus requires at least CMake 3.16.

illwieckz avatar Nov 11 '24 18:11 illwieckz

When building newer Opus on Debian buster (the distro we use for our releases), I get this:

opus/opus-1.5.2/silk/x86/NSQ_del_dec_avx2.c:959:43: warning: implicit declaration of function '_mm_loadu_si64'; did you mean '_mm_loadu_si32'? [-Wimplicit-function-declaration]
         __m256i x = _mm256_cvtepi16_epi64(_mm_loadu_si64(&x16[i]));
                                           ^~~~~~~~~~~~~~
                                           _mm_loadu_si32

Debian Buster provides GCC 8, but GCC 11.3 or GCC 12 may be required: https://stackoverflow.com/questions/72837929/mm-loadu-si32-not-recognized-by-gcc-on-ubuntu

illwieckz avatar Nov 11 '24 18:11 illwieckz

When building newer Opus on Debian buster (the distro we use for our releases), I get this: […]

Telling Opus to not assume more than SSE2 fixed that.

illwieckz avatar Nov 11 '24 18:11 illwieckz

So, the MinGW GLEW and MSVC Vorbis errors are the only ones.

illwieckz avatar Nov 11 '24 18:11 illwieckz

So the Vorbis build error is actually a bug:

  • https://github.com/microsoft/vcpkg/issues/22990
  • https://github.com/microsoft/vcpkg/pull/23761

I added a workaround.

illwieckz avatar Nov 11 '24 19:11 illwieckz

I don't get why I get that GLEW build error with MinGW, the build function has not been modified, and the version has not been updated.

illwieckz avatar Nov 11 '24 19:11 illwieckz

I don't get why I get that GLEW build error with MinGW, the build function has not been modified, and the version has not been updated.

Also the code is the same for both windows-amd64-msvc, windows-i686-msvc, windows-amd64-mingw and windows-i686-mingw, but it only fails with windows-i686-mingw… That doesn't make sense.

illwieckz avatar Nov 11 '24 19:11 illwieckz

So, I don't know what happened, now I don't reproduce the MinGW GLEW error… Maybe I gorgot to prune the prefix folder and some stray files messed-up…

illwieckz avatar Nov 11 '24 19:11 illwieckz

Ah, I now see something: I reproduce the bug with MinGW from Ubuntu 24.04 Noble, not with MinGW from Debian 10 Buster. So, since we produce release builds with Debian Buster, it's not a big problem, but it should be fixed for the future…

illwieckz avatar Nov 11 '24 19:11 illwieckz

What's the purpose of migrating things to build with CMake?

slipher avatar Nov 11 '24 21:11 slipher

So with this a static build for linux-amd64 completes and runs.

illwieckz avatar Nov 16 '24 21:11 illwieckz

So with this a static build for linux-amd64 completes and runs.

Huh? It worked before, so I have a hard time believing that is suddenly necessary to change the build system of 8 packages.

slipher avatar Nov 16 '24 21:11 slipher

I never said this is a response to “What's the purpose of migrating things to build with CMake?”, I'm just reporting the status of me testing that branch. I said in first post:

I haven't tested if the engine builds and runs properly with those.

So now I'm running those tests.

illwieckz avatar Nov 16 '24 21:11 illwieckz

What's the motivation behind this change?

DolceTriade avatar Nov 16 '24 22:11 DolceTriade

Purposes:

  1. Update packages to latest versions,
  2. Update packages build options for latest versions (for example curl adds protocols, so we have to disable them),
  3. Mutualize cmake/configure code as generic reused functions and avoid copy/paste boilerplate,
  4. Rely on CMake as much as possible to make it easy to spot new CMake build options to disable them and other changes.

When using configure it is hard to compare the list of already used options with the new options, one has to read configure's whole output and compare with what's currently used. On the contrary with cmake, one just runs the existing command, then go to the build dir and run ccmake, and see what's enabled and should not, and report the difference to the build script.

illwieckz avatar Nov 18 '24 01:11 illwieckz

windows-amd64-mingw and windows-i686-mingw engine both build and run.

illwieckz avatar Nov 18 '24 03:11 illwieckz

For some reasons libpng now provides png.framework on macOS and the engine build looks for it instead of the static library (even with USE_STATIC_LIBS), so I made generic the copy of any *.framework from deps. I don't know if we prefer static linking or dynamic linking on macOS, static linking allows further LTO optimizations so we may prefer that.

illwieckz avatar Nov 18 '24 18:11 illwieckz

linux-i686-default and macos-amd64-default engine both build and run.

illwieckz avatar Nov 18 '24 18:11 illwieckz

Many libs have a note like "Warning: the maintainers never use CMake; this is provided by contributors and may be out of date." It doesn't necessarily seem like an improvement to switch to CMake for those.

slipher avatar Mar 26 '25 17:03 slipher

Many libs have a note like "Warning: the maintainers never use CMake; this is provided by contributors and may be out of date." It doesn't necessarily seem like an improvement to switch to CMake for those.

This is something I'm afraid of for HarfBuzz. We don't use it yet but I want to use it. There is a CMake file but the authors switched to Meson saying the CMake file may not survive. Unfortunately for the cgame we better build it as a submodule and not having to rely on Meson would be far more convenient…

Here, we can keep CMake as long as it works. This branch now does much more than switching some build to CMake, it deeply refactors both the CMake-based build and the configure-based build, so even switching back some libs to configure-based build would be likely easier than maintaining our current configure-based builds for the same libraries.

illwieckz avatar Mar 26 '25 19:03 illwieckz

This is something I'm afraid of for HarfBuzz. We don't use it yet but I want to use it.

That's only needed for gamelogic, right? I guess we can just write our own little CMake file... like we do for RmlUi for some reason even though it does use CMake.

Here, we can keep CMake as long as it works.

What do you mean "keep" CMake? The PR is proposing to change things from configure scripts to CMake, right?

Any plans to split this PR into smaller pieces?

slipher avatar Mar 26 '25 20:03 slipher

What do you mean "keep" CMake? The PR is proposing to change things from configure scripts to CMake, right?

I will not return to the original configure boiler-plate code. Either libs will uses the new configure wrapper, or the new cmake wrapper, but there is no coming back to the clumsy boiler plate rotten code that was used before.

So, I may keep the new code using the new cmake wrapper, or use the new configure wrapper instead, and I already did that for some libraries that had a CMakeLists.txt but then I noticed it didn't fit the need (like not providing an install target).

Actually switching from cmake wrapper to configure wrapper is easier than maintain the old clumsy configure boiler plate code (as well as maintaining the old clumsy cmake boiler plate we have).

Any plans to split this PR into smaller pieces?

No, not only the cost would be too high and difficultly achievable as changes are deeply interdependent on each others, but splitting the PR may also require to generate many intermediate deps package, which would be a waste.

illwieckz avatar Mar 26 '25 21:03 illwieckz

I tried at first to make specific atomic commits I would squash in different topic commits, but at some point I had to accept that 90% of that work cannot be split in dedicated commits.

illwieckz avatar Mar 26 '25 21:03 illwieckz

No, not only the cost would be too high and difficultly achievable as changes are deeply interdependent on each others, but splitting the PR may also require to generate many intermediate deps package, which would be a waste.

There is no expectation of releasing a deps package every time you modify the script.

slipher avatar Mar 26 '25 22:03 slipher

Yes but even without that,

  • it unifies where macOS framework are saved (in lib/) and read from
  • it updates the disablement/enablement that had not been maintained for a decade
  • it introduces wrappers for configure and cmake
  • it switches some packages to cmake
  • it centralize the url list
  • it bumps some lib versions
  • it adjust the disablement/enablement for them
  • it unifies flags
  • it fixes various bugs
  • it removes lua and freetype (we better not maintain the ability to link against 3 sources for them, submodule and system is enough)

I can't promise to make sure every of this change doesn't require another one. Like some version bump may change the way libs are stored and then read, etc.

illwieckz avatar Mar 26 '25 22:03 illwieckz

Is it ready for review?

slipher avatar Mar 26 '25 22:03 slipher

I now squashed many commits and properly named them. I'm now entering the phase where it is ready to review.

I'm gonna produce test archives so the CI can test them.

illwieckz avatar Mar 26 '25 23:03 illwieckz

It can also be the opportunity to change some other things. For example we use some dylib on macOS from some prebuilt packages, if we don't have the choice, it's fine. But among libs that are either provided prebuilt or that we compile some may provide both a dylib and a static archive, until now we preferred the dylib for some unknown reasons, but we may prefer to ship the static archive instead if it works: it would give us less files to package in the built engine archive, and maybe it allows more LTO to be done at engine build time.

illwieckz avatar Mar 26 '25 23:03 illwieckz