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.

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

BLD: add libquadmath to licences and other tweaks #24753

Merged
merged 10 commits into from
Sep 21, 2023
Merged

Conversation

mattip
Copy link
Member

@mattip mattip commented Sep 20, 2023

As pointed out in MacPython/openblas-libs#114, we are bundling libquadmath which is licensed differently than the rest of the gcc runtimes. This PR

  • adds its license, which is complicated. Most of it is copyright "Sun, with use freely granted". But there are a few files licensed under the LGPL2.0 and LGPL2.1, and even one file that is "Public domain". I am not sure what the implications are.
  • Change the canonical location of OpenBlas to https://github.com/OpenMathLib/OpenBLAS
  • Add OpenBLAS and LAPACK licences to macos, they were missing?
  • Remove the msvc runtime licenses from windows, we do not bundle it (anymore?)
  • tweak the locations of the bundles in the site-packages tree.

cc @rgommers for review?

I think this should be backported. While #24613 to use Accelerate for macos-arm64 was backported, I see the 1.26 wheels are still using OpenBLAS, so the license should not change.

@github-actions github-actions bot added the 36 - Build Build related PR label Sep 20, 2023
@rgommers
Copy link
Member

adds its license, which is complicated. Most of it is copyright "Sun, with use freely granted". But there are a few files licensed under the LGPL2.0 and LGPL2.1, and even one file that is "Public domain". I am not sure what the implications are.

Where are you seeing that? https://github.com/gcc-mirror/gcc/blob/master/libquadmath/COPYING.LIB looks like a standard LGPL license.

we do not bundle it (anymore?)

Correct, we used to bundle libmsvcrt140.dll (from memory, I may have misspelled that by one letter) but aren't doing that anymore.

@mattip
Copy link
Member Author

mattip commented Sep 20, 2023

I took the libquadmath license from the appropriate section in my (debian) /usr/share/doc/gcc-12-base/copyright. Note that that file mistakenly lists the wrong license for ldexpq.c, so I corrected that (I deleted the exception, it is licenses like the default in that library). I am trying to file a debian bug for that.

Edit: here is the debian bug https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1052314

@mattip
Copy link
Member Author

mattip commented Sep 20, 2023

Diving into the actual code will show that each file has a license, most are like this

/* e_acoshl.c -- long double version of e_acosh.c.
 * Conversion to long double by Jakub Jelinek, jj@ultra.linux.cz.
 */

/*
 * ====================================================
 * Copyright (C) 1993 by Sun Microsystems, Inc. All rights reserved.
 *
 * Developed at SunPro, a Sun Microsystems, Inc. business.
 * Permission to use, copy, modify, and distribute this
 * software is freely granted, provided that this notice
 * is preserved.
 * ====================================================
 */

@rgommers
Copy link
Member

I took the libquadmath license from the appropriate section in my (debian) /usr/share/doc/gcc-12-base/copyright

I don't think we need to include very granular per-file copyright statements. The upstream GCC license I linked is standard LGPL as far as I can tell.

Diving into the actual code will show that each file has a license, most are like this

That is perfectly normal, but not really relevant imho. We also have public-domain code in NumPy and SciPy, and since that's BSD-3 compatible we make no attempt to list per-file differences in our own LICENSE.txt.

@mattip
Copy link
Member Author

mattip commented Sep 20, 2023

libgfortan, as the LICENSE text in NumPy currently indicates, is released under gpl v3 with an exception. libquadmath, as you point out, is released under lgpl 2.1 and some files are specifically LGPL2.0 or later. So I can simplify the PR to use LGPL2.1 for libquadmath. A single file has a "Public Domain" license, I guess we can put that in the "more permissive" category and not list it separately.

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.

Thanks Matti. A few minor comments, and one that looks critical: the path to the bundled libraries for the Windows wheels is indeed numpy.libs\ now and not .libs, however the _distributor_init.py still uses .libs. I don't quite understand how this isn't broken. If you have a Windows machine at hand, can you check?

tools/wheels/LICENSE_linux.txt Show resolved Hide resolved
tools/wheels/LICENSE_osx.txt Outdated Show resolved Hide resolved
tools/wheels/LICENSE_win32.txt Outdated Show resolved Hide resolved
@mattip
Copy link
Member Author

mattip commented Sep 21, 2023

The path to the bundled libraries for the Windows wheels is indeed numpy.libs\ now and not .libs, however the _distributor_init.py still uses .libs. I don't quite understand how this isn't broken. If you have a Windows machine at hand, can you check?

A bit of magic that came in with the overly-large #23838 and I didn't realize at the time. We now use delvewheel to repair the wheel, which adds this snippet to the beginning of numpy/__init__.py:

# start delvewheel patch
def _delvewheel_patch_1_5_1():
    import os
    libs_dir = os.path.abspath(os.path.join(os.path.dirname(__file__), os.pardir, 'numpy.libs'))
    if os.path.isdir(libs_dir):
        os.add_dll_directory(libs_dir)


