mirror of
https://github.com/milvus-io/milvus.git
synced 2026-10-12 02:19:10 +00:00
fix: safely reuse Cargo build artifacts (#53522)
### What this PR does
- invokes the Tantivy Cargo build target on every CMake build so Cargo
fingerprinting decides whether Rust artifacts are fresh
- adds the optional `MILVUS_CARGO_TARGET_ROOT` Make variable for
persistent Rust build artifacts
- assigns separate `tantivy` and `milvus-storage` subdirectories below
that root
- redirects Corrosion's existing `cmake_build/cargo/build` target
through a symbolic link, without modifying the generated Cargo command
- migrates an existing generated `cmake_build/cargo` directory when the
persistent root is enabled for the first time
- preserves the current workspace-local build directories when the new
variable is unset
- keeps the existing direct `CARGO_TARGET_DIR` behavior as a
compatibility fallback for current CI callers
### Motivation
The previous `add_custom_command(OUTPUT libtantivy_binding.a)` did not
declare Rust sources, `Cargo.toml`, or `Cargo.lock` as dependencies.
Once the archive existed, CMake/Ninja could skip Cargo entirely and
silently reuse a stale library after Rust inputs changed.
Both Tantivy and milvus-storage's Rust bridge currently place Cargo
build artifacts below `cmake_build`, which CI workspace cleanup removes.
A single Milvus-owned cache root lets CI mount and manage one persistent
directory without knowing Rust component names, while Cargo remains
responsible for freshness within each workspace target.
### Cache layout
When `MILVUS_CARGO_TARGET_ROOT=/path/to/cache` is set:
```text
/path/to/cache/
├── tantivy/
└── milvus-storage/
└── build/
```
The `milvus-storage/build` suffix is retained because Corrosion already
passes `--target-dir=<cmake_build>/cargo/build`.
### Validation
- configured the updated Tantivy CMake file in an isolated CMake/Ninja
harness
- built `install_tantivy_target` repeatedly and verified Cargo was
invoked on every build
- verified an external Cargo target directory was used for the generated
archive
- verified `copy_if_different` preserved the installed archive timestamp
when its content was unchanged
- verified an existing generated `cmake_build/cargo` directory is
replaced by the persistent-root symlink
- verified a non-directory path at `cmake_build/cargo` is rejected and
preserved
- `bash -n scripts/core_build.sh`
- `git diff --check`
Full Milvus compilation with the new shared root has not been run
locally. The follow-up Jenkins change will mount a PR-scoped persistent
root and validate cold/warm builds for all three build-ut-cov variants.
Jenkins integration PR: zilliztech/vdc-jenkins#636.
Previous CI integration experiments: zilliztech/vdc-jenkins#631 and
zilliztech/vdc-jenkins#632.
---------
Signed-off-by: Zhikun Yao <[email protected]>
This commit is contained in:
zhikunyao
@@ -27,6 +27,11 @@ PGO_PATH := $(PWD)/configs/pgo
|
||||
OS := $(shell uname -s)
|
||||
mode = Release
|
||||
|
||||
# Optional persistent root for Cargo build artifacts. Individual Rust
|
||||
# workspaces select their own subdirectories below this root.
|
||||
MILVUS_CARGO_TARGET_ROOT ?=
|
||||
export MILVUS_CARGO_TARGET_ROOT
|
||||
|
||||
# Set disk_index default based on OS
|
||||
# macOS (Darwin) does not support aio, so disable disk_index
|
||||
ifeq ($(OS),Darwin)
|
||||
|
||||
+28
-24
@@ -20,7 +20,13 @@ set(TANTIVY_LIB_DIR "${CMAKE_INSTALL_PREFIX}/lib")
|
||||
set(TANTIVY_INCLUDE_DIR "${CMAKE_INSTALL_PREFIX}/include")
|
||||
set(TANTIVY_NAME "libtantivy_binding${CMAKE_STATIC_LIBRARY_SUFFIX}")
|
||||
|
||||
set(LIB_FILE "${CMAKE_CURRENT_BINARY_DIR}/${TARGET_DIR}/${TANTIVY_NAME}")
|
||||
set(TANTIVY_CARGO_TARGET_DIR "$ENV{CARGO_TARGET_DIR}")
|
||||
if (MILVUS_CARGO_TARGET_ROOT)
|
||||
set(TANTIVY_CARGO_TARGET_DIR "${MILVUS_CARGO_TARGET_ROOT}/tantivy")
|
||||
elseif (NOT TANTIVY_CARGO_TARGET_DIR)
|
||||
set(TANTIVY_CARGO_TARGET_DIR "${CMAKE_CURRENT_BINARY_DIR}")
|
||||
endif ()
|
||||
set(LIB_FILE "${TANTIVY_CARGO_TARGET_DIR}/${TARGET_DIR}/${TANTIVY_NAME}")
|
||||
set(LIB_HEADER_FOLDER "${CMAKE_CURRENT_SOURCE_DIR}/tantivy-binding/include")
|
||||
|
||||
# In fact, cargo was already installed on our builder environment.
|
||||
@@ -30,47 +36,45 @@ set(PATH_VAR $ENV{PATH})
|
||||
set(ENV{PATH} ${HOME_VAR}/.cargo/bin:${PATH_VAR})
|
||||
message($ENV{PATH})
|
||||
|
||||
# Compile tantivy binding using cargo
|
||||
# The OUTPUT must be the actual file produced, so ninja/make knows how to generate it
|
||||
add_custom_command(
|
||||
OUTPUT ${LIB_FILE}
|
||||
# Always invoke Cargo and let its fingerprinting decide whether Rust code needs
|
||||
# to be rebuilt. Treating the static library as an OUTPUT without declaring
|
||||
# every Rust input can otherwise leave a stale library after source changes.
|
||||
add_custom_target(tantivy_build
|
||||
COMMENT "Compiling tantivy binding"
|
||||
COMMAND ${CMAKE_COMMAND} -E env
|
||||
"PATH=${HOME_VAR}/.cargo/bin:${PATH_VAR}"
|
||||
"TOKENIZER_PROTO=${TOKENIZER_PROTO}"
|
||||
"CARGO_TARGET_DIR=${CMAKE_CURRENT_BINARY_DIR}"
|
||||
"CARGO_TARGET_DIR=${TANTIVY_CARGO_TARGET_DIR}"
|
||||
${CARGO_CMD}
|
||||
WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/tantivy-binding
|
||||
BYPRODUCTS ${LIB_FILE}
|
||||
VERBATIM
|
||||
)
|
||||
|
||||
# Set the installed library path
|
||||
set(INSTALLED_LIB_FILE "${TANTIVY_LIB_DIR}/${TANTIVY_NAME}")
|
||||
|
||||
# Install tantivy library and headers to output directory
|
||||
# This custom command produces the installed library file
|
||||
add_custom_command(
|
||||
OUTPUT ${INSTALLED_LIB_FILE}
|
||||
# Install tantivy library and headers to output directory. copy_if_different
|
||||
# keeps downstream C++ targets fresh when Cargo has no work to do.
|
||||
add_custom_target(install_tantivy_target
|
||||
COMMENT "Installing tantivy binding to ${TANTIVY_LIB_DIR}"
|
||||
COMMAND ${CMAKE_COMMAND} -E make_directory ${TANTIVY_LIB_DIR}
|
||||
COMMAND ${CMAKE_COMMAND} -E make_directory ${TANTIVY_INCLUDE_DIR}
|
||||
COMMAND ${CMAKE_COMMAND} -E copy ${LIB_FILE} ${TANTIVY_LIB_DIR}/
|
||||
COMMAND ${CMAKE_COMMAND} -E copy ${LIB_HEADER_FOLDER}/tantivy-binding.h ${TANTIVY_INCLUDE_DIR}/
|
||||
COMMAND ${CMAKE_COMMAND} -E copy ${CMAKE_CURRENT_SOURCE_DIR}/tantivy-wrapper.h ${TANTIVY_INCLUDE_DIR}/
|
||||
COMMAND ${CMAKE_COMMAND} -E copy ${CMAKE_CURRENT_SOURCE_DIR}/rust-binding.h ${TANTIVY_INCLUDE_DIR}/
|
||||
COMMAND ${CMAKE_COMMAND} -E copy ${CMAKE_CURRENT_SOURCE_DIR}/rust-array.h ${TANTIVY_INCLUDE_DIR}/
|
||||
COMMAND ${CMAKE_COMMAND} -E copy ${CMAKE_CURRENT_SOURCE_DIR}/rust-hashmap.h ${TANTIVY_INCLUDE_DIR}/
|
||||
COMMAND ${CMAKE_COMMAND} -E copy ${CMAKE_CURRENT_SOURCE_DIR}/tokenizer.h ${TANTIVY_INCLUDE_DIR}/
|
||||
COMMAND ${CMAKE_COMMAND} -E copy ${CMAKE_CURRENT_SOURCE_DIR}/token-stream.h ${TANTIVY_INCLUDE_DIR}/
|
||||
COMMAND ${CMAKE_COMMAND} -E copy ${CMAKE_CURRENT_SOURCE_DIR}/phrase_match.h ${TANTIVY_INCLUDE_DIR}/
|
||||
COMMAND ${CMAKE_COMMAND} -E copy ${CMAKE_CURRENT_SOURCE_DIR}/time_recorder.h ${TANTIVY_INCLUDE_DIR}/
|
||||
DEPENDS ${LIB_FILE}
|
||||
COMMAND ${CMAKE_COMMAND} -E copy_if_different ${LIB_FILE} ${TANTIVY_LIB_DIR}/
|
||||
COMMAND ${CMAKE_COMMAND} -E copy_if_different ${LIB_HEADER_FOLDER}/tantivy-binding.h ${TANTIVY_INCLUDE_DIR}/
|
||||
COMMAND ${CMAKE_COMMAND} -E copy_if_different ${CMAKE_CURRENT_SOURCE_DIR}/tantivy-wrapper.h ${TANTIVY_INCLUDE_DIR}/
|
||||
COMMAND ${CMAKE_COMMAND} -E copy_if_different ${CMAKE_CURRENT_SOURCE_DIR}/rust-binding.h ${TANTIVY_INCLUDE_DIR}/
|
||||
COMMAND ${CMAKE_COMMAND} -E copy_if_different ${CMAKE_CURRENT_SOURCE_DIR}/rust-array.h ${TANTIVY_INCLUDE_DIR}/
|
||||
COMMAND ${CMAKE_COMMAND} -E copy_if_different ${CMAKE_CURRENT_SOURCE_DIR}/rust-hashmap.h ${TANTIVY_INCLUDE_DIR}/
|
||||
COMMAND ${CMAKE_COMMAND} -E copy_if_different ${CMAKE_CURRENT_SOURCE_DIR}/tokenizer.h ${TANTIVY_INCLUDE_DIR}/
|
||||
COMMAND ${CMAKE_COMMAND} -E copy_if_different ${CMAKE_CURRENT_SOURCE_DIR}/token-stream.h ${TANTIVY_INCLUDE_DIR}/
|
||||
COMMAND ${CMAKE_COMMAND} -E copy_if_different ${CMAKE_CURRENT_SOURCE_DIR}/phrase_match.h ${TANTIVY_INCLUDE_DIR}/
|
||||
COMMAND ${CMAKE_COMMAND} -E copy_if_different ${CMAKE_CURRENT_SOURCE_DIR}/time_recorder.h ${TANTIVY_INCLUDE_DIR}/
|
||||
DEPENDS tantivy_build
|
||||
BYPRODUCTS ${INSTALLED_LIB_FILE}
|
||||
VERBATIM
|
||||
)
|
||||
|
||||
# Create a target that other targets can depend on
|
||||
add_custom_target(install_tantivy_target DEPENDS ${INSTALLED_LIB_FILE})
|
||||
|
||||
add_library(tantivy_binding STATIC IMPORTED)
|
||||
add_dependencies(tantivy_binding
|
||||
install_tantivy_target
|
||||
|
||||
+32
-1
@@ -218,6 +218,36 @@ else
|
||||
CMAKE_GENERATOR="Unix Makefiles"
|
||||
fi
|
||||
|
||||
if [[ -n "${MILVUS_CARGO_TARGET_ROOT:-}" ]]; then
|
||||
if [[ "${MILVUS_CARGO_TARGET_ROOT}" != /* ]]; then
|
||||
echo "ERROR: MILVUS_CARGO_TARGET_ROOT must be an absolute path" >&2
|
||||
exit 1
|
||||
fi
|
||||
|
||||
mkdir -p "${MILVUS_CARGO_TARGET_ROOT}/tantivy" "${MILVUS_CARGO_TARGET_ROOT}/milvus-storage"
|
||||
|
||||
# Corrosion passes --target-dir=${BUILD_OUTPUT_DIR}/cargo/build explicitly,
|
||||
# so CARGO_TARGET_DIR cannot relocate milvus-storage's Rust artifacts.
|
||||
# Keep Corrosion's generated command unchanged and redirect its parent
|
||||
# directory to the persistent cache instead.
|
||||
CORROSION_CARGO_DIR="${BUILD_OUTPUT_DIR}/cargo"
|
||||
MILVUS_STORAGE_CARGO_DIR="${MILVUS_CARGO_TARGET_ROOT}/milvus-storage"
|
||||
if [[ -L "${CORROSION_CARGO_DIR}" ]]; then
|
||||
ln -sfn "${MILVUS_STORAGE_CARGO_DIR}" "${CORROSION_CARGO_DIR}"
|
||||
elif [[ -d "${CORROSION_CARGO_DIR}" ]]; then
|
||||
# Migrate a generated target directory left by builds that predate the
|
||||
# persistent Cargo root. Cargo can safely recreate all of its contents.
|
||||
echo "Migrating ${CORROSION_CARGO_DIR} to persistent Cargo target storage"
|
||||
rm -rf -- "${CORROSION_CARGO_DIR}"
|
||||
ln -s "${MILVUS_STORAGE_CARGO_DIR}" "${CORROSION_CARGO_DIR}"
|
||||
elif [[ -e "${CORROSION_CARGO_DIR}" ]]; then
|
||||
echo "ERROR: ${CORROSION_CARGO_DIR} already exists and is not a symbolic link" >&2
|
||||
exit 1
|
||||
else
|
||||
ln -s "${MILVUS_STORAGE_CARGO_DIR}" "${CORROSION_CARGO_DIR}"
|
||||
fi
|
||||
fi
|
||||
|
||||
# build with diskann index if OS is ubuntu or rocky or amzn
|
||||
if [ -f /etc/os-release ]; then
|
||||
. /etc/os-release
|
||||
@@ -263,7 +293,8 @@ ${CMAKE_EXTRA_ARGS} \
|
||||
-DWITH_CRT=${WITH_CRT} \
|
||||
-DMILVUS_USE_PCH=${USE_PCH} \
|
||||
-DMILVUS_UNITY_BUILD=${USE_UNITY_BUILD} \
|
||||
-DMILVUS_USE_SPLIT_DWARF=${USE_SPLIT_DWARF} "
|
||||
-DMILVUS_USE_SPLIT_DWARF=${USE_SPLIT_DWARF} \
|
||||
-DMILVUS_CARGO_TARGET_ROOT=${MILVUS_CARGO_TARGET_ROOT:-} "
|
||||
# Azure build variables removed as we now use Arrow with Azure support directly
|
||||
CMAKE_CMD=${CMAKE_CMD}"${CPP_SRC_DIR}"
|
||||
|
||||
|
||||
Reference in New Issue
Block a user