-
-
Notifications
You must be signed in to change notification settings - Fork 14.2k
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
python311Packages.cvxopt: use LP64 blas on darwin; fix build #270016
Conversation
darwin builds were using the ILP64 openblas library which causes the build to segufault in the unittests, which might be why the unit tests were disabled on darwin... this change aligns darwin to the linux build to use LP64 blas / lapack libraries. this also aligns with the assert at the top of the file which asserts blas and lapack are LP64. the darwin unit test is re-enabled and passes.
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/prs-ready-for-review/3032/3003 |
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.
Diff LGTM, don't have a mac to test on
@emilytrau if you have a chance to look at this, i'm trying to get this package in as it silently (and not so silently) breaks a bunch of consumers on darwin. thx! |
Result of 32 packages marked as broken and skipped:
4 packages failed to build:
24 packages built:
|
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.
Nice work!
Successfully created backport PR for |
woot! thanks! |
darwin builds were using the ILP64 openblas library which causes the build to segufault in the unittests, which might be why the unit tests were disabled on darwin.
ILP64 is unsupported so it was a bug for darwin to use it in the first place.
this change aligns darwin to the linux build to use LP64 blas / lapack libraries. this also aligns with the assert at the top of the file which asserts blas and lapack are LP64. nixpkgs blas & lapack point to openblasCompat which is the openblas LP64, the opeblas package is ILP64. numpy and most (all) other math python packages require LP64.
the darwin unit test is enabled and now passes after this change as do the consumers of this package which were segfaulting in their unit tests.
Fixes: #271105
ZHF: #265948
https://hydra.nixos.org/build/241904734
note: the hydra failure is for python311Packages.cvxpy but that is just because it's unit tests run and were segfaulting in cvxopt
Result of
nixpkgs-review pr 270016
run on x86_64-darwin 133 packages marked as broken and skipped:
28 packages built:
Result of
nixpkgs-review pr 270016
run on x86_64-linux 128 packages marked as broken and skipped:
48 packages built:
Description of changes
Things done
nix.conf
? (See Nix manual)sandbox = relaxed
sandbox = true
nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD"
. Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/
)Priorities
Add a 👍 reaction to pull requests you find important.