feat(meson): align library modules with CMake - #912
Conversation
|
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
| # --includedir / --datadir arguments, respectively | ||
|
|
||
| option( | ||
| 'bundle', |
There was a problem hiding this comment.
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
| option( | ||
| 'hive', | ||
| type: 'feature', | ||
| value: 'disabled', |
There was a problem hiding this comment.
| value: 'disabled', |
I don't think its worth repeating these everywhere; best to keep as auto
| version: '0.4.0', | ||
| license: 'Apache-2.0', | ||
| meson_version: '>=1.3.0', | ||
| meson_version: '>=1.8.3', |
There was a problem hiding this comment.
Do you know why this needs to be bumped? Generally better to keep support for older versions unless there's a critical feature
| meson_version: '>=1.3.0', | ||
| meson_version: '>=1.8.3', | ||
| default_options: [ | ||
| 'default_library=static', |
There was a problem hiding this comment.
Do you know why this needs to change?
| cmake_minimum_required(VERSION 3.25) | ||
| project(IcebergArrow LANGUAGES C CXX) | ||
|
|
||
| # Meson's CMake module imports library targets but not header installation. |
There was a problem hiding this comment.
Why does we need a CMakeLists.txt file for arrow in the meson subprojects space? There is already a Meson wrapdb entry for arrow
| roaring::roaring) | ||
| list(APPEND | ||
| ICEBERG_STATIC_INSTALL_INTERFACE_LIBS | ||
| ZLIB::ZLIB |
| if get_option('default_library') != 'shared' | ||
| lib = get_variable(name + '_static_lib') | ||
| elif host_machine.system() == 'windows' | ||
| lib = static_library( |
There was a problem hiding this comment.
You can avoid this branching if you use the generic build_target function and specify the target_type
| CXX: g++-14 | ||
| sql-clients: true | ||
| benchmarks: true | ||
| meson-setup-args: --default-library=both --buildtype=release -Db_ndebug=true --force-fallback-for=arrow,avro -Dhive=enabled -Dsql_catalog=enabled -Dsql_sqlite=enabled -Dsql_postgresql=enabled -Dsql_mysql=enabled -Dbenchmarks=enabled -Ds3=enabled -Dsigv4=enabled |
There was a problem hiding this comment.
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
2 participants
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.