Skip to content

GH-55357: [C++] Fix ORC overwriting CMAKE_MODULE_PATH when dependency not found #55415

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
raulcd wants to merge 1 commit into
base: main
Choose a base branch
from
+6 −0
Open

GH-55357: [C++] Fix ORC overwriting CMAKE_MODULE_PATH when dependency not found #55415

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions cpp/cmake_modules/FindorcAlt.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Comment on lines +29 to +35

@kou kou Nov 7, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about using block() like apache/orc#2735 instead of restoring CMAKE_MODULE_PATH manually?

Could you also add a comment that we need this workaround only for ORC <= 2.3.1? We'll remove this workaround once we drop support for ORC <= 2.3.1.

@himanshuchawla0807-gif himanshuchawla0807-gif Nov 8, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

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.

if(orc_FOUND)
set(orcAlt_FOUND TRUE)
set(orcAlt_VERSION ${orc_VERSION})
Expand Down
Loading