-
Notifications
You must be signed in to change notification settings - Fork 83
build(deps): Migrate Boost and msgpack-cxx dependencies to task-based installation (fixes #1243); Drop required Boost version back to 1.81; Add missing ZStd CMake target links. #1254
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
0189799
f0987f8
0f9dcf3
249e1b3
6de558a
542ed09
eec7e05
ddaac0c
b6d5164
4576159
87a0be4
aa8e7c3
a33ac1c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
This file was deleted.
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -37,6 +37,7 @@ The task will download, build, and install (within the build directory) the foll | |||||||||||||||||||
| |-----------------------------------------------------------------------|----------------| | ||||||||||||||||||||
| | [abseil-cpp](https://github.com/abseil/abseil-cpp) | 20250512.0 | | ||||||||||||||||||||
| | [ANTLR](https://www.antlr.org) | v4.13.2 | | ||||||||||||||||||||
| | [Boost](https://github.com/boostorg/boost) | v1.87.0 | | ||||||||||||||||||||
| | [Catch2](https://github.com/catchorg/Catch2.git) | v3.8.0 | | ||||||||||||||||||||
|
Comment on lines
+40
to
41
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Clarify accepted Boost version range in docs to match CMake constraints The table shows the task-installed Boost at v1.87.0, but CMake accepts 1.81–1.88. Adding a brief note prevents confusion when users bring their own Boost. Apply either of the following: Option A — inline parenthetical in the table cell: -| [Boost](https://github.com/boostorg/boost) | v1.87.0 |
+| [Boost](https://github.com/boostorg/boost) | v1.87.0 (CMake accepts 1.81–1.88) |Option B — short note immediately below the table: | [zstd](https://github.com/facebook/zstd) | v1.5.7 |
+Note: The Task-installed Boost default is v1.87.0. The CMake build accepts Boost versions in the range 1.81–1.88.📝 Committable suggestion
Suggested change
Suggested change
|
||||||||||||||||||||
| | [date](https://github.com/HowardHinnant/date.git) | v3.0.1 | | ||||||||||||||||||||
| | [fmt](https://github.com/fmtlib/fmt) | v10.2.1 | | ||||||||||||||||||||
|
|
@@ -45,6 +46,7 @@ The task will download, build, and install (within the build directory) the foll | |||||||||||||||||||
| | [liblzma](https://github.com/tukaani-project/xz) | v5.8.1 | | ||||||||||||||||||||
| | [lz4](https://github.com/lz4/lz4) | v1.10.0 | | ||||||||||||||||||||
| | [mongo-cxx-driver](https://github.com/mongodb/mongo-cxx-driver) | r3.10.2 | | ||||||||||||||||||||
| | [msgpack-cxx](https://github.com/msgpack/msgpack-c/tree/cpp_master) | v7.0.0 | | ||||||||||||||||||||
| | [simdjson](https://github.com/simdjson/simdjson) | v3.13.0 | | ||||||||||||||||||||
| | [spdlog](https://github.com/gabime/spdlog) | v1.14.1 | | ||||||||||||||||||||
| | [SQLite3](https://www.sqlite.org/download.html) | v3.36.0 | | ||||||||||||||||||||
|
|
||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Adding ${zstd_TARGET} as PUBLIC on clp_s::json_constructor: OK, but ensure the build option reflects this.
Linking zstd as PUBLIC here is acceptable if any public header (directly or indirectly) requires zstd.h. Regardless, because zstd is now a required dependency for this target, verify that CLP_NEED_ZSTD has been added to set_clp_s_json_constructor_dependencies in options.cmake as planned in this migration.
Use this quick check:
🏁 Script executed:
Length of output: 2941
Ensure CLP_NEED_ZSTD is added to set_clp_s_json_constructor_dependencies
The
set_clp_s_json_constructor_dependenciesfunction incomponents/core/cmake/Options/options.cmakecurrently only sets:Since
${zstd_TARGET}has been added as a PUBLIC dependency ofclp_s_json_constructor, you must also expose it via theCLP_NEED_ZSTDoption flag. Please update the function as follows:• File: components/core/cmake/Options/options.cmake
• Function:
set_clp_s_json_constructor_dependencies(around line 282)Once this is in place, the public zstd dependency and corresponding build option will be properly wired.
🤖 Prompt for AI Agents