fix: Assorted updates to improve Meson WrapDB entry #803
Conversation
meson.build
Outdated
| version: '0.8.0-SNAPSHOT', | ||
| license: 'Apache 2.0', | ||
| meson_version: '>=1.3.0', | ||
| meson_version: '>=1.1.0', |
There was a problem hiding this comment.
The Meson team advises against artificially raising the version requirement, so that as many people can use the wrap as possible.
We require version 1.1.0 because we have meson.options in the root. If we were to rename that to meson_options.txt, we could go even further back.
With that said, 1.1.0 came out in April 2023, so I think its a reasonable enough floor for now
There was a problem hiding this comment.
meson.options is undoubtedly a nicer name but IMO it's not worth constraining the Meson version just to avoid meson_options.txt.
There was a problem hiding this comment.
Roger that - dropped to 0.58 as a minimum then, which we need for str.replace in the config
| compile_args: nanoarrow_dep_args, | ||
| ) | ||
|
|
||
| meson.override_dependency('nanoarrow', nanoarrow_dep) |
There was a problem hiding this comment.
In today's wrap we have a provides section that looks like:
[provides]
nanoarrow = nanoarrow_dep
nanoarrow_ipc = nanoarrow_ipc_depBut since Meson version 0.54.0, they have advised that a project internally call meson.override_dependency instead, so that the project has total control over their variable naming. As such, on the next release of Meson, we can simplify our wrap entry to just:
[provides]
dependency_names = [nanoarrow, nanoarrow-ipc, ...]and decouple the internal variable name from the wrap system
meson.build
Outdated
| ) | ||
| pkg.generate( | ||
| nanoarrow_ipc_lib, | ||
| filebase: 'nanoarrow-ipc', |
There was a problem hiding this comment.
I am possibly overlooking or misunderstanding the current CMake configuration, but I couldn't find anywhere that we were producing pkgconfig files.
As such, its worth double checking if we want to use hyphens or underscores for the pkgconfig file name. I am using hyphens here as that is what arrow does (ex; arrow-compute.pc)
There was a problem hiding this comment.
This comment might have been dropped by subsequent changes, so just want to make sure you see it @paleolimbot in case you disagree with the chosen naming convention
There was a problem hiding this comment.
I think the arrow convention with hyphens makes sense!
WillAyd
force-pushed
the
meson-wrapdb-updates
branch
from
da7cd9d to
cacb8e1
Compare
|
@bgilbert if you have anything else to add here let me know. Happy to make any changes you think are best for the wrap system |
meson.build
Outdated
| version: '0.8.0-SNAPSHOT', | ||
| license: 'Apache 2.0', | ||
| meson_version: '>=1.3.0', | ||
| meson_version: '>=1.1.0', |
There was a problem hiding this comment.
meson.options is undoubtedly a nicer name but IMO it's not worth constraining the Meson version just to avoid meson_options.txt.
meson.build
Outdated
| link_with: nanoarrow_ipc_lib, | ||
| dependencies: [nanoarrow_dep], | ||
| ) | ||
| meson.override_dependency('nanoarrow-ipc', nanoarrow_dep) |
There was a problem hiding this comment.
| meson.override_dependency('nanoarrow-ipc', nanoarrow_dep) | |
| meson.override_dependency('nanoarrow-ipc', nanoarrow_ipc_dep) |
meson.build
Outdated
| 'c', | ||
| 'cpp', | ||
| version: '0.8.0-SNAPSHOT', | ||
| license: 'Apache 2.0', |
There was a problem hiding this comment.
SPDX license expression is preferred.
| license: 'Apache 2.0', | |
| license: 'Apache-2.0', |
WillAyd
force-pushed
the
meson-wrapdb-updates
branch
from
7341c2a to
9b8bfc3
Compare
|
Thanks @bgilbert for the guidance! |
3 participants
After having submitted more wraps and gotten feedback from the Meson core team, I found out there are a few things we can do to improve our wrap submission and make it easier to include nanoarrow in other projects