Skip to content

ORC-2235: [C++] Preserve CMAKE_MODULE_PATH when dependency discovery fails - #2735

Open
wgtmac wants to merge 2 commits into
apache:mainfrom
wgtmac:codex/orc-2235-cmake-module-path
Open

wgtmac wants to merge 2 commits into
apache:mainfrom
wgtmac:codex/orc-2235-cmake-module-path

Conversation

@wgtmac

@wgtmac wgtmac commented Nov 6, 2026 •

Copy link
Copy Markdown
Member

What changes were proposed in this pull request?

Isolate dependency discovery in orcConfig.cmake with a CMake variable scope, propagating orc_FOUND and orc_NOT_FOUND_MESSAGE while keeping CMAKE_MODULE_PATH local even on an early return.

The patch only changes c++/orcConfig.cmake.in; it does not modify test CMakeLists files or add test infrastructure.

Why are the changes needed?

ORC-2235

find_dependency() returns from the package config when a dependency cannot be found. The previous manual restoration of CMAKE_MODULE_PATH was therefore skipped, leaking ORC's module search path into the consuming project.

This caused a misleading AWS dependency-discovery failure in Arrow after ORC's LZ4 dependency lookup failed. apache/arrow#55415 adds a caller-side workaround; this PR fixes the underlying ORC configuration leak. It does not change LZ4 dependency-selection policy.

How was this patch tested?

On macOS with CMake 4.3.1:

cmake -S . -B build -DBUILD_JAVA=OFF
cmake --build build --target orc-test --parallel 8
cmake --build build --target test-out

After removing the test-infrastructure changes, both existing CTest tests (orc-test and tool-test) passed.

Separately verified six temporary, out-of-tree configure scenarios with CMake 3.25.2 and 4.3.1: successful/missing dependency discovery with CMAKE_MODULE_PATH initially undefined, empty, or containing multiple directories. All six passed with the scoped fix; all three missing-dependency scenarios reproduced the leak against the original template. No regression test files are included in this PR.

Was this patch authored or co-authored using generative AI tooling?

Generated-by: OpenAI Codex (desktop app 26.1029.61003)

@raulcd raulcd left a comment

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.

LGTM, thanks @wgtmac

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants