CI: Add FreeBSD, OpenBSD and NetBSD builds - #571
Conversation
Introduce CI builds for three BSD operating systems using cross-platform-actions, based on QEMU virtual machines accelerated with KVM for near-native performance. The FreeBSD build enables additional Clang hardening checks to detect issues such as integer overflow and to improve memory safety. Addresses davmac314#558
| runs-on: ubuntu-latest | ||
| defaults: | ||
| run: | ||
| shell: cpa.sh {0} --environment-variables CXX |
There was a problem hiding this comment.
I had to look up documentation to find out what "cpa.sh" is and does. It should have comments to explain (at least briefly) what it is and where to find more information.
Is --environment-variables CXX necessary? Wouldn't it be cleaner to specify the environment_variables input to the action?
| # Make "configure" always use clang++. Our NetBSD image has g++ 10 pre-installed, and | ||
| # "configure" prefers it over clang++, but g++ 10 is too old to build Dinit. | ||
| # Note that it's defined here for all the BSDs because it's cleaner than specifying it only | ||
| # for NetBSD. FreeBSD already uses clang++ due to the lack of g++ in its image and OpenBSD | ||
| # uses a static mconfig that specifies clang++ as the C++ compiler. | ||
| CXX: clang++ |
There was a problem hiding this comment.
Specifying CXX as clang++ via the build (and/or configure) command line would be simpler than forcing it into the environment, wouldn't it?
In "Note that it's defined here" why is this a "Note" and what is "it"?
| # uses a static mconfig that specifies clang++ as the C++ compiler. | ||
| CXX: clang++ | ||
| strategy: | ||
| fail-fast: false # Be able to upload src/igr-tests/igr-output/ files in igr-tests step |
There was a problem hiding this comment.
I don't think the comment accurately describes what this is needed for.
See documentation here: https://docs.github.com/en/actions/reference/workflows-and-actions/workflow-syntax#jobsjob_idstrategyfail-fast
| with: | ||
| operating_system: ${{ matrix.name }} | ||
| version: ${{ matrix.version }} | ||
| - name: Getting dependencies (FreeBSD) |
There was a problem hiding this comment.
Rather than "Getting dependencies" maybe "Installing required packages". It is more descriptive and more accurate. The packages being installed are not dependencies of Dinit.
| - name: Print dinit executable file architecture | ||
| run: file ./src/dinit |
There was a problem hiding this comment.
Isn't this redundant considering clang++ -dumpmachine/clang++ --version above?
| clang++ -dumpmachine | ||
| clang++ --version |
There was a problem hiding this comment.
Does clang++ -dumpmachine display anything that clang++ --version doesn't already?
| run: gmake check-igr | ||
| - name: Upload igr-tests output file(s) on failure | ||
| uses: actions/upload-artifact@v6.0.0 | ||
| if: failure() |
There was a problem hiding this comment.
Doesn't this mean igr-tests output is uploaded if any prior step fails even if the integration tests pass?
Introduce CI builds for three BSD operating systems using cross-platform-actions, based on QEMU virtual machines accelerated with KVM for near-native performance.
The FreeBSD build enables additional Clang hardening checks to detect issues such as integer overflow and to improve memory safety.
Changes are self-reviewed and tested in my fork: https://github.com/mobin-2008/dinit/actions/runs/30704553393/job/91381298660
Addresses #558