ARROW-4581: [C++] Do not require googletest_ep or gbenchmark_ep for library targets

[wesm]

No description provided.

[xhochy]

+1, LGTM

[pitrou]

I'm not sure I understand. Shouldn't toolchain-benchmarks and toolchain-tests be used as dependencies of benchmarks and tests (respectively) somewhere?

IOW, currently toolchain is a dependency of arrow_dependencies. We probably want to create additional arrow_benchmark_dependencies and arrow_test_dependencies targets, and declare them as explicit dependencies of benchmarks and unit tests (and of the arrow_testing library).

[wesm]

Well, the benchmarks depend on the benchmark library being available, which has a dependency on the gbenchmark_ep. The arrow_dependencies thing is IIRC a kludge to force the toolchain to build before any of the Arrow library code. In theory this should all be handled by the dependency graph of linker dependencies. We should try to improve things later but I'll put in explicit dependencies for the benchmarks and unit tests for now

[wesm]

OK, so here is the problem.

We are setting dependencies on $LIBRARY_objlib, while the link dependencies are dependencies of the shared or static library targets. So when you ninja arrow_shared, without the explicit dependency it thinks it can build the objlib independent from the toolchain.

I think this is fixable but we are passing some link targets (like "dl" that cannot be used as dependencies), so I'll proceed with the existing kludge and we can try to improve later (not really a priority since things are working fine now)

[codecov-io]

Codecov Report

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

@@            Coverage Diff             @@
##           master    #3698      +/-   ##
==========================================
+ Coverage   87.59%   88.64%   +1.04%     
==========================================
  Files         617      559      -58     
  Lines       79313    75283    -4030     
==========================================
- Hits        69475    66731    -2744     
+ Misses       9729     8552    -1177     
+ Partials      109        0     -109

Impacted Files	Coverage Δ
cpp/src/arrow/vendored/xxhash/xxhash.c	73.6% <0%> (-0.51%)	⬇️
go/arrow/array/table.go
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
go/arrow/datatype_nested.go
go/arrow/array/string.go
go/arrow/math/uint64_avx2_amd64.go
... 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 ef28f20...1fde0b0. Read the comment docs.