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
[staging-20.09] openblas: 0.3.10 -> 0.3.12 #108709
[staging-20.09] openblas: 0.3.10 -> 0.3.12 #108709
Conversation
(Targeting staging as per advice by @TredwellGit in #92458 (comment)) |
@ofborg build python36Packages.scipy |
just saw this, generally we do a cherry-pick, so theres a 1-to-1 correlation between commits in master, to commits in 20.09. Also, the branches seem to have divereged to the point that it might be worthwhile to just bump the package on 20.09 instead of cherry-picking |
Given that on master there are commits and reversion etc, it seemed cleaner to me to just get the state of the package from master (
am I? (I guess that wasn't well described in the PR description, sorry.) |
Ran into this on our HPC cluster. Would be great if it could get accepted. 😄 |
Well, I think what we would want to do for this backport, is only update the version and hash (and remove the upstreamed patch). The static/shared and split-outputs changes, while beneficial, could potentially cause breakage in package dependees. |
Sounds reasonable to me. @nomeata, let me know if you are too busy to modify you pull request, and I can submit an alternative one that just bumps the version and drops the obsolete patch. |
Yea, this is kind of an exceptional case, where it was reverted several times. I guess squashing is fine. However, this is only bumping to |
Right. It is just the subject that is wrong. The bump only needs to be to 0.3.12, as per the actual patch and the referenced commits, to fix the scipy issue. |
Oh, embarrassing. Fixed the title (both PR and commit, which I rebased on latest staging) Is there more for me to do? I have withdrawn my toes, so if someone else does a proper fix before me and closes this one, they would not be stepped on. |
7d647fd
to
a8a0f21
Compare
@r-burns suggested just bumping the version and hash to make the changes as minimal as possible, which seems reasonable to me (not that my opinion matters). What is you thoughts on this @jonringer? Is this good as is now, or are you thinking it would be best to just bump the version and change the hash? |
@GrahamcOfBorg build python3Packages.numpy |
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 think it's find for now, I would like for it to be updated to 0.3.13 though, as 0.3.12 has a threading issue with high core count cpus OpenMathLib/OpenBLAS#2993 (comment)
Are you wanting 0137860 squashed into this as well, or a separate pull request for it? |
It would be my preference to have it included and squashed, as it allows me to review the package without failure. Also would fix runtime issues for people using more than 50 threads on a machine. |
this backports the effect of the following commits from `master` to 20.09: * f52263c treewide: Start to break up static overlay * d1d536c openblas: 0.3.10 -> 0.3.12 * f715602 Revert "openblas: 0.3.10 -> 0.3.12" * 840c201 Merge pull request NixOS#101715 from r-ryantm/auto-update/openblas * e1a59dd openblas: 0.3.10 -> 0.3.12 * 4e29151 Revert "Merge pull request NixOS#101780 from glittershark/bump-openblas" * 3b4cd4f openblas: 0.3.10 -> 0.3.12 * 692d219 Merge staging-next into staging * 7902256 openblas: enable multiple outputs * 92d7b38 openblas: enable on ppc64le * 0137860 openblas: 0.3.12 -> 0.3.13 The motivation is to unbreak building `python36Packages.scipy` (see issue 92458)
a8a0f21
to
397c376
Compare
Done |
Is this ready for a merge? |
I believe so. @NoMeta has addressed everything that @jonringer has brought up. |
Thanks! Merging into |
this backports the following commits from
master
to 20.09:The motivation is to unbreak building
python36Packages.scipy
(seeissue 92458)