Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Separate CI jobs for macOS on x86 #284

Merged
merged 4 commits into from
Jun 10, 2024
Merged

Conversation

bjosv
Copy link
Collaborator

@bjosv bjosv commented Jun 4, 2024

Split the CI jobs for testing Mac to run on both "old" x86 runners and the new M1 runner.

The pre-installed gcc 13.3 (brew package gcc-13) only seems to work with an older MacOSX.sdk version than the default in a macos-12 runner.
Lets update to gcc-14 which has been corrected to be able to use any SDK version.

This PR also adds a check if the compiler is already installed, which silences a warning.

Weekly run:
https://github.com/Nordix/flatcc/actions/runs/9413941834

The pre-installed gcc 13.3 (brew package 'gcc-13') only seems to work
with an older MacOSX.sdk version than the default in a macos-12 runner.
By setting a specific version gcc will use matching headers like:
/Applications/Xcode_14.0.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/Availability.h

Add a check if the compiler is already installed, which silences a warning.
@mikkelfj
Copy link
Contributor

mikkelfj commented Jun 4, 2024

Thanks for looking into this. Would it make more sense to move to a newer macos runner? Staying compatible with multiple versions old macos might not be worthwhile since Apple generally doesn't seem pay much attention to backwards compat. Also, if there is a newer it might be a more important test case?
Generally, weekly should test old versions of compilers, I'm just not sure exactly how that translates to macos.

@bjosv
Copy link
Collaborator Author

bjosv commented Jun 5, 2024

Would it make more sense to move to a newer macos runner?

I guess it would be better to run on later runner releases. The only "issue" I know is that macos-14 is running on M1 machines only and older packages from Hombrew releases are not always available. But building on both x86_64 and arm64 would be good thing anyway.

Maybe we should have a build matrix in the job macos-clang to build on both macos-14(arm64) and macos-13(x86_64), and simlilar for the macos-gcc job?
gcc10 is not available on arm64 though so there is a need to fiddle with gcc versions depending on runner and set the required Xcode version.

@mikkelfj
Copy link
Contributor

mikkelfj commented Jun 5, 2024

yeah, my only concern is that this gets a bit complicated, and the explicit xcode version should hopefully not be necessary, but have no clear understanding of this.

I'm thinking maybe it is better to separate out the x86 build to a separate target and try to keep the M1+ as clean as possible? The x86 will likely get deprecated at some point, and if there are any issues, it will be easier to kill off.

@bjosv
Copy link
Collaborator Author

bjosv commented Jun 5, 2024

It looks like there a fixes incoming for a (new) gcc-14 that handles various SDK headers:
Homebrew/homebrew-core@61581e6
so maybe I should just have waited a bit as you said from the beginning :)

Yea, it gets a bit complicated with theses matrixes.. I can make an attempt by moving x86 to its own jobs, and maybe see if we could go for gcc-14 instead if that works without switching Xcode version..

@mikkelfj
Copy link
Contributor

mikkelfj commented Jun 5, 2024

sounds like a plan, thanks

@bjosv bjosv changed the title Set Xcode version in CI for gcc builds on macOS Separate CI jobs for macOS on x86 Jun 7, 2024
@mikkelfj
Copy link
Contributor

mikkelfj commented Jun 7, 2024

Looks fine, but:
It seems the checks have only run the small build, which is maybe not surprising.
If you look at summary for that build, there are some CMake warnings (I will eventually upgrade to 3.x FWIW)

Lift macos runner in CI and only install needed tools.
@bjosv
Copy link
Collaborator Author

bjosv commented Jun 10, 2024

So, been of a bit with the flu, but now the warnings should be gone in both CI and weekly.
Lifted macos in the small build and fixed a check in weekly.
Example run in weekly:
https://github.com/Nordix/flatcc/actions/runs/9443621149

@mikkelfj mikkelfj merged commit 896db54 into dvidelabs:master Jun 10, 2024
5 checks passed
@mikkelfj
Copy link
Contributor

Thanks, looks great, get well soon!

@bjosv bjosv deleted the mac-gcc13-fix branch June 11, 2024 08:40
@bjosv
Copy link
Collaborator Author

bjosv commented Jun 11, 2024

Thanks!
Wellwell, new day, new warnings from Github :)

[Clang 7](https://github.com/dvidelabs/flatcc/actions/runs/9448729894/job/26023376576#step:2:60)
Failed to add apt key via server keyserver.ubuntu.com:

They are probably temporary, but still.. I'll put them in my backlog to see if there are ways to avoid them

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants