Skip to content

Conversation

@mjp41
Copy link
Member

@mjp41 mjp41 commented Jan 29, 2026

This restructures the CI to make it clearer what is different between each build by using reusable workflows.

@mjp41 mjp41 force-pushed the ci_refactor branch 5 times, most recently from 5321a67 to 9cf9e52 Compare January 30, 2026 15:46
@SchrodingerZhu
Copy link
Collaborator

SchrodingerZhu commented Jan 30, 2026

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.
@mjp41 mjp41 marked this pull request as ready for review February 1, 2026 10:08
@mjp41 mjp41 requested a review from Copilot February 1, 2026 10:08
Copy link
Contributor

Copilot AI left a 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.yml to share CMake configure/build/test logic across Ubuntu, macOS, and Windows jobs, including optional self-host and size checks.
  • Introduces reusable-vm-build.yml to unify FreeBSD and NetBSD VM-based builds, with parameterized VM type, version, and dependency commands.
  • Restructures .github/workflows/main.yml to 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 the snmalloc-rs subdirectory.

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}}
Copy link

Copilot AI Feb 1, 2026

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.

Suggested change
sudo apt install ${{matrix.arch.extra-packages}}
if [ -n "${{ matrix.arch.extra-packages }}" ]; then
sudo apt install ${{ matrix.arch.extra-packages }}
fi

Copilot uses AI. Check for mistakes.
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++
Copy link

Copilot AI Feb 1, 2026

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.

Suggested change
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++

Copilot uses AI. Check for mistakes.
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