RDART-962: Use xcode 15.4 #1548
Conversation
Pull Request Test Coverage Report for Build 9285323168Details
💛 - Coveralls |
nielsenko
force-pushed
the
kn/xcode-15
branch
3 times, most recently
from
19be2b2
to
17827e8
Compare
nielsenko
force-pushed
the
kn/xcode-15
branch
5 times, most recently
from
0fe31e9
to
3d02b62
Compare
| @@ -3,18 +3,10 @@ name: Dart desktop tests | |||
| on: | |||
| workflow_call: | |||
| inputs: | |||
| os: | |||
There was a problem hiding this comment.
I'm not sure I'm a huge fan of the changes in this file. I understand that it removes some redundant pieces of information, but it complicates the setup somewhat and forces readers to familiarize themselves with the architectures of the various github-hosted runners.
There was a problem hiding this comment.
The previous code had assumptions about the architecture, that didn't actually match the runners, implicitly assuming arch was x86_64 unless architecture was set to arm. That is not correct for macos-14, which is being rolled out as macos-latest as of yesterday (github/roadmap#926).
One thing I regret is that I cannot show the arch in the job title, as we don't know what runner is being used yet, so it is only shown in the steps. And obviously I would have preferred for github action expressions to be expressive enough to lowercase a simple string 🙄
What if I reintroduce the architecture argument, and make a step that explicitly test that it actually match runner.arch?
.github/workflows/ci.yml
Outdated
| submodules: 'recursive' | ||
| submodules: "recursive" | ||
|
|
||
| - name: Select XCode for MacOS |
There was a problem hiding this comment.
We need to increase the ulimit here it seems.
| @@ -7,4 +7,4 @@ set -o pipefail | |||
| cd "$(dirname "$0")/.." | |||
|
|
|||
| cmake --preset macos | |||
| cmake --build --preset macos --config MinSizeRel -- -destination "generic/platform=macOS" | |||
| cmake --build --preset macos --config Release -- -destination "generic/platform=macOS" | |||
There was a problem hiding this comment.
MinSizeRel becomes -Os which apparently clang++ (from XCode 15.x) don't like, hence this change.
| @@ -168,7 +168,7 @@ | |||
| "configurePreset": "macos", | |||
| "nativeToolOptions": [ | |||
| "-destination", | |||
| "platform=macOS" | |||
| "generic/platform=macOS" | |||
There was a problem hiding this comment.
As of XCode 15.3 it is apparently important to use generic/platform=macOS during preset, to actually end up with a universal binary (ie. supporting both arm64 and x86_64).
The downside of this change is that we also build for x86_64 on our dev machines, but given that we use ccache it is perhaps a non-issue in practice.
nielsenko
force-pushed
the
kn/xcode-15
branch
8 times, most recently
from
1da63ba
to
f69615f
Compare
nielsenko
force-pushed
the
kn/xcode-15
branch
2 times, most recently
from
375ceae
to
ab64653
Compare
nielsenko
force-pushed
the
kn/xcode-15
branch
3 times, most recently
from
6fb86bd
to
0247de5
Compare
nielsenko
force-pushed
the
kn/xcode-15
branch
2 times, most recently
from
3c999f4
to
49aa8af
Compare
Update CHANGELOG Use Release instead of MinSIzeRel
| - name: Select XCode for MacOS | ||
| if: startsWith(inputs.runner, 'macos') | ||
| run: | | ||
| xcodes installed |
There was a problem hiding this comment.
We don't, but I find it useful to see what versions are available when looking at the output of the step
| run: ulimit -n 10240 | ||
| if: ${{ contains(inputs.os, 'macos') }} | ||
| run: | | ||
| ulimit -n |
There was a problem hiding this comment.
To see the current limit in the output of the step before setting it.
Co-authored-by: Nikola Irinchev <[email protected]>
Fixes: #1547