himanshuchawla0807-gif commented on code in PR #55415:
URL: https://github.com/apache/arrow/pull/55415#discussion_r4505525647
##########
cpp/cmake_modules/FindorcAlt.cmake:
##########
@@ -26,7 +26,13 @@ endif()
if(orcAlt_FIND_QUIETLY)
list(APPEND find_package_args QUIET)
endif()
+# orcConfig.cmake overwrites CMAKE_MODULE_PATH and doesn't restore it
+# when one of its dependencies isn't found because find_dependency()
+# calls return() in the config file.
+set(orcAlt_CMAKE_MODULE_PATH_KEEP "${CMAKE_MODULE_PATH}")
find_package(orc ${find_package_args})
+set(CMAKE_MODULE_PATH "${orcAlt_CMAKE_MODULE_PATH_KEEP}")
+unset(orcAlt_CMAKE_MODULE_PATH_KEEP)
Review Comment:
I checked the proposed `block()` variant with a small CMake fixture at
3d0445e. A stub `orcConfig.cmake` changes the module path and either returns
early via a missing `find_dependency()` or sets a version and creates
`orc::orc`.
```cmake
block(SCOPE_FOR VARIABLES PROPAGATE orc_FOUND orc_VERSION)
find_package(orc ${find_package_args})
endblock()
```
The base leaked the stub's module path in the missing-dependency case. Both
the current PR and the block variant preserved Arrow's original two-entry path
there. The block variant also preserved `orcAlt_FOUND`, `orcAlt_VERSION`, and
the imported target on success. Codex generated and ran this isolated CMake
check with stub packages.
--
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]
