PHILO-HE commented on code in PR #10650:
URL:
https://github.com/apache/incubator-gluten/pull/10650#discussion_r2339121547
##########
dev/builddeps-veloxbe.sh:
##########
@@ -159,9 +159,9 @@ do
done
if [[ "$(uname)" == "Darwin" ]]; then
- INSTALL_PREFIX=${INSTALL_PREFIX:-${VELOX_HOME}/deps-install}
+ export INSTALL_PREFIX=${INSTALL_PREFIX:-${VELOX_HOME}/deps-install}
else
- INSTALL_PREFIX=${INSTALL_PREFIX:-"/usr/local"}
+ export INSTALL_PREFIX=${INSTALL_PREFIX:-"/usr/local"}
Review Comment:
With this line, if `INSTALL_PREFIX` is not explicitly set in env, Velox will
use "/user/local" to set `CMAKE_PREFIX_PATH`, which is unnecessary since
"/user/local" is already CMake's default searching path. This setting may
somehow cause failures in finding the installed thrift and arrow on Centos-8,
as shown in CI.
I think we can remove line 164 and only keep setting the env variable
`INSTALL_PREFIX` for MacOS, since a non-default path is used.
If users explicitly set env variable `INSTALL_PREFIX` regardless of MacOS or
others, I assume their setting is respected by build_arrow.sh and Velox build.
--
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]