Skip to content
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

BLD: avoid running run_command(py3, ...), for better cross-compiling #18034

Merged
merged 1 commit into from Mar 6, 2023

Conversation

rgommers
Copy link
Member

@rgommers rgommers commented Feb 23, 2023

Adding the ability to specify the paths to the numpy, pybind11 and pythran include directories was discussed in gh-14812. Short of pkg-config files for these packages and/or builtin support for them in Meson, this is a decent solution.

Closes gh-16783.

To do:

  • verify that this indeed solves the problem for conda-forge (Cc @h-vetinari) or another packaging system
  • [ ] add a CI job for cross-compiling (probably Linux x86-64 -> aarch64)

@rgommers rgommers added enhancement A new feature or improvement Meson Items related to the introduction of Meson as the new build system for SciPy labels Feb 23, 2023
@rgommers rgommers marked this pull request as draft February 23, 2023 15:21
@eli-schwartz
Copy link
Contributor

eli-schwartz commented Feb 23, 2023

Note that I've coded a custom dependency lookup for pybind11 (and made one PR to upstream to make pybind11-config --version work). I'll be submitting it to meson shortly. So you don't need that one anymore once meson 1.1.0 is out. It will pick up a pkg-config/cmake version if NOT installed via pip, and it will pick up a config-tool version if installed via build isolation (the same way the meson command is locatable) and of course you can override the program location via a machine file in a cross build, to point to a native version.

@rgommers
Copy link
Member Author

Thanks @eli-schwartz, that's great. I did see your PR on the pybind11 repo. Seeing your upcoming Meson PR will also help me understand how that works better, so I can implement dependency('numpy') and we don't need this manual hardcoding of paths approach anymore. For now I think it's still helpful to merge this.

@ahesford
Copy link

I can cross-build SciPy with Meson on Void Linux using (a roughly backported) version of these changes: ahesford/void-packages@5e5bae2. This seems like a straightforward way to solve the problem.

@rgommers rgommers added the backport-candidate This fix should be ported by a maintainer to previous SciPy versions. label Feb 24, 2023
@rgommers
Copy link
Member Author

That's great, thanks for testing @ahesford!

@rgommers rgommers added this to the 1.11.0 milestone Feb 24, 2023
@rgommers rgommers marked this pull request as ready for review February 26, 2023 16:44
@rgommers
Copy link
Member Author

This has been tested now, and since we need it also for conda-forge and it's going to take a while before I'll be able to add a new CI job for this, I think we should merge this as is now.

@rgommers
Copy link
Member Author

@ahesford I was interested in the rest of how Void Linux specifies dependency information. I'm not sure I understand this completely:

build_helper="numpy"
hostmakedepends="gcc-fortran python3-setuptools pythran python3-Cython python3-pybind11 pkg-config"
makedepends="python3-devel python3-pybind11 pythran $(vopt_if openblas openblas-devel lapack-devel)"
depends="python3-numpy"

Questions:

  • Why isn't python3-numpy present in makedepends or hostmakedepends? Does depends mean "both of those", or is depends the runtime dependency and the build_helper bit covers the build dep?
  • Why is pybind11 present on host? I'd think it should be in makedepends only.
  • Is there any sort of "virtual dependency" concept, like on BLAS/LAPACK? The openblas handling looks sort of like that, but there's more to it than switching the name in build_options="openblas" it seems.

@eli-schwartz
Copy link
Contributor

Why isn't python3-numpy present in makedepends or hostmakedepends? Does depends mean "both of those", or is depends the runtime dependency and the build_helper bit covers the build dep?

depends is, according to Manual.md, not installed for building at all.

The build_helper does handle this though. A build_helper is loaded from the parent directory of this particular build_helper, which is https://github.com/void-linux/void-packages/blob/master/common/build-helper/numpy.sh

It adds additional host/makedepends, and injects some CFLAGS/LDFLAGS.

@ahesford
Copy link

As @eli-schwartz notes, the build_helper="numpy" causes NumPy to be pulled into hostmakedepends and, for cross builds, makedepends. The helper also manually injects include and library paths so they appear before those of the build architecture (in Void, we call the build machine the "host" and use "target" for the union of "host" and "target" terminology used in Meson). Next, we ensure that the build system can find a FORTRAN compiler.

The hostmakedepends are those that must be available for the build machine and will generally be run during the build process. The makedepends are those that provide headers and libraries for the target architecture that will be compiled into the package contents. As you observed, makedepends is the right place for python3-pybind11 here because we need the target headers. However, for a lot of Python packages, we also need these things in hostmakedepends because the build machine tends to run a Python interpreter to check for the availability of these packages. This is generally true for packages that build with setuptools (and is a holdover of the current python3-scipy template), but is probably no longer necessary for Meson.

The depends list is, as noted, a collection of runtime dependencies. The xbps-src build system will automatically detect shared-library dependencies, so our practice is not to include those in the list. Hence, you do not see things like openblas here.

XBPS does support virtual dependencies, but it's kind of hacky and difficult to control. (The biggest problem with XBPS virtuals is that it's possible to define a default provider for each virtual only in the build system; the package repository has no concept of a preferred default provider, so you will get whatever is listed first in the repository. Replacing virtuals with other providers also doesn't quite work right.) That isn't the mechanism at use here. Instead, note the line in makedepends:

$(vopt_if openblas openblas-devel lapack-devel)

This function willl substitute openblas-devel as a makedepends if openblas is set; it will substitute lapack-devel otherwise. Thus, turning off the option will pull in our standard lapack and, transivitively, blas. Later on, in the pre_build function, I write a site.cfg that refers to the proper BLAS/LAPACK implementation. I suspect that this, like the python3-pybind11 entry in hostmakedepends, is no longer necessary. However, the changes I made were quick and dirty, intended only to test your Meson fix. Once the fix lands in a release (and maybe not until setuptools support is dropped), I'll do a proper template cleanup and officially switch to Meson.

@rgommers
Copy link
Member Author

Thanks a lot @ahesford and @eli-schwartz, very helpful! I'm trying to wrap my head around how this works across different distros, and figure out some cross compilation improvements that we can make in Python packaging land. I added Void Linux to a page on this topic at pypackaging-native/pypackaging-native#27.

Adding the ability to specify the paths to the numpy, pybind11 and
pythran include directories was discussed in scipygh-14812. Short of
pkg-config files for these packages and/or builtin support for them in
Meson, this is a decent solution.
@rgommers
Copy link
Member Author

rgommers commented Mar 6, 2023

This was reviewed and shown to solve the problem for Void Linux. I also need this for next steps, so I'll go ahead and self-merge it.

@rgommers rgommers merged commit 6a4b15c into scipy:main Mar 6, 2023
@rgommers rgommers deleted the cross-include-paths branch March 6, 2023 14:40
@tylerjereddy
Copy link
Contributor

Merge conflicts in meson.build vs. maintenance/1.10.x are nasty, punting on backporting this at the moment, but I'll make a note of this when I open the backport PR in case someone wants to manually pull in the change.

@tylerjereddy tylerjereddy removed the backport-candidate This fix should be ported by a maintainer to previous SciPy versions. label Jun 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement A new feature or improvement Meson Items related to the introduction of Meson as the new build system for SciPy
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BUG: meson fails to properly detect numpy, pybind11 and (sort of) pythran when cross compiling
4 participants