ORC-2013: [C++] Better CMake integration with Apache Arrow by wgtmac · Pull Request #2437 · apache/orc

wgtmac · 2025-10-12T06:58:13Z

What changes were proposed in this pull request?

To make it easier for Apache Arrow to integrate with Apache ORC.

Why are the changes needed?

Detect existing targets of 3rd-party dependencies from Apache Arrow, including: Protobuf, ZLIB, Snappy, and zstd.
Note that lz4 does not need this change because Apache Arrow has already used FetchContent for it.

How was this patch tested?

Test it locally against Apache Arrow main branch.

-- Apache ORC: Using existing arrow::protobuf::libprotobuf
-- Apache ORC: Using existing Snappy::snappy-static
-- Apache ORC: Using existing ZLIB::ZLIB
-- Apache ORC: Using existing zstd::libzstd_static
-- Apache ORC: Using vendored lz4

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

No.

wgtmac · 2025-10-12T07:33:16Z

@kou What do you think of this change? I've tested Apache Arrow in apache/arrow#47792

kou

Thanks!

kou · 2025-10-12T12:48:45Z

cmake_modules/ThirdpartyToolchain.cmake

+elseif (TARGET ${ARROW_PROTOBUF_LIBPROTOBUF})
+  # Used by Apache Arrow only, remove this once Arrow leverages FetchContent for Protobuf
+  add_library (orc_protobuf INTERFACE IMPORTED)
+  target_link_libraries(orc_protobuf INTERFACE ${ARROW_PROTOBUF_LIBPROTOBUF})
+  set (PROTOBUF_EXECUTABLE ${ARROW_PROTOBUF_PROTOC})
+  message(STATUS "Using existing ${ARROW_PROTOBUF_LIBPROTOBUF}")


How about using the standard target names instead of ARROW_PROTOBUF_* so that other downstream projects can use Apache ORC by FetchContent?

Suggested change

elseif (TARGET ${ARROW_PROTOBUF_LIBPROTOBUF})

# Used by Apache Arrow only, remove this once Arrow leverages FetchContent for Protobuf

add_library (orc_protobuf INTERFACE IMPORTED)

target_link_libraries(orc_protobuf INTERFACE ${ARROW_PROTOBUF_LIBPROTOBUF})

set (PROTOBUF_EXECUTABLE ${ARROW_PROTOBUF_PROTOC})

message(STATUS "Using existing ${ARROW_PROTOBUF_LIBPROTOBUF}")

elseif (TARGET protobuf::libprotobuf AND TARGET protobuf::protoc)

# Used for FetchContent by downstream projects

add_library (orc_protobuf INTERFACE IMPORTED)

target_link_libraries(orc_protobuf INTERFACE protobuf::libprotobuf)

set (PROTOBUF_EXECUTABLE protobuf::protoc)

message(STATUS "Using existing protobuf::libprotobuf and protobuf::protoc")

Apache Arrow can provide protobuf::libprotobuf/protobuf::protoc targets by add_library(ALIAS).

I was supposed to add this as a temporary solution for Apache Arrow. Apache Arrow has already migrated lz4 to use FetchContent so I don't need to do anything and FetchContent_Declare in Apache ORC can automatically find it. I assume Apache Arrow will eventually migrate all dependencies to use FetchContent, am I correct?

Ah, I haven't used FetchContent_Declare(FIND_PACKAGE_ARGS) yet. Sorry.

If Apache Arrow migrates Protobuf, Snappy, zlib and Zstandard to FetchContent, we don't need this in Apache ORC, right? If so, let's improve Apache Arrow.

Yes, I agree!

As I have successfully migrated dependencies in the Apache ORC to use FetchContent with FIND_PACKAGE_ARGS, I think Snappy and Zstandard should be pretty easy to migrate. It is a little bit tricky for Protobuf to find protoc. ZLIB is more messy since it does not have a release with good CMake support so I am currently using an unreleased git tag. I can work on this in the Apache Arrow one by one but may need your help :)

