GH-45225: [C++] Upgrade ORC to 2.1.0 by wgtmac · Pull Request #45226 · apache/arrow

wgtmac · 2025-01-11T16:17:26Z

Rationale for this change

Apache ORC has just released 2.1.0: https://orc.apache.org/news/2025/01/09/ORC-2.1.0/

We need to upgrade it to avoid occasional download failures of orc-format.

What changes are included in this PR?

Bump Apache ORC to its latest version 2.1.0.

Are these changes tested?

Pass CIs.

Are there any user-facing changes?

No.

GitHub Issue: [C++] Upgrade ORC to 2.1.0 #45225

github-actions · 2025-01-11T16:17:50Z

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

kou · 2025-01-12T02:13:20Z

Hmm. There are some CI failures with ORC 2.1.0...
Could you take a look at them?

Could you remove a workaround for ORC 2.0.2?

arrow/cpp/cmake_modules/ThirdpartyToolchain.cmake

Lines 4670 to 4672 in 913cb58

    
               # We can remove this with ORC 2.0.2 or later. 
        
               list(PREPEND CMAKE_MODULE_PATH 
        
                    ${CMAKE_CURRENT_BINARY_DIR}/_deps/orc-src/cmake_modules)

wgtmac · 2025-01-12T13:59:16Z

Yes, I will take a look and fix it.

Could you remove a workaround for ORC 2.0.2?

Just to confirm that we can drop the support of ORC versions prior to 2.1.0, right?

wgtmac · 2025-01-12T15:52:23Z

My bad! I found an issue here: https://github.com/apache/orc/blob/5bbafbb847f6e23b5a25d83c4d817741d36d9cc8/c%2B%2B/src/CMakeLists.txt#L221-L222

target_include_directories (orc
  INTERFACE
    $<INSTALL_INTERFACE:${CMAKE_INSTALL_INCLUDEDIR}>
  PUBLIC
    $<BUILD_INTERFACE:${CMAKE_SOURCE_DIR}/c++/include>
    $<BUILD_INTERFACE:${CMAKE_BINARY_DIR}/c++/include>
  PRIVATE
    ${CMAKE_CURRENT_SOURCE_DIR}
    ${CMAKE_CURRENT_BINARY_DIR}
    ${LIBHDFSPP_INCLUDE_DIR}
)

I should use PROJECT_SOURCE_DIR and PROJECT_BINARY_DIR instead.

kou · 2025-01-13T00:18:09Z

Just to confirm that we can drop the support of ORC versions prior to 2.1.0, right?

Right. But it's only for bundled ORC. We keep old ORC support for system ORC (not bundled ORC).
The old ORC workaround code is used only with bundled ORC.

wgtmac · 2025-01-13T03:37:01Z

@kou Is it fine to disable ORC in C++ / ARM64 Ubuntu 20.04 C++? The patch command is unavailable there. Otherwise we need to install patch command on the docker images IMO.

kou · 2025-01-13T04:38:23Z

We can install patch in Ubuntu 20.04 image:

diff --git a/ci/docker/ubuntu-20.04-cpp.dockerfile b/ci/docker/ubuntu-20.04-cpp.dockerfile
index 8dc778d544..259c5fb77f 100644
--- a/ci/docker/ubuntu-20.04-cpp.dockerfile
+++ b/ci/docker/ubuntu-20.04-cpp.dockerfile
@@ -106,6 +106,7 @@ RUN apt-get update -y -q && \
         ninja-build \
         nlohmann-json3-dev \
         npm \
+        patch \
         pkg-config \
         protobuf-compiler \
         python3-dev \

wgtmac · 2025-01-13T07:33:58Z

@github-actions crossbow submit -g cpp

github-actions · 2025-01-13T07:36:39Z

Revision: 2b0a4ab

Submitted crossbow builds: ursacomputing/crossbow @ actions-2893fb5e45

Task	Status
example-cpp-minimal-build-static
example-cpp-minimal-build-static-system-dependency
example-cpp-tutorial
test-alpine-linux-cpp
test-build-cpp-fuzz
test-conda-cpp
test-conda-cpp-valgrind
test-cuda-cpp-ubuntu-20.04-cuda-11.2.2
test-cuda-cpp-ubuntu-22.04-cuda-11.7.1
test-debian-12-cpp-amd64
test-debian-12-cpp-i386
test-fedora-39-cpp
test-ubuntu-20.04-cpp
test-ubuntu-20.04-cpp-bundled
test-ubuntu-22.04-cpp
test-ubuntu-22.04-cpp-20
test-ubuntu-22.04-cpp-emscripten
test-ubuntu-22.04-cpp-no-threading
test-ubuntu-24.04-cpp
test-ubuntu-24.04-cpp-bundled-offline
test-ubuntu-24.04-cpp-gcc-13-bundled
test-ubuntu-24.04-cpp-gcc-14
test-ubuntu-24.04-cpp-minimal-with-formats
test-ubuntu-24.04-cpp-thread-sanitizer

