Copilot commented on code in PR #12321:
URL: https://github.com/apache/gluten/pull/12321#discussion_r3617060470
##########
ep/build-velox/src/setup-openeuler24.sh:
##########
@@ -143,11 +156,14 @@ function install_folly {
function install_wangle {
wget_and_untar
https://github.com/facebook/wangle/archive/refs/tags/${FB_OS_VERSION}.tar.gz
wangle
- cmake_install_dir wangle/wangle -DBUILD_TESTS=OFF
+ cmake_install_dir wangle/wangle -DBUILD_TESTS=OFF -DBUILD_EXAMPLES=OFF
}
function install_fbthrift {
wget_and_untar
https://github.com/facebook/fbthrift/archive/refs/tags/${FB_OS_VERSION}.tar.gz
fbthrift
+ cd ${DEPENDENCY_DIR}/fbthrift
+ git apply
${VELOX_HOME:-${DEPENDENCY_DIR}/../}/CMake/resolve_dependency_modules/fbthrift/compactv1-protocol-refiller.patch
2>/dev/null || true
+ cd ${DEPENDENCY_DIR}
cmake_install_dir fbthrift -Denable_tests=OFF -DBUILD_TESTS=OFF
-DBUILD_SHARED_LIBS=OFF
Review Comment:
`install_fbthrift` suppresses all output and ignores failures from `git
apply ... || true`. If the patch path is missing or the patch no longer
applies, the script will continue and fail later in harder-to-debug ways.
Prefer an idempotent apply that (a) skips when already applied and (b) fails
loudly when it can’t be applied.
##########
ep/build-velox/src/setup-centos8.sh:
##########
@@ -172,7 +173,10 @@ function install_wangle {
function install_fbthrift {
wget_and_untar
https://github.com/facebook/fbthrift/archive/refs/tags/${FB_OS_VERSION}.tar.gz
fbthrift
- cmake_install_dir fbthrift -Denable_tests=OFF -DBUILD_SHARED_LIBS=OFF
-DBUILD_TESTS=OFF -DBUILD_SHARED_LIBS=OFF
+ cd ${DEPENDENCY_DIR}/fbthrift
+ git apply
${VELOX_HOME:-${DEPENDENCY_DIR}/../}/CMake/resolve_dependency_modules/fbthrift/compactv1-protocol-refiller.patch
2>/dev/null || true
+ cd ${DEPENDENCY_DIR}
+ cmake_install_dir fbthrift -Denable_tests=OFF -DBUILD_TESTS=OFF
-DBUILD_SHARED_LIBS=OFF
Review Comment:
This applies `compactv1-protocol-refiller.patch` with `2>/dev/null || true`,
which hides real patch failures (missing file, conflicts, etc.) and makes later
build errors much harder to diagnose. Consider an idempotent apply that skips
when already applied but fails loudly otherwise; also use a subshell to avoid
leaking `cd` state.
--
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]