Sure. Let's work on this together. I'll also work on this one by one in a few weeks. (I'll fix 22.0.0 release blockers before this.)

kou · 2025-10-12T12:50:45Z

cmake_modules/ThirdpartyToolchain.cmake

  list (APPEND ORC_INSTALL_INTERFACE_TARGETS "$<INSTALL_INTERFACE:Snappy::snappy>")
  orc_provide_find_module (SnappyAlt)
+elseif (TARGET Snappy::snappy)
+  # Used by Apache Arrow only, remove this once Arrow leverages FetchContent for Snappy


Snappy::snappy is the standard target name provided by Snappy. So all downstream projects can use this by FetchContent:

Suggested change

# Used by Apache Arrow only, remove this once Arrow leverages FetchContent for Snappy

# Used for FetchContent by downstream projects

kou · 2025-10-12T12:51:01Z

cmake_modules/ThirdpartyToolchain.cmake

+  target_link_libraries(orc_snappy INTERFACE Snappy::snappy)
+  message(STATUS "Using existing Snappy::snappy")
+elseif (TARGET Snappy::snappy-static)
+  # Used by Apache Arrow only, remove this once Arrow leverages FetchContent for Snappy


Suggested change

# Used by Apache Arrow only, remove this once Arrow leverages FetchContent for Snappy

# Used for FetchContent by downstream projects

kou · 2025-10-12T12:55:20Z

cmake_modules/ThirdpartyToolchain.cmake

+elseif (TARGET Snappy::snappy)
+  # Used by Apache Arrow only, remove this once Arrow leverages FetchContent for Snappy
+  add_library (orc_snappy INTERFACE IMPORTED)
+  target_link_libraries(orc_snappy INTERFACE Snappy::snappy)
+  message(STATUS "Using existing Snappy::snappy")
+elseif (TARGET Snappy::snappy-static)


find_package(Snappy) may provide both of Snappy::snappy and Snappy::snappy-static. Apache Arrow wants to control which is used. (Apache Arrow uses ``ARROW_SNAPPY_USE_SHARED` for it.)

Can Apache ORC provide a CMake variable to control it? For example:

elseif (ORC_SNAPPY_USE_SHARED AND TARGET Snappy::snappy) ... elseif (NOT ORC_SNAPPY_USE_SHARED AND TARGET Snappy::snappy-static) ... else ()

github-actions bot added the BUILD label Oct 12, 2025

wgtmac force-pushed the arrow_cmake branch from 3321867 to 3156046 Compare October 12, 2025 06:59

wgtmac marked this pull request as ready for review October 12, 2025 07:00

wgtmac mentioned this pull request Oct 12, 2025

MINOR: [C++] Simplify CMake integration with Apache ORC apache/arrow#47792

Draft

ORC-2013: [C++][FOLLOWUP] Better CMake integration with Apache Arrow

509182d

wgtmac force-pushed the arrow_cmake branch from 3156046 to 509182d Compare October 12, 2025 09:21

wgtmac changed the title ~~ORC-2013: [C++][FOLLOWUP] Better CMake integration with Apache Arrow~~ ORC-2013: [C++] Better CMake integration with Apache Arrow Oct 12, 2025

kou reviewed Oct 12, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ORC-2013: [C++] Better CMake integration with Apache Arrow#2437

ORC-2013: [C++] Better CMake integration with Apache Arrow#2437
wgtmac wants to merge 1 commit intoapache:mainfrom
wgtmac:arrow_cmake

wgtmac commented Oct 12, 2025 •

edited

Loading

Uh oh!

wgtmac commented Oct 12, 2025

Uh oh!

kou left a comment

Uh oh!

kou Oct 12, 2025

Uh oh!

wgtmac Oct 12, 2025

Uh oh!

kou Oct 12, 2025

Uh oh!

wgtmac Oct 12, 2025

Uh oh!

wgtmac Oct 12, 2025

Uh oh!

kou Oct 12, 2025

Uh oh!

kou Oct 12, 2025

Uh oh!

kou Oct 12, 2025

Uh oh!

kou Oct 12, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	# Used by Apache Arrow only, remove this once Arrow leverages FetchContent for Snappy
	# Used for FetchContent by downstream projects

Conversation

wgtmac commented Oct 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

Why are the changes needed?

How was this patch tested?

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

Uh oh!

wgtmac commented Oct 12, 2025

Uh oh!

kou left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

wgtmac commented Oct 12, 2025 •

edited

Loading