Skip to content

ci: add macos test#327

Merged
spe-ciellt merged 3 commits intogerbv:developfrom
chenrui333:ci-macos-followup-303
Mar 2, 2026
Merged

ci: add macos test#327
spe-ciellt merged 3 commits intogerbv:developfrom
chenrui333:ci-macos-followup-303

Conversation

@chenrui333
Copy link
Copy Markdown
Contributor

@chenrui333 chenrui333 commented Mar 1, 2026 •
edited
Loading

Add a native macOS CI smoke build on macos-15 for CMake/Homebrew validation.

Follow-up to #303.

@chenrui333 chenrui333 mentioned this pull request Mar 1, 2026
Merged
@chenrui333 chenrui333 force-pushed the ci-macos-followup-303 branch from 83429c1 to d685f05 Compare March 1, 2026 19:24
@chenrui333 chenrui333 changed the title ci: add macOS 15 smoke job (follow-up #303) ci: add macos test Mar 1, 2026
@chenrui333
ci: add macos test
be519f0 
Signed-off-by: Rui Chen <rui@chenrui.dev>
@chenrui333 chenrui333 force-pushed the ci-macos-followup-303 branch from d685f05 to be519f0 Compare March 1, 2026 19:25
@spe-ciellt
Copy link
Copy Markdown
Contributor

spe-ciellt commented Mar 1, 2026 •
edited
Loading

Thank you for the MacOS support för building.

I would ask if you possible could create a preset and a toolchain file for MacOS in cmake/preset and cmake/toolchain. And then use it in the script. If not here, we could create an issue about it and fix it later.

Also, you dont have to refer to previous commit in code.

@chenrui333
Copy link
Copy Markdown
Contributor Author

chenrui333 commented Mar 2, 2026

thanks for the feedback, I am updating the pr now.

@chenrui333
ci: add macos preset and toolchain
b27005a 
Signed-off-by: Rui Chen <rui@chenrui.dev>
@chenrui333
Copy link
Copy Markdown
Contributor Author

chenrui333 commented Mar 2, 2026

Followed up on this in b27005a.

Changes made:

  • Added a dedicated macOS preset: cmake/preset/macos-clang.json
  • Added macOS toolchain file: cmake/toolchains/macos-clang.cmake
  • Included the new preset in CMakePresets.json
  • Updated macOS CI steps to use presets directly:
    • cmake --preset macos-clang
    • cmake --build --preset macos-clang --parallel ...
    • ctest --preset macos-clang -V

I also set CMAKE_COMPILE_WARNING_AS_ERROR=OFF in the macOS preset so the smoke job does not fail on existing Clang-only warnings unrelated to this CI wiring.

rampageservices
rampageservices reviewed Mar 2, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@rampageservices rampageservices left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few observations from reviewing:

_GNU_SOURCE in cmake/toolchains/macos-clang.cmake — consistent with the Linux toolchain which also defines it, so this makes sense for parity.

continue-on-error: true on the test step — is this intentional as a temporary measure while macOS tests stabilize, or should it stay permanently? If temporary, it might be worth adding a comment noting why, so it doesn't get forgotten. If permanent, it means macOS test failures will never block CI, which reduces the value of running them.

CMAKE_COMPILE_WARNING_AS_ERROR: OFF — the Linux preset doesn't set this at all (inherits whatever the project default is). Is macOS hitting warnings that Linux doesn't? If so, it might be worth documenting which warnings are macOS-specific, since silencing them project-wide could mask real issues on that platform.

bzip2 pkg-config file generation — this is the standard approach for Homebrew keg-only packages that don't ship .pc files. Looks correct.

Job rename from ci to ci-ubuntu-22.04 — worth double-checking that no branch protection rules reference the old ci status check name, since GitHub treats these as distinct checks.

actions/checkout@v6 — consistent with the existing Ubuntu job. Neither is SHA-pinned, which is fine for a project of this size, but noting it for completeness.

Overall looks solid as a smoke-test addition. The main question is whether continue-on-error should remain long-term.

@spe-ciellt
Copy link
Copy Markdown
Contributor

spe-ciellt commented Mar 2, 2026

Thank you for the cmake/preset and cmake/toolchains files. Now we can signal that we support MacOS as well.

I just checked the CI build and it reported the following error https://github.com/gerbv/gerbv/actions/runs/22560526941/workflow

Seems there can not be number in ci work names. And also the comnment above by @rampageservices on at least commenting if it is temporary or permanently off.

It is great to test it on Clang as well and (later) making sure it is warning free there as well. But it is a journey...

@chenrui333
Copy link
Copy Markdown
Contributor Author

chenrui333 commented Mar 2, 2026

yeah, that should easy fix (I ran into that issue the other day, that the job id does not support dot in 22.04

@chenrui333
ci: fix gha job id ref, ci-ubuntu-22.04 -> ci-ubuntu-22-04
30ae0a6 
Signed-off-by: Rui Chen <rui@chenrui.dev>
@chenrui333
Copy link
Copy Markdown
Contributor Author

chenrui333 commented Mar 2, 2026

@spe-ciellt should be good now

@spe-ciellt
Copy link
Copy Markdown
Contributor

spe-ciellt commented Mar 2, 2026 •
edited
Loading

Thanks for the updates.

OK, I see the warnings from Clang now. Interesting, Clang and GCC seems to disagree how much [[fallthrough]] to add, This is something to analyze.

I will merge this now.

@spe-ciellt spe-ciellt merged commit 05194c6 into gerbv:develop Mar 2, 2026
3 checks passed
@chenrui333
Copy link
Copy Markdown
Contributor Author

chenrui333 commented Mar 2, 2026

Thanks @spe-ciellt.

@spe-ciellt spe-ciellt added the buildsystem Updates to the buildsystem (CMake/CTest/CPack) label Mar 3, 2026
@spe-ciellt spe-ciellt self-assigned this Mar 3, 2026
@BrewTestBot BrewTestBot mentioned this pull request Mar 8, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@rampageservices rampageservices rampageservices left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@spe-ciellt spe-ciellt

Labels

buildsystem Updates to the buildsystem (CMake/CTest/CPack)

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@chenrui333 @spe-ciellt @rampageservices