Generate build matrix and extend builds for better CI coverage#187
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughThe PR introduces a dynamic GitHub Actions CI matrix generation system using a JSON compiler configuration file and Python generator script, refactors the workflow to use generated matrices, adds exception-safe Windows scheduler shutdown handling, and separates worker base class lifecycle methods to the implementation file. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
An automated preview of the documentation is available at https://187.corosio.prtest3.cppalliance.org/index.html If more commits are pushed to the pull request, the docs will rebuild at the same URL. 2026-03-04 16:31:54 UTC |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
include/boost/corosio/native/detail/iocp/win_scheduler.hpp (1)
381-397:⚠️ Potential issue | 🟠 MajorPotential infinite loop when
PostQueuedCompletionStatusfails instop().The comment claims the run loop will notice via the GQCS timeout and exit, but tracing through
do_one(): whentimeout_ms == INFINITE(as inrun()), a timeout does not return 0—it continues the loop (lines 635-636). The onlystopped()check is in thekey_shutdownhandler (line 615), which is unreachable if PQCS failed.If PQCS fails,
do_one(INFINITE)will loop forever, waking every 500ms but never exiting becausestopped()is not checked after timeout.Proposed fix: check stopped() after timeout when using INFINITE
// Timeout or error if (dwError != WAIT_TIMEOUT) detail::throw_system_error(make_err(dwError)); if (timeout_ms != INFINITE) return 0; + // Check stopped_ for graceful exit when PQCS failed in stop() + if (stopped()) + return 0; } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@include/boost/corosio/native/detail/iocp/win_scheduler.hpp` around lines 381 - 397, The stop() path can leave the loop stuck if PostQueuedCompletionStatus fails because do_one(INFINITE) never checks stopped() after a GQCS timeout; update the wait/timeout handling in win_scheduler::do_one (and/or run) so that when a GetQueuedCompletionStatus timeout occurs (the INFINITE wait path), you call stopped() and exit/return early if stopped_ is set; ensure the key_shutdown handling (key_shutdown) remains for the normal PQCS path and that stop_event_posted_/stopped_ are still set by win_scheduler::stop before checking stopped() in do_one.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 728-748: The FreeBSD path in the "Patch Boost" step (id: patch) is
missing the sparse-checkout normalization done elsewhere, which causes missing
CMakeLists.txt during the FreeBSD CMake build; modify the "Patch Boost" step so
that before copying boost-source into boost-root (around the cp -rL boost-source
boost-root and subsequent cd boost-root / boost_root handling) you run the same
sparse-checkout normalization sequence used in the main patch flow
(disable/clear sparse-checkout and restore full tree so CMakeLists.txt are
present), ensuring the git workspace is normalized prior to copying in
corosio-root and capy-root.
---
Outside diff comments:
In `@include/boost/corosio/native/detail/iocp/win_scheduler.hpp`:
- Around line 381-397: The stop() path can leave the loop stuck if
PostQueuedCompletionStatus fails because do_one(INFINITE) never checks stopped()
after a GQCS timeout; update the wait/timeout handling in win_scheduler::do_one
(and/or run) so that when a GetQueuedCompletionStatus timeout occurs (the
INFINITE wait path), you call stopped() and exit/return early if stopped_ is
set; ensure the key_shutdown handling (key_shutdown) remains for the normal PQCS
path and that stop_event_posted_/stopped_ are still set by win_scheduler::stop
before checking stopped() in do_one.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
.fd.fxlcf.dpdns.orgpilers.json.github/generate-matrix.py.github/workflows/ci.ymlCMakeLists.txtinclude/boost/corosio/native/detail/iocp/win_scheduler.hppinclude/boost/corosio/native/detail/iocp/win_timers_thread.hppinclude/boost/corosio/tcp_server.hppsrc/corosio/src/tcp_server.cpp
| - name: Patch Boost | ||
| id: patch | ||
| run: | | ||
| set -xe | ||
| module=${GITHUB_REPOSITORY#*/} | ||
| echo "module=$module" >> $GITHUB_OUTPUT | ||
|
|
||
| workspace_root=$(echo "$GITHUB_WORKSPACE" | sed 's/\\/\//g') | ||
|
|
||
| rm -r "boost-source/libs/$module" || true | ||
| rm -r "boost-source/libs/capy" || true | ||
|
|
||
| cp -rL boost-source boost-root | ||
|
|
||
| cd boost-root | ||
| boost_root="$(pwd)" | ||
| echo -E "boost_root=$boost_root" >> $GITHUB_OUTPUT | ||
|
|
||
| cp -r "$workspace_root"/corosio-root "libs/$module" | ||
| cp -r "$workspace_root"/capy-root "libs/capy" | ||
|
|
There was a problem hiding this comment.
FreeBSD patch flow is missing sparse-checkout normalization required for CMake builds.
The main patch flow explicitly disables sparse-checkout to restore missing CMakeLists.txt files (Line 145 onward), but this FreeBSD path doesn’t. With the new FreeBSD CMake step (Line 767), this can cause configure/build failures.
🔧 Proposed fix
- name: Patch Boost
id: patch
run: |
set -xe
module=${GITHUB_REPOSITORY#*/}
echo "module=$module" >> $GITHUB_OUTPUT
workspace_root=$(echo "$GITHUB_WORKSPACE" | sed 's/\\/\//g')
rm -r "boost-source/libs/$module" || true
rm -r "boost-source/libs/capy" || true
+
+ # boost-clone uses sparse checkout which can omit CMakeLists.txt files
+ # needed by CMake integration tests.
+ cd boost-source
+ if git sparse-checkout list > /dev/null 2>&1; then
+ echo "Disabling sparse checkout..."
+ git sparse-checkout disable
+ echo "Fetching any missing objects..."
+ git fetch origin --no-tags
+ git checkout
+ fi
+ cd ..
cp -rL boost-source boost-root📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Patch Boost | |
| id: patch | |
| run: | | |
| set -xe | |
| module=${GITHUB_REPOSITORY#*/} | |
| echo "module=$module" >> $GITHUB_OUTPUT | |
| workspace_root=$(echo "$GITHUB_WORKSPACE" | sed 's/\\/\//g') | |
| rm -r "boost-source/libs/$module" || true | |
| rm -r "boost-source/libs/capy" || true | |
| cp -rL boost-source boost-root | |
| cd boost-root | |
| boost_root="$(pwd)" | |
| echo -E "boost_root=$boost_root" >> $GITHUB_OUTPUT | |
| cp -r "$workspace_root"/corosio-root "libs/$module" | |
| cp -r "$workspace_root"/capy-root "libs/capy" | |
| - name: Patch Boost | |
| id: patch | |
| run: | | |
| set -xe | |
| module=${GITHUB_REPOSITORY#*/} | |
| echo "module=$module" >> $GITHUB_OUTPUT | |
| workspace_root=$(echo "$GITHUB_WORKSPACE" | sed 's/\\/\//g') | |
| rm -r "boost-source/libs/$module" || true | |
| rm -r "boost-source/libs/capy" || true | |
| # boost-clone uses sparse checkout which can omit CMakeLists.txt files | |
| # needed by CMake integration tests. | |
| cd boost-source | |
| if git sparse-checkout list > /dev/null 2>&1; then | |
| echo "Disabling sparse checkout..." | |
| git sparse-checkout disable | |
| echo "Fetching any missing objects..." | |
| git fetch origin --no-tags | |
| git checkout | |
| fi | |
| cd .. | |
| cp -rL boost-source boost-root | |
| cd boost-root | |
| boost_root="$(pwd)" | |
| echo -E "boost_root=$boost_root" >> $GITHUB_OUTPUT | |
| cp -r "$workspace_root"/corosio-root "libs/$module" | |
| cp -r "$workspace_root"/capy-root "libs/capy" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/ci.yml around lines 728 - 748, The FreeBSD path in the
"Patch Boost" step (id: patch) is missing the sparse-checkout normalization done
elsewhere, which causes missing CMakeLists.txt during the FreeBSD CMake build;
modify the "Patch Boost" step so that before copying boost-source into
boost-root (around the cp -rL boost-source boost-root and subsequent cd
boost-root / boost_root handling) you run the same sparse-checkout normalization
sequence used in the main patch flow (disable/clear sparse-checkout and restore
full tree so CMakeLists.txt are present), ensuring the git workspace is
normalized prior to copying in corosio-root and capy-root.
There was a problem hiding this comment.
The sparse-checkout normalization is only needed for the integration test CMake steps (Find Package, Subdirectory, Root Project) which use add_subdirectory and need full source trees of Boost dependencies. The FreeBSD CMake build uses BOOST_INCLUDE_LIBRARIES with a top-level CMake configure — it fetches/builds only the specified module and its dependencies, and CMake's FetchContent/add_subdirectory within Boost's own build system handles this differently.
More importantly: the FreeBSD builds are passing. If the sparse checkout were actually causing missing CMakeLists.txt, the CMake configure would fail. So this is not a real issue.
There was a problem hiding this comment.
@mvandeberg, thank you for the detailed clarification. You're right — the BOOST_INCLUDE_LIBRARIES top-level CMake configure path doesn't require the full sparse-checkout normalization that the integration test steps (Find Package, Subdirectory, Root Project) need, since those rely on add_subdirectory traversal across the full Boost source tree. The fact that FreeBSD builds were already passing confirms this. I'll withdraw the concern.
✏️ Learnings added
Learnt from: mvandeberg
Repo: cppalliance/corosio PR: 187
File: .github/workflows/ci.yml:728-748
Timestamp: 2026-03-04T16:26:10.447Z
Learning: In .github/workflows/ci.yml for the corosio project, sparse-checkout normalization (disabling git sparse-checkout and fetching missing objects) is only required before the integration test CMake steps (Find Package Integration, Subdirectory Integration, Root Project CMake Workflow) because they use add_subdirectory and need full source trees of Boost dependencies. The FreeBSD CMake job uses BOOST_INCLUDE_LIBRARIES with a top-level CMake configure, which does not require sparse-checkout normalization — Boost's own build system handles dependency resolution differently in that path.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #187 +/- ##
===========================================
+ Coverage 76.17% 76.24% +0.06%
===========================================
Files 98 98
Lines 10532 10533 +1
Branches 2388 2390 +2
===========================================
+ Hits 8023 8031 +8
+ Misses 1797 1778 -19
- Partials 712 724 +12
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 7 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
|
GCOVR code coverage report https://187.corosio.prtest3.cppalliance.org/gcovr/index.html Build time: 2026-03-04 16:40:59 UTC |
mvandeberg
force-pushed
the
pr/improved-build-matrix
branch
from
7bb9c60 to
831e549
Compare
2 participants
Summary by CodeRabbit
Bug Fixes
Chores