ARROW-3934: [Gandiva] Only add precompiled tests if ARROW_GANDIVA_BUILD_TESTS

[pcmoritz]

No description provided.

[wesm]

Hm, shouldn't this just be if (ARROW_BUILD_TESTS)? I think we should remove the separate flag

[pcmoritz]

That's fine with me, I'll change the PR accordingly.

[wesm]

Sounds good. I think it would be better to use custom targets to control what subcomponents get built (subject to some global options like tests on/off). So we could easily add a custom target so we can write ninja gandiva like we can currently write ninja parquet (or make parquet) and only build the dependencies attached to that target

[pcmoritz]

Oh, now I remember why it is done this way: In the Python build matrix entry (the longest one on travis), we build only the arrow tests (for code coverage?) and not the gandiva tests (if we did, the matrix entry would time out). Do you have any suggestion how to handle this? I think it's the only matrix entry where the python gandiva cython wrappers can be reasonably tested.

[pcmoritz]

With your suggestion the idea would be to build twice, one with make (tests on) and once with make gandiva (tests off)? Not sure what the impact on build time is in that case...

[wesm]

Oh right. Let's leave the flag in your patch like it is. We need to https://issues.apache.org/jira/browse/ARROW-3803 and there we can deal with the CMake flags

[pcmoritz]

SGTM!

[wesm]

+1