ci: build the macOS job with Apple clang#1802
Closed
nickk02 wants to merge 2 commits into
Closed
Conversation
find_package(Eigen3 3.3 REQUIRED NO_MODULE) rejects Eigen 5, because Eigen's config-version file no longer treats a new major as compatible with an older request. Eigen 5 now ships on Arch and Homebrew, so configure aborts there before anything compiles. This is the cause of issue SFTtech#1793 and of the long-failing macOS CI on master. openage only uses the stable core and geometry API (Vector2/3/4f, Matrix4f, Affine3f, AngleAxisf, dot, prescale, UnitZ), none of which Eigen 5 drops, so dropping the version constraint is enough. A version range would be tidier but needs cmake 3.19 and our floor is 3.16. Verified by building openage and running the full test suite against a local Eigen 5.0.1: configures, compiles, and all tests pass. Closes SFTtech#1793.
The macOS workflow forced Homebrew's LLVM (`brew --prefix llvm`/clang++), which on the current runner is clang 22 whose libc++ can't find its own <math.h>/<stddef.h> ahead of the SDK's C headers, so every C++ TU fails to compile with "header search paths are not configured properly". None of the errors are in openage code. doc/building.md already recommends `--compiler=clang` (the Xcode clang) as the hassle-free macOS toolchain, so point CI at it and drop the now unused Homebrew LLVM install. Apple clang is built for the macOS SDK and its libc++, so the header-ordering problem goes away.
Contributor
Author
|
Folded into #1801, which now carries both the Eigen 5 fix and this macOS-CI change and goes green end to end. Closing this in favor of the single PR. The branch stays up in case it is easier to review the workflow change in isolation. |
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Merge Checklist
make checkmerge(changed file is the workflow YAML; clean)Description
The macOS-CI job forces Homebrew's LLVM (
--compiler="$(brew --prefix llvm)/bin/clang++"). On the current runner that is clang 22, whose libc++ can't find its own<math.h>/<stddef.h>ahead of the SDK's C headers, so every C++ translation unit fails to compile with "header search paths are not configured properly". None of the errors are in openage code.doc/building.mdalready recommends--compiler=clang(the Xcode clang) as the hassle-free macOS toolchain. This points CI at it and drops the now-unused Homebrew LLVM install. Apple clang is built for the macOS SDK and its libc++, so the header-ordering problem goes away.Note on the diff (stacked on #1801)
The macOS job can't reach compilation today regardless of the compiler, because
find_package(Eigen3 3.3 REQUIRED NO_MODULE)rejects the Eigen 5 the runner now ships (that is #1801, closing #1793). So this branch is based on top of #1801, otherwise CI would still abort at configure and this change couldn't be exercised. The branch therefore shows two commits; once #1801 lands I will rebase this down to just the workflow commit. Happy to fold it into #1801 instead if you would rather have one "fix the macOS build" PR.