BLD: Fix features.h detection and blocklist complex trig funcs on musl #25092
Conversation
Fixes function blocklisting for glibc<2.18, reported in issue numpygh-25087. Signed-off-by: Marcel Bargull <marcel.bargull@udo.edu>
|
Could we add a test for the reproducer |
|
Thanks for adding the test, Matti! |
|
Grrr, sounds like MUSL libc bugs again, which isn't surprising, I guess. Maybe should just xfail the test on MUSL for now? |
Yes, makes sense
stack overflow says I will try it |
|
I added an |
Signed-off-by: mattip <matti.picus@gmail.com>
6e51dae to
cab680c
Compare
Yeah, not sure if we should start minimal, I doubt we can trust many of them but maybe some are good. Ideally we would see if some of them actually work, but it will probably also be version dependent. I would hope that we have a few xfailed/skipped tests that actually start working with the blocklist. |
|
I am OK with trying this, it looks a bit like we would block not just musl, but also other glibc-like implementations. It also may block too much, but... OTOH, all of that is probably very niche. MUSL seems to insists on making detection hard/impossible. I am not sure if they want to make us run the code to know if their implementations are faulty, or whether they just never thought of the fact that functions might exist but be deficient. |
|
CI passes on the last commit, so the mechanism must be working. There is issue #23049, which has this list of failures: I unskipped them, tests passed locally, so I will mark this as Edit: the issue linking is not working, when this gets merged please be sure to close that issue. |
I think erring on the side of caution is preferable. If one of those glibc implementations get annoyed at us using a non-native routine (or demi quad float precision routines), they can suggest how to unblock the functions |
|
Yeah, replacing them all for musl is probably not too bad given:
I one were to use them selectively one would have to do things like this: if cc.run(
'''
#include <complex.h>
#include <math.h>
int main(void) {
double complex x = -I;
return signbit(creal(catanh(x))) ? 0 : 1;
}
''',
dependencies: m_dep,
).returncode() == 0
# add HAVE_CATANH* defines
endifwhich isn't too bad (those compilations/checks can be run very quickly).
|
|
Hmm. More work is needed to blocklist frexp. |
|
Do we want the last 2 commits (testing more musl) cherry-picked for gh-25093 as well? |
|
The usual backport workflow starts from a successful PR to |
|
There is a (new?) test failure on macos_arm64. Does macos (based on freebsd) pick up the blocklisting? |
It should not (i.e., |
I don't really remember it recently at least, is that with openblas or accelerate? In either case, it says in matmul... try rerunning, but it seems clearly unrelated. (If it turns out non-flaky, I would ask if openblas version might have changed.) |
|
CI is passing. Let's leave the fpexp blocklist for another day, it is not connected to complex trig and was there before #23049 |
|
Looks good, thanks for working through this!
Please don't do that. @seberg is correct here:
We currently have a single place where we run non-native code (for detecting the |
Definitely, yes. |
|
I put the backport (#25093) in, it doesn't look quite the same. Does it need something more? |
As long as CI is passing I think it is fine. The test changes that were not backported, as far as I can tell, remove some skips so the main branch should be more sensitive to new failures. |
|
I think this is ready for review |
|
Yes, it looks fine, just wanted to keep it briefly in case anyone has an opinion on the details of blocklisting. I suppose backporting all of it makes sense, even if the blocklist might go a bit further than strictly necessary? |
Only the test changes were missing and I'd rather not fool with those, so not going to worry about backporting this. |
Fixes function blocklisting for glibc<2.18, reported in issue gh-25087.
closes gh-25087