GH-46375: [C++] Add adapters/orc directory to Meson

[WillAyd]

Rationale for this change

Apache ORC now has native support for Meson, so we can rather easily add support now in Arrow

What changes are included in this PR?

This integrates the adapters/orc directory into the Meson configuration

Are these changes tested?

Yes

Are there any user-facing changes?

No

• GitHub Issue: [C++] Add adapters/orc directory to Meson #46375

[WillAyd]

I don't believe the AppVeyor failure is related

[wgtmac]

I'm not familiar with Meson internals but it looks reasonable to me. cc @kou

[kou]

Can we use a released version instead?

[WillAyd]

Not yet - a release with the Meson configuration hasn't happened (it's brand new)

[kou]

Can we merge this PR after the new ORC release?

[WillAyd]

Sure we can wait. I know originally the next release for Orc was planned far out, but there have been discussions for it happening sooner (next month or so).

Will keep tabs on that discussion

[WillAyd]

Hmm actually it would be good if we reconsider for now. The orc implementation here is a dependency for the dataset work, which in turn can unlock substrait work, which is the last component needed before we can more seriously look at the Python build system again.

There are of course ways to work around that, but I think going in a linear fashion is the easiest way to avoid missing anything compared against CMake

[kou]

OK. But can we do this after 21.0.0 feature freeze?
We should not release a version that depends on non "official" Apache product releases.

[WillAyd]

Definitely!

[WillAyd]

Hey @kou - looks like the wrapdb merged apache-orc today, so this uses the official 2.2.0 release

[WillAyd]

Hmm looks like the build is failing due to some UB:

[ RUN      ] TestORCWriterSingleArray.WriteStructOfStruct
../src/arrow/adapters/orc/util.cc:216:27: runtime error: signed integer overflow: -4702111234474983746 * 1000000000 cannot be represented in type 'long int'

I don't remember this from testing against a development version of orc, so its possibly a regression in the 2.2.0 release? Have to investigate more

[kou]

@wgtmac may help us.

[wgtmac]

Let me try to reproduce it on my side. Is it a stable failure on your side? @WillAyd

[WillAyd]

It is reproducible. If you take this branch and do:

cd cpp
meson setup builddir -Dtests=enabled -Dorc=enabled -Db_sanitize=address,undefined

Then you can either run the test suite with

meson test arrow-orc-adapter-test --print-errorlogs

or run the individual test in builddir/src/arrow/adapters/orc/arrow-orc-adapter-test

However, I erred in my comment before that this did not appear previously. It looks like even back to orc commit 15d86dfcad6fc80ee24cf785419a820a55db103b where this PR started that UB can be detected; we may have just gotten lucky with CI not failing

[wgtmac]

It seems that I cannot execute -Db_sanitize=address,undefined on my MacOS M1.

[WillAyd]

Are you getting any error?

[wgtmac]

ERROR: Linker c++ does not support sanitizer arguments ['-fsanitize=address,undefined']

I got the above error. If I removed them, I see some compiling error from boost like below:

FAILED: [code=1] src/arrow/libarrow_testing.a.p/testing_process.cc.o
c++ -Isrc/arrow/libarrow_testing.a.p -Isrc/arrow -I../src/arrow -Isrc -I../src -I/opt/homebrew/include -I/opt/homebrew/Cellar/googletest/1.14.0/include -I//opt/homebrew/opt/llvm@18/include -fdiagnostics-color=always -D_LIBCPP_HARDENING_MODE=_LIBCPP_HARDENING_MODE_FAST -Wall -Winvalid-pch -Wextra -std=c++17 -O0 -g -Wno-unused-parameter -Wno-array-bounds -Wno-nonnull -DGTEST_HAS_PTHREAD=1 -DBOOST_FILESYSTEM_DYN_LINK=1 -DBOOST_ALL_NO_LIB -MD -MQ src/arrow/libarrow_testing.a.p/testing_process.cc.o -MF src/arrow/libarrow_testing.a.p/testing_process.cc.o.d -o src/arrow/libarrow_testing.a.p/testing_process.cc.o -c ../src/arrow/testing/process.cc
../src/arrow/testing/process.cc:268:12: error: no type named 'environment' in namespace 'boost::process'
  268 |   process::environment env_;
      |   ~~~~~~~~~^
../src/arrow/testing/process.cc:269:28: error: no member named 'child' in namespace 'boost::process'
  269 |   std::unique_ptr<process::child> process_;
      |                   ~~~~~~~~~^
../src/arrow/testing/process.cc:270:28: error: no member named 'group' in namespace 'boost::process'
  270 |   std::unique_ptr<process::group> process_group_;
      |                   ~~~~~~~~~^
../src/arrow/testing/process.cc:130:21: error: unexpected namespace name 'environment': expected expression
  130 |     env_ = process::environment(boost::this_process::environment());
      |                     ^

