Skip to content

[Store] Fix Rust store build path#2114

Open
ZhenyuePan wants to merge 1 commit into
kvcache-ai:mainfrom
ZhenyuePan:codex/store-rust-build-path-2069
Open

[Store] Fix Rust store build path#2114
ZhenyuePan wants to merge 1 commit into
kvcache-ai:mainfrom
ZhenyuePan:codex/store-rust-build-path-2069

Conversation

@ZhenyuePan
Copy link
Copy Markdown

Description

Fixes #2069.

This PR fixes the Mooncake Store Rust standalone build path so Rust examples and tests link against the same native libraries selected by the CMake build.

The main issue was that standalone Cargo builds could pick up a different system copy of native dependencies, such as glog, instead of the dependency used by CMake. mooncake-store/rust/build.rs now reads library search paths from the CMake build context, including MOONCAKE_BUILD_DIR, the default ../../build/CMakeCache.txt, and CMAKE_PREFIX_PATH.

This PR also adds Rust Store build/test documentation and fixes the stale WITH_WITH_RUST_EXAMPLE CMake option in the docs.

Module

  • Transfer Engine (mooncake-transfer-engine)
  • Mooncake Store (mooncake-store)
  • Mooncake EP (mooncake-ep)
  • Integration (mooncake-integration)
  • P2P Store (mooncake-p2p-store)
  • Python Wheel (mooncake-wheel)
  • PyTorch Backend (mooncake-pg)
  • Mooncake RL (mooncake-rl)
  • CI/CD
  • Docs
  • Other

Type of Change

  • Bug fix
  • New feature
  • Refactor
  • Breaking change
  • Documentation update
  • Other

How Has This Been Tested?

git diff --cached --check

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces support for Mooncake Store Rust bindings by updating documentation and enhancing the build system. Key changes include adding libclang as a dependency, correcting CMake option names in the documentation, and providing a comprehensive README for the Rust component. The build.rs script was updated to automatically discover library paths by parsing CMakeCache.txt. A review comment identifies an improvement opportunity in the build script to correctly handle semicolon-separated library lists in the CMake cache, ensuring all dependency directories are properly identified.

Comment on lines +73 to +78
if key.ends_with("_LIBRARY:FILEPATH") || key.ends_with("_LIBRARIES:FILEPATH") {
let library_path = PathBuf::from(value);
if let Some(parent) = library_path.parent() {
push_existing_dir(search_dirs, parent.to_path_buf());
}
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

CMake variables ending in _LIBRARIES often contain a semicolon-separated list of library paths (for example, GLOG_LIBRARIES:FILEPATH=/usr/lib/libglog.so;/usr/lib/libgflags.so). The current implementation treats the entire string as a single path, which will fail to resolve the parent directory correctly if multiple paths are present. Splitting the value by ; ensures that all library directories are correctly identified.

        if key.ends_with("_LIBRARY:FILEPATH") || key.ends_with("_LIBRARIES:FILEPATH") {
            for path_str in value.split(';').filter(|s| !s.is_empty()) {
                let library_path = PathBuf::from(path_str);
                if let Some(parent) = library_path.parent() {
                    push_existing_dir(search_dirs, parent.to_path_buf());
                }
            }
        }

@codecov-commenter
Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Performance]: Mooncake Store Rust API Perf.

2 participants