GH-55357: [C++] Fix ORC overwritting CMAKE_MODULE_PATH when dependency not found by raulcd · Pull Request #55415 · apache/arrow

raulcd · 2026-11-05T11:48:54Z

Rationale for this change

When a dependency is not found on ORC its find_dependency returns without resetting previous CMAKE_MODULE_PATH:
https://github.com/apache/orc/blob/32046606774da481182dce2956f336dc8e79a579/c%2B%2B/orcConfig.cmake.in#L45-L55

What changes are included in this PR?

Maintain our CMAKE_MODULE_PATH before calling ORC.

Are these changes tested?

Not on our CI but this has been tested on downstream arrow-cpp-feedstock here:
conda-forge/arrow-cpp-feedstock#2100

Are there any user-facing changes?

No

Was AI used for this PR?

In accordance to the AI generation guidelines, please disclose below whether and how AI was used in this PR.

PR code and description written by:

Human
AI

Reviewed before submission by:

Human
AI
Not reviewed

GitHub Issue: [C++] AWS SDK not found for osx-arm #55357

…endency not found

github-actions · 2026-11-05T11:49:24Z

⚠️ GitHub issue #55357 has been automatically assigned in GitHub to PR creator.

Copilot

🟢 Approval recommended

No active review issues are reported for the change preserving Arrow’s CMake module search path during ORC discovery.

No open issues 🎉

What changed in this PR

Preserves Arrow’s CMake module search path when ORC dependency discovery fails, preventing later lookups such as AWS SDK discovery from losing access to Arrow’s find modules.

Changes:

Saves and restores CMAKE_MODULE_PATH around find_package(orc), then removes the temporary variable.
Documents why ORC’s early return requires this workaround.

File	Description
`cpp/cmake_modules/FindorcAlt.cmake`	Preserves the module search path across ORC discovery.

_{Review effort: Balanced}

h-vetinari · 2026-11-06T01:32:45Z

Thanks a lot for debugging this. I think this is good as an intermediate fix; better would be for orc to fix this for itself.

raulcd · 2026-11-06T08:14:50Z

Yes, I agree this is something that should be fixed on ORC side, this seems slightly related to the following issue (eventhough not exactly the same problem):

Orc unable to find dependencies, even when they exist orc#2408

ping @wgtmac what do you think?

edit: not the specific clobbing but the initial issue that created this as ORC wasn't able to find lz4 from conda due to homebrew also having lz4 and then (this new issue) not clearing CMAKE_MODULE_PATH

wgtmac · 2026-11-06T09:19:31Z

Ah, indeed ORC needs to fix this: apache/orc#2735

…fails ### 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](https://issues.apache.org/jira/browse/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: ```sh 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) Closes #2735 from wgtmac/codex/orc-2235-cmake-module-path. Authored-by: Gang Wu <ustcwg@gmail.com> Signed-off-by: Gang Wu <ustcwg@gmail.com>

…fails ### 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](https://issues.apache.org/jira/browse/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: ```sh 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) Closes #2735 from wgtmac/codex/orc-2235-cmake-module-path. Authored-by: Gang Wu <ustcwg@gmail.com> Signed-off-by: Gang Wu <ustcwg@gmail.com> (cherry picked from commit 0c0d0f0) Signed-off-by: Gang Wu <ustcwg@gmail.com>

apacheGH-55357: [C++] Fix ORC overwritting CMAKE_MODULE_PATH when dep…

3d0445e

…endency not found

github-actions Bot added Component: C++ awaiting committer review Awaiting committer review labels Nov 5, 2026

raulcd marked this pull request as ready for review November 5, 2026 12:21

Copilot AI balanced review requested due to automatic review settings November 5, 2026 12:21

Copilot started reviewing on behalf of raulcd November 5, 2026 12:21

View session

Copilot AI reviewed Nov 5, 2026

View reviewed changes

This was referenced Nov 6, 2026

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

Closed

fix(cmake): preserve module path when dependency discovery fails apache/iceberg-cpp#1097

Open

Navigation Menu

Repository navigation

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

GH-55357: [C++] Fix ORC overwritting CMAKE_MODULE_PATH when dependency not found#55415
raulcd wants to merge 1 commit into
apache:mainfrom
raulcd:GH-55357

raulcd commented Nov 5, 2026 •

edited by github-actions Bot

Loading

Uh oh!

github-actions Bot commented Nov 5, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

h-vetinari commented Nov 6, 2026

Uh oh!

raulcd commented Nov 6, 2026 •

edited

Loading

Uh oh!

wgtmac commented Nov 6, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

raulcd commented Nov 5, 2026 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

Was AI used for this PR?

Uh oh!

github-actions Bot commented Nov 5, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

h-vetinari commented Nov 6, 2026

Uh oh!

raulcd commented Nov 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wgtmac commented Nov 6, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

raulcd commented Nov 5, 2026 •

edited by github-actions Bot

Loading

raulcd commented Nov 6, 2026 •

edited

Loading