zhjwpku commented on code in PR #1097:
URL: https://github.com/apache/iceberg-cpp/pull/1097#discussion_r4493958332
##########
src/iceberg/iceberg-config.cmake.in:
##########
@@ -47,23 +47,14 @@ set(ICEBERG_FIND_EXTRA_ARGS_AWSSDK
"@ICEBERG_FIND_EXTRA_ARGS_AWSSDK@")
include(CMakeFindDependencyMacro)
macro(iceberg_find_dependencies dependencies)
- if(DEFINED CMAKE_MODULE_PATH)
- set(ICEBERG_CMAKE_MODULE_PATH_OLD ${CMAKE_MODULE_PATH})
- else()
- unset(ICEBERG_CMAKE_MODULE_PATH_OLD)
- endif()
- set(CMAKE_MODULE_PATH "${CMAKE_CURRENT_LIST_DIR}")
-
- foreach(dependency ${dependencies})
- find_dependency(${dependency} ${ICEBERG_FIND_EXTRA_ARGS_${dependency}})
- endforeach()
-
- if(DEFINED ICEBERG_CMAKE_MODULE_PATH_OLD)
- set(CMAKE_MODULE_PATH ${ICEBERG_CMAKE_MODULE_PATH_OLD})
- unset(ICEBERG_CMAKE_MODULE_PATH_OLD)
- else()
- unset(CMAKE_MODULE_PATH)
- endif()
+ # find_dependency() returns from this file when a dependency is not found.
+ # Keep changes to CMAKE_MODULE_PATH local even on that early return.
+ block(SCOPE_FOR VARIABLES PROPAGATE iceberg_FOUND iceberg_NOT_FOUND_MESSAGE)
Review Comment:
The `PROPAGATE` list hard-codes `iceberg_FOUND` /
`iceberg_NOT_FOUND_MESSAGE`, but `find_dependency()` sets
`${CMAKE_FIND_PACKAGE_NAME}_FOUND` / `_NOT_FOUND_MESSAGE` before `return()`.
`CMAKE_FIND_PACKAGE_NAME` is the name the caller passed to `find_package()`.
With `find_package(Iceberg)`, `Iceberg_FOUND` is set inside the block and never
reaches the caller. When a dependency is missing, the package is then reported
as found with no targets.
```suggestion
block(SCOPE_FOR VARIABLES
PROPAGATE ${CMAKE_FIND_PACKAGE_NAME}_FOUND
${CMAKE_FIND_PACKAGE_NAME}_NOT_FOUND_MESSAGE)
```
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
