-
Notifications
You must be signed in to change notification settings - Fork 447
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[VL] Set Arrow_SOURCE to AUTO to allow using system arrow libs #6325
Conversation
Thanks for opening a pull request! Could you open an issue for this pull request on Github Issues? https://github.com/apache/incubator-gluten/issues Then could you also rename commit message and pull request title in the following format?
See also: |
if use system arrow, seems gluten still use |
@Yohahaha, thanks for your comment! I just removed the setting for arrow include directories, as they are actually not needed in Gluten. |
this line should also be removed. |
@Yohahaha, yes, just removed. Thanks! |
@Yohahaha, the corresponding velox patch has been merged. Do you have any other comment on this gluten PR? |
If arrow has installed in other places, seems we still set ARROW_HOME under velox path, could you clarify?
|
mayby it's not an issue, |
@Yohahaha, thanks for your comment! The arrow home is set to let cmake firstly find arrow libs from that path. If it does not exist (because velox finds the lib from system, then it doesn't build arrow from source), cmake will find the libs from system. Though we always expect cmake in Gluten/Velox finds the libs from system now, it may be good to still keep the arrow home path for debugging (e.g., by |
apache#6325)" This reverts commit b93f254.
apache#6325)" This reverts commit b93f254.
apache#6325)" This reverts commit b93f254.
apache#6325)" This reverts commit b93f254.
apache#6325)" This reverts commit b93f254.
What changes were proposed in this pull request?
Depends on the fix in facebookincubator/velox#10355.
How was this patch tested?
CI.