Skip to content

WIP / DNM: build with meson on unix#205

Closed
h-vetinari wants to merge 33 commits into
conda-forge:mainfrom
h-vetinari:rc
Closed

WIP / DNM: build with meson on unix#205
h-vetinari wants to merge 33 commits into
conda-forge:mainfrom
h-vetinari:rc

Conversation

@h-vetinari

@h-vetinari h-vetinari commented Jun 27, 2022

Copy link
Copy Markdown
Member

I don't expect this to work yet. While upstream support meson support is still maturing, we'll probably need to make the gfortan + clang hack work again. But such a fundamental build system change seemed like a good opportunity to try to see which work-arounds are still necessary.

@conda-forge-linter

Copy link
Copy Markdown

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 (recipe) and found it was in an excellent condition.

@h-vetinari

Copy link
Copy Markdown
Member Author

Seems like conda's build isolation mechanism (with the placehold_placehold_...) is breaking ninja

ninja: error: stat(scipy/special/_specfun.cpython-310-x86_64-linux-gnu.so.p/_home_conda_feedstock_root_build_artifacts_scipy_1656354640264__h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_place_lib_python3.10_site-packages_numpy_core_include_.._.._f2py_src_fortranobject.c.o): File name too long

Quite surprised about the appearance of windows-flavoured problems on unix... 🤔

@h-vetinari

Copy link
Copy Markdown
Member Author

Seems like conda's build isolation mechanism (with the placehold_placehold_...) is breaking ninja

Interestingly, this does not seem to happen for aarch64/ppc64le, which then fail with:

  File "[...]/lib/python3.9/site-packages/scipy/_lib/_ccallback.py", line 1, in <module>
    from . import _ccallback_c
ImportError: cannot import name '_ccallback_c' from 'scipy._lib' ([...]/lib/python3.9/site-packages/scipy/_lib/__init__.py)

@h-vetinari

Copy link
Copy Markdown
Member Author

Does meson on windows only work with mingw? Surprised that MSVC seems to be detected as not working:

..\..\meson.build:1:0: ERROR: Compiler cl.exe can not compile programs.

All things considered, I'm probably going to need a bit of help on this PR @rgommers @eli-schwartz

@h-vetinari

Copy link
Copy Markdown
Member Author

Finally, for osx-arm, we might have to be more explicit about the fact that this should be cross-compiled:

../../meson.build:1:0: ERROR: Could not invoke sanity test executable: [Errno 86] Bad CPU type in executable

@rgommers

Copy link
Copy Markdown
Contributor

While upstream support meson support is still maturing, we'll probably need to make the gfortan + clang hack work again

I don't think so, since that's not what we're using on Windows for wheels - there is no build system support for this. There's two options for 1.9.0:

  1. Make the Windows build work with Mingw-w64
  2. Still build the Windows packages with numpy.distutils.

Does meson on windows only work with mingw? Surprised that MSVC seems to be detected as not working:

It works with any compiler toolchain that has coherent support for C/C++/Fortran. Which in practice is:

  1. Mingw-w64 (with a bunch of fiddling needed to deal with forcing Mingw to treat long double as 64-bit rather than 80-bit)
  2. (MSVC or ICC) + Intel Fortran

Finally, for osx-arm, we might have to be more explicit about the fact that this should be cross-compiled:

Ah, that should work but I have not dealt with the specifics of cross-compiling yet (also not for the macOS arm64 wheel builds). For documentation, see https://mesonbuild.com/Cross-compilation.html. I'm guessing we'll have to figure out what the cross file for conda-forge builds should look like.

@eli-schwartz

eli-schwartz commented Jun 28, 2022

Copy link
Copy Markdown

From the failing logs:

..\..\meson.build:1:0: ERROR: Compiler cl.exe can not compile programs.

A full log can be found at C:\bld\scipy_1656354772652\work\.mesonpy-902z_jt4\build\meson-logs\meson-log.txt

This log file contains debug information about the failed state including the command lines, input files, and console output for compiler checks.