Let me investigate it.

[WillAyd]

Ah that's interesting. Maybe just doing -b_sanitize=undefined will work? I suppose ASAN isn't required to debug this.

If not, will look further into what the macOS linker is doing

[WillAyd]

Maybe the sanitizers aren't supported on arm macs? This homebrew conversation is a few years old but I don't see a resolution:

https://github.com/orgs/Homebrew/discussions/3384

[wgtmac]

I have successfully reproduced it without meson. Simply run cmake .. -DCMAKE_BUILD_TYPE=Debug -DARROW_BUILD_TESTS=ON -DARROW_ORC=ON -DARROW_USE_UBSAN=ON

[----------] 3 tests from TestORCWriterNoConversion
[ RUN      ] TestORCWriterNoConversion.writeNoNulls
[       OK ] TestORCWriterNoConversion.writeNoNulls (87 ms)
[ RUN      ] TestORCWriterNoConversion.writeMixed
/Users/gangwu/Projects/arrow/cpp/src/arrow/adapters/orc/util.cc:216:27: runtime error: signed integer overflow: 6949478456235920402 * 1000000000 cannot be represented in type 'int64_t' (aka 'long long')
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /Users/gangwu/Projects/arrow/cpp/src/arrow/adapters/orc/util.cc:216:27

[wgtmac]

