Repository navigation
Modernization Phase 1.1: build-system hygiene (Ninja, CMake 3.28 floor, presets, cmake-file sync) - #21
Merged
Conversation
- Ninja generator everywhere: install-prerequisities.sh exports CMAKE_GENERATOR=Ninja (shell, setenvs.sh, GITHUB_ENV) and installs ninja via brew; all test.sh scripts switch from `cmake ..; make` to the generator-agnostic `cmake --build build`. Ninja is faster than Makefiles and required by CMake's C++ modules support. - test.sh scripts unified from one template (per-package env vars preserved); fixes a latent bug where most package test.sh scripts dropped ctest's exit code and could never fail. - CMake floor raised: cmake_minimum_required(VERSION 3.28...4.3) in all 9 CMakeLists (3.28 = first release with stable C++ modules support; satisfiable with stock cmake on ubuntu-24.04). - set(CMAKE_CXX_STANDARD_REQUIRED ON) next to every CMAKE_CXX_STANDARD so the standard can no longer silently decay. - Canonical cmake/ dir + sync-cmake-files.sh: the per-package copies of homebrewClang.cmake/monorepoPackage.cmake stay (intentional, for future standalone vcpkg publishing) but are now generated from canonical files; lint.sh/CI fail if they drift. While unifying: monorepoPackage.cmake now appends monorepo dep dirs to both CMAKE_PREFIX_PATH and CMAKE_FIND_ROOT_PATH (previously only streamr-logger's copy had the FIND_ROOT_PATH variant, which is the one that works under cross-compiling toolchains). - Root CMakePresets.json (host/ios/android x debug/release) encoding the generator/triplet/toolchain combinations for IDEs and manual cmake --preset use. - README updated accordingly (including the one-time ./clean.sh needed after the generator switch). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The failing package's output tail is emitted as a ::error:: workflow command so the cause shows up in the Checks UI and the annotations API instead of only in the raw step log. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The cmake_minimum_required bump to 3.28 enabled policy CMP0155, which scans all C++20+ sources for module imports. With Ninja + GCC this injects GCC-specific scanning flags (-fmodules-ts, -fmodule-mapper, -fdeps-format=p1689r5) into compile_commands.json; gcc accepts them but clangd rejects them as unknown driver arguments, breaking clangd-tidy (lint). There are no module sources in the codebase yet, so scanning is pure overhead: set CMAKE_CXX_SCAN_FOR_MODULES OFF everywhere the C++ standard is set. The modules migration will re-enable it deliberately. Also: test.sh now emits the failed test names as a GitHub error annotation (ctest's LastTestsFailed.log), making CI test failures visible in the Checks UI. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Collaborator
Author
|
CI outcome on the final commit (
The test failures are identical to main and are exactly three tests on both legs (now visible via the new annotations): Ready for review. 🤖 Generated with Claude Code |
This was referenced Jul 2, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Phase 1.1 of the modernization plan: build-system hygiene with no compiler or dependency changes. Everything here is a prerequisite for the LLVM 22 upgrade (Phase 1.2) and the C++ modules migration (Part 2).
Changes
install-prerequisities.shexportsCMAKE_GENERATOR=Ninja(current shell +setenvs.sh+ CI'sGITHUB_ENV) andbrew install ninjaon macOS (Linux already installedninja-build). Ninja is faster than Makefiles and is required by CMake's C++ modules support../clean.shonce after this lands — CMake errors on generator change in an existing build dir (README updated).test.shscripts unified from one template:cmake ..; make→ generator-agnosticcmake --build build; per-package env vars preserved. Fixes a latent bug: thestreamr-utils/streamr-proto-rpcvariants dropped ctest's exit code and could never report failure.3.28...4.3in all 9 CMakeLists (was 3.22). 3.28 = first CMake with stable C++ modules support; ubuntu-24.04's stock cmake is 3.28.3.CMAKE_CXX_STANDARD_REQUIRED ONnext to everyCMAKE_CXX_STANDARD 26.CMAKE_CXX_SCAN_FOR_MODULES OFFnext to every standard setting — see "found during CI" below.cmake/dir +sync-cmake-files.sh. Per-package copies ofhomebrewClang.cmake/monorepoPackage.cmakestay (standalone vcpkg publishing goal) but are generated from canonical files;lint.sh/CI fail if a copy drifts.monorepoPackage.cmakedivergence fixed: canonical file appends dep dirs to bothCMAKE_PREFIX_PATHandCMAKE_FIND_ROOT_PATH(previously onlystreamr-loggerhad the FIND_ROOT_PATH variant — the one that works under cross-compiling toolchains).CMakePresets.json(host/ios/android × debug/release).install.shintentionally untouched this phase.lint.shemits the failing package's output tail as a GitHub::error::annotation;test.shemits failed test names (ctest'sLastTestsFailed.log). These show up in the Checks UI and the anonymous annotations API.Found during CI iteration on this PR
-fmodules-ts -fmodule-mapper=... -fdeps-format=p1689r5intocompile_commands.json; gcc accepts them, but clangd rejects them as unknown arguments → lint failed on both Linux legs. Fixed withCMAKE_CXX_SCAN_FOR_MODULES OFF(there are no module sources yet; the modules migration re-enables scanning deliberately). Good news for Part 2: with Clang, scanning happens via a separateclang-scan-depsstep and compile commands stay clean, so the lint pipeline survives the modules era once Linux moves to Clang (Phase 1.2).teststep is red. Both on the PR Modernization Phase 1.0: baseline & repo fixes #20 run and on main's own post-merge run, both Linux legs go install ✓ → lint ✓ → test ✗. The failed test names are now visible via the new test.sh annotation. Recommend treating the test failures as a separate workstream; this PR's gate is therefore: install + lint green on both Linux legs, test failing identically to main.Expected CI status
llvm@17, unchanged).Next
Phase 1.2: compiler upgrade — Homebrew LLVM 17 → latest, Linux gcc-14 → clang + libc++, keg-only LLVM handling via
LLVM_PREFIX, with the iOS runtime-libc++ compatibility gate (device test) you specified.🤖 Generated with Claude Code