Not sure how easy it is to get at that underneath the mesonpy build backend (it might clean up the build directory automatically? Don't know).

In general, lots of people use cl.exe instead of mingw, and the meson test matrix includes several MSVC jobs (different versions of visual studio).

@h-vetinari

Copy link
Copy Markdown
Member Author

There's two options for 1.9.0:

  1. Make the Windows build work with Mingw-w64

I'll need to defer to @isuruf if he considers that feasible here (i.e. why he chose MSVC as the main compiler when adding windows support).

  1. Still build the Windows packages with numpy.distutils.

That's obviously possible for 1.9, but not a permanent solution AFAIU - isn't non-meson support planned to be removed in 1.10?

Does meson on windows only work with mingw? Surprised that MSVC seems to be detected as not working:

It works with any compiler toolchain that has coherent support for C/C++/Fortran. Which in practice is:

Regarding compilers, it's perhaps worth considering if we can move everything to the LLVM stack on windows? We'd probably have to update the (classic-)flang builds a bit, but in principle that should be an option?

@h-vetinari

Copy link
Copy Markdown
Member Author

Thanks for the input @eli-schwartz! I think Ralf pinpointed the issue regarding the consistent toolchain, or rather the lack thereof. Windows builds here currently use an unholy combination of MSVC+gfortran, which presumably meson doesn't recognize (and arguably it shouldn't).

@rgommers

Copy link
Copy Markdown
Contributor

(i.e. why he chose MSVC as the main compiler when adding windows support).

It was the only option and matching the way SciPy wheels were built at the time.

That's obviously possible for 1.9, but not a permanent solution AFAIU - isn't non-meson support planned to be removed in 1.10?

Yes, but that's fine. That gives us 6 months to solve the problem. And either way, switching the build away from distutils should be done in a separate PR because it's a very much nontrivial change. So let's not mix it with changes for the other supported platforms (where I would not expect significant trouble).

@rgommers

Copy link
Copy Markdown
Contributor

@h-vetinari note that there's some lingering problems with wheels builds, I won't be sure that this all works as advertised until MacPython/scipy-wheels#171 is complete.

@h-vetinari

Copy link
Copy Markdown
Member Author

@h-vetinari note that there's some lingering problems with wheels builds

Normally I'd say wheel builds don't concern us directly, but now I don't know if they necessarily get built as an intermediate step in the meson builds?

In any case, I opened #206 for a distutils-based test of the rc, which looks fine except for highs on windows.

@rgommers

Copy link
Copy Markdown
Contributor

Normally I'd say wheel builds don't concern us directly, but now I don't know if they necessarily get built as an intermediate step in the meson builds?

They're not necessary technically (with meson or with distutils), but we're already using them: https://github.com/conda-forge/scipy-feedstock/blob/main/recipe/build.sh#L9-L13 as it's an easier interface.

That's not quite what I meant though. Before we have done a single release with wheels, you may run into issues here that we have not yet found (example: I just noticed that the main LICENSE.txt file will be missing, which will also affect this PR).

@h-vetinari

Copy link
Copy Markdown
Member Author

They're not necessary technically (with meson or with distutils), but we're already using them: https://github.com/conda-forge/scipy-feedstock/blob/main/recipe/build.sh#L9-L13 as it's an easier interface.

Sure, but we don't have to do anything but install them and can discard them right away (no auditwheel, etc.)

That's not quite what I meant though. Before we have done a single release with wheels, you may run into issues here that we have not yet found (example: I just noticed that the main LICENSE.txt file will be missing, which will also affect this PR).

Understood. Similarly for the HiGHS compilation error on windows I presume. Do you want me to raise an issue?

@rgommers

Copy link
Copy Markdown
Contributor

Similarly for the HiGHS compilation error on windows I presume. Do you want me to raise an issue?

No that's fine, I think that belongs on this PR, the error is expected.

@h-vetinari

Copy link
Copy Markdown
Member Author

Similarly for the HiGHS compilation error on windows I presume. Do you want me to raise an issue?

No that's fine, I think that belongs on this PR, the error is expected.

I don't understand. It happens in #206 (not this PR), and that one's using the non-meson build path, so I'd expect a green test suite everywhere, but get a build error in the new submodule (haven't found a similar issue or PR tackling that on the upstream repo).

