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

[RFC] common/build-helper/meson.sh: new build helper, used by meson build style #46117

Merged
merged 3 commits into from
Sep 22, 2023

Conversation

ahesford
Copy link
Member

The python3-scipy package now builds with python3-meson-python, with some more coming down the pike. The Python build process is necessary to include Python package metadata. For SciPy, this is accomplished by using the meson build style to configure a build directory, then overriding do_build and do_install to build things the PEP517 way (using the pre-configured build directory).

It might be helpful to define a meson build helper that can write out the standard cross file for builds, allowing packages like SciPy to use the python3-pep517 build style with the meson helper. There are some issues to be worked out:

  1. It seems like the xbps_meson.cross is written before CFLAGS, et al. are properly defined, so this cross file is actually broken. (This is the process by which the qmake helper writes its configurations, and they reference CFLAGS and others; is the qmake helper writing broken configs?)
  2. The meson configuration (which would need to happen even with python3-pep517 plus a meson helper) still includes a lot of magic that would have to be brought over to templates. This isn't really easier than the current practice of overriding the one-line do_build and do_install.

A possible solution would be to define a special-purpose function in the helper, called vmesoncross or somesuch, that will only write the configured when called---from a build function where these are properly defined (e.g., pre_configure, do_configure). We could also define a special do_configure-like function in the helper that can be used by the meson style to actually do the configuration. This might be a bit ugly, but avoids repetition or violating the typical practice that the actual build functions are only defined in templates or build styles.

A third option is just defining a new python3-meson build style that mashes the meson do_configure bits from the meson style with the do_build and do_install bits from the python3-pep517 style.

All of these options are sub-optimal, but I'm not sure which one is least offsensive.

@ahesford ahesford marked this pull request as draft September 18, 2023 19:53
@tornaria
Copy link
Contributor

An idea to consider: adding a hook in common/hooks/pre-configure (or similar) that applies this when build_helper includes meson, etc. similar to hooks/pre-configure/00-gnu-configure-asneeded.sh etc.

@tornaria
Copy link
Contributor

Please see my proof of concept in #46109, which seems to be working for python3-contourpy. I didn't try the same approach with scipy (or numpy 1.26 which is also moving to meson).

@ahesford
Copy link
Member Author

Alright, it turns out that meson respects $CFLAGS, $LDFLAGS and $CXXFLAGS in the environment at configure time, so we don't need to write them to the cross file; leaving these out produces a valid cross file and lets us customize the flags in templates before do_configure is run.

As for re-using do_configure, this might not be necessary for the few packages that will be moving to mesonpy PEP517 builds in the future, so I'm going to ignore it now.

I've updated python3-scipy to use the new helper in conjunction with the python3-pep517 build style, and will be rebuilding all of the packages with build_style=meson to confirm that this doesn't break anything.

@tornaria
Copy link
Contributor

Looks nice. I will try it on python3-contourpy.

Random thoughts:

  1. Would it make sense to set -Csetup-args=--cross-file=${XBPS_WRAPPERDIR}/meson/xbps_meson.cross in the python3-pep517 style (only if using meson helper), or is that "too much magic"?
  2. Would it make sense to place the configuration for numpy and
    pythran in xbps_meson.cross itself? I assume unused properties are harmless.
  3. The helper seems to be called several times at different stages, so the crossfile would be written several times, maybe with different content? Could this be an issue in some case?

@ahesford ahesford changed the title [RFC, WIP] common/build-helper/meson.sh: new build helper, used by meson build style [RFC] common/build-helper/meson.sh: new build helper, used by meson build style Sep 21, 2023
@ahesford ahesford marked this pull request as ready for review September 21, 2023 00:35
@ahesford
Copy link
Member Author

Well I didn't build all of them, but I built a large subset and have no reason to believe this breaks any cross builds.

@ahesford
Copy link
Member Author

ahesford commented Sep 21, 2023

  1. Would it make sense to set -Csetup-args=--cross-file=${XBPS_WRAPPERDIR}/meson/xbps_meson.cross in the python3-pep517 style (only if using meson helper), or is that "too much magic"?

I think adding this argument to the PEP517 style is reasonable. It shouldn't be "too much magic" because it confines its manipulations to the actual build functions where this kind of thing is expected.

  1. Would it make sense to place the configuration for numpy and
    pythran in xbps_meson.cross itself? I assume unused properties are harmless.

This would probably better in the numpy build helper, which can check if the meson helper is also enabled and write a second cross file. We can then add another check in the PEP517 helper to automatically add the second file to the build arguments.

  1. The helper seems to be called several times at different stages, so the crossfile would be written several times, maybe with different content? Could this be an issue in some case?

I thought about this and decided to follow the precedent of the qmake helper, but we could also add a check to avoid writing the file if it already exists. I'm not sure it matters.

While I'm inclined to adopt the numpy and pep517 improvements, let's consider those in a separate PR after this more general change is merged.

@ahesford ahesford merged commit 8979397 into void-linux:master Sep 22, 2023
8 checks passed
@ahesford ahesford deleted the meson branch September 22, 2023 14:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
xbps-src xbps-src related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants