fix(build): Support CMake v4+ and GCC 15 toolchains, and upgrade C++ dependencies (fixes #795). - #2454
fix(build): Support CMake v4+ and GCC 15 toolchains, and upgrade C++ dependencies (fixes #795).#2454jackluo923 wants to merge 4 commits into
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:
WalkthroughThe changes update source dependency versions, libarchive installation, pkg-config discovery, CMake version handling, and yaml-cpp linkage across supported build environments. ChangesBuild dependency and toolchain updates
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
428b4cc to
8419c98
Compare
8419c98 to
6f5cc57
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@components/core/tools/scripts/lib_install/pipx-packages/install-cmake.sh`:
- Around line 17-21: Update the CMake requirement documented in the
components-core development documentation to match the policy represented by
installed_version_major_max_plus_1: accept CMake 3.23.0 and newer, including
version 4+. Remove or revise the stale “less than 4.0.0” constraint note while
preserving the existing minimum-version requirement.
- Around line 23-36: Update the pre-install condition in install-cmake.sh to
install the pipx CMake package whenever cmake is missing or its detected version
is below the same minimum required by the existing installed-version failure
check. Reuse the existing required_version_major_min and
required_version_minor_min policy so acceptable system installations remain
unchanged, and ensure version detection occurs before deciding whether
installation is needed.
- Around line 24-30: Update the installation branch in install-cmake.sh after
pipx install/ensurepath to make the directory reported by pipx environment
--value PIPX_BIN_DIR available in the current process before the
installed_version command resolves cmake. Preserve the existing PATH behavior
when CMake is already installed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: d6eb2cde-2cda-440a-ad4b-b49ecc88c060
📒 Files selected for processing (11)
components/core/src/glt/glt/CMakeLists.txtcomponents/core/tools/docker-images/clp-env-base-centos-stream-9/Dockerfilecomponents/core/tools/scripts/lib_install/centos-stream-9/install-packages-from-source.shcomponents/core/tools/scripts/lib_install/centos-stream-9/install-prebuilt-packages.shcomponents/core/tools/scripts/lib_install/libarchive.shcomponents/core/tools/scripts/lib_install/manylinux_2_28/install-packages-from-source.shcomponents/core/tools/scripts/lib_install/musllinux_1_2/install-packages-from-source.shcomponents/core/tools/scripts/lib_install/pipx-packages/install-cmake.shcomponents/core/tools/scripts/lib_install/ubuntu-jammy/install-packages-from-source.shdocs/src/dev-docs/components-core/index.mdtaskfiles/deps/main.yaml
b2a814a to
a10dc73
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/src/dev-docs/components-core/index.md`:
- Line 47: Synchronize the log-surgeon revision between the dependency table
entry and the build manifest: update the documented revision in the log-surgeon
row to match the revision configured by the relevant dependency manifest entry,
or update both consistently if the installed revision is intentionally changing.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 2dc914bb-e6a6-4925-83f8-43f58974b90c
📒 Files selected for processing (11)
components/core/src/glt/glt/CMakeLists.txtcomponents/core/tools/docker-images/clp-env-base-centos-stream-9/Dockerfilecomponents/core/tools/scripts/lib_install/centos-stream-9/install-packages-from-source.shcomponents/core/tools/scripts/lib_install/centos-stream-9/install-prebuilt-packages.shcomponents/core/tools/scripts/lib_install/libarchive.shcomponents/core/tools/scripts/lib_install/manylinux_2_28/install-packages-from-source.shcomponents/core/tools/scripts/lib_install/musllinux_1_2/install-packages-from-source.shcomponents/core/tools/scripts/lib_install/pipx-packages/install-cmake.shcomponents/core/tools/scripts/lib_install/ubuntu-jammy/install-packages-from-source.shdocs/src/dev-docs/components-core/index.mdtaskfiles/deps/main.yaml
💤 Files with no reviewable changes (1)
- components/core/tools/scripts/lib_install/centos-stream-9/install-prebuilt-packages.sh
…dependencies (fixes y-scope#795). CMake v4 removed compatibility with `cmake_minimum_required` below v3.5, breaking yaml-cpp, libarchive, and the googletest that ANTLR's test build fetches. CLP previously capped this by pinning CMake below v4, which has since become actively harmful: the install script fails outright wherever CMake v4+ is the system package, since it only installs the pinned version when no `cmake` is already present. The version check now accepts v4+, while the pipx install keeps its `<4` bound so no environment changes which CMake it builds with. Separately, libstdc++ 15 stopped transitively including <cstdint>, so yaml-cpp 0.7.0 fails to compile under GCC 15. The yaml-cpp upgrade addresses both that and its CMake v4 incompatibility. Also upgrades libarchive (3.5.1/3.8.0 -> 3.8.9), mongo-cxx-driver (r4.1.1 -> r4.4.1, the only release carrying an upstream mongo-c-driver fix that CMake v4.4 requires), SQLite3 (3.36.0 -> 3.53.4), Catch2, Microsoft.GSL, nlohmann_json, simdjson, utfcpp, zlib, and date. libarchive is now built from source on centos-stream-9 too, so all four distro scripts agree on one version. None of these required changes to CLP's own source. Supersedes y-scope#794, which first diagnosed the CMake v4 problem, and y-scope#2453, which surfaced it again on Ubuntu 26.04. Co-authored-by: Bingran Hu <bingran.hu@yscope.com> Co-authored-by: Artem Zuikov <chertus@gmail.com> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
a10dc73 to
39abb57
Compare
mariadb-connector-cpp 1.1.5 declares its CMake minimum conditionally: `VERSION 3.23` on Apple and `VERSION 3.1` everywhere else. CMake v4 rejects the non-Apple branch, so `task package` fails to configure on Linux while macOS is unaffected. Upstream raised the else-branch to `VERSION 3.5` in 1.1.8; 1.1.6 and 1.1.7 still declare 3.1. Reached only via `task package` -> `deps:spider`, so the core-only builds used to validate this branch did not exercise it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
- install-cmake.sh: `pipx ensurepath` updates shell startup files but not the running process, so a freshly pipx-installed CMake could fail to resolve at the version check, exiting 127 with `cmake: command not found`. Prepend pipx's application directory when CMake still doesn't resolve. `pipx environment` would report that directory but doesn't exist in pipx v1.0 (Ubuntu 22.04), so fall back to pipx's default. - docs: drop the `< 4.0.0` CMake constraint, which contradicted this PR, along with its now-unused link reference. - docs: correct log-surgeon to 193e1f9, matching taskfiles/deps/main.yaml. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Tested that e2e compression+search workflows succeeds for:
|
gibber9809
left a comment
There was a problem hiding this comment.
LGTM. I don't expect any of these dependency updates to be a problem for clp-s, but still tested that:
- Unit tests pass as expected
- Core integration tests pass as expected
Description
Makes CLP capable of building with CMake v4+ and GCC 15, and batches in a set of low-risk dependency upgrades while the build is already being touched.
Scope: this removes the blockers so an environment providing CMake v4+ is accepted. It does not switch CLP to CMake v4 — when CLP's
install-cmake.shhas to provide CMake itself, it still installs v3, exactly as before.Background
Two independent toolchain shifts were breaking CLP.
CMake v4+ changed two things that affect CLP. v4.0 made a
cmake_minimum_requiredminimum below v3.5 a hard error, so any dependency declaring less no longer configures. v4.4 then started validating the warning categories in-Warguments passed to CMake itself, rejecting any it doesn't recognize where earlier versions let them through. The first of these surfaced on macOS, wherebrew install cmakehad moved to v4 — that's #795, filed in April 2025. CLP's response then was to pin CMake<4in #1271, which capped the problem rather than fixing it.Separately, libstdc++ 15 stopped transitively including
<cstdint>, so yaml-cpp v0.7.0 fails to compile with'uint16_t' was not declared in this scope. CMake plays no part here — this hits GCC 15 on any CMake version.The blockers
Five dependencies blocked CMake v4, and all five had to be fixed for support to mean anything. Four simply declare a minimum below the v3.5 threshold; the fifth trips the v4.4 flag validation instead, which is why it moves the ceiling to v4.4 rather than v4.0:
The mongo-cxx-driver upgrade is the critical one. The bundled driver passes
-Werror— a compiler flag — through atry_compile()'sCMAKE_FLAGS, and v4.4 reads it as a CMake warning category instead, failing withThe warning category "error ..." is not known. Upstream moved the flag toCOMPILE_DEFINITIONSin mongo-c-driver#2346.Without that upgrade, "CMake v4 support" would mean only v4.0 through v4.3 — yet v4.4 is what Homebrew, the
musllinuxbase image, and a plainpipx install cmakeall provide today.zlib, Microsoft.GSL, and nlohmann_json declare v3.12, v3.14, and v3.5 respectively — all at or above v3.5 — so v4 accepts them unchanged, and their upgrades below are maintenance rather than v4 fixes.
The GCC 15 failure needs no separate change: yaml-cpp v0.9.0 fixes the
<cstdint>include as well, so one upgrade closes both toolchain shifts.Prior work
Two earlier attempts preceded this one.
@Bill-hbrhbr hit the CMake v4 problem in #794 (April 2025) and bumped yaml-cpp to an unreleased commit; when that didn't land, they added the
<4pin in #1271, which this PR removes. yaml-cpp v0.9.0 is now a released version declaring a compatible minimum. They also proposed-DANTLR_BUILD_CPP_TESTS=OFFearlier, in #1479 (October 2025, still open), as part of standardizing the dependency install tasks rather than as a CMake v4 fix.@4ertus2 hit it again on Ubuntu 26.04 in #2453, arriving at the same ANTLR flag independently. This PR carries their yaml-cpp upgrade and ANTLR change, with two deviations:
-DCMAKE_POLICY_VERSION_MINIMUM=3.5, which forces v4 to accept older declarations, isn't carried over — it's redundant once yaml-cpp declares v3.5 — and-DANTLR_BUILD_CPP_TESTS=OFFfollows the existing alphabetical ordering.Dependency upgrades
None required changes to CLP's own source.
<cstdint>failure; also raises its declared minimum to CMake v3.5.CMAKE_MINIMUM_REQUIRED(VERSION 3.1)on non-Apple platforms (VERSION 3.23on Apple), which v4 rejects; upstream raised the else-branch to v3.5 in 1.1.8, and 1.1.6/1.1.7 still declare 3.1. Reached only throughtask package→deps:spider. Distinct from the MariaDB C connector CLP core uses (libmariadb), which comes from distro packages and is unaffected.On r4.4.1's recency: it was published 2026-07-23, roughly two weeks before this PR, and is both the newest release and the oldest carrying the fix — there is no version to fall back to. It is a three-minor jump from r4.1.1 across the 16 CLP source files using mongocxx/bsoncxx. Reviewers may prefer to let it age; the alternative is keeping the CMake ceiling, since CMake v4.4+ can't be supported without it.
Build-configuration changes
install-cmake.sh: accept CMake v4+pipx installkeeps its<4bound, so no environment changes which CMake it uses.install-cmake.sh: prepend pipx's application directory when neededpipx ensurepathupdates shell startup files but not the running process, so a freshly installed CMake could fail to resolve at the version check, exiting 127. Guarded oncommand -v, so it is a no-op where the directory is already onPATH(including all four images, which setPIPX_BIN_DIR).< 4.0.0CMake constraintcomponents-core/index.mdstill documented the ceiling this PR removes, citing #795 as the reason. Also refreshes the dependency version table.-DANTLR_BUILD_CPP_TESTS=OFFBUILD_VERSION._*) entries, which collide in the examples' target names. The GitHub archive is clean but omitsVERSION_CURRENT, so the version must be passed explicitly.PKG_CONFIG_PATH=/usr/local/lib/pkgconfig:/usr/local/lib64/pkgconfiglib64on RHEL-family distros, but pkg-config doesn't search there, so it never finds libarchive's.pcfile.pkg_check_modulesfinding nothing isn't an error, so-lzsilently drops off the link line andclpfails oninflateEnd. Naming both directories fixes the lookup itself, and holds whichever of the two a given distro installs into.centos-stream-9: build the compression stack from sourcePKG_CONFIG_PATHentry above.yaml-cpp::yaml-cppubuntu-jammy: build libarchive after liblzma/lz4/zstdThe CMake floor stays at v3.23, required by ystdlib's use of
FILE_SET: CMake 3.22.6 fails withinstall TARGETS given unknown argument "FILE_SET"even with the declared minimum lowered, and 3.23.3 configures cleanly.Deliberately not upgraded
Everything above is drop-in. The following are major-version bumps with breaking APIs, most needing CLP source changes, and some constrained by other components:
clp-plugin-presto-connectorpins fmt to Velox's version.find_package(msgpack-cxx 7.0.0 REQUIRED)updated.Checklist
Validation performed
Eight environments built end-to-end (
task deps:corethentask core), each producing 24/24 dependencies and all 7 binaries, with no CLP source changes. Those eight were arm64 builds only; the CMake v4 matrix below adds x86-64 and unit tests.Shipped configuration
Every CLP image rebuilt from this branch. CMake comes from
install-cmake.shunder its unchanged<4bound, so all four stay on v3:ubuntu-jammycentos-stream-9manylinux_2_28musllinux_1_2CMake v4 and GCC 15
Every run overrides
install-cmake.sh's<4install bound, so the capability is demonstrated rather than assumed:ubuntu-jammycentos-stream-9manylinux_2_28musllinux_1_2✓ = all dependencies and 7/7 binaries built, and the unit-test suite passed (159 test cases, ~5.77M assertions). macOS has no x86-64 row because the runner is arm64.
install-cmake.shinstalls v4 — the only way to make CI exercise the v4 path, since this PR's own jobs always build with v3. Both runs are green:clp-artifact-build(every job, includingpackage-image-amd64and the*-binariesjobs that run the test suite) andclp-core-build-macos.install-all.shran unmodified, andtask deps:corebuilt yaml-cpp 0.9.0, the release whose<cstdint>fix GCC 15 needs.Each fix confirmed against the failure it resolves
-DANTLR_BUILD_CPP_TESTS=OFFtask packageunder 4.4.0 — found by CI, not by any local runPKG_CONFIG_PATHmanylinux_2_28(inflateEnd); 3.8.0 builds 7/7install-cmake.shaccepts v4install-cmake.shPATH fixcmake: command not foundwhen pipx's directory isn't already onPATHOther checks
task deps:spider— the only path reaching mariadb-connector-cpp — built under CMake 4.4.0 onubuntu-jammyx86-64 after the 1.1.8 upgrade, with no configure errors. The environment builds above usetask deps:core+task core, which never reach it.centos-stream-9,manylinux_2_28x86-64,musllinux_1_2x86-64,ubuntu-jammy, and macOS (shared and static).PKG_CONFIG_PATHscope. CLP consumes pkg-config output only asfind_library/find_pathhints, a version string, and the static-dependency name list — no-Lreaches the link line, so all that matters is that the.pcis found.task lint:yamlpasses.Known gaps
software-properties-common, which CLP's install scripts still request. Neither is needed: ANTLR runs on Java 21, and CLP never usessoftware-properties-common. This is pre-existing and unrelated to this PR; with both applied locally, the two † rows built cleanly.Summary by CodeRabbit
Improvements
Bug Fixes