@rgommers

Copy link
Copy Markdown
Contributor

I don't understand. It happens in #206 (not this PR), and that one's using the non-meson build path, so I'd expect a green test suite everywhere, but get a build error in the new submodule (haven't found a similar issue or PR tackling that on the upstream repo).

Ah I missed that part. In that case, yes please.

@h-vetinari

Copy link
Copy Markdown
Member Author

In that case, yes please.

Done

@rgommers

rgommers commented Jan 4, 2023

Copy link
Copy Markdown
Contributor

It's probably time to switch anything but the Windows builds to Meson now. SciPy released 1.9.3 and 1.10.0 wheels built with Meson, things should work. And the --config-settings UX change in meson-python is present now and can be used to build against the desired BLAS/LAPACK flavor for conda-forge.

WDYT @h-vetinari?

@h-vetinari

Copy link
Copy Markdown
Member Author

WDYT @h-vetinari?

SGTM!

I wanted to get 1.10 out the door (and the CI was green...), before going down the rabbit hole of how much changes will actually be necessary.

@rgommers

rgommers commented Jan 4, 2023

Copy link
Copy Markdown
Contributor

I'd expect very little, leaving aside the Windows issues:

  • switch back to pip install . use python -m build in build.sh
  • use the --use-g77-abi CLI flag instead of the env var
  • specify blas and lapack through CLI options

So (untested) something like:

# -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=true
pip install dist/scipy*.whl

(yes this is a bit ugly, and can't use pip because it doesn't accept multiple times the same key for --config-settings - see mesonbuild/meson-python#230 (comment))

@h-vetinari h-vetinari changed the title WIP: scipy 1.9.0rc's, now with meson WIP: build with meson on unix Jan 27, 2023
@conda-forge-webservices

Copy link
Copy Markdown

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 (recipe) and found it was in an excellent condition.

@h-vetinari

Copy link
Copy Markdown
Member Author

So (untested) something like:

# -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=true
pip install dist/scipy*.whl

Good guess, this worked on unix in native compilation.

I guess it's expected that cross-compilation fails?

ERROR Backend subprocess exited when trying to invoke build_wheel

Happy to dig into this of course, but if you have some references handy of where I need to tackle this, that would be very nice. :)

@rgommers

Copy link
Copy Markdown
Contributor

Relevant part of the build log:

+CXX=$BUILD_PREFIX/bin/aarch64-conda-linux-gnu-c++
-CXX=$PREFIX/bin/aarch64-conda-linux-gnu-c++
-DEBUG_CXXFLAGS=-fvisibility-inlines-hidden -fmessage-length=0 -ftree-vectorize -fPIC -fstack-protector-all -fno-plt -Og -g -Wall -Wextra -fvar-tracking-assignments -pipe -isystem $PREFIX/include -fdebug-prefix-map=$SRC_DIR=/usr/local/src/conda/scipy-1.10.0 -fdebug-prefix-map=$PREFIX=/usr/local/src/conda-prefix
+DEBUG_CXXFLAGS=-fvisibility-inlines-hidden -fmessage-length=0 -ftree-vectorize -fPIC -fstack-protector-all -fno-plt -Og -g -Wall -Wextra -fvar-tracking-assignments -pipe -isystem $PREFIX/include -fdebug-prefix-map=$SRC_DIR=/usr/local/src/conda/scipy-1.10.0 -fdebug-prefix-map=$PREFIX=/usr/local/src/conda-prefix -fvisibility-inlines-hidden -fmessage-length=0 -ftree-vectorize -fPIC -fstack-protector-all -fno-plt -Og -g -Wall -Wextra -fvar-tracking-assignments -pipe -isystem $PREFIX/include -fdebug-prefix-map=$SRC_DIR=/usr/local/src/conda/scipy-1.10.0 -fdebug-prefix-map=$PREFIX=/usr/local/src/conda-prefix
+GXX=$BUILD_PREFIX/bin/aarch64-conda-linux-gnu-g++
-GXX=$PREFIX/bin/aarch64-conda-linux-gnu-g++
Setting up cross-python
Finished setting up cross-python
+ /home/conda/feedstock_root/build_artifacts/scipy_1674797616497/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_place/bin/python -m build -w -n -x -Csetup-args=-Dblas=blas -Csetup-args=-Dlapack=lapack -Csetup-args=-Duse-g77-abi=true
* Building wheel...
+ meson setup --prefix=$PREFIX $SRC_DIR $SRC_DIR/.mesonpy-8brkxeub/build --native-file=$SRC_DIR/.mesonpy-native-file.ini -Ddebug=false -Doptimization=2 -Dblas=blas -Dlapack=lapack -Duse-g77-abi=true

