-
Notifications
You must be signed in to change notification settings - Fork 71
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
build(core): Upgrade msgpack to v7.0.0. #575
build(core): Upgrade msgpack to v7.0.0. #575
Conversation
WalkthroughThe changes in this pull request primarily involve updating the version of the Changes
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 (
|
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.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
components/core/tools/scripts/lib_install/centos-stream-9/install-packages-from-source.sh (1)
18-18
: Add a version sync commentConsider adding a comment to indicate that this version needs to be kept in sync with other platforms (macOS Homebrew version).
-"${lib_install_scripts_dir}/msgpack.sh" 7.0.0 +# Keep this version in sync with macOS Homebrew (currently 7.0.0) +"${lib_install_scripts_dir}/msgpack.sh" 7.0.0
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
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/ubuntu-focal/install-packages-from-source.sh
(1 hunks)components/core/tools/scripts/lib_install/ubuntu-jammy/install-packages-from-source.sh
(1 hunks)
🔇 Additional comments (4)
components/core/tools/scripts/lib_install/ubuntu-jammy/install-packages-from-source.sh (1)
16-16
: Verify compatibility with dependent components.
The version update from 6.0.0 to 7.0.0 aligns with the PR objective of matching the macOS Homebrew default. However, as this is a major version bump, we should verify compatibility with dependent components.
Let's check for potential compatibility issues:
✅ Verification successful
Let's check the version constraints in these files to ensure compatibility.
The msgpack version upgrade is compatible with all dependencies
The version bump to msgpack 7.0.0 is safe as:
- CMake configuration already expects version 7.0.0
- Python components use msgpack-python (^1.0.7) which is independent of msgpack-cxx
- No explicit version constraints found in Node.js dependencies
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for msgpack usage patterns that might be affected by the version upgrade
# Check for msgpack includes and usage patterns
echo "Checking msgpack usage patterns..."
rg -l "msgpack" | grep -v "install-packages-from-source.sh"
# Look for specific version requirements in build files
echo "Checking for version constraints..."
rg -l "msgpack.*[0-9]+\.[0-9]+\.[0-9]+" | grep -v "install-packages-from-source.sh"
Length of output: 2502
Script:
#!/bin/bash
# Check version constraints in package files and CMakeLists.txt
echo "=== Python Dependencies ==="
rg "msgpack.*=.*" components/job-orchestration/pyproject.toml components/clp-package-utils/pyproject.toml
echo -e "\n=== Node.js Dependencies ==="
rg '"msgpack.*":.*"[0-9]' components/webui/package.json components/log-viewer-webui/server/package.json
echo -e "\n=== CMake Version Requirements ==="
rg "msgpack.*7\.0\.0" components/core/CMakeLists.txt
Length of output: 658
components/core/tools/scripts/lib_install/centos-stream-9/install-packages-from-source.sh (1)
18-18
: Verify msgpack 7.0.0 compatibility
The version update aligns with the PR objective to standardize msgpack version across platforms. However, as this is a major version bump, we should verify compatibility.
✅ Verification successful
Based on the search results, I can now provide a final response as the code analysis shows consistent and safe usage of msgpack that is compatible with version 7.0.0:
Msgpack 7.0.0 upgrade is safe to proceed
The codebase uses standard msgpack features that are stable across major versions:
- Basic type serialization/deserialization (integers, floats, strings, booleans)
- Standard containers (maps, arrays)
- Core msgpack APIs (pack, unpack, sbuffer)
No deprecated or version-specific features are used that would be affected by the 6.0.0 to 7.0.0 upgrade.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for potential breaking changes in msgpack usage
# Search for msgpack includes and usage patterns
rg -A 3 "msgpack" --type cpp
# Look for specific msgpack features that might have changed between versions
ast-grep --pattern 'MSGPACK_$_'
Length of output: 19600
components/core/tools/scripts/lib_install/ubuntu-focal/install-packages-from-source.sh (1)
19-19
: The msgpack version update looks good, but let's verify compatibility.
The version update to 7.0.0 aligns with the PR objective of matching Homebrew's default version.
Let's verify compatibility with dependent packages:
✅ Verification successful
Let me check the CMake configurations to verify the version requirements.
The msgpack 7.0.0 update is safe to proceed
The verification shows that:
- CMake explicitly requires msgpack-cxx 7.0.0
- The codebase uses standard msgpack features that are stable across versions:
- Basic serialization/deserialization
- Standard type handling (integers, floats, strings, arrays)
- No usage of version-specific APIs or deprecated features
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for potential compatibility issues with msgpack 7.0.0
# Search for msgpack includes and usage
echo "Checking msgpack usage patterns:"
rg -l "msgpack" | grep -v "install-packages-from-source.sh"
# Look for version-specific msgpack features or API calls
echo "Checking for version-specific msgpack features:"
rg "MSGPACK_VERSION|msgpack::version"
# Check CMake configuration for msgpack version requirements
echo "Checking CMake configuration:"
find . -name "CMakeLists.txt" -exec grep -l "msgpack" {} \;
Length of output: 2407
Script:
#!/bin/bash
# Check CMake files for msgpack version requirements
echo "Checking CMake files for msgpack version requirements:"
for file in ./components/core/src/clp/clo/CMakeLists.txt ./components/core/src/clp_s/CMakeLists.txt ./components/core/src/reducer/CMakeLists.txt ./components/core/CMakeLists.txt; do
echo "=== $file ==="
cat "$file" | grep -A 5 -B 5 "msgpack"
done
# Check msgpack usage in C++ files for version-specific features
echo -e "\nChecking C++ files for msgpack usage:"
for file in components/core/src/reducer/ServerContext.cpp components/core/src/clp_s/search/OutputHandler.cpp components/core/src/reducer/reducer_server.cpp components/core/src/reducer/DeserializedRecordGroup.cpp components/core/src/clp/ffi/ir_stream/Serializer.cpp; do
echo "=== $file ==="
cat "$file" | grep -A 2 -B 2 "msgpack"
done
# Check Python files for msgpack usage
echo -e "\nChecking Python files for msgpack usage:"
for file in components/job-orchestration/job_orchestration/scheduler/query/reducer_handler.py components/job-orchestration/job_orchestration/scheduler/compress/partition.py components/clp-package-utils/clp_package_utils/scripts/native/utils.py; do
echo "=== $file ==="
cat "$file" | grep -A 2 -B 2 "msgpack"
done
Length of output: 14461
components/core/CMakeLists.txt (1)
Line range hint 197-203
: LGTM! Version update aligns with PR objectives.
The update of msgpack-cxx requirement from 6.0.0 to 7.0.0 matches the PR's goal of maintaining consistency with the default Homebrew version.
Let's verify if there are any breaking changes between msgpack 6.0.0 and 7.0.0 that might affect the codebase:
✅ Verification successful
Let me verify the msgpack version upgrade impact by checking the changelog for breaking changes.
The msgpack-cxx version upgrade from 6.0.0 to 7.0.0 appears safe to proceed with.
The codebase uses stable msgpack-cxx features that have remained consistent across versions:
- Basic serialization/deserialization with
pack/unpack
- Core types like
object_map
,object_kv
, andsbuffer
- No usage of deprecated or removed features
The usage patterns found in the codebase align with the core functionality that has remained stable between these versions. The primary changes in msgpack-cxx 7.0.0 were focused on modernization and internal improvements rather than breaking API changes.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for msgpack usage patterns that might be affected by breaking changes
# between versions 6.0.0 and 7.0.0
# Test 1: Search for msgpack includes to identify usage
echo "Searching for msgpack includes..."
rg -l "msgpack.hpp|msgpack/.*\.hpp"
# Test 2: Search for specific msgpack patterns that might need attention
echo "Searching for msgpack serialization/deserialization patterns..."
rg -A 2 "MSGPACK_DEFINE|msgpack::pack|msgpack::unpack|msgpack::object"
Length of output: 5044
Script:
#!/bin/bash
# Search for any msgpack-specific types or features that might be affected
echo "Searching for msgpack array/map usage..."
rg -A 2 "msgpack::(array|map|object_array|object_map|object_kv)"
echo "Searching for msgpack type definitions..."
rg -A 2 "MSGPACK_DEFINE_MAP|MSGPACK_DEFINE_ARRAY"
echo "Searching for specific msgpack features..."
rg -A 2 "msgpack::zone|msgpack::sbuffer|msgpack::unpacker"
Length of output: 2227
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.
For the PR title, how about:
build(core): Upgrade msgpack to v7.0.0.
* ffi: Add support for serializing/deserializing auto-generated and user-generated schema tree node IDs. (y-scope#557) Co-authored-by: kirkrodrigues <[email protected]> * clp: Add missing C++ standard library includes in IR parsing files. (y-scope#561) Co-authored-by: kirkrodrigues <[email protected]> * log-viewer-webui: Update `yscope-log-viewer` to the latest version (which uses `clp-ffi-js`). (y-scope#562) * package: Upgrade dependencies to resolve security issues. (y-scope#536) * clp-s: Implement table packing (y-scope#466) Co-authored-by: wraymo <[email protected]> Co-authored-by: Kirk Rodrigues <[email protected]> Co-authored-by: wraymo <[email protected]> * log-viewer-webui: Update `yscope-log-viewer` to the latest version. (y-scope#565) * ci: Switch GitHub macOS build workflow to use macos-13 (x86) and macos-14 (ARM) runners. (y-scope#566) * core: Add support for user-defined HTTP headers in `NetworkReader`. (y-scope#568) Co-authored-by: Lin Zhihao <[email protected]> Co-authored-by: Xiaochong Wei <[email protected]> * chore: Update to the latest version of yscope-dev-utils. (y-scope#574) * build(core): Upgrade msgpack to v7.0.0. (y-scope#575) * feat(ffi): Update IR stream protocol version handling in preparation for releasing the kv-pair IR stream format: (y-scope#573) - Bump the IR stream protocol version to 0.1.0 for the kv-pair IR stream format. - Treat the previous IR stream format's versions as backwards compatible. - Differentiate between backwards-compatible and supported versions during validation. Co-authored-by: kirkrodrigues <[email protected]> * fix(taskfiles): Trim trailing slash from URL prefix in `download-and-extract-tar` (fixes y-scope#577). (y-scope#578) * fix(ffi): Correct `clp::ffi::ir_stream::Deserializer::deserialize_next_ir_unit`'s return value when failing to read the next IR unit's type tag. (y-scope#579) * fix(taskfiles): Update `yscope-log-viewer` sources in `log-viewer-webui-clients` sources list (fixes y-scope#576). (y-scope#580) * fix(cmake): Add Homebrew path detection for `mariadb-connector-c` to fix macOS build failure. (y-scope#582) Co-authored-by: kirkrodrigues <[email protected]> * refactor(ffi): Make `get_schema_subtree_bitmap` a public method of `KeyValuePairLogEvent`. (y-scope#581) * ci: Schedule GitHub workflows to daily run to detect failures due to upgraded dependencies or environments. (y-scope#583) * docs: Update the required version of task. (y-scope#567) * Add pr check workflow --------- Co-authored-by: kirkrodrigues <[email protected]> Co-authored-by: Junhao Liao <[email protected]> Co-authored-by: Henry8192 <[email protected]> Co-authored-by: Devin Gibson <[email protected]> Co-authored-by: wraymo <[email protected]> Co-authored-by: wraymo <[email protected]> Co-authored-by: Xiaochong(Eddy) Wei <[email protected]> Co-authored-by: Xiaochong Wei <[email protected]> Co-authored-by: haiqi96 <[email protected]>
Description
This PR updates msgpack the latest release (version 7.0.0) since the default version installed through
brew
is 7.0.0 on macOS. The PR also updates the scripts to install 7.0.0 for Linux builds.Validation performed
Summary by CodeRabbit
New Features
msgpack
library version to7.0.0
across various installation scripts.Bug Fixes