wgtmac · 2025-01-13T07:45:17Z

I think this is ready for review @kou

kou · 2025-01-13T08:42:39Z

@github-actions crossbow submit -g linux

kou · 2025-01-13T08:42:51Z

@github-actions crossbow submit -g wheel -g r

cpp/src/arrow/adapters/orc/adapter_test.cc

github-actions · 2025-01-13T08:45:18Z

Revision: 2b0a4ab

Submitted crossbow builds: ursacomputing/crossbow @ actions-6d7e7577fe

Task	Status
almalinux-8-amd64
almalinux-8-arm64
almalinux-9-amd64
almalinux-9-arm64
amazon-linux-2023-amd64
amazon-linux-2023-arm64
centos-7-amd64
centos-8-stream-amd64
centos-8-stream-arm64
centos-9-stream-amd64
centos-9-stream-arm64
debian-bookworm-amd64
debian-bookworm-arm64
debian-trixie-amd64
debian-trixie-arm64
ubuntu-focal-amd64
ubuntu-focal-arm64
ubuntu-jammy-amd64
ubuntu-jammy-arm64
ubuntu-noble-amd64
ubuntu-noble-arm64

github-actions · 2025-01-13T08:47:06Z

Revision: 2b0a4ab

Submitted crossbow builds: ursacomputing/crossbow @ actions-852243f72c

Task	Status
python-sdist
r-binary-packages
r-recheck-most
test-r-arrow-backwards-compatibility
test-r-clang-sanitizer
test-r-depsource-bundled
test-r-depsource-system
test-r-dev-duckdb
test-r-devdocs
test-r-extra-packages
test-r-gcc-11
test-r-gcc-12
test-r-install-local
test-r-install-local-minsizerel
test-r-linux-as-cran
test-r-linux-rchk
test-r-linux-valgrind
test-r-macos-as-cran
test-r-minimal-build
test-r-offline-maximal
test-r-offline-minimal
test-r-rhub-debian-gcc-devel-lto-latest
test-r-rhub-debian-gcc-release-custom-ccache
test-r-rhub-ubuntu-release-latest
test-r-rocker-r-ver-latest
test-r-rstudio-r-base-4.1-opensuse155
test-r-rstudio-r-base-4.2-focal
test-r-ubuntu-22.04
test-r-versions
test-ubuntu-r-sanitizer
wheel-macos-monterey-cp310-cp310-amd64
wheel-macos-monterey-cp310-cp310-arm64
wheel-macos-monterey-cp311-cp311-amd64
wheel-macos-monterey-cp311-cp311-arm64
wheel-macos-monterey-cp312-cp312-amd64
wheel-macos-monterey-cp312-cp312-arm64
wheel-macos-monterey-cp313-cp313-amd64
wheel-macos-monterey-cp313-cp313-arm64
wheel-macos-monterey-cp313-cp313t-amd64
wheel-macos-monterey-cp313-cp313t-arm64
wheel-macos-monterey-cp39-cp39-amd64
wheel-macos-monterey-cp39-cp39-arm64
wheel-manylinux-2-28-cp310-cp310-amd64
wheel-manylinux-2-28-cp310-cp310-arm64
wheel-manylinux-2-28-cp311-cp311-amd64
wheel-manylinux-2-28-cp311-cp311-arm64
wheel-manylinux-2-28-cp312-cp312-amd64
wheel-manylinux-2-28-cp312-cp312-arm64
wheel-manylinux-2-28-cp313-cp313-amd64
wheel-manylinux-2-28-cp313-cp313-arm64
wheel-manylinux-2-28-cp313-cp313t-amd64
wheel-manylinux-2-28-cp313-cp313t-arm64
wheel-manylinux-2-28-cp39-cp39-amd64
wheel-manylinux-2-28-cp39-cp39-arm64
wheel-manylinux-2014-cp310-cp310-amd64
wheel-manylinux-2014-cp310-cp310-arm64
wheel-manylinux-2014-cp311-cp311-amd64
wheel-manylinux-2014-cp311-cp311-arm64
wheel-manylinux-2014-cp312-cp312-amd64
wheel-manylinux-2014-cp312-cp312-arm64
wheel-manylinux-2014-cp313-cp313-amd64
wheel-manylinux-2014-cp313-cp313-arm64
wheel-manylinux-2014-cp313-cp313t-amd64
wheel-manylinux-2014-cp313-cp313t-arm64
wheel-manylinux-2014-cp39-cp39-amd64
wheel-manylinux-2014-cp39-cp39-arm64
wheel-windows-cp310-cp310-amd64
wheel-windows-cp311-cp311-amd64
wheel-windows-cp312-cp312-amd64
wheel-windows-cp313-cp313-amd64
wheel-windows-cp313-cp313t-amd64
wheel-windows-cp39-cp39-amd64

