github-actions[bot] commented on code in PR #67523:
URL: https://github.com/apache/doris/pull/67523#discussion_r3933492678
##########
thirdparty/build-thirdparty.sh:
##########
@@ -45,6 +45,17 @@ if [[ -f "${DORIS_HOME}/env.sh" ]]; then
export DO_NOT_CHECK_JAVA_ENV=
fi
+# Do not let ambient CMake injection hooks or package-manager environments
+# alter third-party dependency resolution. Keep this after env.sh so custom
+# environment setup cannot reintroduce these values.
+unset CMAKE_TOOLCHAIN_FILE \
+ CMAKE_PROJECT_INCLUDE \
+ CMAKE_PROJECT_INCLUDE_BEFORE \
+ CMAKE_PROJECT_TOP_LEVEL_INCLUDES \
+ VCPKG_ROOT \
Review Comment:
[P1] Clear Azure's effective vcpkg override variables — This does not clear
the environment variables that actually control vcpkg on the default Azure
build. The pinned `azure-core_1.16.0` calls `az_vcpkg_integrate()` before
`project()`: `AZURE_SDK_DISABLE_AUTO_VCPKG` makes it skip the managed vcpkg
toolchain and fall back to caller/system resolution, while
`AZURE_SDK_VCPKG_COMMIT` replaces the tested vcpkg revision. Both still survive
`env.sh` and reach `build_azure`, so the new isolation guarantee is bypassable.
Please unset these Azure-specific variables too (or pass a Doris-owned pinned
toolchain/revision explicitly), and cover the poisoned-environment case.
##########
thirdparty/build-thirdparty.sh:
##########
@@ -45,6 +45,17 @@ if [[ -f "${DORIS_HOME}/env.sh" ]]; then
export DO_NOT_CHECK_JAVA_ENV=
fi
+# Do not let ambient CMake injection hooks or package-manager environments
+# alter third-party dependency resolution. Keep this after env.sh so custom
+# environment setup cannot reintroduce these values.
+unset CMAKE_TOOLCHAIN_FILE \
Review Comment:
[P1] Clear cached toolchains on sanitized retries — Unsetting this
environment variable only protects a fresh CMake build tree. `build_arrow`
reuses `release/` (`mkdir -p` without removing it), and CMake persists an
initially injected `CMAKE_TOOLCHAIN_FILE` in `CMakeCache.txt`. If an Arrow
configure/build fails, `set -e` exits before source cleanup; the next sanitized
retry or `--continue arrow` reconfigures the same cache and still loads that
toolchain. Please recreate Arrow's build directory (as the other package
builders do) or explicitly invalidate the cached injection variables, and test
a poisoned failed-run/retry case.
##########
thirdparty/build-thirdparty.sh:
##########
@@ -2093,8 +2104,22 @@ build_lance_c() {
echo "failed to get cargo version for lance-c. Install Rust
${required_rust_version} or set LANCE_C_CARGO/RUSTUP_TOOLCHAIN."
exit 1
fi
- if [[ "${cargo_version}" != "${required_rust_version}" ]]; then
- echo "lance-c requires Rust/Cargo ${required_rust_version}, but found
${cargo_version}."
+ # Rust 1.91.0 is the minimum supported version. Allow newer toolchains when
Review Comment:
[P1] Honor explicit Cargo before bootstrapping rustup — The new comparison
allows an explicitly selected Cargo newer than 1.91, but this path still runs
the rustup block above first. If `LANCE_C_CARGO` (or `CARGO`) points to an
already-installed newer toolchain while rustup is on `PATH` and
`RUSTUP_TOOLCHAIN` is unset, the script still tries to install 1.91.0 and can
fail offline before it ever checks the selected Cargo; if it succeeds, it also
forces `RUSTUP_TOOLCHAIN=1.91.0` for that Cargo. Please skip the automatic
install/pin when an explicit Cargo override is present and validate that
executable directly, reserving the 1.91 bootstrap for the default Cargo path.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
