-
-
Notifications
You must be signed in to change notification settings - Fork 160
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
Don't double install mac deps, cleanups #2513
Conversation
829d36b
to
596354b
Compare
596354b
to
f880811
Compare
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 and, more importantly here, the mac CI is happy.
Question: does this lead to a noticeable speedup of the mac deps generation? That's not necessary, but it would be a quantifiable win for this PR.
I used Meson for a C/C++ project last year and I had a good impression of it. Other libraries seem to be getting good use of it, I think numpy and scipy use meson now. |
@oddbookworm or @novialriptide Would it be convenient for either of you to give these mac wheels a test? |
In theory it should, but the speedup would be small. There is usually a lot of randomness in the time the build step takes, for some reason. But it runs rarely so I guess it won't be a big deal. |
I got multiple errors running macOS 14.0 (M2 Max 32GB)
|
It could be because I'm using ARM (Apple Silicon), not x86 (Intel)? |
I get those scrap fails locally too so that's fine IG. What's weird is the timer fail and the blur fail (but again, both happen on the older version too). I don't think either is caused by this PR though. Could you also go though a few examples and stuff to check those are not broken |
I did a quick run of the following
Nothing seems broken so far. |
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.
Since the errors seem like they're irrelevant to this PR, I'll give it the thumbsup
This PR is one part of the series of changes I'm making. The goal of this PR is to update the scripts to only install mac deps in the custom prefix (and not in
/usr/local
) and do related cleanups that should eventually help us get on using self-compiled deps on windows, and also being able to use systems like meson for the dependencies that either only support it, or recommend using it