### 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 <[email protected]> Signed-off-by: Gang Wu <[email protected]>

### 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 <[email protected]> Signed-off-by: Gang Wu <[email protected]> (cherry picked from commit 3eb423a) Signed-off-by: Gang Wu <[email protected]>

kou

+1

cpp/cmake_modules/ThirdpartyToolchain.cmake

Co-authored-by: Sutou Kouhei <[email protected]>

conbench-apache-arrow · 2025-01-16T19:23:30Z

After merging your PR, Conbench analyzed the 4 benchmarking runs that have been run so far on merge-commit 7fc8222.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details. It also includes information about 12 possible false positives for unstable benchmarks that are known to sometimes produce them.

apacheGH-45225: [C++] Upgrade ORC to 2.1.0

856516e

github-actions bot added Component: C++ awaiting review Awaiting review labels Jan 11, 2025

apply patch to avoid CMAKE_SOURCE_DIR

b27ea21

wgtmac marked this pull request as draft January 12, 2025 16:16

add license header

45a8128

wgtmac added 5 commits January 13, 2025 09:21

disable CMakeFormat from ORC

12eb153

fix name conflict of provide_cmake_module

634e137

resolve more name conflict

988c8c6

fix test

d888b50

fix test_orc.py

b18c5c1

github-actions bot added the Component: Python label Jan 13, 2025

wgtmac added 3 commits January 13, 2025 13:07

fix ubuntu 20.04

ae9a8a8

fix target visibility for ubuntu 20.04

bf0368f

fix zlib

f9cf61c

wgtmac marked this pull request as ready for review January 13, 2025 07:06

wgtmac requested review from assignUser, jonkeane, kou and raulcd as code owners January 13, 2025 07:06

This comment was marked as outdated.

Sign in to view

install patch cmd

2b0a4ab

wgtmac mentioned this pull request Jan 13, 2025

ORC-1833: [C++] Fix CMake script to be used inside another project apache/orc#2108

Closed

kou reviewed Jan 13, 2025

View reviewed changes

cpp/src/arrow/adapters/orc/adapter_test.cc Show resolved Hide resolved

github-actions bot added awaiting changes Awaiting changes and removed awaiting review Awaiting review labels Jan 13, 2025

kou approved these changes Jan 14, 2025

View reviewed changes

cpp/cmake_modules/ThirdpartyToolchain.cmake Outdated Show resolved Hide resolved

Update cpp/cmake_modules/ThirdpartyToolchain.cmake

a3493d3

Co-authored-by: Sutou Kouhei <[email protected]>

github-actions bot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Jan 14, 2025

wgtmac merged commit 7fc8222 into apache:main Jan 14, 2025
36 of 37 checks passed

wgtmac removed the awaiting change review Awaiting change review label Jan 14, 2025

raulcd mentioned this pull request Feb 11, 2025

[main] Rebuild for orc 2.1.1 conda-forge/arrow-cpp-feedstock#1697

Merged

Conversation

wgtmac commented Jan 11, 2025 • 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?

Uh oh!

github-actions bot commented Jan 11, 2025

Uh oh!

kou commented Jan 12, 2025

Uh oh!

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

Uh oh!

Uh oh!

wgtmac commented Jan 12, 2025

Uh oh!

kou commented Jan 13, 2025

Uh oh!

wgtmac commented Jan 13, 2025

Uh oh!

kou commented Jan 13, 2025

Uh oh!

This comment was marked as outdated.

This comment was marked as outdated.

wgtmac commented Jan 13, 2025

Uh oh!

github-actions bot commented Jan 13, 2025

Uh oh!

wgtmac commented Jan 13, 2025

Uh oh!

kou commented Jan 13, 2025

Uh oh!

kou commented Jan 13, 2025

Uh oh!

Uh oh!

github-actions bot commented Jan 13, 2025

Uh oh!

github-actions bot commented Jan 13, 2025

Uh oh!

kou left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

conbench-apache-arrow bot commented Jan 16, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

wgtmac commented Jan 11, 2025 •

edited by github-actions bot

Loading

wgtmac commented Jan 12, 2025 •

edited

Loading