-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
[Bug]: LICENSE_QHULL is included in wheel only the second time #25212
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
Comments
I cannot reproduce it locally (although with Python 3.9). Does the log print that it adds the license files? I get this at the end of the log and it seems to be in the wheel as well
Any other license files missing? |
Did you already have the Qhull code/license downloaded? The problem I see happens only the first time, so a |
This is a race condition between the qhull custom build step (which puts the license file in the right place) in Hopefully there is some nice setuptools hook we can call as part of the copying to re-discover the files (or just manually update the manifest metadata). |
The following wheels from the 3.7.0 final release DO NOT include LICENSE_QHULL:
i.e. all of the manylinux wheels except pypy 3.9 plus one 32 bit windows wheel |
We should fix this for 3.7.1 (which I expect to be with in the month) and going forward. |
Same results for 3.7.0rc1 wheels, but not for wheels generated on the 3.6.x branch, which had all of the qhull license files (I just pulled the zip from the "zenodo doi" run shortly after 3.6.3 was released) I spot checked a couple manylinux 3.6.3 wheels from pypi and found they had LICENSE_QHULL. |
Actually the problem for me happens also in 3.6.0, when built from source. PyPI does have the LICENSE_QHULL file, but it doesn't if you re-build it (from clean env) from the git tag v3.6.0. |
Well, it may be true that newly built wheels have the same problem, it may still be caused by a relatively recent toolchain update, which is useful information. |
diff --git a/setup.py b/setup.py
index bbaa56f3d6..f3b3b29c5b 100644
--- a/setup.py
+++ b/setup.py
@@ -42,12 +42,13 @@ from setupext import print_raw, print_status
# These are the packages in the order we want to display them.
+qhull_setup = setupext.Qhull()
mpl_packages = [
setupext.Matplotlib(),
setupext.Python(),
setupext.Platform(),
setupext.FreeType(),
- setupext.Qhull(),
+ qhull_setup,
setupext.Tests(),
setupext.BackendMacOSX(),
]
@@ -267,6 +268,8 @@ if not (any('--' + opt in sys.argv
package_data.setdefault(key, [])
package_data[key] = list(set(val + package_data[key]))
+qhull_setup.download()
+
setup( # Finally, pass this all along to setuptools to do the heavy lifting.
name="matplotlib",
description="Python plotting package",
diff --git a/setupext.py b/setupext.py
index 7cd5216a59..acba7ea70e 100644
--- a/setupext.py
+++ b/setupext.py
@@ -756,7 +756,7 @@ class Qhull(SetupPackage):
else:
cls._extensions_to_update.append(ext)
- def do_custom_build(self, env):
+ def download(self):
if options.get('system_qhull'):
return
@@ -767,6 +767,12 @@ class Qhull(SetupPackage):
)
shutil.copyfile(toplevel / "COPYING.txt", "LICENSE/LICENSE_QHULL")
+ def do_custom_build(self, env):
+ if options.get('system_qhull'):
+ return
+
+ self.download()
+
for ext in self._extensions_to_update:
qhull_path = Path(f'build/qhull-{LOCAL_QHULL_VERSION}/src')
ext.include_dirs.insert(0, str(qhull_path)) A super quick and dirty workaround, not sure I like it, but it seems to work. Essentially I copied the download/copy license file from One small difference (which may be important) is that the sdist includes the QHULL license file (which it didn't before) and is not actually distributing qhull at all. Have had no success at pinning down what local version of build deps may have changed this behavior, but I did notice that the 3.6.x branch uses cibuildwheel 0.11.2, whereas the main branch (and 3.7.x) uses 0.12.0, so that could have something to do with it... though nothing jumped at me in looking at the changelog there, so not sure that is relevant (and certainly would expect something lower level to be the actual root cause even if that is the proximal cause) |
We couldn't work out what exactly changed to stop the license being included, so work around that by pre-downloading it. Fixes matplotlib#25212
We couldn't work out what exactly changed to stop the license being included, so work around that by pre-downloading it. Fixes matplotlib#25212
We couldn't work out what exactly changed to stop the license being included, so work around that by pre-downloading it. Fixes matplotlib#25212
We couldn't work out what exactly changed to stop the license being included, so work around that by pre-downloading it. Fixes matplotlib#25212
Note that this is only fixed for our CI-built wheels. We are still not sure what changed in what package that caused 3.7 wheels to drop the license while 3.6 wheels were previously fine. |
Bug summary
The first time I run
python3.10 setup.py bdist_wheel
on both 3.6.0 and 3.7.0, the resulting wheel does not includematplotlib-0.0.0.dist-info/LICENSE_QHULL
. The second time theLICENSE_QHULL
file is included.Code for reproduction
Actual outcome
Expected outcome
Additional information
No response
Operating system
quay.io/pypa/manylinux_2_28_aarch64
Matplotlib Version
3.6.0 and 3.7.0
Matplotlib Backend
No response
Python version
3.8 and 3.10
Jupyter version
No response
Installation
git checkout
The text was updated successfully, but these errors were encountered: