build(deps): Install version-constrained dev tools via pipx rather than brew formulae on macOS; Bump minimum Boost version to 1.89 (fixes #1218, #1223).
#1219
Conversation
WalkthroughIncrease Boost minimum to 1.89 and remove Boost.System usage from CMake/linkage; update Boost version in multiple install-from-source scripts; rewrite macOS install flow to avoid temporary Homebrew formula files by using pipx for some tools and direct brew installs for others; update linting doc to require uv >= 0.8. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant CI as CI Runner (macOS)
participant Script as install-all.sh
participant Pipx as pipx
participant Brew as brew
CI->>Script: run install-all.sh
Script->>Pipx: install pinned cmake and go-task (and uv if selected) via pipx
Script->>Brew: brew install remaining dependencies (including boost)
Script-->>CI: exit with status
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related issues
Possibly related PRs
Suggested reviewers
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
Bill-hbrhbr
changed the title
CMake and Task via pipx on macOS; Upgrade Boost to 1.89.
Bill-hbrhbr
changed the title
CMake and Task via pipx on macOS; Upgrade Boost to 1.89.CMake and Task via pipx on macOS; Bump and restrict Boost version to 1.89.
Bill-hbrhbr
changed the title
CMake and Task via pipx on macOS; Bump and restrict Boost version to 1.89.CMake and Task via pipx on macOS; Bump and restrict Boost version to >=1.89.
Bill-hbrhbr
changed the title
CMake and Task via pipx on macOS; Bump and restrict Boost version to >=1.89.CMake and Task via pipx on macOS; Bump and restrict Boost version to >=1.89 (fixes #1218, #1223).
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
components/core/CMakeLists.txt(1 hunks)components/core/src/reducer/CMakeLists.txt(0 hunks)components/core/tools/scripts/lib_install/centos-stream-9/install-packages-from-source.sh(1 hunks)components/core/tools/scripts/lib_install/macos/install-all.sh(2 hunks)components/core/tools/scripts/lib_install/manylinux_2_28/install-packages-from-source.sh(1 hunks)components/core/tools/scripts/lib_install/musllinux_1_2/install-packages-from-source.sh(1 hunks)components/core/tools/scripts/lib_install/ubuntu-jammy/install-packages-from-source.sh(1 hunks)
💤 Files with no reviewable changes (1)
- components/core/src/reducer/CMakeLists.txt
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: Bill-hbrhbr
PR: y-scope/clp#1122
File: components/core/src/clp/clp/CMakeLists.txt:175-195
Timestamp: 2025-07-23T09:54:45.185Z
Learning: In the CLP project, when reviewing CMakeLists.txt changes that introduce new compression library dependencies (BZip2, LibLZMA, LZ4, ZLIB), the team prefers to address conditional linking improvements in separate PRs rather than expanding the scope of focused migration PRs like the LibArchive task-based installation migration.
Learnt from: Bill-hbrhbr
PR: y-scope/clp#1156
File: components/core/CMakeLists.txt:772-772
Timestamp: 2025-08-09T04:07:27.083Z
Learning: In the CLP project's CMakeLists.txt, when reviewing changes related to the ${zstd_TARGET} variable usage in test linking, the team is planning a refactoring PR to improve this mechanism. Guards for undefined target variables should be deferred to that separate PR rather than being added in focused dependency migration PRs.
📚 Learning: 2025-08-03T18:56:07.366Z
Learnt from: Bill-hbrhbr
PR: y-scope/clp#1124
File: components/core/cmake/Modules/FindBZip2.cmake:49-65
Timestamp: 2025-08-03T18:56:07.366Z
Learning: In CLP's custom CMake Find modules, `FindStaticLibraryDependencies` populates the `*_DYNAMIC_LIBS` variable even when building with static libraries. When the main library is static, all dependencies must also use static libraries - there's no fallback to shared libraries for dependencies.
Applied to files:
components/core/CMakeLists.txt
📚 Learning: 2025-08-04T17:26:17.098Z
Learnt from: Bill-hbrhbr
PR: y-scope/clp#1124
File: components/core/CMakeLists.txt:167-170
Timestamp: 2025-08-04T17:26:17.098Z
Learning: In CLP's CMake configuration for BZip2, Bill-hbrhbr prefers using HINTS parameter for path-based resolution rather than version pinning in find_package(). The primary concern is ensuring the task-installed BZip2 is found instead of system copies, not enforcing specific versions.
Applied to files:
components/core/CMakeLists.txt
📚 Learning: 2025-07-24T21:56:05.806Z
Learnt from: Bill-hbrhbr
PR: y-scope/clp#1124
File: components/core/cmake/Modules/FindBZip2.cmake:60-60
Timestamp: 2025-07-24T21:56:05.806Z
Learning: In CLP's custom CMake Find modules (like FindBZip2.cmake), when building with dynamic libraries, the code intentionally uses `bzip2_PKGCONF_STATIC_LIBRARIES` as input to `FindDynamicLibraryDependencies`. This is a deliberate fallback mechanism to ensure all library dependencies are resolved even in dynamic builds, using the static library list from pkg-config as a source of dependency information.
Applied to files:
components/core/CMakeLists.txt
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: ubuntu-jammy-static-linked-bins
- GitHub Check: ubuntu-jammy-dynamic-linked-bins
- GitHub Check: ubuntu-jammy-lint
- GitHub Check: centos-stream-9-dynamic-linked-bins
- GitHub Check: centos-stream-9-static-linked-bins
- GitHub Check: build-macos (macos-15, true)
- GitHub Check: build-macos (macos-15, false)
🔇 Additional comments (6)
components/core/CMakeLists.txt (1)
159-160: No lingering references toBoost::systemdetectedThe CMake scan confirms:
- No occurrences of
Boost::systemas a linked target.- No
find_package(Boost … system)component requests.- A single
find_package(Boost 1.89 REQUIRED …)declaration.All clear—the bump to Boost 1.89 and removal of
Boost::systemare properly applied.components/core/tools/scripts/lib_install/macos/install-all.sh (1)
9-15: Nice mitigation for the Homebrew tap change; CMake via pipx keeps us under v4 as required.Moving off ad-hoc formulae addresses issue #1218’s “formula must be in a tap” failure and enforces the CMake<4 constraint. Good call.
components/core/tools/scripts/lib_install/ubuntu-jammy/install-packages-from-source.sh (1)
13-13: LGTM: Boost version bump to 1.89.0 matches the new minimum.No other flow changes; downstream packages remain pinned as before.
components/core/tools/scripts/lib_install/centos-stream-9/install-packages-from-source.sh (1)
13-13: LGTM: Consistent Boost 1.89.0 across distros.No concerns with the change in this script.
components/core/tools/scripts/lib_install/musllinux_1_2/install-packages-from-source.sh (2)
10-10: Boost version bump to 1.89.0 aligns with the PR’s dependency update plan.Change is straightforward and consistent with the CMake/other installer updates described in the PR summary.
10-10: All checks passed for Boost 1.89.0 across install scripts, CMake, and installer
Cross-repo consistency and installer compatibility confirmed:
- Every platform’s install-packages-from-source.sh now invokes
install-boost.sh 1.89.0.components/core/CMakeLists.txthasfind_package(Boost 1.89 REQUIRED …).install-boost.shcorrectly converts “1.89.0” → “1_89_0” and downloadsboost_1_89_0.tar.gz.No further changes required.
components/core/tools/scripts/lib_install/manylinux_2_28/install-packages-from-source.sh
Show resolved
Hide resolved
components/core/tools/scripts/lib_install/musllinux_1_2/install-packages-from-source.sh
Show resolved
Hide resolved
Co-authored-by: kirkrodrigues <2454684+kirkrodrigues@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
components/core/CMakeLists.txt(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: Bill-hbrhbr
PR: y-scope/clp#1122
File: components/core/src/clp/clp/CMakeLists.txt:175-195
Timestamp: 2025-07-23T09:54:45.185Z
Learning: In the CLP project, when reviewing CMakeLists.txt changes that introduce new compression library dependencies (BZip2, LibLZMA, LZ4, ZLIB), the team prefers to address conditional linking improvements in separate PRs rather than expanding the scope of focused migration PRs like the LibArchive task-based installation migration.
Learnt from: Bill-hbrhbr
PR: y-scope/clp#1156
File: components/core/CMakeLists.txt:772-772
Timestamp: 2025-08-09T04:07:27.083Z
Learning: In the CLP project's CMakeLists.txt, when reviewing changes related to the ${zstd_TARGET} variable usage in test linking, the team is planning a refactoring PR to improve this mechanism. Guards for undefined target variables should be deferred to that separate PR rather than being added in focused dependency migration PRs.
🔇 Additional comments (2)
components/core/CMakeLists.txt (2)
159-165: Boost >=1.89 requirement and component set look correctRaising the minimum to 1.89 and omitting Boost.System aligns with the PR goals and the linked reducer changes. The component list matches the link targets later in this file.
159-165: No lingering Boost::system references found
All CMake files were scanned forBoost::systemlink usages and forfind_package(Boost … system …)declarations. No occurrences were detected.
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (3)
components/core/tools/scripts/lib_install/macos/install-all.sh (3)
3-8: Add pipefail for safer error propagation in pipelinesWith set -e, -u already set, add pipefail to avoid masking errors in piped commands (e.g., grep | awk).
Apply:
set -e @@ set -u +set -o pipefail
30-41: Optional: assert Boost >= 1.89 after brew install to align with CMake min requirementBrew’s “boost” version can drift; adding a quick post-install check will fail fast if runners lag below 1.89.
Apply:
zstd + +# Verify Boost >= 1.89 +if brew list --versions boost >/dev/null 2>&1; then + boost_ver="$(brew list --versions boost | awk '{print $2}')" + req_ver="1.89.0" + # Check boost_ver >= req_ver + if [ "$(printf '%s\n' "$req_ver" "$boost_ver" | sort -V | head -n1)" != "$req_ver" ]; then + echo "Boost $boost_ver is installed, but >= $req_ver is required." >&2 + exit 1 + fi +fi
47-49: Silence command detection output for cleaner logsMinor: redirect
command -vto avoid printing paths in CI logs.Apply:
-if ! command -v pkg-config ; then +if ! command -v pkg-config >/dev/null 2>&1 ; then
♻️ Duplicate comments (2)
components/core/tools/scripts/lib_install/macos/install-all.sh (2)
13-16: Make uninstall idempotent and prepare pipx/PATH before first use (prevents set -e aborts)
command -v cmakedoesn’t imply Homebrew owns it;brew uninstallwill fail if cmake wasn’t installed by brew.- Some runners don’t have pipx on PATH; ensure pipx is installed and PATH prepared prior to use.
- Force-install with pipx to guarantee the constrained version is applied if an older pipx-managed version exists.
Apply:
-if command -v cmake ; then - brew uninstall --force cmake -fi -pipx install "cmake~=3.31" +if brew list --formula cmake >/dev/null 2>&1; then + brew uninstall --force cmake +fi +if ! command -v pipx >/dev/null 2>&1; then + brew install pipx +fi +# Ensure pipx shims are on PATH for this shell +pipx ensurepath >/dev/null 2>&1 || true +export PATH="${HOME}/.local/bin:${PATH}" +hash -r +pipx install --force 'cmake~=3.31'
19-23: Guard brew uninstall for task/go-task and verify installed Task version
- Similar to CMake,
command -v taskdoesn’t mean brew owns it; guard brew uninstalls.- Homebrew has historically used the formula name
go-task; handle both.- Force-install ensures the constrained version is applied.
- Optional but useful: assert the installed Task version to catch PATH issues early.
Apply:
-if command -v task ; then - brew uninstall --force task -fi -pipx install "go-task-bin>=3.42,<3.43" +if brew list --formula task >/dev/null 2>&1 || brew list --formula go-task >/dev/null 2>&1; then + brew uninstall --force task || true + brew uninstall --force go-task || true +fi +pipx install --force 'go-task-bin>=3.42,<3.43' +# Basic sanity check; expected "Task version 3.42.x" +if ! task --version | grep -Eq 'Task version 3\.42\.'; then + echo "Unexpected task version or not on PATH:" >&2 + command -v task || true + task --version || true + exit 1 +fi
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
components/core/tools/scripts/lib_install/macos/install-all.sh(1 hunks)docs/src/dev-guide/contributing-linting.md(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: Bill-hbrhbr
PR: y-scope/clp#1122
File: components/core/src/clp/clp/CMakeLists.txt:175-195
Timestamp: 2025-07-23T09:54:45.185Z
Learning: In the CLP project, when reviewing CMakeLists.txt changes that introduce new compression library dependencies (BZip2, LibLZMA, LZ4, ZLIB), the team prefers to address conditional linking improvements in separate PRs rather than expanding the scope of focused migration PRs like the LibArchive task-based installation migration.
Learnt from: Bill-hbrhbr
PR: y-scope/clp#1156
File: components/core/CMakeLists.txt:772-772
Timestamp: 2025-08-09T04:07:27.083Z
Learning: In the CLP project's CMakeLists.txt, when reviewing changes related to the ${zstd_TARGET} variable usage in test linking, the team is planning a refactoring PR to improve this mechanism. Guards for undefined target variables should be deferred to that separate PR rather than being added in focused dependency migration PRs.
📚 Learning: 2025-08-04T18:38:33.130Z
Learnt from: haiqi96
PR: y-scope/clp#0
File: :0-0
Timestamp: 2025-08-04T18:38:33.130Z
Learning: User haiqi96 requested creating a GitHub issue to document a documentation discrepancy where Task version requirements in docs weren't updated after yscope-utils upgrade in PR #1158.
Applied to files:
docs/src/dev-guide/contributing-linting.md
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: centos-stream-9-deps-image
- GitHub Check: ubuntu-jammy-deps-image
- GitHub Check: build-macos (macos-15, true)
- GitHub Check: build-macos (macos-15, false)
- GitHub Check: lint-check (ubuntu-24.04)
- GitHub Check: lint-check (macos-15)
🔇 Additional comments (1)
docs/src/dev-guide/contributing-linting.md (1)
21-21: LGTM: uv version requirement consistently updatedVerified that the only explicit version constraint for uv is
>= 0.8in both the docs and the macOS install script. No references to older uv constraints were found elsewhere. No further changes needed here.
Bill-hbrhbr
changed the title
CMake and Task via pipx on macOS; Bump and restrict Boost version to >=1.89 (fixes #1218, #1223).pipx on macOS; Bump and restrict Boost version to >=1.89 (fixes #1218, #1223).
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
components/core/tools/scripts/lib_install/macos/install-all.sh (3)
11-11: Comment/version mismatch with CMake constraintThe comment says v3.31.6 but the spec installs any 3.31.x. Align the comment or pin the exact version.
-# Install CMake v3.31.6 as ANTLR and yaml-cpp do not yet support CMake v4+. +# Install CMake 3.31.x as ANTLR and yaml-cpp do not yet support CMake v4+.
19-22: Use brew-owned check for uninstall and force the Task version; optionally assert version post-installAs with CMake, uninstall only if Homebrew owns task to avoid set -e aborts, and use pipx --force so the constraint is enforced even if task is already installed.
Apply within these lines:
-if command -v task ; then - brew uninstall --force task -fi -pipx install "go-task-bin>=3.40,<3.43" +if brew list --formula task >/dev/null 2>&1; then + brew uninstall --force task +fi +pipx install --force 'go-task-bin>=3.40,<3.43'Optionally add a quick sanity check (after installation) to catch PATH/resolution issues at runtime:
# Expect Task version 3.40–3.42 if ! task --version | grep -Eq 'Task version 3\.(40|41|42)\.'; then echo "Unexpected task version or not on PATH:" >&2 command -v task || true task --version || true exit 1 fi
24-27: Prefer a pipx-managed uv and enforce the minimum versionInstalling uv only if missing won’t upgrade pre-existing installs that are below the minimum. Also, if uv was installed by Homebrew, keep PATH unambiguous by uninstalling the brew formula first. Use pipx --force to apply the constraint.
Apply within these lines:
-# Install uv -if ! command -v uv ; then - pipx install "uv>=0.8" -fi +# Install uv via pipx; prefer pipx-managed install and enforce minimum version +if brew list --formula uv >/dev/null 2>&1; then + brew uninstall --force uv +fi +pipx install --force 'uv>=0.8'Optional post-install sanity check:
if ! uv --version >/dev/null 2>&1; then echo "uv not found on PATH after install" >&2 command -v uv || true exit 1 fi
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
components/core/tools/scripts/lib_install/macos/install-all.sh(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: centos-stream-9-deps-image
- GitHub Check: ubuntu-jammy-deps-image
- GitHub Check: lint-check (ubuntu-24.04)
There was a problem hiding this comment.
For the PR title, how about:
build(deps): Install version-constrained dev tools via `pipx` rather than brew formulae on macOS; Bump minimum Boost version to 1.89 (fixes #1218, #1223).
Bumping the Boost version is a little bit of a brute force fix. Perhaps we should open an issue to switch to the Boost install task; that way we don't have to conform to the versions installed in GH runners.
Bill-hbrhbr
changed the title
pipx on macOS; Bump and restrict Boost version to >=1.89 (fixes #1218, #1223).pipx rather than brew formulae on macOS; Bump minimum Boost version to 1.89 (fixes #1218, #1223).
2 participants
Description
This PR fixes #1218 and #1223. These issues occur due to MacOS runner upgrade.
Checklist
breaking change.
Validation performed
Summary by CodeRabbit
Chores
Documentation
Notes