-
Notifications
You must be signed in to change notification settings - Fork 124
Conversation
zhjwpku
commented
Sep 5, 2026
I'm not a meson expert, I use codex to help to align meson build with CMake, @WillAyd I'd appreciate it if you can help to review this change.
ca7ec4b to
c994090
Compare
Provide the same library modules through Meson and CMake, including the bundle, Hive, and SQL catalogs, so applications can use either build system. Keep installed static and shared libraries usable by downstream consumers and cover their public interfaces with installation checks. Closes apache#256 AI assistance was used in this change.
c994090 to
6c46eea
Compare
@WillAyd
WillAyd
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @zhjwpku . This is a rather large change - my suggestion would be to break it up into smaller pieces.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Meson provides functionality that CMake doesn't have for wrangling dependencies and choosing how they should (or shouldn't) be packaged in a source distribution / install. As such, I don't think this option makes a lot of sense to implement in Meson
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think its worth repeating these everywhere; best to keep as auto
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you know why this needs to be bumped? Generally better to keep support for older versions unless there's a critical feature
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you know why this needs to change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why does we need a CMakeLists.txt file for arrow in the meson subprojects space? There is already a Meson wrapdb entry for arrow
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this related to meson?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can avoid this branching if you use the generic build_target function and specify the target_type
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would be better expressed via -Dauto_features=enabled instead of specifying each option; that way the CI scales across all features as they get added
Provide the same library modules through Meson and CMake, including the bundle, Hive, and SQL catalogs, so applications can use either build system.
Keep installed static and shared libraries usable by downstream consumers and cover their public interfaces with installation checks.
Closes #256
AI assistance was used in this change.