ARROW-5638: [C++][CMake] Fixes for xcode project builds

[hatemhelal]

This patch fixes the following build failures with Xcode projects generated with the -G Xcode option for Cmake:

• Link failure with google test
• Cmake error with ARROW_GANDIVA_JAVA
• Build failure for lz4_ep

This patch effectively disables the ability to do multi-configuration builds with the generated Xcode project and makes them behave more like a single-configuration Makefile driven build. I see no problem with this as supporting it would add a lot of extra complexity to the rest of the build system and one can always generate multiple projects with different configurations as needed.

[hatemhelal]

@pprudhvi, could you try this out?

[pprudhvi]

Yes, I am able to create the xcodeproj file now. Thanks.
Would be nice if we can build/run from Xcode.

[hatemhelal]

Yes, I am able to create the xcodeproj file now. Thanks.

Great!

Would be nice if we can build/run from Xcode.

I'm not sure I follow, my workflow with Xcode is to build, run, debug, etc different arrow test executables. I mostly work with the parquet component targets so I'm not sure if I'm missing something specific to your workflow?

[pprudhvi]

some *_ep modules fail to build when I run from Xcode

[hatemhelal]

A workaround is to use brew installed dependencies. I can take a look with BUNDLED dependency sources to see if I can make sense of them while this is still fresh in my head.

[pprudhvi]

Could you tell me how to use installed dependencies and disable building bundled ep's?

[hatemhelal]

Sure I think I just did this:

git clone https://github.com/apache/arrow.git
cd arrow
brew update && brew bundle --file=cpp/Brewfile

(copied from here)

Then with the default dependency source ("AUTO") it should find the system installed versions.

[hatemhelal]

I see that the lz4_ep fails to build in the Xcode project with the following syndrome:

...
compiling static library
compiling dynamic library 1.8.3
ld: unknown option: -soname=liblz4.so.1
clang: error: linker command failed with exit code 1 (use -v to see invocation)
make[2]: *** [liblz4.so.1.8.3] Error 1
make[1]: *** [lib-release] Error 2

Which won't ever work on macOS...

[hatemhelal]

I think I found the problem with the lz4_ep. The makefile for building the shared library checks for the OS like this (see lz4/lib/Makefile)

OS ?= $(shell uname)

which I think is necessary to support cross-compiling. The test for OS == "Darwin" fails since the Xcode driven build appears to have OS set to MACOS so we could patch the above line to force invoking uname to correctly infer that we really are on a Darwin machine:

OS := $(shell uname)

[kou]

Could you use list(APPEND GTEST_CMAKE_ARGS ...)?

[hatemhelal]

Good tip, thanks @kou for reviewing these changes!

[kou]

I think that we can always set these variables.

We should use UPPERCASE_BUILD_TYPE instead of CMAKE_BUILD_TYPE because users may specify Debug as CMAKE_BUILD_TYPE.

[hatemhelal]

I think that we can always set these variables.

I'm a bit worried about settings these variables unconditionally and possibly breaking multi-configuration generators (MSVC?).

[kou]

AppVeyor build will answer this... Oh, we don't have a job that uses "Visual Studio XXX" generator.

OK. We can set them only for Xcode for now.

[kou]

+1
We can merge this with a small fix. Could you see my comments for details?

[kou]

Could you put this to https://github.com/apache/arrow/blob/master/cpp/CMakeLists.txt#L30 and remove https://github.com/apache/arrow/blob/master/cpp/cmake_modules/ThirdpartyToolchain.cmake#L420 ?

[kou]

AppVeyor build will answer this... Oh, we don't have a job that uses "Visual Studio XXX" generator.

OK. We can set them only for Xcode for now.

[kou]

And could you rebase on master to resolve macOS CI failures?

[hatemhelal]

@kou, I've incorporated your suggestions and rebased on master. Thanks for your help on this.

[codecov-io]

Codecov Report

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

@@            Coverage Diff             @@
##           master    #5046      +/-   ##
==========================================
+ Coverage   87.51%   89.23%   +1.71%     
==========================================
  Files         912      731     -181     
  Lines      139034   104535   -34499     
==========================================
- Hits       121679    93277   -28402     
+ Misses      17003    11258    -5745     
+ Partials      352        0     -352

Impacted Files	Coverage Δ
cpp/src/plasma/thirdparty/ae/ae.c	70.75% <0%> (-0.95%)	⬇️
go/arrow/math/uint64_amd64.go
go/arrow/memory/memory_avx2_amd64.go
rust/datafusion/src/execution/filter.rs
rust/arrow/src/csv/writer.rs
rust/datafusion/src/bin/main.rs
go/arrow/ipc/file_reader.go
rust/parquet/src/arrow/converter.rs
go/arrow/array/builder.go
go/arrow/type_traits_boolean.go
... and 173 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 3420d30...b9417f7. Read the comment docs.

[kou]

+1
Thanks! I'll merge this.