CI: Add macOS CI build.#3409
Conversation
|
Looking into why the PR run failed when it worked in my fork. 👀 Edit: Found it, the runner image got updated between my fork run and the PR run:
AppleClang 21 added |
mserajnik
force-pushed
the
add-macos-ci-build
branch
from
67993e0 to
a6d01e7
Compare
|
|
||
| std::strcat(path, " -s run"); | ||
| size_t pathLen = strlen(path); | ||
| snprintf(path + pathLen, sizeof(path) - pathLen, " -s run"); |
There was a problem hiding this comment.
By the name of this file I would expect that it shouldn't get compiled at all on MacOS.
There was a problem hiding this comment.
Yep, want me to drop it or keep it for consistency?
There was a problem hiding this comment.
Yep, want me to drop it or keep it for consistency?
std::strcat() would still warn (and thus error with -Werror) if it were outside of a Windows-only context, so imo it would make sense to keep everything consistent and not have different ways of doing basic things that will work on some platforms/compilers but not on others (even if in this particular case it doesn't matter since it's gated in #ifdef).
|
Should we also have a release published for MacOS or would those binaries not be portable? Also since we are building on clang with MacOS, perhaps the Linux clang CI action is redundant? |
Should be possible in a similar manner to the Linux release I believe. I will look into it next week or so.
I would leave that as it is still useful to have a CI build with regular Clang on Linux. Apple Clang is built with different options and defaults and as such is not a great general indicator of "can VMaNGOS be compiled with Clang?". |
2 participants
🍰 Pullrequest
This adds a macOS CI build (as requested by @ratkosrb), running on
macos-26(Apple Silicon only; Intel Macs will be abandoned with macOS 27 this year anyway) for client version1.12.1.5875, matching the existing Ubuntu Clang and Windows builds.Split into two commits:
Fix macOS build.: code and CMake changes needed to compile on macOS with-Werror:cmake/find/FindMySQL.cmake: use thePATHSkeyword properly infind_program(the old syntax was silently treating paths as additional names; it only worked on Linux because/usr/binis in the default search) and add Homebrew search paths (for both Apple Silicon and Intel, so Intel Mac users should still be able to compile on older macOS systems), mirroring whatFindOpenSSL.cmakealready does.cmake/platform/unix/settings.cmake:STREQUAL "Clang"was changed toMATCHES "Clang"so Apple'sAppleClangcompiler ID doesn't end up using the GCC branch and pick up GCC-only flags like-Wno-nonnull-compare.dep/include/nlohmann/json.hpp: Made two whitespace-only changes because the PR runner image funnily got updated from Xcode 26.2 / AppleClang 17 to Xcode 26.4.1 / AppleClang 21 within the ~30 minutes between my fork run and the PR run, and AppleClang 21 enables-Wdeprecated-literal-operatorby default.sprintf/strcpy/strcatcall sites acrosssrc/andcontrib/have been rewritten tosnprintf/memcpy. Apple's SDK marks these with__deprecated_msg, so-Werror,-Wdeprecated-declarationsfails the build otherwise. The replacements should be safe, but worth reviewing closely (I threw Claude Opus 4.7 Max at these replacements).CI: Add macOS CI build.: adds the actual CI build.The alternative to the
sprintf/strcpy/strcatrewrites would be suppressing the deprecation on Apple Clang only by adding-Wno-deprecated-declarationsincmake/platform/unix/settings.cmakegated onCMAKE_CXX_COMPILER_ID STREQUAL "AppleClang"and drop the relevant changes from the first commit. Happy to do that instead if preferred.Edit: and the same thing for
-Wdeprecated-literal-operatorsince the PR runner image got updated.Tested end-to-end in my fork: https://github.com/mserajnik/vmangos-core/actions/runs/25850039327
Proof
Issues
How2Test
Todo / Checklist