Fix CI with stricter meson-python#232
Conversation
|
Hi! This is the friendly automated conda-forge-linting service. I just wanted to let you know that I linted all conda-recipes in your PR ( |
|
@rgommers @eli-schwartz @isuruf The build runs fine (at least so it appears; no new or scary warnings from what I can tell), but then the very first import (AFAICT) during testing stumbles: |
|
We merged |
|
From gh-231: This PR: The reason is that reconfiguring an already-configured directory changed. In gh-231: In this PR: IIRC that change was on purpose, and the "run |
|
This is the current way of doing things in # need to run meson first for cross-compilation case
$PYTHON $(which meson) setup ${MESON_ARGS} \
-Dblas=blas \
-Dlapack=lapack \
-Duse-g77-abi=true \
builddir || (cat builddir/meson-logs/meson-log.txt && exit 1)
# -wnx flags mean: --wheel --no-isolation --skip-dependency-check
$PYTHON -m build -w -n -x -Cbuilddir=builddirI think we need to change it to: # -wnx flags mean: --wheel --no-isolation --skip-dependency-check
$PYTHON -m build -w -n -x -Csetup-args=-Dblas=blas -Csetup-args=-Dlapack=lapack -Csetup-args=-Duse-g77-abi=trueand then append the That's all fine, but |
|
@h-vetinari, I'm guessing you know so I'll ask: is |
Yes it is. It's constructed here for osx, and here for linux.
This is going to be a general issue for all meson-python feedstocks, right? Perhaps it's a good idea to have a |
The setup script for macOS uses bash, but the one for Linux indicates it tries to be compatible with POSIX sh. So no bash goodies. (Like arrays!!! Arrays are really, really, really nice.) |
That sounds quite nice, +1 from me. |
|
I'm not a fan of adding a new variable to a big package like the compilers just for 2 feedstocks as meson-python is pretty niche right now. You can add the one liner |
|
Looks like it's 7 feedstocks right now: https://github.com/search?q=org%3Aconda-forge%20meson-python&type=code. And that will grow: PyWavelets completed the switchover, pandas is very close, numpy will follow too before Python 3.12. More smaller packages too I'd expect. That said, if it's preferred to carry this one-liner (thanks for that providing it!) in the relevant feedstocks, that should work perfectly fine too. |
Co-Authored-By: Isuru Fernando <isuruf@gmail.com> Co-Authored-By: Ralf Gommers <ralf.gommers@gmail.com>
|
OK, it seems the scipy setup is setting Guess I'll strip out that bit from |
|
It's meson-python doing that. Yes, should be filtered out from |
|
OK, something is still off. Despite correctly setting up the arguments now meson starts building x86 libs on aarch/ppc The one thing that I see in the invocation is the addition of |
It looks like this might be related to mesonbuild/meson-python@c8dbaee, which landed in 0.13.0, or we simply didn't pick it up due to the fact that previously it only got appended for the second call to configure (and presumably ignored). From the docs about the options, there also doesn't seem to be a way to switch of this native file, only add more to it:
I'm not sure if this is due to the not yet finished support for cross-compilation, but AFAICT, this automatic addition of |
|
Answered in mesonbuild/meson-python#321 (comment). With that fix, the logging should change from this: to showing |
Third time's the charm it seems - thanks a lot for the solution (I almost can't believe I got e1a936c correct first try 😅). From my POV this should be ready to merge. |
|
Did you see my comment on the native file part in that commit? |
No sorry, I hadn't seen that in the UI here. Sorry if I got the instructions wrong, my impression was we need to pass a user-provided native-file to override the default one? Are you saying that it should be enough to write the python host binary into the cross-file? |
Yes indeed. |
Co-Authored-By: Ralf Gommers <ralf.gommers@gmail.com>
|
|
||
| # meson-python already sets up a -Dbuildtype=release argument to meson, so | ||
| # we need to strip --buildtype out of MESON_ARGS or fail due to redundancy | ||
| MESON_ARGS_REDUCED="$(echo $MESON_ARGS | sed 's/--buildtype release //g')" |
There was a problem hiding this comment.
@h-vetinari do you think we could change MESON_ARGS to use -Dbuildtype instead? It is always documented like that (see https://mesonbuild.com/Builtin-options.html#details-for-buildtype) and is used a lot more than --buildtype if you look at the Meson issue tracker (in fact, it's hard to find --buildtype usage at all).
There was a problem hiding this comment.
We are using -Dbuildtype; the line you commented on is stripping out the alternative formulation --buildtype.
There was a problem hiding this comment.
But admittedly, that's the one coming from meson-python. We could change the spelling on the activation feedstock(s), that'd be fine by me.
There was a problem hiding this comment.
Yes, that spelling change is what I meant. That will avoid having to use this hack in all feedstocks that use meson-python.
Unexpected breakage in #200 (after #231 passed a few days ago). Investigating