build(core): Update Boost to v1.87.0 in order to pull in boost::urls; Replace calls to boost::asio's deprecated expires_from_now with expires_after.
#636
Conversation
|
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 ast-grep (0.31.1)components/core/src/reducer/reducer_server.cppAn unexpected error occurred while running ast-grep. WalkthroughThe pull request updates the Boost library version across multiple scripts and configuration files within the Changes
Possibly related PRs
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
| @@ -15,9 +15,6 @@ DEBIAN_FRONTEND=noninteractive apt-get install --no-install-recommends -y \ | |||
| build-essential \ | |||
| git \ | |||
| jq \ | |||
| libboost-filesystem-dev \ | |||
| libboost-iostreams-dev \ | |||
| libboost-program-options-dev \ | |||
There was a problem hiding this comment.
Chose to switch to the boost install script on jammy because libboost-url-dev is only available from ubuntu noble onwards.
There was a problem hiding this comment.
How about the MacOS install scripts?
There was a problem hiding this comment.
We use the brew install formula for boost, which appears to be on 1.86 right now, so it satisfies the requirement.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
components/core/tools/scripts/lib_install/ubuntu-focal/install-packages-from-source.sh (1)
13-13: Consider enhancing the prerequisite comment.While the Boost version update is correct, consider expanding the comment to document the specific requirement for boost::urls in version 1.81.0.
Suggested comment enhancement:
-# NOTE: boost must be installed first since the remaining packages depend on it +"# NOTE: boost >= 1.81.0 must be installed first for boost::urls support and package dependencies"components/core/tools/scripts/lib_install/install-boost.sh (2)
Line range hint
29-31: Consider adding checksum verification for security.The script downloads the source tarball but doesn't verify its integrity. Consider adding SHA256/GPG verification.
# Download source +curl -fsSL https://boostorg.jfrog.io/artifactory/main/release/${version}/source/${tar_filename}.sha256 -o ${tar_filename}.sha256 curl -fsSL https://boostorg.jfrog.io/artifactory/main/release/${version}/source/${tar_filename} -o ${tar_filename} +echo "Verifying tarball integrity..." +sha256sum -c ${tar_filename}.sha256 tar xzf ${tar_filename}
Line range hint
16-16: Consider using a unique temp directory.Using a static temp directory name could cause issues with concurrent installations.
-temp_dir=/tmp/${package_name}-installation +temp_dir=$(mktemp -d /tmp/${package_name}-installation-XXXXXX)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
components/core/CMakeLists.txt(1 hunks)components/core/tools/scripts/lib_install/centos-stream-9/install-packages-from-source.sh(1 hunks)components/core/tools/scripts/lib_install/install-boost.sh(1 hunks)components/core/tools/scripts/lib_install/ubuntu-focal/install-packages-from-source.sh(1 hunks)components/core/tools/scripts/lib_install/ubuntu-jammy/install-packages-from-source.sh(1 hunks)components/core/tools/scripts/lib_install/ubuntu-jammy/install-prebuilt-packages.sh(0 hunks)
💤 Files with no reviewable changes (1)
- components/core/tools/scripts/lib_install/ubuntu-jammy/install-prebuilt-packages.sh
🔇 Additional comments (4)
components/core/tools/scripts/lib_install/centos-stream-9/install-packages-from-source.sh (1)
13-13: LGTM! Verify version compatibility with dependent packages.
The Boost version update to 1.81.0 aligns with the PR objective to support boost::urls. However, we should verify compatibility with dependent packages like fmt, spdlog, and msgpack.
✅ Verification successful
Let me gather more information about package dependencies and their Boost requirements.
Boost 1.81.0 update is compatible with all dependencies
The CMake configuration explicitly requires Boost 1.81.0, which matches the version being installed. All dependent packages (fmt 8.0.1, spdlog 1.9.2, and msgpack-cxx 7.0.0) have no specific Boost version constraints, indicating compatibility with Boost 1.81.0.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if any package documentation mentions specific Boost version requirements
# Search for Boost version requirements in package documentation
echo "Checking package documentation for Boost version requirements..."
rg -i "boost.*version|requires.*boost" --type txt --type md
Length of output: 282
Script:
#!/bin/bash
# Check CMake files for Boost version requirements and package dependencies
echo "Checking CMake files for Boost version requirements..."
rg -i "boost.*version|find_package.*boost|boost.*required" --type cmake -A 3
echo -e "\nChecking for fmt, spdlog, and msgpack dependencies..."
rg -i "find_package.*(fmt|spdlog|msgpack)" --type cmake -A 2
Length of output: 1710
components/core/tools/scripts/lib_install/ubuntu-jammy/install-packages-from-source.sh (1)
12-14: Excellent addition of prerequisite documentation and formatting.
The changes maintain version consistency while improving readability. Consider standardizing this comment format across all installation scripts for better maintainability.
✅ Verification successful
Comment style is already well-standardized across installation scripts
The prerequisite documentation for Boost installation is consistently formatted with "NOTE:" prefix across all distribution-specific scripts. While Ubuntu variants (Focal and Jammy) share identical wording, CentOS uses a slightly different but equally clear phrasing. The script headers also follow a consistent style with error handling comments.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check comment consistency across installation scripts
# Search for Boost-related comments in installation scripts
echo "Checking comment consistency across installation scripts..."
fd install-packages-from-source.sh --exec grep -H "boost.*must" {}
Length of output: 537
Script:
#!/bin/bash
# Check comment style consistency across all installation scripts
fd install-packages-from-source.sh --exec grep -H "^#.*NOTE" {}
# Check if there are other similar prerequisite comments
fd install-packages-from-source.sh --exec grep -H "^#.*must" {}
# Look for other types of comments in these files for style comparison
fd install-packages-from-source.sh --exec grep -H "^#" {}
Length of output: 2593
components/core/tools/scripts/lib_install/install-boost.sh (1)
37-37: LGTM! Addition of the 'url' library aligns with the PR objectives.
The change correctly adds the required boost::urls library to the build configuration.
components/core/CMakeLists.txt (1)
101-101: LGTM! Boost requirements correctly updated.
The change appropriately updates the minimum Boost version to 1.81 and adds the 'url' component.
Let's verify CI environment compatibility:
Picked 1.81.0 in general because it is the earliest version with boost::urls. Could switch to pulling 1.83.0 in environments where we pull specific versions and just keep the requirement at 1.81.0 if desired though. I don't think we have install scripts for 24.04 (noble) -- newest scripts seem to be for 22.04 (jammy). For jammy specifically it seemed like the individual libboost-url-dev package wasn't available (and only became available in noble), so I switched to using the install script. For noble the normal libboost-X-dev packages should work without issue. |
Right, I was just thinking of picking one version to satisfy all requirements, but picking 1.81 in CMake makes sense. Can we build and install 1.87 (the latest afaik) when installing in environments where we build & install? My thinking is that we may as well pick the latest and greatest if we're going to build and install (unless there's some compatibility issues). |
|
Seems like expires_from_now was removed in 1.87. I think the docs suggest switching to expires_after: https://www.boost.org/doc/libs/1_86_0/doc/html/boost_asio/reference/basic_waitable_timer.html This is necessary outside the PR since I see the macOS build is now failing due to this. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
components/core/src/reducer/reducer_server.cpp (1)
208-209: Consider improving readability of the timer expiration callWhile the change to
expires_afteris correct, the line split makes it slightly harder to read. Consider storing the interval in a variable first.Here's a more readable version:
- upsert_timer.expires_after(std::chrono::milliseconds(m_server_ctx->get_upsert_interval() - )); + auto interval = std::chrono::milliseconds(m_server_ctx->get_upsert_interval()); + upsert_timer.expires_after(interval);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
components/core/src/reducer/reducer_server.cpp(2 hunks)components/core/tools/scripts/lib_install/centos-stream-9/install-packages-from-source.sh(1 hunks)components/core/tools/scripts/lib_install/ubuntu-focal/install-packages-from-source.sh(1 hunks)components/core/tools/scripts/lib_install/ubuntu-jammy/install-packages-from-source.sh(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- components/core/tools/scripts/lib_install/ubuntu-focal/install-packages-from-source.sh
- components/core/tools/scripts/lib_install/ubuntu-jammy/install-packages-from-source.sh
- components/core/tools/scripts/lib_install/centos-stream-9/install-packages-from-source.sh
🧰 Additional context used
📓 Path-based instructions (1)
components/core/src/reducer/reducer_server.cpp (1)
Pattern **/*.{cpp,hpp,java,js,jsx,ts,tsx}: - Prefer false == <expression> rather than !<expression>.
🔇 Additional comments (1)
components/core/src/reducer/reducer_server.cpp (1)
124-124: Timer expiration method change looks good!
The change from expires_from_now to expires_after is correct and aligns with Boost's API changes. The semantic meaning remains unchanged.
Let's verify if there are any remaining instances of the deprecated expires_from_now:
✅ Verification successful
Timer expiration method update is correctly implemented
The codebase search confirms that all timer expiration methods are using the modern expires_after API, and there are no remaining instances of the deprecated expires_from_now. The changes are consistent throughout the reducer server implementation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining usage of deprecated expires_from_now
rg "expires_from_now" --type cpp
Length of output: 32
Script:
#!/bin/bash
# Let's search for any timer-related code to ensure comprehensive verification
rg -A 2 "expires_" --type cpp
Length of output: 663
gibber9809
changed the title
expires_from_now with expires_after.
Description
This PR updates our boost dependency to a minimum version of 1.81.0 in order to pull in the boost::urls library. This library will be used in a subsequent PR in order to determine the archive ID for a single-file-archive based on a url pointing to that archive.
The find_package(Boost 1.81 ... url) line in the cmake script effectively validates that the correct version of boost is available in all of our supported environments after this change.
Also note that the namespace the library uses is boost::urls, but the library is libboost-url.
Validation performed
Summary by CodeRabbit
New Features
Bug Fixes
Chores