ORC-1833: [C++] Fix CMake script to be used inside another project #2108
Conversation
| "Arbitrary string that identifies the kind of package" | ||
| "") | ||
|
|
||
| option(ORC_ENABLE_CLANG_TOOLS |
There was a problem hiding this comment.
Maybe we should check CMAKE_CXX_COMPILER_ID MATCHES "Clang" when the option is ON
There was a problem hiding this comment.
I think they are unrelated. Sometimes I want to use gcc to build but Clang is available on my machine to do the checking.
There was a problem hiding this comment.
I think they are unrelated. Sometimes I want to use gcc to build but Clang is available on my machine to do the checking.
Fine, but there are some compile options used by GCC that clang did not understand.
There was a problem hiding this comment.
Actually I think we can disable it by default because we have switched to use cpp-linter-action already. But this PR is required to backport to 2.1.1 so I decided to keep it to make the CI happy on branch-2.1
|
cc @kou |
### What changes were proposed in this pull request? The change to add support for exporting CMake config and target has introduced some minor issues when the ORC C++ library is used inside a larger CMake project. Mainly there are three issues: - We should prepend (not append) our CMake modules so we always use our own modules when there are naming conflict. - Do not use CMAKE_SOURCE_DIR and CMAKE_BINARY_DIR because they are tied to the root project and it is no longer ORC project when we are inside another project. - Add orc_ prefix to our CMake functions to avoid potential conflict. See: apache/arrow#45226 ### Why are the changes needed? We had a problem with upgrading Apache ORC to 2.1.0 in the Apache Arrow. See: apache/arrow#45226 ### How was this patch tested? - Pass all CIs. - Manually integrated it with Apache Arrow. ### Was this patch authored or co-authored using generative AI tooling? No. Closes #2108 from wgtmac/fix_cmake. Authored-by: Gang Wu <ustcwg@gmail.com> Signed-off-by: Gang Wu <ustcwg@gmail.com> (cherry picked from commit 3eb423a) Signed-off-by: Gang Wu <ustcwg@gmail.com>
dongjoon-hyun
left a comment
There was a problem hiding this comment.
+1, LGTM. Thank you, @wgtmac and @ffacs .
cc @williamhyun
What changes were proposed in this pull request?
The change to add support for exporting CMake config and target has introduced some minor issues when the ORC C++ library is used inside a larger CMake project. Mainly there are three issues:
See: apache/arrow#45226
Why are the changes needed?
We had a problem with upgrading Apache ORC to 2.1.0 in the Apache Arrow. See: apache/arrow#45226
How was this patch tested?
Was this patch authored or co-authored using generative AI tooling?
No.