Add separation design doc and reorganize doc navigation#161
Add separation design doc and reorganize doc navigation#161vinniefalco merged 5 commits intocppalliance:developfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds mimalloc v2.2.7 to the allocation example build, links Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ 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://161.capy.prtest3.cppalliance.org/index.html If more commits are pushed to the pull request, the docs will rebuild at the same URL. 2026-02-14 02:10:41 UTC |
|
GCOVR code coverage report https://161.capy.prtest3.cppalliance.org/gcovr/index.html Build time: 2026-02-13 23:19:20 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: 3
🤖 Fix all issues with AI agents
In `@include/boost/capy/ex/channel.hpp`:
- Line 83: The doc comment for template parameter T is incorrect: the
implementation currently requires T to be default-constructible and
move-assignable (uses std::make_unique<T[]> to value-initialize storage and
assigns with buf_[idx] = std::move(v)), not just move-constructible. Fix by
either updating the `@tparam` to list default-constructible and move-assignable,
or (preferred) change the ring buffer implementation in channel.hpp
(functions/methods buf_push, buf_pop and the allocation that uses
std::make_unique<T[]>) to use raw aligned storage + placement-new and explicit
destructor calls so elements are constructed only on push and destroyed on pop,
and change error-return paths to return std::optional<T> (or another sentinel)
instead of returning T{}.
- Around line 246-257: The move constructor read_awaiter(read_awaiter&&)
currently transfers has_value_ but does not move the actual stored value in
value_buf_, leading to garbage in the destination when has_value_ is true;
update the move ctor in class read_awaiter to, after transferring flags, check
the source's has_value_ and if true move-construct (or memcpy for trivially
copyable types) the value from o.value_buf_ into this->value_buf_ (using
placement new or std::launder as appropriate), then destroy or mark the source
value as moved (set o.has_value_ = false) to maintain invariants; reference
symbols: read_awaiter(read_awaiter&&), value_buf_, has_value_, and ensure
noexcept/active_/in_list_ logic remains unchanged.
- Around line 568-571: Destructor ~channel() currently calls close(), which
posts coroutine handles that will resume and dereference ch_ leading to
use-after-free if the channel is destroyed; replace that behavior by asserting
no suspended coroutines remain: in ~channel() remove the call to close() and
instead add BOOST_CAPY_ASSERT(readers_.empty() && writers_.empty()) (or
equivalent) to catch misuse early, referencing the close(), ~channel(),
readers_, writers_, ch_, and await_resume() symbols when locating the change.
🧹 Nitpick comments (1)
include/boost/capy/ex/channel.hpp (1)
73-132: Async/awaitable javadoc forread()andwrite()is incomplete per coding guidelines.The guidelines require completion conditions (
@lilist), cancellation documentation,@throws,@par Exampleper function,@see, concurrency/overlap notes, and error condition details. The current docs cover the basics but omit most of these elements.Not blocking, but worth filling in over time—especially the cancellation behavior (stop_token support, resulting
error::canceled) which is required per the project's doc-awaitable rule.Also applies to: 591-626
include/boost/capy/ex/channel.hpp
Outdated
| a write suspends until a reader arrives (and vice versa), and | ||
| the value is transferred directly. | ||
|
|
||
| @tparam T The value type. Must be move-constructible. |
There was a problem hiding this comment.
T requirements are broader than documented.
The @tparam says "Must be move-constructible", but the actual code imposes additional requirements:
- Default-constructible:
std::make_unique<T[]>(limit)(line 587) value-initializes every element, andT{}is used in error-return paths (lines 309, 367). - Move-assignable:
buf_push(line 155) usesbuf_[idx] = std::move(v).
Either tighten the documentation to reflect the true constraints, or (preferably) relax the implementation so only move-constructibility is required—e.g., back the ring buffer with raw aligned storage + placement-new/manual destruction, and return std::optional<T> or a sentinel on error paths instead of T{}.
Sketch: aligned-storage ring buffer (eliminates default-constructible + move-assignable requirements)
- std::unique_ptr<T[]> buf_;
+ std::unique_ptr<unsigned char[]> buf_storage_;
+ // Access element i: *std::launder(reinterpret_cast<T*>(buf_storage_.get() + i * sizeof(T)))
// Constructor
explicit channel(std::size_t limit = 0)
: limit_(limit)
- , buf_(limit > 0
- ? std::make_unique<T[]>(limit) : nullptr)
+ , buf_storage_(limit > 0
+ ? std::make_unique<unsigned char[]>(limit * sizeof(T)) : nullptr)
{
}For buf_push / buf_pop, use placement-new and explicit destructor calls instead of move-assignment.
Also applies to: 145-145, 308-309, 367-367, 584-588
🤖 Prompt for AI Agents
In `@include/boost/capy/ex/channel.hpp` at line 83, The doc comment for template
parameter T is incorrect: the implementation currently requires T to be
default-constructible and move-assignable (uses std::make_unique<T[]> to
value-initialize storage and assigns with buf_[idx] = std::move(v)), not just
move-constructible. Fix by either updating the `@tparam` to list
default-constructible and move-assignable, or (preferred) change the ring buffer
implementation in channel.hpp (functions/methods buf_push, buf_pop and the
allocation that uses std::make_unique<T[]>) to use raw aligned storage +
placement-new and explicit destructor calls so elements are constructed only on
push and destroyed on pop, and change error-return paths to return
std::optional<T> (or another sentinel) instead of returning T{}.
include/boost/capy/ex/channel.hpp
Outdated
| read_awaiter(read_awaiter&& o) noexcept | ||
| : ch_(o.ch_) | ||
| , h_(o.h_) | ||
| , ex_(o.ex_) | ||
| , claimed_(o.claimed_.load( | ||
| std::memory_order_relaxed)) | ||
| , canceled_(o.canceled_) | ||
| , active_(std::exchange(o.active_, false)) | ||
| , in_list_(std::exchange(o.in_list_, false)) | ||
| , has_value_(std::exchange(o.has_value_, false)) | ||
| { | ||
| } |
There was a problem hiding this comment.
Latent bug: move constructor doesn't relocate value_buf_ contents.
has_value_ is transferred via std::exchange, but the actual value in value_buf_ is never moved to the new object. If the move constructor ever runs after has_value_ is set to true, the new awaiter would hold garbage in its value_buf_.
Currently this is safe because the compiler-generated move happens before await_suspend (and has_value_ is only set during suspension). Still, it is fragile—a future refactor or a different coroutine transform could break the invariant silently.
Defensive fix
read_awaiter(read_awaiter&& o) noexcept
: ch_(o.ch_)
, h_(o.h_)
, ex_(o.ex_)
, claimed_(o.claimed_.load(
std::memory_order_relaxed))
, canceled_(o.canceled_)
, active_(std::exchange(o.active_, false))
, in_list_(std::exchange(o.in_list_, false))
, has_value_(std::exchange(o.has_value_, false))
{
+ if(has_value_)
+ ::new(value_buf_) T(std::move(o.value_ref_()));
}🤖 Prompt for AI Agents
In `@include/boost/capy/ex/channel.hpp` around lines 246 - 257, The move
constructor read_awaiter(read_awaiter&&) currently transfers has_value_ but does
not move the actual stored value in value_buf_, leading to garbage in the
destination when has_value_ is true; update the move ctor in class read_awaiter
to, after transferring flags, check the source's has_value_ and if true
move-construct (or memcpy for trivially copyable types) the value from
o.value_buf_ into this->value_buf_ (using placement new or std::launder as
appropriate), then destroy or mark the source value as moved (set o.has_value_ =
false) to maintain invariants; reference symbols: read_awaiter(read_awaiter&&),
value_buf_, has_value_, and ensure noexcept/active_/in_list_ logic remains
unchanged.
include/boost/capy/ex/channel.hpp
Outdated
| ~channel() | ||
| { | ||
| close(); | ||
| } |
There was a problem hiding this comment.
Destructor + close() can post handles that outlive the channel.
close() posts suspended coroutine handles back to the executor. Those coroutines will call await_resume(), which dereferences ch_. If the channel is destroyed immediately after close() returns (which it is here—this is the destructor), every posted resumption is a use-after-free.
This is safe only if the caller guarantees all coroutines have already completed or been canceled before the channel is destroyed. Consider adding an assertion (BOOST_CAPY_ASSERT(readers_.empty() && writers_.empty())) in the destructor instead of calling close(), to catch misuse early.
Suggested safer destructor
~channel()
{
- close();
+ // All awaiters must have completed before the
+ // channel is destroyed.
+ BOOST_CAPY_ASSERT(readers_.empty());
+ BOOST_CAPY_ASSERT(writers_.empty());
+ closed_ = true;
}🤖 Prompt for AI Agents
In `@include/boost/capy/ex/channel.hpp` around lines 568 - 571, Destructor
~channel() currently calls close(), which posts coroutine handles that will
resume and dereference ch_ leading to use-after-free if the channel is
destroyed; replace that behavior by asserting no suspended coroutines remain: in
~channel() remove the call to close() and instead add
BOOST_CAPY_ASSERT(readers_.empty() && writers_.empty()) (or equivalent) to catch
misuse early, referencing the close(), ~channel(), readers_, writers_, ch_, and
await_resume() symbols when locating the change.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@example/allocation/allocation.cpp`:
- Around line 160-178: The printed percent values pct_rc_std, pct_mi_std and
pct_rc_mi currently always have a hardcoded '+' prefix which yields awkward
"+-X.X%" for negative values; update the print formatting in the output block
that prints "Recycling allocator", "mimalloc" and "std::allocator" to either
enable std::showpos on the stream before printing those percent values (so
positives show '+', negatives show '-' correctly) or remove the hardcoded '+'
and prepend a '+' only when the computed value is >= 0 (i.e., conditionally
format the sign for pct_rc_std, pct_rc_mi and pct_mi_std). Ensure you restore
the stream flags if you use std::showpos so other numeric output is unaffected.
🧹 Nitpick comments (4)
example/allocation/Jamfile (1)
20-22: Hardcoded-lmimallocis not portable to MSVC.
<linkflags>-lmimallocis GCC/Clang-specific. On MSVC this flag will be silently ignored or cause an error. Consider using a conditional or a Boost.Buildlibrule:Suggested approach
+lib mimalloc : : <name>mimalloc ; + exe allocation : [ glob *.cpp ] : - <linkflags>-lmimalloc + <library>mimalloc ;Alternatively, wrap the flag in a toolset condition:
- <linkflags>-lmimalloc + <toolset>gcc:<linkflags>-lmimalloc + <toolset>clang:<linkflags>-lmimalloc + <toolset>msvc:<linkflags>mimalloc.libexample/allocation/CMakeLists.txt (1)
10-15: Consider disabling unneeded mimalloc build targets.FetchContent will build all of mimalloc's default targets (shared lib, tests, etc.). Since only
mimalloc-staticis needed, you can reduce build time by disabling extras:Suggested addition
FetchContent_Declare(mimalloc GIT_REPOSITORY https://github.com/microsoft/mimalloc GIT_TAG v2.2.7 GIT_SHALLOW TRUE) +set(MI_BUILD_SHARED OFF CACHE BOOL "" FORCE) +set(MI_BUILD_TESTS OFF CACHE BOOL "" FORCE) FetchContent_MakeAvailable(mimalloc)example/allocation/allocation.cpp (2)
42-73: Themi_memory_resourceclass lacks a docstring.Per the coding guidelines, classes should have documentation. A brief comment exists on line 42, but a Doxygen-style block would be more appropriate. This is an example file so it's less critical, but a brief
/** */block describing purpose and noting that it's non-thread-safe (or thread-safe, per mimalloc's guarantees) would be helpful.
127-139: Minor:mi_mrconstruction is included in untimed region butcounter.store(0)placement is inconsistent.In the recycling and std::allocator benchmarks,
counter.store(0)is immediately followed by the timestamp capture. Here,mi_memory_resource mi_mris constructed between the counter reset (line 128) and the timestamp (line 130). Whilemi_mrconstruction is trivially cheap, moving it before the counter reset (or outside the timed block entirely) would keep the pattern consistent.Suggested reorder
+ mi_memory_resource mi_mr; + // With mimalloc counter.store(0); - mi_memory_resource mi_mr; auto t2 = std::chrono::steady_clock::now();
| auto pct_rc_std = std::round( | ||
| (ms_standard / ms_recycling - 1.0) * 1000.0) / 10.0; | ||
| auto pct_mi_std = std::round( | ||
| (ms_standard / ms_mimalloc - 1.0) * 1000.0) / 10.0; | ||
| auto pct_rc_mi = std::round( | ||
| (ms_mimalloc / ms_recycling - 1.0) * 1000.0) / 10.0; | ||
|
|
||
| std::cout | ||
| << iterations << " iterations, " | ||
| << "4-deep coroutine chain\n\n" | ||
| << std::fixed << std::setprecision(1) | ||
| << " Recycling allocator: " | ||
| << ms_recycling << " ms\n" | ||
| << ms_recycling << " ms (+" | ||
| << pct_rc_std << "% vs std, +" | ||
| << pct_rc_mi << "% vs mimalloc)\n" | ||
| << " mimalloc: " | ||
| << ms_mimalloc << " ms (+" | ||
| << pct_mi_std << "% vs std)\n" | ||
| << " std::allocator: " |
There was a problem hiding this comment.
Hardcoded + prefix will produce +-X.X% when a ratio is negative.
If one allocator is unexpectedly faster than another (e.g., mimalloc faster than recycling), the computed percentage goes negative, and the output becomes +-3.5% vs mimalloc. Either drop the hardcoded + and use std::showpos, or conditionally format:
Suggested fix using showpos
std::cout
<< iterations << " iterations, "
<< "4-deep coroutine chain\n\n"
- << std::fixed << std::setprecision(1)
+ << std::fixed << std::setprecision(1) << std::showpos
<< " Recycling allocator: "
- << ms_recycling << " ms (+"
- << pct_rc_std << "% vs std, +"
- << pct_rc_mi << "% vs mimalloc)\n"
- << " mimalloc: "
- << ms_mimalloc << " ms (+"
- << pct_mi_std << "% vs std)\n"
+ << std::noshowpos << ms_recycling << " ms ("
+ << std::showpos << pct_rc_std << "% vs std, "
+ << pct_rc_mi << "% vs mimalloc)\n"
+ << " mimalloc: "
+ << std::noshowpos << ms_mimalloc << " ms ("
+ << std::showpos << pct_mi_std << "% vs std)\n"
<< " std::allocator: "
- << ms_standard << " ms\n";
+ << std::noshowpos << ms_standard << " ms\n";🤖 Prompt for AI Agents
In `@example/allocation/allocation.cpp` around lines 160 - 178, The printed
percent values pct_rc_std, pct_mi_std and pct_rc_mi currently always have a
hardcoded '+' prefix which yields awkward "+-X.X%" for negative values; update
the print formatting in the output block that prints "Recycling allocator",
"mimalloc" and "std::allocator" to either enable std::showpos on the stream
before printing those percent values (so positives show '+', negatives show '-'
correctly) or remove the hardcoded '+' and prepend a '+' only when the computed
value is >= 0 (i.e., conditionally format the sign for pct_rc_std, pct_rc_mi and
pct_mi_std). Ensure you restore the stream flags if you use std::showpos so
other numeric output is unaffected.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@example/allocation/Jamfile`:
- Around line 21-22: The Jamfile currently hardcodes "<linkflags>-lmimalloc",
which is GCC/Clang-specific and conflicts with the project's CMake static
mimalloc; update the Jamfile to avoid the non-portable flag by either (A)
conditionalizing the link flag per toolset (e.g., apply
"<toolset>gcc:<linkflags>-lmimalloc" and "<toolset>clang:<linkflags>-lmimalloc"
and add an MSVC-specific alternative or no-op for "<toolset>msvc"), or (B)
declare mimalloc as a Boost.Build library target with a lib rule and use that
target in your target requirements so both MSVC and GCC/Clang resolve it
consistently with the CMake static build (refer to the existing
"<linkflags>-lmimalloc" usage to locate where to replace it and to the new lib
declaration to unify resolution).
| : | ||
| <linkflags>-lmimalloc |
There was a problem hiding this comment.
Hardcoded -lmimalloc is not portable across toolsets.
<linkflags>-lmimalloc is a GCC/Clang-ism and will fail (or be silently ignored) on MSVC. Also, the CMake build fetches and statically links mimalloc v2.2.7, whereas this Jamfile expects a system-installed shared library — the two build paths are inconsistent.
Consider either:
- Conditionalizing on toolset (e.g.,
<toolset>gcc:<linkflags>-lmimalloc) and adding an MSVC equivalent, or - Using a Boost.Build
librule to declare mimalloc so both toolsets resolve it.
🤖 Prompt for AI Agents
In `@example/allocation/Jamfile` around lines 21 - 22, The Jamfile currently
hardcodes "<linkflags>-lmimalloc", which is GCC/Clang-specific and conflicts
with the project's CMake static mimalloc; update the Jamfile to avoid the
non-portable flag by either (A) conditionalizing the link flag per toolset
(e.g., apply "<toolset>gcc:<linkflags>-lmimalloc" and
"<toolset>clang:<linkflags>-lmimalloc" and add an MSVC-specific alternative or
no-op for "<toolset>msvc"), or (B) declare mimalloc as a Boost.Build library
target with a lib rule and use that target in your target requirements so both
MSVC and GCC/Clang resolve it consistently with the CMake static build (refer to
the existing "<linkflags>-lmimalloc" usage to locate where to replace it and to
the new lib declaration to unify resolution).
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @.drone/drone.bat:
- Around line 1-47: The .drone/drone.bat file must use CRLF line endings to
avoid Windows batch parser failures; convert .drone/drone.bat to CRLF line
endings (ensure the file in the repo is saved with CRLF) and add a
.gitattributes entry to force CRLF for .bat files (e.g., add a line targeting
.drone/*.bat such as " .drone/*.bat text eol=crlf" in .gitattributes) so future
commits preserve CRLF; ensure you commit the updated .drone/drone.bat and
.gitattributes so CI on Windows parses labels and commands like the IF/SET/CALL
blocks correctly.
- Around line 22-23: Replace the Unix commands in the batch script: instead of
using "cp -prf boost-ci-cloned/ci .", copy the directory with the native Windows
command xcopy using recursive and overwrite flags to copy boost-ci-cloned\ci
into the current folder (preserve subdirectories and files); and instead of "rm
-rf boost-ci-cloned", remove the source directory with rmdir using /s /q to
recursively delete without prompts. Update the commands referencing
boost-ci-cloned and ci accordingly (use xcopy and rmdir) so the .drone/drone.bat
script uses only native Windows commands.
🧹 Nitpick comments (3)
.drone/drone.sh (2)
83-86: Use[[ ]]or separate[ ]tests instead of-a/-ooperators.The
-aand-ooperators inside[ ]are POSIX-deprecated and can produce unexpected results when operands resemble operators. Since the script already uses[[ ]]elsewhere (line 39), prefer it here for consistency and correctness.Proposed fix
-if [ -n "${COVERITY_SCAN_NOTIFICATION_EMAIL}" -a \( "$TRAVIS_BRANCH" = "develop" -o "$TRAVIS_BRANCH" = "master" \) -a \( "$DRONE_BUILD_EVENT" = "push" -o "$DRONE_BUILD_EVENT" = "cron" \) ] ; then +if [[ -n "${COVERITY_SCAN_NOTIFICATION_EMAIL}" && ( "$TRAVIS_BRANCH" = "develop" || "$TRAVIS_BRANCH" = "master" ) && ( "$DRONE_BUILD_EVENT" = "push" || "$DRONE_BUILD_EVENT" = "cron" ) ]]; then
22-22: Minor:export SELF=\basename ...`` masks the return value (SC2155).This pattern appears on several lines (9, 13, 16, 22, 34, 99). Under
set -e, if the command substitution fails, theexportstill succeeds and the error is silently swallowed. Since this is a CI script where silent failures can waste debugging time, consider splitting declare and assign on at least the critical ones.Example for line 22:
- export SELF=`basename $REPO_NAME` + SELF=$(basename "$REPO_NAME") + export SELF.drone/drone.bat (1)
10-10: Quote theSETassignment for safety.Unquoted
SETvalues can include trailing whitespace or cause issues with special characters.- SET TRAVIS_BRANCH=!DRONE_BRANCH! + SET "TRAVIS_BRANCH=!DRONE_BRANCH!"
|
|
||
| @ECHO ON | ||
| setlocal enabledelayedexpansion | ||
|
|
||
| set TRAVIS_OS_NAME=windows | ||
|
|
||
| IF "!DRONE_BRANCH!" == "" ( | ||
| for /F %%i in ("!GITHUB_REF!") do @set TRAVIS_BRANCH=%%~nxi | ||
| ) else ( | ||
| SET TRAVIS_BRANCH=!DRONE_BRANCH! | ||
| ) | ||
|
|
||
| if "%DRONE_JOB_BUILDTYPE%" == "boost" ( | ||
|
|
||
| echo "Running boost job" | ||
| echo '==================================> INSTALL' | ||
| REM there seems to be some problem with b2 bootstrap on Windows | ||
| REM when CXX env variable is set | ||
| SET "CXX=" | ||
|
|
||
| git clone https://github.com/boostorg/boost-ci.git boost-ci-cloned --depth 1 | ||
| cp -prf boost-ci-cloned/ci . | ||
| rm -rf boost-ci-cloned | ||
| REM source ci/travis/install.sh | ||
| REM The contents of install.sh below: | ||
|
|
||
| for /F %%i in ("%DRONE_REPO%") do @set SELF=%%~nxi | ||
| SET BOOST_CI_TARGET_BRANCH=!TRAVIS_BRANCH! | ||
| SET BOOST_CI_SRC_FOLDER=%cd% | ||
| if "%BOOST_BRANCH%" == "" ( | ||
| SET BOOST_BRANCH=develop | ||
| if "%BOOST_CI_TARGET_BRANCH%" == "master" set BOOST_BRANCH=master | ||
| ) | ||
|
|
||
| call ci\common_install.bat | ||
|
|
||
| echo '==================================> ZLIB' | ||
| git clone --branch v1.2.13 https://github.com/madler/zlib.git !BOOST_ROOT!\zlib-src --depth 1 | ||
| set ZLIB_SOURCE=!BOOST_ROOT!\zlib-src | ||
|
|
||
| echo '==================================> COMPILE' | ||
|
|
||
| REM set B2_TARGETS=libs/!SELF!/test libs/!SELF!/example | ||
| set B2_TARGETS=libs/!SELF!/test | ||
| call !BOOST_ROOT!\libs\!SELF!\ci\build.bat | ||
|
|
||
| ) |
There was a problem hiding this comment.
Unix line endings (LF) in a .bat file will cause parsing failures on Windows.
The Windows batch parser has known issues with LF-only line endings, including label parsing failures at 512-byte boundaries. This file must use CRLF line endings. Consider adding a .gitattributes entry:
.drone/*.bat text eol=crlf
🧰 Tools
🪛 Blinter (1.0.112)
[error] 1-1: Unix line endings detected. Explanation: Batch file uses Unix line endings (LF-only) which can cause GOTO/CALL label parsing failures and script malfunction due to Windows batch parser 512-byte boundary bugs. Recommendation: Convert file to Windows line endings (CRLF). Use tools like dos2unix, notepad++, or configure git with 'git config core.autocrlf true'. Context: File uses Unix line endings (LF-only) - 47 LF sequences found
(E018)
[error] 10-10: Unsafe SET command usage. Explanation: SET commands without proper validation or quoting can cause security issues. Recommendation: Always quote SET values and validate input: SET "var=safe value". Context: SET command value should be quoted for safety
(SEC002)
🤖 Prompt for AI Agents
In @.drone/drone.bat around lines 1 - 47, The .drone/drone.bat file must use
CRLF line endings to avoid Windows batch parser failures; convert
.drone/drone.bat to CRLF line endings (ensure the file in the repo is saved with
CRLF) and add a .gitattributes entry to force CRLF for .bat files (e.g., add a
line targeting .drone/*.bat such as " .drone/*.bat text eol=crlf" in
.gitattributes) so future commits preserve CRLF; ensure you commit the updated
.drone/drone.bat and .gitattributes so CI on Windows parses labels and commands
like the IF/SET/CALL blocks correctly.
| cp -prf boost-ci-cloned/ci . | ||
| rm -rf boost-ci-cloned |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, locate the .drone/drone.bat file
fd -g "drone.bat" -t f
# Then examine its contents with line numbers
if [ -f ".drone/drone.bat" ]; then
wc -l ".drone/drone.bat"
cat -n ".drone/drone.bat"
fiRepository: cppalliance/capy
Length of output: 1690
🏁 Script executed:
#!/bin/bash
# Check for .gitattributes files in the repository
fd -g '.gitattributes' -t f --exec cat {}Repository: cppalliance/capy
Length of output: 267
🏁 Script executed:
#!/bin/bash
# Look for context about the CI/CD setup (check if Git for Windows is explicitly required/configured)
fd -g '*.md' -t f | head -5 | xargs grep -l -i "git.*windows\|windows.*git" 2>/dev/null || echo "No direct references found"
fd -g 'appveyor*' -t f || echo "No AppVeyor config found"
fd -g '.drone.yml' -t fRepository: cppalliance/capy
Length of output: 311
🏁 Script executed:
#!/bin/bash
# Search for .gitattributes with hidden files enabled
fd -g '.gitattributes' -H -t f --exec cat {}Repository: cppalliance/capy
Length of output: 78
🏁 Script executed:
#!/bin/bash
# Search for any CI documentation or setup files
fd -E 'node_modules' -E '.git' -t f \( -name '*.md' -o -name 'README*' -o -name 'INSTALL*' \) | head -10Repository: cppalliance/capy
Length of output: 290
🏁 Script executed:
#!/bin/bash
# Look for references to cp or rm in batch/bash scripts to see execution pattern
rg -t batch -t shell "cp|rm" --max-count 20Repository: cppalliance/capy
Length of output: 89
🏁 Script executed:
#!/bin/bash
# Check if there are shell scripts that might set up the environment
fd -g '*.sh' -t f | head -10 | xargs -I {} sh -c 'echo "=== {} ===" && head -5 {}'Repository: cppalliance/capy
Length of output: 305
🏁 Script executed:
#!/bin/bash
# Search for .drone.yml or drone configuration
fd -g '.drone.yml' -o -g '.drone.yaml' -H -t f --exec cat {}Repository: cppalliance/capy
Length of output: 176
🏁 Script executed:
#!/bin/bash
# Search for README or CI documentation
fd -H -t f -E 'node_modules' -E '.git' '(README|CI|CONTRIBUTE)' | head -5 | xargs -I {} sh -c 'echo "=== {} ===" && head -20 {}'Repository: cppalliance/capy
Length of output: 3403
🏁 Script executed:
#!/bin/bash
# Look for other .bat files to see patterns
fd -g '*.bat' -t f --exec sh -c 'echo "=== {} ===" && cat {}'Repository: cppalliance/capy
Length of output: 717
🏁 Script executed:
#!/bin/bash
# Search for any documentation about Windows builds
rg -i "windows|batch|cmd|drone" --type md --max-count 30 | head -40Repository: cppalliance/capy
Length of output: 2502
Replace Unix commands with native Windows equivalents in batch script.
Lines 22–23 use cp and rm, which are not standard Windows commands and require external tools (Git for Windows, etc.). The repository's own batch file (doc/build_antora.bat) demonstrates the correct pattern by using only native Windows commands. Use xcopy and rmdir for consistency and robustness.
Proposed fix
-cp -prf boost-ci-cloned/ci .
-rm -rf boost-ci-cloned
+xcopy /s /e /y /q boost-ci-cloned\ci ci\
+rmdir /s /q boost-ci-cloned📝 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.
| cp -prf boost-ci-cloned/ci . | |
| rm -rf boost-ci-cloned | |
| xcopy /s /e /y /q boost-ci-cloned\ci ci\ | |
| rmdir /s /q boost-ci-cloned |
🤖 Prompt for AI Agents
In @.drone/drone.bat around lines 22 - 23, Replace the Unix commands in the
batch script: instead of using "cp -prf boost-ci-cloned/ci .", copy the
directory with the native Windows command xcopy using recursive and overwrite
flags to copy boost-ci-cloned\ci into the current folder (preserve
subdirectories and files); and instead of "rm -rf boost-ci-cloned", remove the
source directory with rmdir using /s /q to recursively delete without prompts.
Update the commands referencing boost-ci-cloned and ci accordingly (use xcopy
and rmdir) so the .drone/drone.bat script uses only native Windows commands.
3 participants
Summary by CodeRabbit
New Features
Chores