[VL] Apply patch to Velox for compilation #3520
Conversation
|
Thanks for opening a pull request! Could you open an issue for this pull request on Github Issues? https://github.com/oap-project/gluten/issues Then could you also rename commit message and pull request title in the following format? See also: |
ep/build-velox/src/build_velox.sh
Outdated
| sudo mkdir -p ${velox_home}/third_party/arrow_patches/ | ||
| sudo cp ${current_dir}/helpers.patch ${velox_home}/third_party/arrow_patches/ | ||
| cd ${velox_home} | ||
| git apply compilation.patch |
There was a problem hiding this comment.
If git apply fails, will the build script fail or continue the build even though the patch is not applied? Could you confirm it? Thanks!
There was a problem hiding this comment.
If it fails, I suppose whether to continue or to stop, the build process will fail anyway. How do you think?
There was a problem hiding this comment.
not actually, as it may build the original code?
There was a problem hiding this comment.
I think we can do a check after executing git apply. And leave some message to help developers easily figure out why build failed.
Reference: https://askubuntu.com/questions/29370/how-to-check-if-a-command-succeeded
rui-mo
force-pushed
the
wip_apply
branch
3 times, most recently
from
e02288a to
c6b06fd
Compare
| cd ${velox_home} | ||
| git apply compilation.patch | ||
| if [ $? -ne 0 ]; then | ||
| echo "Failed to apply compilation fixes to Velox: $?." |
There was a problem hiding this comment.
Please add exit 1 to directly make the build exit. Suppose it is not acceptable to continue the build without the patch applied.
rui-mo
force-pushed
the
wip_apply
branch
2 times, most recently
from
da67950 to
622277a
Compare
| current_dir=$1 | ||
| velox_home=$2 | ||
| sudo cp ${current_dir}/modify_velox.patch ${velox_home}/ | ||
| sudo cp ${current_dir}/modify_arrow.patch ${velox_home}/third_party/ |
There was a problem hiding this comment.
@rui-mo Do we need to git apply modify_arrow.patch like modify_velox.patch in line 74?
There was a problem hiding this comment.
It is applied to Arrow during compilation: https://github.com/oap-project/gluten/pull/3520/files#diff-b41402fae332ead78c4646843c9bcf66a2813203cdc82521c9c9f130b48146dbR100.
|
===== Performance report for TPCH SF2000 with Velox backend, for reference only ====
|
What changes were proposed in this pull request?
Removes build script changes to Velox by applying patch from Gluten to Velox.
oap-project/velox#425
How was this patch tested?
under test