GH-36379: [C++] Bundled dependency include paths should override system include dirs

[bkietz]

Rationale for this change

Bundled dependencies' include directories should override system include dirs. Otherwise an incompatible header in the system might be included when we wanted a header from the bundled dependency.

What changes are included in this PR?

bundled dependencies explicitly insert their own include dirs ahead of others

• Closes: [C++] Bundled GTest CI jobs seem to #includeing GTest headers from somewhere else #36379

[github-actions]

⚠️ GitHub issue #36379has been automatically assigned in GitHub to PR creator.

[pitrou]

@github-actions crossbow submit -g cpp

[github-actions]

Revision: 7073a98

Submitted crossbow builds: ursacomputing/crossbow @ actions-1750e0d372

Task	Status
test-alpine-linux-cpp
test-build-cpp-fuzz
test-conda-cpp
test-conda-cpp-valgrind
test-cuda-cpp
test-debian-11-cpp-amd64
test-debian-11-cpp-i386
test-fedora-35-cpp
test-ubuntu-20.04-cpp
test-ubuntu-20.04-cpp-bundled
test-ubuntu-20.04-cpp-minimal-with-formats
test-ubuntu-20.04-cpp-thread-sanitizer
test-ubuntu-22.04-cpp
test-ubuntu-22.04-cpp-20
test-ubuntu-22.04-cpp-no-threading

[kou]

+1

We need to run cmake-format before we merge this.

[assignUser]

This change makes sense imo, a more involved (and brittle) alternative might be to move gtest to the front in target_link_libraries calls. I tried to reproduce the error but failed, where you able to test the fix with the error locally @bkietz ?

[pitrou]

Should we try to revert #37483 in this PR?

[kou]

Should we try to revert #37483 in this PR?

Is it just for confirming whether this fixes the #36379 problem or not? Do we re-revert #37483 after we confirm it?

#37483 is needed to fix #37067. So we should not revert #37483 from main.

[pitrou]

Ah, I thought the two approaches were alternatives. If not then nevermind.

[bkietz]

FetchContent_MakeAvailable automatically adds include directories and (at least while I was testing this) seems to prepend the -I flags by default. If I'm correct, then #37483 actually resolved this issue for bundled GTest (but the other bundled dependencies are still vulnerable).

[kou]

the other bundled dependencies are still vulnerable

I think so too.
So we need both of #37483 and this.

I merge this.

[conbench-apache-arrow]

After merging your PR, Conbench analyzed the 6 benchmarking runs that have been run so far on merge-commit 44811ba.

There were 5 benchmark results indicating a performance regression:

• Commit Run on ursa-i9-9960x at 2023-09-08 22:41:44Z
  • tpch (R) with engine=arrow, format=native, language=R, memory_map=False, query_id=TPCH-06, scale_factor=1
  • tpch (R) with engine=arrow, format=parquet, language=R, memory_map=False, query_id=TPCH-08, scale_factor=1
• and 3 more (see the report linked below)

The full Conbench report has more details. It also includes information about possible false positives for unstable benchmarks that are known to sometimes produce them.