-
Notifications
You must be signed in to change notification settings - Fork 119
Refactor CI #806
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
base: main
Are you sure you want to change the base?
Refactor CI #806
Conversation
5321a67 to
9cf9e52
Compare
|
If it is easier and helpful, you may want to directly absorb #808 into this. |
There was a lot of redundancy in the CI. This attempts to reduce that using reusable workflows.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR refactors the CI configuration to centralize common build logic into reusable workflows, making the matrix of platforms and build variants clearer and easier to maintain.
Changes:
- Introduces
reusable-cmake-build.ymlto share CMake configure/build/test logic across Ubuntu, macOS, and Windows jobs, including optional self-host and size checks. - Introduces
reusable-vm-build.ymlto unify FreeBSD and NetBSD VM-based builds, with parameterized VM type, version, and dependency commands. - Restructures
.github/workflows/main.ymlto consume the reusable workflows for the various build jobs (Ubuntu, macOS, BSDs, Windows, QEMU cross, GWP-ASan, etc.) and narrows Dependabot’s Cargo updates to thesnmalloc-rssubdirectory.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
.github/workflows/reusable-vm-build.yml |
New reusable workflow encapsulating FreeBSD/NetBSD/OpenBSD VM setup, CMake configuration, Ninja build and ctest invocation, parameterized by VM type and version. |
.github/workflows/reusable-cmake-build.yml |
New reusable workflow that standardizes dependency installation, CMake configuration, build, testing, size checks, and optional self-hosting across OSes and generators. |
.github/workflows/main.yml |
Refactored main CI workflow to use the new reusable workflows for Ubuntu, macOS, BSD, Windows, QEMU cross-builds, fuzzing, GWP-ASan, and the final gate, and extended build/test matrix variants (sanitizers, self-vendored STL, etc.). |
.github/dependabot.yml |
Updated Dependabot configuration so Cargo updates target the snmalloc-rs subdirectory instead of the repository root and removed the gitsubmodule ecosystem entry. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| run: | | ||
| sudo apt update | ||
| sudo apt install libstdc++-12-dev-${{ matrix.arch.name }}-cross qemu-user ninja-build binfmt-support | ||
| sudo apt install ${{matrix.arch.extra-packages}} |
Copilot
AI
Feb 1, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the qemu-crossbuild job, matrix.arch.extra-packages is only defined for the riscv64 entry, but the Install cross-compile toolchain and QEMU step always runs sudo apt install ${{matrix.arch.extra-packages}}. For architectures like armhf and arm64 where extra-packages is unset, this expands to sudo apt install with no package names, which will cause the step (and therefore those jobs) to fail. Consider either folding extra-packages into the first apt install command or guarding the second apt invocation with a condition so it only runs when extra-packages is non-empty.
| sudo apt install ${{matrix.arch.extra-packages}} | |
| if [ -n "${{ matrix.arch.extra-packages }}" ]; then | |
| sudo apt install ${{ matrix.arch.extra-packages }} | |
| fi |
| vm-version: '9.2' | ||
| build-type: Release | ||
| dependencies: "/usr/sbin/pkg_add cmake ninja-build gcc10" | ||
| cmake-flags: -DCMAKE_CXX_COMPILER=/usr/pkg/gcc10/bin/g++ |
Copilot
AI
Feb 1, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the NetBSD VM build, the previous workflow explicitly set both the C and C++ compilers to the GCC 10 toolchain, but the new reusable workflow only passes -DCMAKE_CXX_COMPILER=/usr/pkg/gcc10/bin/g++ via cmake-flags. This means the C compiler will now fall back to the system default, which is inconsistent with the earlier configuration and can make debugging compiler / ABI issues harder; if the intent is still to build both C and C++ with GCC 10, consider also setting -DCMAKE_C_COMPILER=/usr/pkg/gcc10/bin/gcc in cmake-flags.
| cmake-flags: -DCMAKE_CXX_COMPILER=/usr/pkg/gcc10/bin/g++ | |
| cmake-flags: -DCMAKE_C_COMPILER=/usr/pkg/gcc10/bin/gcc -DCMAKE_CXX_COMPILER=/usr/pkg/gcc10/bin/g++ |
This restructures the CI to make it clearer what is different between each build by using reusable workflows.