We're not telling meson that we're cross-compiling here. That needs a cross file, see https://mesonbuild.com/Cross-compilation.html. I think conda-forge has a way of doing that already for Meson. General docs are at https://conda-forge.org/docs/maintainer/knowledge_base.html#cross-compilation, but they don't tell us enough. https://conda-forge.org/blog/posts/2020-10-29-macos-arm64/ says to add meson ${MESON_ARGS} builddir/, and that should fix things. I hope that's still unchanged.

Note: if that works, then it's likely that there will be a problem with libnpymath, I've seen that before when cross-compiling. Not sure though, let's figure that out if and when it happens.

@h-vetinari

Copy link
Copy Markdown
Member Author

I'm a little fuzzy though on why there's more than one version of meson installed to begin with. Does by default every build dependency get installed for both build and host?

No, only as specified in build/host envs. It's possible we need meson only in build...

@rgommers

Copy link
Copy Markdown
Contributor

It's possible we need meson only in build...

Yes indeed, that should be the case when we're cross-compiling. Same for other build tools and code generators, like ninja, meson-python, cython. And probably pythran too (I'm double-checking that one).

@h-vetinari

Copy link
Copy Markdown
Member Author

So far, cython was in both build and host even when cross-compiling. I'll try moving meson & ninja to build-only first. Probably will take a bit of iteration, but have to drop off here for now.

@h-vetinari

Copy link
Copy Markdown
Member Author

Mainly I'm waiting for the patch for mesonbuild/meson-python#321 to land, so we can backport it to conda-forge's meson. I think with that we'd solve things (as of c9afb89).

In the meantime, trying scipy/scipy#18034 + specifying the include directories, I get

scipy/meson.build:81:0: ERROR: Include dir $PREFIX/lib/python3.9/site-packages/numpy/core/include does not exist.

even if those definitely exist.

@rgommers

Copy link
Copy Markdown
Contributor

ERROR: Include dir $PREFIX

The PREFIX variable here doesn't look like it's substituted for the actual prefix. The string shown by Meson should be an absolute path.

@h-vetinari

h-vetinari commented Feb 25, 2023

Copy link
Copy Markdown
Member Author

The PREFIX variable here doesn't look like it's substituted for the actual prefix. The string shown by Meson should be an absolute path.

That was my suspicion as well, but with the last debug commit, I'm using the same vanilla sed that worked before, and I'm 99% certain the content in the crossfile is correct, i.e. an absolute path. (It should work at least for python 3.9, because I had to choose one python version to replace SP_DIR with)

PS. It's confusing that cat will sometimes replace expanded path variables back to environment variables.

@h-vetinari h-vetinari changed the title WIP: build with meson on unix WIP / DNM: build with meson on unix Mar 7, 2023
@isuruf

isuruf commented Mar 27, 2023

Copy link
Copy Markdown
Member

The PREFIX variable here doesn't look like it's substituted for the actual prefix. The string shown by Meson should be an absolute path.

conda-build captures stdout and replaces the values with $PREFIX. It makes the logs much easier to read, but might be confusing to new users.

@h-vetinari

Copy link
Copy Markdown
Member Author

FYI: I opened up a cleaned up version of this PR in #231, with the intention of merging it in the near future.

@isuruf, after rebasing out the later-reverted-anyway bf43134, the modifications you made in aa19c00 & c4a44be only needed to be done once each, rather than twice (because there's now one less meson-invocation). To my mind, the adaptation to this fact does not impact the faithfulness of the rebase to your changes, but please let me know if you feel otherwise.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants