ARROW-3331: [Gandiva][C++] Add re2 to toolchain #2695

pravindra · 2018-10-03T16:38:59Z

No description provided.

codecov-io · 2018-10-03T17:47:36Z

Codecov Report

Merging #2695 into master will increase coverage by 1.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #2695      +/-   ##
==========================================
+ Coverage    87.5%   88.51%   +1.01%     
==========================================
  Files         402      341      -61     
  Lines       61401    57596    -3805     
==========================================
- Hits        53728    50981    -2747     
+ Misses       7603     6615     -988     
+ Partials       70        0      -70

Impacted Files	Coverage Δ
rust/src/record_batch.rs
go/arrow/datatype_nested.go
rust/src/util/bit_util.rs
go/arrow/math/uint64_amd64.go
go/arrow/internal/testing/tools/bool.go
go/arrow/internal/bitutil/bitutil.go
go/arrow/memory/memory_avx2_amd64.go
go/arrow/array/null.go
rust/src/lib.rs
rust/src/array.rs
... and 51 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f16148f...4408b85. Read the comment docs.

wesm

Minor change re the $ARROW_BUILD_TOOLCHAIN which is used in Travis CI and Python builds. I will test this out locally, add that, and then merge

wesm · 2018-10-04T08:59:54Z

cpp/cmake_modules/FindRE2.cmake

@@ -22,48 +22,57 @@
 # This module defines
 #  RE2_INCLUDE_DIR, directory containing headers
 #  RE2_STATIC_LIB, path to libre2.a
-#  re2 imported static library
 #  RE2_FOUND, whether re2 has been found


This may or may not work on Windows. We're working on this in conda-forge/re2-feedstock#15 -- I'll add a follow up JIRA to validate this on Windows https://issues.apache.org/jira/browse/ARROW-3433

wesm · 2018-10-04T09:02:18Z

cpp/cmake_modules/ThirdpartyToolchain.cmake

+# RE2 (required for Gandiva)
+if (ARROW_GANDIVA)
+  # re2
+  if ("${RE2_HOME}" STREQUAL "")


This also needs to be added to the $ARROW_BUILD_TOOLCHAIN part at the top of the file. I will add and push here

This was already there. This is failing for me for another reason so I'll push a fix

wesm · 2018-10-04T09:19:39Z

This appears to have a race condition, encountered when running make -j8. Looking into it

[ 21%] Building CXX object src/gandiva/CMakeFiles/gandiva_obj_lib.dir/bitmap_accumulator.cc.o
[ 22%] Building CXX object src/gandiva/CMakeFiles/gandiva_obj_lib.dir/configuration.cc.o
[ 22%] Building CXX object src/gandiva/CMakeFiles/gandiva_obj_lib.dir/engine.cc.o
[ 22%] Building CXX object src/gandiva/CMakeFiles/gandiva_obj_lib.dir/expr_decomposer.cc.o
In file included from /home/wesm/code/arrow/cpp/src/gandiva/function_holder_registry.h:26:0,
                 from /home/wesm/code/arrow/cpp/src/gandiva/expr_decomposer.cc:27:
/home/wesm/code/arrow/cpp/src/gandiva/like_holder.h:24:21: fatal error: re2/re2.h: No such file or directory
 #include <re2/re2.h>
                     ^
compilation terminated.
-- glog_ep configure command succeeded.  See also /home/wesm/code/arrow/cpp/fresh-build/glog_ep-prefix/src/glog_ep-stamp/glog_ep-configure-*.log

wesm · 2018-10-04T09:30:54Z

Still some more dependency issues:

$ make -j8 epoch_time_point_test
Scanning dependencies of target epoch_time_point_test
make[3]: *** No rule to make target `googletest_ep-prefix/src/googletest_ep/lib/libgtest.a', needed by `debug/epoch_time_point_test'.  Stop.
make[3]: *** Waiting for unfinished jobs....
Building CXX object src/gandiva/precompiled/CMakeFiles/epoch_time_point_test.dir/epoch_time_point_test.cc.o
/home/wesm/code/arrow/cpp/src/gandiva/precompiled/epoch_time_point_test.cc:20:25: fatal error: gtest/gtest.h: No such file or directory
 #include <gtest/gtest.h>
                         ^
