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

Add meson awareness to numpy helper and python3-pep517 style #46186

Closed
wants to merge 4 commits into from

Conversation

ahesford
Copy link
Member

Now that a meson build helper exists, we can further remove redundancy in some numeric python packages. In addition to the two cleaned up here, I believe we have some pending PRs that would benefit from these changes.

While I'm touching the numpy helper, I might as well fix its missing documentation.

cc: @tornaria

Testing the changes

  • I tested the changes in this PR: YES

@tornaria
Copy link
Contributor

LGTM, do you want to cleanup make_build_args in pylint?

A (very) minor nitpick: maybe [[ "${build_helper}" = *meson* ]] should be [[ " ${build_helper} " = *" meson "* ]], etc.

But the idiom is used all over the place and currently there is no conflict possible.

@ahesford
Copy link
Member Author

A (very) minor nitpick: maybe [[ "${build_helper}" = *meson* ]] should be [[ " ${build_helper} " = *" meson "* ]], etc.

Adding spaces might thwart a match when the variable embeds a newline, like

build_helper="
 meson
 numpy
"

which probably never ought to happen, but could cause hard-to-discover issues when it does. What we really need is to turn these space-separated variables into proper arrays so that we can test for membership the right way.

Thanks for reminding me about pylint; I'll add a fix here.

@tornaria
Copy link
Contributor

You may want to cherry-pick 0a2141b from #46109, to further cleanup the scipy template (no need for the pybind11 thing).

I wonder about this:

	if [ "$CROSS_BUILD" ]; then
		# Meson can't tolerate $CC with arguments as set by the build helper
		CC="${XBPS_CROSS_TRIPLET}-gcc"
		# CXX needs to know where to find Python headers
		CXXFLAGS+=" -I${XBPS_CROSS_BASE}/${py3_inc}"
	fi

Is this necessary? I didn't add this to python3-contourpy and I had no trouble... This block is there both in scipy and scikit-image.

@tornaria
Copy link
Contributor

Aha, it probably refers to the python3 build helper, which is no longer used...

@ahesford
Copy link
Member Author

The python3 build helper is automatically pulled in by the python3 build styles. I haven't tested whether it can be removed now, but it was definitely needed when it was added.

@ahesford
Copy link
Member Author

Closed in 0fcff10 with some additional changes:

@ahesford ahesford closed this Sep 23, 2023
@ahesford ahesford deleted the meson-numpy branch September 23, 2023 16:52
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.

None yet

2 participants