_delvewheel_patch_1_5_1()
del _delvewheel_patch_1_5_1
# end delvewheel patch

so actually, we don't need the _developer_init.py rewrite anymore.

@mattip
Copy link
Member Author

mattip commented Sep 21, 2023

The _distributor_init.py rewrite changes after the scipy-openblas PR #24749 anyway. If we can use the scipy-openblas wheel at runtime, then all platforms will need a import scipy-openblas in the _dsitributor_init.py, so I won't bother with a cleanup PR to remove the unneeded file change for 1.26.1.

@mattip
Copy link
Member Author

mattip commented Sep 21, 2023

I fixed the review comments and changed all the specifiers to be spdx compliant

@rgommers
Copy link
Member

so actually, we don't need the _developer_init.py rewrite anymore.

We do on Python 3.9: conda/conda#10897 (comment). We should fix _distributor_init.py in 1.26.1, it's a 1 line fix.

@mattip
Copy link
Member Author

mattip commented Sep 21, 2023

For python3.9 or conda, delvewheel does it differently:

# start delvewheel patch
def _delvewheel_patch_1_5_1():
    import ctypes
    import os
    import platform
    import sys
    libs_dir = os.path.abspath(os.path.join(os.path.dirname(__file__), os.pardir, 'numpy.libs'))
    is_conda_cpython = platform.python_implementation() == 'CPython' and (hasattr(ctypes.pythonapi, 'Anaconda_GetVersion') or 'packaged by conda-forge' in sys.version)
    if sys.version_info[:2] >= (3, 8) and not is_conda_cpython or sys.version_info[:2] >= (3, 10):
        if os.path.isdir(libs_dir):
            os.add_dll_directory(libs_dir)
    else:
        load_order_filepath = os.path.join(libs_dir, '.load-order-numpy-1.26.0')
        if os.path.isfile(load_order_filepath):
            with open(os.path.join(libs_dir, '.load-order-numpy-1.26.0')) as file:
                load_order = file.read().split()
            for lib in load_order:
                lib_path = os.path.join(os.path.join(libs_dir, lib))
                if os.path.isfile(lib_path) and not ctypes.windll.kernel32.LoadLibraryExW(ctypes.c_wchar_p(lib_path), None, 0x00000008):
                    raise OSError('Error loading {}; {}'.format(lib, ctypes.FormatError()))


_delvewheel_patch_1_5_1()
del _delvewheel_patch_1_5_1
# end delvewheel patch


Edit: Note .load-order-numpy-1.26.0 contains the mangled name of the DLL libopenblas64__v0.3.23-293-gc2f4bdbb-gcc_10_3_0-65e29aac85b9409a6008e2dc84b1cc09.dll

I agree we should make CI more like delvewheel repair and use numpy.libs in the azure runs.

@mattip
Copy link
Member Author

mattip commented Sep 21, 2023

I cherry-picked the change from #24765 into this PR

@mattip
Copy link
Member Author

mattip commented Sep 21, 2023

Closes #24764

@rgommers rgommers added this to the 2.0.0 release milestone Sep 21, 2023
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 Matti! I'll squash-merge this for ease of backporting.

@rgommers rgommers merged commit 085665c into numpy:main Sep 21, 2023
70 checks passed
@charris
Copy link
Member

charris commented Sep 21, 2023

This is an uncertain backport due to the changes made during the testing cleanup. I'm tempted to just take all the changes into 1.26.x, but that requires some checking that nothing gets lost . 2.0.0 and 1.26.x do build the same wheels at this point.

@rgommers
Copy link
Member

It looks like all changes are okay to backport here, except possibly the env var removal in linux.yml - but that's unrelated to the rest of this PR so can be discarded.

charris pushed a commit to charris/numpy that referenced this pull request Sep 23, 2023
Other improvements:

- use SPDX short specifiers, fixes from review
- only use `openblas_support.make_init()` in CI, fix paths
- fix license concatenation for wheels we distribute

Closes numpygh-24764
charris pushed a commit to charris/numpy that referenced this pull request Sep 24, 2023
Other improvements:

- use SPDX short specifiers, fixes from review
- only use `openblas_support.make_init()` in CI, fix paths
- fix license concatenation for wheels we distribute

Closes numpygh-24764
charris added a commit that referenced this pull request Sep 24, 2023
BLD: add libquadmath to licences and other tweaks (#24753)
@charris charris removed the 09 - Backport-Candidate PRs tagged should be backported label Sep 26, 2023
@charris charris removed this from the 2.0.0 release milestone Sep 26, 2023
charris pushed a commit to charris/numpy that referenced this pull request Nov 11, 2023
Other improvements:

- use SPDX short specifiers, fixes from review
- only use `openblas_support.make_init()` in CI, fix paths
- fix license concatenation for wheels we distribute

Closes numpygh-24764
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
36 - Build Build related PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants