ORC-1940: Add threads dependency to orc lib in Meson#2301
ORC-1940: Add threads dependency to orc lib in Meson#2301WillAyd wants to merge 1 commit intoapache:mainfrom
Conversation
| incdir = include_directories('../include') | ||
| orc_format_proto_dep = dependency('orc_format_proto') | ||
| # zstd requires us to add the threads | ||
| threads_dep = dependency('threads') |
There was a problem hiding this comment.
Does it work on Windows? Or does Meson support Windows?
There was a problem hiding this comment.
Good question.
BTW, FYI, we lost Windows test coverage recently due to the EOL of Windows 2019.
There was a problem hiding this comment.
Yes this works across platforms
There was a problem hiding this comment.
Tangentially we can look to add Meson CI for Windows. Typically the hardest part for Windows is getting the export macros correct, but if that's already taken care of it should be a trivial amount of effort
There was a problem hiding this comment.
@dongjoon-hyun Thanks for the info! It seems that we need to bump version of protobuf to make the Windows 2025 CI happy.
@WillAyd It may or may not work out of box. But I think we can postpone this until required to do.
There was a problem hiding this comment.
BTW, do you mean this which requires ORC 3.0.0 or something else, @wgtmac ?
3 participants
What changes were proposed in this pull request?
This explicitly adds the threads dependency to the orc library when built in the Meson configuration
Why are the changes needed?
In the current CI, no failure is reported because the threads dependency is transitively provided by the protobuf dependency, when protobuf is included from source. However, some older protobuf libraries do not correctly specify in their pkg-config files that threads is a transitive dependency, so if built on a system where an older protobuf version exists orc will fail to build. This is exactly the case in the Apache Arrow CI, which you can see https://github.com/apache/arrow/actions/runs/15878114506/job/44770826425?pr=46906#step:6:1669
How was this patch tested?
Compiled and tested locally
Was this patch authored or co-authored using generative AI tooling?
No