diff --git a/cpp/src/arrow/adapters/orc/util.cc b/cpp/src/arrow/adapters/orc/util.cc
index 6974faae59..68d062f125 100644
--- a/cpp/src/arrow/adapters/orc/util.cc
+++ b/cpp/src/arrow/adapters/orc/util.cc
@@ -212,7 +212,10 @@ Status AppendTimestampBatch(liborc::ColumnVectorBatch* column_vector_batch,
   const int64_t* seconds = batch->data.data() + offset;
   const int64_t* nanos = batch->nanoseconds.data() + offset;

-  auto transform_timestamp = [seconds, nanos](int64_t index) {
+  auto transform_timestamp = [seconds, nanos, valid_bytes](int64_t index) -> int64_t {
+    if (valid_bytes && !valid_bytes[index]) {
+      return 0;
+    }
     return seconds[index] * kOneSecondNanos + nanos[index];
   };

Could you please apply this patch?

[WillAyd]

I believe the core of the issue is that orc does not distribute pkgconfig information like most (possibly all) of the other conda-installed libraries, which place their respective files in /opt/conda/envs/arrow/lib/pkgconfig. If that could be detected, then there would be no need to even build a local copy

[wgtmac]

Unfortunately I'm not familiar with pkgconfig so it may still be the case in the near future. Is there any workaround?

[kou]

Meson can use use not only pkg-config package (orc.pc) but also CMake package (orc/orcConfig.cmake). ORC provides its CMake package. Why is pkg-config package required to use system ORC?

[WillAyd]

The CMake config could be used as well but it is also not part of the conda installation

[kou]

Can we improve conda package to include the CMake config?

[kou]

I found lib/cmake/orc/orcConfig.cmake in linux-64/orc-2.2.0-h1bc01a4_0.conda listed at https://anaconda.org/conda-forge/orc/files .

[WillAyd]

Ah nice find! Unfortunately, the conda installation seems to be pinning orc to the 1.9.0 installation (I noticed this also in #47455 (comment)).

The 1.9.0 conda package doesn't seem to distribute that same file, so we might have to dissect what is causing that version pin

[WillAyd]

OK so the issue is definitely the fact that there are multiple versions of Orc installed, and the conda-forge installed version of 1.9.0 is too old to distribute any cmake/pkgconfig information that makes it detectable.

Looking at ci/conda_env_cpp.txt, it looks like the grpc_cpp package is what is responsible for indirectly pinning orc at the old 1.9.0 version. Unfortunately it doesn't look like grpc_cpp has had an updated conda package in 2.5 years, although the upstream library does distribute newer versions.

There are a lot of potential options to fix this, but I think the cleanest (and easiest?) would be to get an updated conda package for grpc_cpp somehow

[kou]

It seems that grpc_cpp was split to https://anaconda.org/conda-forge/libgrpc/ , https://anaconda.org/conda-forge/grpcio and https://anaconda.org/conda-forge/grpcio-tools .

[kou]

See also: https://github.com/conda-forge/grpc-cpp-feedstock/blob/main/recipe/meta.yaml

[WillAyd]

Nice find! That helped us get unpinned.

The newest issue appears to be something with the cmake files that orc distributes. The dependency call currently fails with the following warning:

NOTICE: CMake reported that the package orc was not found with the following reason:
orc could not be found because dependency ZSTD could not be found.
Run-time dependency orc found: NO 

Within the conda environment, I see orc distributes its own files for CMake package resolution:

${CONDA_PREFIX}/lib/cmake/orc/FindLZ4.cmake
${CONDA_PREFIX}/lib/cmake/orc/FindProtobuf.cmake
${CONDA_PREFIX}/lib/cmake/orc/FindSnappy.cmake
${CONDA_PREFIX}/lib/cmake/orc/FindZLIB.cmake
${CONDA_PREFIX}/lib/cmake/orc/FindZSTD.cmake

Renaming or removing any of these files seems to let CMake find the required packages. Hopefully there's some type of setting for CMake we can use to ignore these?

[kou]

@wgtmac Can Apache ORC also try finding CMake config packages? For example, can https://github.com/apache/orc/blob/main/cmake_modules/FindSnappy.cmake call find_package(Snappy CONFIG) like Apache Arrow's FindSnappyAlt.cmake does

arrow/cpp/cmake_modules/FindSnappyAlt.cmake

Lines 34 to 66 in d2dace9

	set(find_package_args)
	if(SnappyAlt_FIND_VERSION)
	list(APPEND find_package_args ${SnappyAlt_FIND_VERSION})
	endif()
	if(SnappyAlt_FIND_QUIETLY)
	list(APPEND find_package_args QUIET)
	endif()
	find_package(Snappy ${find_package_args})
	if(Snappy_FOUND)
	if(ARROW_SNAPPY_USE_SHARED)
	set(Snappy_TARGET Snappy::snappy)
	set(SnappyAlt_FOUND TRUE)
	return()
	else()
	if(TARGET Snappy::snappy-static)
	# The official SnappyTargets.cmake uses Snappy::snappy-static for
	# static version.
	set(Snappy_TARGET Snappy::snappy-static)
	set(SnappyAlt_FOUND TRUE)
	return()
	else()
	# The Conan's Snappy package always uses Snappy::snappy and it's
	# an INTERFACE_LIBRARY.
	get_target_property(Snappy Snappy::snappy TYPE)
	if(Snappy_TYPE STREQUAL "STATIC_LIBRARY" OR Snappy_TYPE STREQUAL
	"INTERFACE_LIBRARY")
	set(Snappy_TARGET Snappy::snappy)
	set(SnappyAlt_FOUND TRUE)
	return()
	endif()
	endif()
	endif()
	endif()

? (FindSnappyAlt.cmake doesn't need explicit CONIFG because it uses different name (not using FindSnappy.cmake).)

[wgtmac]

Can we try set(ORC_PACKAGE_KIND "conan")?

https://github.com/apache/orc/blob/main/cmake_modules/ThirdpartyToolchain.cmake#L269

[WillAyd]

Hmm I'm not sure - that file is not distributed with the conda install, so it may not have an effect?

I'm not sure of all the rules CMake uses, but from Meson I tried to do:

orc_dep = dependency('orc', cmake_args: ['-DORC_PACKAGE_KIND=conan'])

I can confirm in the logs that argument is getting forwarded to cmake:

Calling CMake (['/home/will/miniforge3/envs/arrow-dev/bin/cmake']) in /home/will/clones/arrow/cpp/foo/meson-private/cmake_orc with:
  - "-DNAME=orc"
  - "-DARCHS=i386-linux-gnu;x86_64-linux-gnu"
  - "-DVERSION="
  - "-DCOMPS="
  - "-DSTATIC=OFF"
  - "-DORC_PACKAGE_KIND=conan"
  - "--trace-expand"
  - "--trace-format=json-v1"
  - "--no-warn-unused-cli"
  - "--trace-redirect=cmake_trace.txt"
  - "-DCMAKE_TOOLCHAIN_FILE=/home/will/clones/arrow/cpp/foo/meson-private/cmake_orc/CMakeMesonToolchainFile.cmake"
  - "."
  - "-DCMAKE_PREFIX_PATH=/home/will/miniforge3/envs/arrow-dev"
NOTICE: CMake reported that the package orc was not found with the following reason:
orc could not be found because dependency Snappy could not be found.
Dependency lookup for orc with method 'cmake' failed: Dependency orc not found: PACKAGE_FOUND is false
Run-time dependency orc found: NO (tried pkgconfig)

But it appears to make no difference

[kou]

ORC_PACKAGE_KIND=conan is used only when we build bundled Apache ORC. It's not used when find_package(orc).

Let's improve Apache ORC before this.

[wgtmac]

I'm not sure if I can find some time to work on this recently. I'm also a little bit concerned of breaking compatibility of downstream...

[WillAyd]

FWIW I see this starting back with the orc 2.1.0 release. To be clear, I don't think it is stricly a Meson <> Cmake bridge issue, as it seems like the orc distributed FindXXX.cmake files are interefering with CMake's normal dependency resolution mechanism(s) (see the failing non-Meson jobs in the above CI run)

[wgtmac]

Let me know if apache/orc#2416 can help fix the issue here.