Skip to content

Conversation

illwieckz
Copy link
Member

@illwieckz illwieckz commented Nov 11, 2024

Current external_deps build status:

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

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 and others added 2 commits November 11, 2024 00:01
O3 because we use CMake Release build type that sets O3 anyway, so things are consistent.
@illwieckz illwieckz marked this pull request as draft November 11, 2024 16:57
@illwieckz
Copy link
Member Author

illwieckz commented Nov 11, 2024

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
Copy link
Member Author

illwieckz commented Nov 11, 2024

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
Copy link
Member Author

illwieckz commented Nov 11, 2024

  • 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
Copy link
Member Author

illwieckz commented Nov 11, 2024

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
Copy link
Member Author

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
Copy link
Member Author

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

@illwieckz
Copy link
Member Author

So the Vorbis build error is actually a bug:

I added a workaround.

@illwieckz
Copy link
Member Author

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
Copy link
Member Author

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
Copy link
Member Author

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
Copy link
Member Author

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…

@slipher
Copy link
Member

slipher commented Nov 11, 2024

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

@illwieckz
Copy link
Member Author

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

@slipher
Copy link
Member

slipher commented Nov 16, 2024

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.

@illwieckz
Copy link
Member Author

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.

@DolceTriade
Copy link
Contributor

What's the motivation behind this change?

@illwieckz
Copy link
Member Author

illwieckz commented Nov 18, 2024

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 illwieckz force-pushed the illwieckz/deps branch 2 times, most recently from ac1c6e1 to 5653846 Compare November 18, 2024 01:26
@illwieckz
Copy link
Member Author

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

@illwieckz
Copy link
Member Author

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
Copy link
Member Author

I got Wayland working with the cross-compiled arm64 engine.

@slipher
Copy link
Member

slipher commented Sep 4, 2025

external_deps: we now keep directories is erroneous. I just tried this and install does not install an empty bin directory. Maybe the CMake bug is fixed in SDL3 though?

Please remove non-essential commits like external_deps: improve logging and external_deps: deduplicate build iterating for another PR. This one is already too big.

@illwieckz
Copy link
Member Author

external_deps: we now keep directories is erroneous. I just tried this and install does not install an empty bin directory. Maybe the CMake bug is fixed in SDL3 though?

Commit renamed to external_deps: remove the SDL workaround for missing bin folder.

Please remove non-essential commits like external_deps: improve logging and external_deps: deduplicate build iterating for another PR. This one is already too big.

Done. I needed them for debugging but that can be merged later indeed.

Mystery solved: kcat/openal-soft#1093 actually does decrease the minimum MacOS version in the newest OpenAL by removing std::filesystem. So you can remove this comment and remove my authorship from this commit.

Done

The Nettle headers include the GMP header. So this may be here just for the warning fix. You could get rid of the GMP include but the MSVC warning adjustment would still need to be used for the Nettle headers.

Commit removed.

@illwieckz illwieckz force-pushed the illwieckz/deps branch 2 times, most recently from 4b2a1c5 to ecb25b7 Compare September 4, 2025 18:59
@illwieckz
Copy link
Member Author

I rebuilt and uploaded the current deps archives for testing.

I also rebuilt an unizip using docker.

So here is the current “unfinished unizip”, but good enough for testing engines:

@illwieckz
Copy link
Member Author

illwieckz commented Sep 4, 2025

Oh no, lol, forget about that unizip for anything that is not linux, they used previous deps.

Well, the deps for macOS and Windows shouldn't be that different, since the last time I built them, what I did was to enable Wayland.

I'm rebuilding anyway.

@illwieckz
Copy link
Member Author

@illwieckz
Copy link
Member Author

So, armhf engine on armhf distro has no input issue. The issue only happens on armhf engine on arm64 distro. Since we have a working arm64 engine, there is no reason to worry.

So I'm not aware about remaining input bugs. @slipher did you have a remaining one with macOS?

A new SDL3 release was published some days ago. It mentions some macOS fixes, I'll rebuild the deps and the test unizip soon.

@illwieckz
Copy link
Member Author

I noticed we still install some libjpeg-turbo CMake files. We can strip them out I guess.

@slipher
Copy link
Member

slipher commented Sep 5, 2025

The Mac issue was that the game doesn't grab focus when it starts (tested in fullscreen mode). You have to alt tab out and in for the game to start accepting inputs. I don't see it as blocking this PR. I'll want to fix it before any Unvanquished release, but we can do it after merging the deps update.

@illwieckz
Copy link
Member Author

So I uploaded new deps with SDL 3.2.22 and the removal of libjpeg-turbo CMake stuff, and rebuilt the test unizip:

@illwieckz
Copy link
Member Author

The Mac issue was that the game doesn't grab focus when it starts (tested in fullscreen mode). You have to alt tab out and in for the game to start accepting inputs. I don't see it as blocking this PR. I'll want to fix it before any Unvanquished release, but we can do it after merging the deps update.

OK thanks. All the other issues I have faced with SDL3 are now gone. So it looks like we are good to go! 😀

@illwieckz
Copy link
Member Author

illwieckz commented Sep 5, 2025

There is something that makes the CI loops the CMake call, in both CodeQL and Azure, I have no idea what!

@slipher
Copy link
Member

slipher commented Sep 5, 2025

The code LGTM. Some commit reordering/squashing may be in order:

  • The SDL2->SDL3 code rewrite should go in the same commit as the 10->11 version bump in CMakeLists.txt to keep all commits buildable. (build.sh rewriting commits can go before that.)
  • sdl3: APIENTRY no longer defined should be squashed with SDL3 (needed to build)
  • cmake: make MinGW toolchains also look for CMAKE_PREFIX_PATH was superseded by cmake: don't set CMAKE_FIND_ROOT_PATH in toolchain files

@slipher
Copy link
Member

slipher commented Sep 5, 2025

There is something that makes the CI loops the CMake call, in both CodeQL and Azure, I have no idea what!

Seems to be those bogus timestamps that you are using in the deps archives. (This time the fake timestamp was in the future.)

@illwieckz
Copy link
Member Author

Funny!

@illwieckz
Copy link
Member Author

After a restart, the CI passed. I'll do the remaining squashes later, and I believe we will be ready! Finally! 🚀

@illwieckz illwieckz force-pushed the illwieckz/deps branch 2 times, most recently from f484286 to ff61bd8 Compare September 6, 2025 12:23
@illwieckz illwieckz merged commit 5e536bc into master Sep 6, 2025
8 checks passed
@illwieckz
Copy link
Member Author

Done! 🎉

SDL3 is now a thing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Build T-Improvement Improvement for an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants