-
-
Notifications
You must be signed in to change notification settings - Fork 10.9k
BLD: use macos-11 image on azure, macos-1015 is deprecated #22043
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
Conversation
Good idea to update. Some of the The |
The error is a failure to compile. Do the tests work locally on macOS 11?
|
Yes, but macOS 11 isn't really a thing many people use; it was followed by 12.0 shortly after, and we're at 12.4 now. There is gh-20677 which looks like the same problem. However, it's kind of weird in this PR that the regular build works but the test suite fails on compiling something. So perhaps it has to do with the way the |
What would be the downside of moving to a |
For |
Even with macOS-12 the error of
|
I think you can go back to This looks suspicious but is no longer used, so doesn't seem like the problem:
Somehow |
On a mac, I see sysconfig variables
|
Ah, it depends on the Python install:
|
Trying to reproduce this on a local M1 machine leads me down a rabbit hole. When I build NumPy with |
macOS passed in 07a4352. I am not sure why. Trying again with a little less debugging. |
CI is passing, twice (both 07a4352 and the latest commit) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, recommend adding a comment but overall good to merge.
displayName: 'Run Full NumPy Test Suite' | ||
condition: eq(variables['USE_OPENBLAS'], '1') | ||
env: | ||
LIBRARY_PATH: /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/lib |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about above this line adding:
# The Python install in GitHub Actions adds `-lSystem`, which fails without adding this (see gh-22043)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it is coming from the macOS-provided python, rather from gcc/gfortran (as opposed to clang, which does not add it). I did not find SHLIBS in NumPy nor in distutils/setuptools, and the stack overflows I linked to did not mention python. Adding a comment in that vein.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
makes sense. that's a gfortran that anyway needs an update soon, so hopefully that goes away in the next update
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All green, so in it goes. Thanks @mattip!
I think this explains what is going on: https://developer.apple.com/forums/thread/655588?answerId=665804022#665804022. The stub library
From the link above: OTOH, if your third-party tools are using their own linker then you’ll have to determine whether that linker has been updated to use stub libraries. So yes, that makes sense. The |
Marked for backport, looks like this will be needed for 1.23.x soon. It would be good if 12.0 could be made to work, I suspect we will need it soon. |
There is a warning in the CI runs that the macos-1015 image is deprecated.