compilation terminated.
make[3]: *** [src/gandiva/precompiled/CMakeFiles/epoch_time_point_test.dir/epoch_time_point_test.cc.o] Error 1
make[2]: *** [src/gandiva/precompiled/CMakeFiles/epoch_time_point_test.dir/all] Error 2
make[1]: *** [src/gandiva/precompiled/CMakeFiles/epoch_time_point_test.dir/rule] Error 2
make: *** [epoch_time_point_test] Error 2

$ make -j8 expression_registry_test
<SNIP>
Scanning dependencies of target arrow_shared
[ 94%] Linking CXX shared library ../../debug/libarrow.so
[ 94%] Built target arrow_shared
Scanning dependencies of target expression_registry_test
[ 97%] Building CXX object src/gandiva/CMakeFiles/expression_registry_test.dir/llvm_types.cc.o
[ 97%] Building CXX object src/gandiva/CMakeFiles/expression_registry_test.dir/expression_registry_test.cc.o
make[3]: *** No rule to make target `googletest_ep-prefix/src/googletest_ep/lib/libgtest.a', needed by `debug/expression_registry_test'.  Stop.
make[3]: *** Waiting for unfinished jobs....
[ 97%] Building CXX object src/gandiva/CMakeFiles/expression_registry_test.dir/expression_registry.cc.o
[ 97%] Building CXX object src/gandiva/CMakeFiles/expression_registry_test.dir/function_signature.cc.o
[100%] Building CXX object src/gandiva/CMakeFiles/expression_registry_test.dir/function_registry.cc.o
/home/wesm/code/arrow/cpp/src/gandiva/expression_registry_test.cc:23:25: fatal error: gtest/gtest.h: No such file or directory
 #include <gtest/gtest.h>
                         ^
compilation terminated.
make[3]: *** [src/gandiva/CMakeFiles/expression_registry_test.dir/expression_registry_test.cc.o] Error 1
make[2]: *** [src/gandiva/CMakeFiles/expression_registry_test.dir/all] Error 2
make[1]: *** [src/gandiva/CMakeFiles/expression_registry_test.dir/rule] Error 2
make: *** [expression_registry_test] Error 2

Ideally we should replace all usages of add_gandiva_unit_test with ADD_ARROW_TEST as it handles the toolchain dependencies etc.

wesm · 2018-10-04T09:38:43Z

This looks good new. Let me know if my changes are OK and then I'll merge. I'm getting a weird error building on Ubuntu 14.04 that's unrelated to this:

