kou commented on code in PR #14059:
URL: https://github.com/apache/arrow/pull/14059#discussion_r967453059
##########
cpp/src/arrow/ArrowConfig.cmake.in:
##########
@@ -172,3 +172,16 @@ endmacro()
arrow_keep_backward_compatibility(Arrow arrow)
check_required_components(Arrow)
+
+function(arrow_show_details PACKAGE_NAME)
Review Comment:
Could you use lower case for variable name?
```suggestion
function(arrow_show_details package_name)
```
##########
cpp/src/arrow/ArrowTestingConfig.cmake.in:
##########
@@ -34,3 +34,5 @@ include("${CMAKE_CURRENT_LIST_DIR}/ArrowTestingTargets.cmake")
arrow_keep_backward_compatibility(ArrowTesting arrow_testing)
check_required_components(ArrowTesting)
+
+arrow_show_details(arrow_testing)
Review Comment:
Ah, you expect `arrow_testing` not `ArrowTesting` here.
`arrow_testing` isn't a (CMake) package name. `ArrowTesting` is the (CMake)
package name for this.
`ArrowTesting_FIND_QUIETLY` is defined but `arrow_testing_FIND_QUIETLY`
isn't defined.
##########
cpp/src/arrow/ArrowConfig.cmake.in:
##########
@@ -172,3 +172,16 @@ endmacro()
arrow_keep_backward_compatibility(Arrow arrow)
check_required_components(Arrow)
+
+function(arrow_show_details PACKAGE_NAME)
+ if(NOT ${PACKAGE_NAME}_FIND_QUIETLY)
+ string(TOUPPER ${PACKAGE_NAME} package_name_upper)
+ message(STATUS "${PACKAGE_NAME} version: ${${package_name_upper}_VERSION}")
+ message(STATUS "Found the ${PACKAGE_NAME} shared library:
${${package_name_upper}_SHARED_LIB}")
Review Comment:
Does this work with `ArrowDataset`?
It seems that we need to pass 2 arguments to this function:
`arrow_show_details(package_name variable_prefix)` (e.g.:
`arrow_show_details(ArrowDataset ARROW_DATASET)`)
##########
python/pyarrow/src/ArrowPythonConfig.cmake.in:
##########
@@ -37,3 +37,5 @@ include("${CMAKE_CURRENT_LIST_DIR}/ArrowPythonTargets.cmake")
arrow_keep_backward_compatibility(ArrowPython arrow_python)
check_required_components(ArrowPython)
+
+arrow_show_details(arrow_python)
Review Comment:
Yes.
##########
cpp/src/arrow/flight/ArrowFlightConfig.cmake.in:
##########
@@ -34,3 +34,5 @@ include("${CMAKE_CURRENT_LIST_DIR}/ArrowFlightTargets.cmake")
arrow_keep_backward_compatibility(ArrowFlight arrow_flight)
check_required_components(ArrowFlight)
+
+arrow_show_details(arrow_flight)
Review Comment:
I think that it's not a problem but we can suppress it by defining a
variable that keeps whether we already showed these messages or not:
```cmake
function(arrow_show_details package_name)
if(${package_name}_SHOWED_DETAILS)
return()
endif()
# ...
set(${package_name}_SHOWED_DETAILS TRUE)
endfunction()
```
(We may need to a macro instead of a function to use the variable over
function calls. If we use a macro, we can't use `return()` here:
https://cmake.org/cmake/help/latest/command/return.html
##########
cpp/src/arrow/ArrowConfig.cmake.in:
##########
@@ -172,3 +172,16 @@ endmacro()
arrow_keep_backward_compatibility(Arrow arrow)
check_required_components(Arrow)
+
+function(arrow_show_details PACKAGE_NAME)
+ if(NOT ${PACKAGE_NAME}_FIND_QUIETLY)
+ string(TOUPPER ${PACKAGE_NAME} package_name_upper)
+ message(STATUS "${PACKAGE_NAME} version: ${${package_name_upper}_VERSION}")
Review Comment:
Can we use `${PACKAGE_NAME}_VERSION` here?
`ARROW_VERSION` is defined but `ARROW_DATASET_VERSION` and so on aren't
defined.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
