-
Notifications
You must be signed in to change notification settings - Fork 3
BLD: migrate to the mesonpy build backend #269
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
6a4eb7d to
f423c14
Compare
|
@rgommers, since you offered, would you have any clue as to what I'm doing wrong here ? |
rgommers
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.
Sure, here are some hints.
meson.build
Outdated
|
|
||
|
|
||
| # https://github.com/mesonbuild/meson/issues/9598 | ||
| incdir_numpy = run_command(py, |
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.
For the fancier version, you can copy the code block at https://github.com/PyWavelets/pywt/blob/becef5486174c77727ea2a60e6392744b2cc1d4a/pywt/_extensions/meson.build#L13
Once we get to numpy >=2.0 only (one more year ....), it becomes simply:
numpy_dep = dependency('numpy')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.
One other advantage is that the longer code block takes care of in-tree venv's . Meson by default doesn't like to see include_directories('abspath-within-source-tree'), which is usually a mistake, but in-tree venv's overlap with this.
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.
There's also a build warning related to NPY_NO_DEPRECATED_API that you can take care of with the code linked above. Warning-free builds are one of the upsides of moving to meson-python:)
The define_macros thing in setuptools has more generic alternatives in Meson. In this case, a c_args keyword for passing any argument to a C compiler. In the case of NPY_TARGET_VERSION (which doesn't need setting I think, but it can't hurt either): c_args: ['-DNPY_TARGET_VERSION=NPY_1_25_API_VERSION']
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.
Once we get to numpy >=2.0 only (one more year ....), it becomes simply:
just to be sure, you mean that the required: false argument can be removed once numpy 1 is dropped ?
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.
No I mean the 25 lines of code at https://github.com/PyWavelets/pywt/blob/becef5486174c77727ea2a60e6392744b2cc1d4a/pywt/_extensions/meson.build#L13-L43 can be replaced by the one-liner then.
To support that, I had to add the numpy-config executable, and that only landed in NumPy 2.0
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.
When dropping the required: false keyword argument, the remainder of those 25 lines can be removed due to dead code elimination. Numpy 2 will always be detectable via dependency('numpy', required: true) (the default is true) and this .found() can never evaluate to false so the code block becomes unreachable.
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 see. This specific package isn't distributed anywhere other than PyPI (that I know of), so I suppose I don't need to support building with numpy 1, but I'll try to make this PR a canonical example when I later try to migrate heavier projects, so I guess I'll keep this complexity for now. Thanks !
meson.build
Outdated
| inc_np = include_directories(incdir_numpy) | ||
|
|
||
| py.extension_module( | ||
| 'gpgi._lib', |
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 doesn't look right, it produces an extension module named gpgi._lib.cpython-xxx.so, rather than a module _lib inside a package gpgi. This will fix it:
diff --git a/meson.build b/meson.build
index fcd7a4e..53da894 100644
--- a/meson.build
+++ b/meson.build
@@ -19,8 +19,9 @@ incdir_numpy = run_command(py,
inc_np = include_directories(incdir_numpy)
py.extension_module(
- 'gpgi._lib',
+ '_lib',
'src/gpgi/_lib.pyx',
install: true,
include_directories : [inc_np],
+ subdir: 'gpgi',
)|
I'm still having a hard time figuring out what I need to do to make this work. This might be a silly question but since I can't find an example that does this in the wild: @rgommers, could you confirm that meson-python supports src layouts ? |
|
Yes, src layout should be supported. Let me try and find some time tonight and test this branch locally again. The basic points are:
I try this locally with: $ meson setup build --prefix=$PWD/build-install
$ ninja -C build
$ meson install -C buildThat gives easier to introspect results than going via |
meson.build
Outdated
| endif | ||
|
|
||
| py.extension_module( | ||
| '_lib', |
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.
Installing the cython module is a good first step. In pyproject.toml you currently have setuptools.packages.find which hasn't been ported to meson yet, so you can do that with:
py.install_sources(
'src/gpgi/foo.py',
'src/gpgi/bar.py',
subdir: 'gpgi'
)Or if you want to just install the whole subdirectory you could try:
install_subdir('src/gpgi', install_dir: py.get_install_dir())though this would install all files, including *.pyx. There is an exclude_files argument, but my personal inclination would be to list the files that do get installed, not the ones that don't get installed. I've been bitten by weird autodetection results with stale files and directories that shouldn't have existed, in the past, including "we refactored some code into a subpackage but did not run git clean so we ended up with two copies autodetected in the wheel, and the wheel had to be yanked because it was so broken and then nobody was ever able to pip install that version without building from an sdist because PyPI doesn't support fixing existing releases".
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 a lot, this seems to be the one crucial piece I was missing !
0e3bb6f to
087aa4e
Compare
|
Thanks a lot @rgommers and @eli-schwartz for your help so far !
|
|
Ninja is a C++ program, you should be able to install it via apt or brew, and for windows I guess You can also get it from pip because members of the Python community who make use of ninja in python build backends, wanted it to be easier to install / rely on its availability, so they copied the ninja C++ program into a wheel and used PyPI as a handy distribution channel. There's no particular reason why one or the other method needs to be used. If ninja is installed already, meson-python won't add it via the build-backend hook as an added build requirement. It will just use the one you already have. I'm not sure why it can't find a prebuilt free-threading wheel. The ninja wheel is "py-none-${manylinux_tag}" so it should be compatible? |
|
Wait. Why isn't it adding this to |
|
Having a look at the free-threading issue. I'm not yet friends with uv build --wheel --python $(which python)This uses build isolation, it fails because of Cython 3.0.12. After committing (not just saving the file, actually committing) this diff, the isolated build works: --- a/pyproject.toml
+++ b/pyproject.toml
@@ -1,7 +1,7 @@
[build-system]
requires = [
"meson-python",
- "Cython>=3.0",
+ "Cython>=3.1.0a1",
"numpy>=2.0.0",
]
build-backend = "mesonpy"I have For the CI job the explanation seems simpler: build isolation is disabled, and this is run: uv pip install meson-python numpy Cython
uv pip install --no-build-isolation --no-deps .
Because of |
|
@neutrinoceros for context: I'll note that |
|
The Windows CI failure is fixed by changing the first line of You have to add the Passing CI log: https://github.com/rgommers/gpgi/actions/runs/13319374517/job/37200822857
It may be useful to force it to show the log, since the details there (versions of compilers, numpy, etc.) are often helpful. |
Oh right, duh. 🤡 No idea why I blanked on that. |
@rgommers how about this ? |
|
ah that works, thanks! |
9fb4420 to
529f65c
Compare
|
I'm attempting to re-implement my build-time option, previously implemented via an env var ( |
0b4aff5 to
d8a3f37
Compare
|
https://mesonbuild.com/meson-python/reference/config-settings.html
You are using the latter but I think you want the former... |
|
You'll want In general, custom options in a |
You're right; I think I'm simply confused by the amount of novelty I'm facing and trying to combine. I'm sure I'll get it sorted out in the end, and I cannot thank you enough for your support thus far! I finally managed to get the pure-python build to work on CI by ... disabling In the mean time, I just added a comment to the issue about lock files and build-time dependencies, which I think should be relevant to anyone willing to combine |
33445b9 to
d143e89
Compare
This used to be something that |
d143e89 to
ffc38dd
Compare
|
While inspecting distribution artifacts in search for missing files (or files wrongfully included), I discovered that the generated I would assume that this is because meson-python defaults to a release build while, presumably, setuptools doesn't, but I couldn't check this hypothesis reading through its docs yet. |
|
That one is complicated. Setuptools can inherit default flags from cpython itself instead of using e.g. $CFLAGS, which means that it may be getting debug info settings from however cpython was built, and generally also gets It's honestly possible this is almost entirely down to the debug info alone. You could try stripping the setuptools versions to check if that makes them closer to the size produced by meson. |
|
Indeed it is apparent that setuptools calls gcc with
|
|
Good news:) Re build flags, there's Here's the output for this project for Details[
{
"name": "_lib.cpython-313-darwin",
"id": "_lib.cpython-313-darwin.so@sha",
"type": "shared module",
"defined_in": "/Users/rgommers/code/tmp/gpgi/meson.build",
"filename": [
"/Users/rgommers/code/tmp/gpgi/build/_lib.cpython-313-darwin.so"
],
"build_by_default": true,
"target_sources": [
{
"language": "c",
"compiler": [
"arm64-apple-darwin20.0.0-clang"
],
"parameters": [
"-I/Users/rgommers/code/tmp/gpgi/build/_lib.cpython-313-darwin.so.p",
"-I/Users/rgommers/code/tmp/gpgi/build",
"-I/Users/rgommers/code/tmp/gpgi",
"-I/Users/rgommers/mambaforge/envs/scipy-dev-313/lib/python3.13/site-packages/numpy/_core/include",
"-I/Users/rgommers/mambaforge/envs/scipy-dev-313/include/python3.13",
"-fvisibility=hidden",
"-fdiagnostics-color=always",
"-Wall",
"-Winvalid-pch",
"-O0",
"-g",
"-ftree-vectorize",
"-fPIC",
"-fstack-protector-strong",
"-O2",
"-pipe",
"-isystem",
"/Users/rgommers/mambaforge/envs/scipy-dev-313/include",
"-D_FORTIFY_SOURCE=2",
"-isystem",
"/Users/rgommers/mambaforge/envs/scipy-dev-313/include",
"-DNPY_TARGET_VERSION=NPY_1_25_API_VERSION",
"-DNPY_NO_DEPRECATED_API=NPY_1_25_API_VERSION"
],
"sources": [],
"generated_sources": [
"/Users/rgommers/code/tmp/gpgi/build/_lib.cpython-313-darwin.so.p/src/gpgi/_lib.pyx.c"
],
"unity_sources": []
},
{
"linker": [
"arm64-apple-darwin20.0.0-clang"
],
"parameters": [
"-L/Users/rgommers/mambaforge/envs/scipy-dev-313/lib",
"-Wl,-dead_strip_dylibs",
"-Wl,-headerpad_max_install_names",
"-Wl,-undefined,dynamic_lookup",
"-bundle",
"-Wl,-undefined,dynamic_lookup",
"-Wl,-headerpad_max_install_names",
"-Wl,-dead_strip_dylibs",
"-Wl,-rpath,/Users/rgommers/mambaforge/envs/scipy-dev-313/lib",
"-ftree-vectorize",
"-fPIC",
"-fstack-protector-strong",
"-O2",
"-pipe",
"-isystem",
"/Users/rgommers/mambaforge/envs/scipy-dev-313/include",
"-D_FORTIFY_SOURCE=2",
"-isystem",
"/Users/rgommers/mambaforge/envs/scipy-dev-313/include"
]
}
],
"extra_files": [],
"subproject": null,
"dependencies": [
"dep4401978960",
"numpy",
"python-3.13"
],
"depends": [],
"installed": true,
"install_filename": [
"/usr/local/lib/python3.13/site-packages/gpgi/_lib.cpython-313-darwin.so"
]
}
](disclaimer: for the result to be accurate for wheel builds, you should run it on a builddir generated by
In my experience, that continues happening once in a while. Recent example: we just started using and set up AddressSanitizer and ThreadSanitizer CI jobs in NumPy, and found that those had built-in support in Meson, it's a single argument to pass and things just worked. |
ca8a4a7 to
023f466
Compare
|
Last point on my list before I call this a success: diff --git a/meson.build b/meson.build
index dd69fe8..34c3889 100644
--- a/meson.build
+++ b/meson.build
@@ -58,6 +58,7 @@ else
install: true,
dependencies : [np_dep],
c_args: numpy_compile_flags,
+ limited_api: '3.11', # keep in sync with requires-python (pyproject.toml)
)
endif
diff --git a/pyproject.toml b/pyproject.toml
index ed6aff0..afc895b 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -20,6 +20,7 @@ classifiers = [
"Programming Language :: Cython",
"Programming Language :: Python :: 3",
]
+# keep in sync with limited_api (meson.build)
requires-python = ">=3.11"
dependencies = [
# keep in sync with build-time requirements (dev group)and built locally with Cython 3.1 (master). The resulting wheel isn't actually tagged as |
|
You'll want to tell
|
023f466 to
7330948
Compare
|
Thanks ! I'm getting closer with the following patch diff --git a/meson.build b/meson.build
index dd69fe8..34c3889 100644
--- a/meson.build
+++ b/meson.build
@@ -58,6 +58,7 @@ else
install: true,
dependencies : [np_dep],
c_args: numpy_compile_flags,
+ limited_api: '3.11', # keep in sync with requires-python (pyproject.toml)
)
endif
diff --git a/pyproject.toml b/pyproject.toml
index e8eb87b..8e0c752 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -20,6 +20,7 @@ classifiers = [
"Programming Language :: Cython",
"Programming Language :: Python :: 3",
]
+# keep in sync with limited_api (meson.build)
requires-python = ">=3.11"
dependencies = [
# keep in sync with build-time requirements (dev group)
@@ -79,6 +80,9 @@ dev = [
"numpy>=1.25.0, <3", # keep in sync with runtime requirement
]
+[tool.meson-python]
+limited-api = true
+
[tool.uv]
no-build-isolation-package = ["gpgi"]However:
is this expected, am I still anything else, or is this a bug in meson-python that I should report ? |
|
Is that the whole patch? I'm getting this, when building in a 3.13 env: Cython's Limited C API support is still WIP, so you may see something different in another env. The |
|
I did install Cython from source. The |
|
I'm not sure how abi3audit works, but I was under the impression that abi3 notably doesn't have any version tagging at all, and version tagging is handled solely through the wheel filename (which should match the highest in-use ABI for all abi3 extensions in the project). Where on earth is it getting "3.2" from? Does it simply report vague nonsense when run directly on a The only thing necessary here should be to rename the wheel file in accordance with 3.11, then audit the wheel as a whole. |
|
Thanks, should have remembered that Cython is WIP. After installing Cython master: File name diff --git a/meson.build b/meson.build
index dd69fe8..03ea4f6 100644
--- a/meson.build
+++ b/meson.build
@@ -56,6 +56,7 @@ else
'src/gpgi/_lib.pyx',
subdir: 'gpgi',
install: true,
+ limited_api: '3.11',
dependencies : [np_dep],
c_args: numpy_compile_flags,
)
diff --git a/pyproject.toml b/pyproject.toml
index e8eb87b..3f87e7f 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -177,3 +177,6 @@ archs = "auto64"
[tool.cibuildwheel.windows]
archs = "AMD64"
+
+[tool.meson-python]
+limited-api = trueAnd |
|
Oh, I think I know what happened: did you use |
|
From the abi3audit README:
That seems... wrongly designed, dunno what else to say. |
no, I indeed committed the change, but thanks for pointing this out, I didn't realize that uncommitted changes wouldn't make it to the artifact. It also pushed me in the right direction: I think the reason I got a broken build was that I had some meson-setup generated files left over from a previous build. Running again from a clean slate I get the expected, correct result !
Oh dear... I really should have paid more attention to that readme. Sorry about that ! Anyway, I think we're all done here, and I'm now convinced this is a net win. |
|
Awesome 🎉 |
I don't think it's your fault, honestly. If we look at their readme... It seems trivially obvious at least to me, that given a
event, it is wrongly designed to respond with
and needlessly causes deep confusion and potential panic and threatens to undermine the fundamental goal of having an abi checker application in the first place. The straightforward and logical design would be, instead of this: to do this: |
This is highly experimental and I'm not sure I'll merge this, but this is a dump of my current progress.
TODO:
NPY_TARGET_VERSION)numpy.include()if possiblesetuptools-specific stuff (setup.py, MANIFEST.in)GPGI_PY_LIBif I can