[ 90%] Linking CXX executable ../../../debug/hash_test_gandiva_static
/usr/lib/llvm-6.0/lib/libLLVMX86CodeGen.a(X86FloatingPoint.cpp.o): In function `(anonymous namespace)::FPS::runOnMachineFunction(llvm::MachineFunction&) [clone .part.165]':
(.text._ZN12_GLOBAL__N_13FPS20runOnMachineFunctionERN4llvm15MachineFunctionE.part.165+0x481): undefined reference to `std::__throw_out_of_range_fmt(char const*, ...)'
/usr/lib/llvm-6.0/lib/libLLVMX86CodeGen.a(X86InstructionSelector.cpp.o): In function `llvm::PredicateBitsetImpl<107ul>::PredicateBitsetImpl(std::initializer_list<unsigned int>)':
(.text._ZN4llvm19PredicateBitsetImplILm107EEC2ESt16initializer_listIjE[_ZN4llvm19PredicateBitsetImplILm107EEC5ESt16initializer_listIjE]+0x75): undefined reference to `std::__throw_out_of_range_fmt(char const*, ...)'
/usr/lib/llvm-6.0/lib/libLLVMAsmPrinter.a(CodeViewDebug.cpp.o): In function `llvm::CodeViewDebug::getFullFilepath(llvm::DIFile const*)':
(.text._ZN4llvm13CodeViewDebug15getFullFilepathEPKNS_6DIFileE+0x452): undefined reference to `std::__throw_out_of_range_fmt(char const*, ...)'
/usr/lib/llvm-6.0/lib/libLLVMAsmPrinter.a(CodeViewDebug.cpp.o): In function `llvm::CodeViewDebug::getFullFilepath(llvm::DIFile const*)':
(.text._ZN4llvm13CodeViewDebug15getFullFilepathEPKNS_6DIFileE+0x46a): undefined reference to `std::__throw_out_of_range_fmt(char const*, ...)'
/usr/lib/llvm-6.0/lib/libLLVMCodeGen.a(TargetLoweringBase.cpp.o): In function `getOpEnabled(bool, llvm::EVT, llvm::StringRef)':
(.text._ZL12getOpEnabledbN4llvm3EVTENS_9StringRefE+0x3f3): undefined reference to `std::__throw_out_of_range_fmt(char const*, ...)'
/usr/lib/llvm-6.0/lib/libLLVMCodeGen.a(TargetLoweringBase.cpp.o):(.text._ZL20getOpRefinementStepsbN4llvm3EVTENS_9StringRefE+0x367): more undefined references to `std::__throw_out_of_range_fmt(char const*, ...)' follow
collect2: error: ld returned 1 exit status
make[2]: *** [debug/hash_test_gandiva_static] Error 1
make[1]: *** [src/gandiva/tests/CMakeFiles/hash_test_gandiva_static.dir/all] Error 2
make: *** [all] Error 2

wesm · 2018-10-04T09:42:25Z

I think the reason I'm getting that error is because LLVM is compiled with a newer libstdc++ (for LLVM 6 this is required on Ubuntu 14.04), but since I'm compiling with gcc 4.8.x the flags -static-libstdc++ -static-libgcc are causing a conflict

pravindra · 2018-10-04T09:51:01Z

@wesm Your changes look good. I ran on ubuntu 14.04 with gcc-4.9 and so, didn't see the LLVM linking error.

wesm · 2018-10-04T09:56:11Z

OK. I think the static-libgcc static-libstdc++ stuff should be handled by LDFLAGS rather than hardcoded. I can open a JIRA about it?

pravindra · 2018-10-04T09:58:36Z

OK. I think the static-libgcc static-libstdc++ stuff should be handled by LDFLAGS rather than hardcoded. I can open a JIRA about it?

sure, pls do.

wesm · 2018-10-04T10:01:24Z

Cool. https://issues.apache.org/jira/browse/ARROW-3437. I'll rebase this patch and then merge it

Author: Pindikura Ravindra <ravindra@dremio.com> Closes #2695 from pravindra/re2 and squashes the following commits: 4408b85 <Pindikura Ravindra> ARROW-3331: Add re2 to toolchain

wesm approved these changes Oct 4, 2018

View reviewed changes

ARROW-3331: [Gandiva][C++] Add re2 to toolchain

4408b85

wesm closed this in 5284ad0 Oct 4, 2018

wesm mentioned this pull request Nov 26, 2018

ARROW-3860: [WIP][Gandiva][C++] Link std c++ statically. #3020

Closed

asfimport mentioned this pull request Oct 8, 2018

[C++] Add re2 to ThirdpartyToolchain #19666

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ARROW-3331: [Gandiva][C++] Add re2 to toolchain #2695

ARROW-3331: [Gandiva][C++] Add re2 to toolchain #2695

pravindra commented Oct 3, 2018

codecov-io commented Oct 3, 2018 •

edited

Loading

wesm left a comment

wesm Oct 4, 2018

wesm Oct 4, 2018

wesm Oct 4, 2018

wesm commented Oct 4, 2018

wesm commented Oct 4, 2018

wesm commented Oct 4, 2018

wesm commented Oct 4, 2018

pravindra commented Oct 4, 2018

wesm commented Oct 4, 2018

pravindra commented Oct 4, 2018

wesm commented Oct 4, 2018

ARROW-3331: [Gandiva][C++] Add re2 to toolchain #2695

ARROW-3331: [Gandiva][C++] Add re2 to toolchain #2695

Conversation

pravindra commented Oct 3, 2018

codecov-io commented Oct 3, 2018 • edited Loading

Codecov Report

wesm left a comment

Choose a reason for hiding this comment

wesm Oct 4, 2018

Choose a reason for hiding this comment

wesm Oct 4, 2018

Choose a reason for hiding this comment

wesm Oct 4, 2018

Choose a reason for hiding this comment

wesm commented Oct 4, 2018

wesm commented Oct 4, 2018

wesm commented Oct 4, 2018

wesm commented Oct 4, 2018

pravindra commented Oct 4, 2018

wesm commented Oct 4, 2018

pravindra commented Oct 4, 2018

wesm commented Oct 4, 2018

codecov-io commented Oct 3, 2018 •

edited

Loading