[C++] Comments in FindArrow.cmake misleading

[asfimport]

When you try to find out how to use Arrow from a third-party project, you may read the comments in FindArrow.cmake:

# This module defines
#  ARROW_FOUND, whether Arrow has been found
#  ARROW_FULL_SO_VERSION, full shared object version of found Arrow "100.0.0"
#  ARROW_IMPORT_LIB, path to libarrow's import library (Windows only)
#  ARROW_INCLUDE_DIR, directory containing headers
#  ARROW_LIBS, deprecated. Use ARROW_LIB_DIR instead
#  ARROW_LIB_DIR, directory containing Arrow libraries
#  ARROW_SHARED_IMP_LIB, deprecated. Use ARROW_IMPORT_LIB instead
#  ARROW_SHARED_LIB, path to libarrow's shared library
#  ARROW_SO_VERSION, shared object version of found Arrow such as "100"
#  ARROW_STATIC_LIB, path to libarrow.a
#  ARROW_VERSION, version of found Arrow
#  ARROW_VERSION_MAJOR, major version of found Arrow
#  ARROW_VERSION_MINOR, minor version of found Arrow
#  ARROW_VERSION_PATCH, patch version of found Arrow

... only to find out that find_package(Arrow) doesn't set most of those variables:

• it sets Arrow_FOUND, not ARROW_FOUND

• ARROW_INCLUDE_DIR, ARROW_SHARED_LIB, ARROW_STATIC_LIB are not set

• to link with Arrow C++, you have to link with either arrow_shared or arrow_static

  Also, I wonder if Arrow should follow the more recent convention of naming its targets e.g. Arrow::Arrow or arrow::arrow (or specifically arrow::shared).

Reporter: Antoine Pitrou / @pitrou
Assignee: Kouhei Sutou / @kou

Related issues:

• [C++] CMake's find_package(Parquet) does not find Parquet with Arrow 3.0.0 (is related to)
• [C++] CMake's find_package(Parquet) does not find Parquet with Arrow 3.0.0 (is superceded by)

_{Note: This issue was originally created as ARROW-9171. Please see the migration documentation for further details.}

[asfimport]

Antoine Pitrou / @pitrou:
cc @kou @xhochy

[asfimport]

Kouhei Sutou / @kou:
I agree that we need to update the comments.

For ARROW_INCLUDE_DIR, it must be set. If it isn't set, find_package(Arrow) must be failed because we specify it as a variable of REQUIRED_VARS for find_package_handle_standard_args: https://github.com/apache/arrow/blob/master/cpp/cmake_modules/FindArrow.cmake#L416

For target names, I like arrow::arrow if we can unify arrow::arrow_shared and arrow::arrow_static. If we can't unify them, I like arrow::arrow_shared and arrow::arrow_static.

[asfimport]

Kouhei Sutou / @kou:
We may should rename ARROW_* to Arrow_* to follow CMake convention.

[asfimport]

Wes McKinney / @wesm:
Do we want to change anything for 1.0.0?

[asfimport]

Kouhei Sutou / @kou:
I think that we should work on this for 1.0.0 because it may break backward compatibility.
But I will not have enough time to work on this for 1.0.0...

[asfimport]

Wes McKinney / @wesm:
Okay, @pitrou or @xhochy what do you think?

[asfimport]

Antoine Pitrou / @pitrou:
Well, the comments at least should be fixed. I can try to do it, though I fear only Kou and Uwe really understand what this all does ;-)

[asfimport]

Uwe Korn / @xhochy:
I can take care of this. I agree that we should change to Arrow case and. Also we should change the targets to Arrow::Arrow, Arrow::Compute,...

[asfimport]

Wes McKinney / @wesm:
@xhochy do you think this is still possible for 1.0.0?

[asfimport]

Wes McKinney / @wesm:
It seems this change may not make 1.0.0. I moved out of the milestone for now

[asfimport]

Uwe Korn / @xhochy:
I would be able to squeeze this in but I have the feeling that this might break some downstream users again and thus lead to delays in the release process, so delaying it shortly after the release.

[asfimport]

Krisztian Szucs / @kszucs:
Postponing to 7.0

[asfimport]

Antoine Pitrou / @pitrou:
@kou It would probably be worthwhile cleaning up our own FindXXX files and making them easier to use/understand.

(perhaps even switch to more modern CMake conventions if they exist?)

[asfimport]

Kouhei Sutou / @kou:
I think so too. :-)
I want to do this in ARROW-12175 but I didn't have time to do this.
But I will have time to do this for 9.0.0.

[asfimport]

Kouhei Sutou / @kou:
ARROW-12175 / #13892 changes arrow_shared to Arrow::arrow_shared and removes all our Find*.cmake.