Skip to content
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.

Sign up for GitHub

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

Jump to bottom

MAINT: fix libquadmath licence #19299

Merged
merged 4 commits into from Sep 26, 2023
Merged

MAINT: fix libquadmath licence #19299

merged 4 commits into from Sep 26, 2023

Conversation

andyfaff
Copy link
Contributor

See https://github.com/numpy/numpy/pull/24785/files and MacPython/openblas-libs#114 for relevance.

I think these should be backported to all active maintenance branches.

@andyfaff andyfaff added the backport-candidate This fix should be ported by a maintainer to previous SciPy versions. label Sep 25, 2023
@tylerjereddy tylerjereddy added this to the 1.12.0 milestone Sep 25, 2023
tylerjereddy
tylerjereddy reviewed Sep 25, 2023
View reviewed changes
tools/wheels/LICENSE_win32.txt


Name: libquadmath
Files: scipy.libs\libopenblas*.dll
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most of the PR looks similar to what Matti/Ralf did upstream, and so does this, but this looks incorrect because it is quadmath with OpenBLAS shared lib, and also if we statically linked I'm not sure it would be a dll vs. i.e., .a?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess that is indeed what is in the distributed wheel, so maybe it is just the line about static linking below that confuses me re: DLL.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Discussed in numpy/numpy#24793.

rgommers
rgommers approved these changes Sep 25, 2023
View reviewed changes
Copy link
Member

@rgommers rgommers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks @andyfaff. And yes, +1 for backporting.

Waiting for a bit with merging to see if there are any more comments on this PR or the numpy one.

@tylerjereddy
Copy link
Contributor

I still think that, from a license consumption standpoint, if you statically link i.e., libquadmath to OpenBLAS, and then dynamically load openblas, you're ultimately a runtime dynamic link consumer of the binary asset, but I'm happy to consider myself outvoted 2-1 here.

Also, because Ralf's upstream analysis was that the distinction was apparently irrelevant from the standpoint of the quadmath license anyway, so spending energy on this would be doubly pointless: numpy/numpy#24793 (comment)

@tylerjereddy tylerjereddy merged commit b713344 into scipy:main Sep 26, 2023
23 of 24 checks passed
@tylerjereddy tylerjereddy modified the milestones: 1.12.0, 1.11.3 Sep 26, 2023
tylerjereddy pushed a commit to tylerjereddy/scipy that referenced this pull request Sep 26, 2023
@andyfaff @tylerjereddy
MAINT: fix libquadmath licence (scipy#19299)
ecc2ff2 
* MAINT: linux amendments

* MAINT: osx amendments

* MAINT: win licence amendments

* MAINT: licence typo
@tylerjereddy tylerjereddy mentioned this pull request Sep 26, 2023
Merged
8 tasks
@tylerjereddy tylerjereddy removed the backport-candidate This fix should be ported by a maintainer to previous SciPy versions. label Sep 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tylerjereddy tylerjereddy tylerjereddy left review comments

@rgommers rgommers rgommers approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
1.11.3
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@andyfaff @tylerjereddy @rgommers