-
Notifications
You must be signed in to change notification settings - Fork 337
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
Fix Errors Due to Deps #433
Conversation
The pillow change fixes pip trying to compile pillow from source but is missing zlib headers(maybe others), you could set these up on your system/CI/CD or 9.5.0 works. They also recommend using the wheel if you don't want setup the headers. The change to pyinstaller fixes a typelib error in freeze on windows saying freetype 2.0 is missing. |
I don't know if you can do both greater and less then in requirements but also not sure that it matters in this case as I think the versions of the deps are old enough that versions older are not likely to be used with the versions of python deluge supports, but idk. |
So I thought zope.interface change fixes python 3.7 but apparently not test still pass on 3.10, so I guess this is a python 3.7 issue? The other two deps changes I believe are valid. |
It's same issue also for py3.9(zope.interface), and I just reverted pyopenssl and cryptography to earlier versions(22.0.0/37.0.4) for quick fix(trial/error) when I ran into it last week. Anyway, I looked little into it now, and don't have more time now, but found seemingly for some reason an older setuptools(47.1.0) is installed for python's under 3.10(tested 3.7, 3.9 and 3.10), and when upgrading setuptools afterwards to newest, then works. Probably need pin setuptools to 68.0.0, as latter two versions are py3.8 and up. Also, in requirements.txt, then I believe twisted should be pinned to 22.10(or below), like in CI, as else: "Nameerror fdesc is not defined", when newest twisted installed. Thanks! :) Edit: Setuptools version variances because included already(as dep) with pip/python. Edit2(last): In quick test here(not CI though), then pyinstaller 4.10 doesn't fail(py3.9.8), though have as always displayed a bunch missing typelib errors, originating from one of the hooks being buggy, though not important for us, as a misunderstanding those messages. Pyinstaller 5 broke freezing completely(hence 4.10 pin added), as respected those errors instead of just forward/log and ignore, but they gracefully fixed it for us in next version(maybe next again, can't remember, and I made PR to update pyinstaller and drop 4.10 pin, but I closed it later, and Cas thought not important to update at the time). Sorry if misunderstanding and 4.10 has actual error blocking on CI now and not just cosmetic, or that you already knew these things. Granted might as well update pyinstaller as well, and is confusing to end-users looking at, and just elaborating. Sorry for wordyness, and butting in - please let me know if I can help with something, and i'll but out again now. |
seems related: zopefoundation/zope.interface#255 |
@mhertz the pyinstaller issue is when running deluge after packaging not in the packaging process sorry I was not clear. |
Good catch, yeah must've been fixed here presumably then: pypa/setuptools#3153 for setuptools 62. Just strange because first thing I tried myself was mess with that(change install name), to no avail, and see requirements.txt and setup.py has the canonical name regardless, but whatever. I guess the setuptools change should be added little up too in CI file, as still old version in log. Sorry didn't ment to butt in here again, unless called, but will adhere to now promise ;) Thin line to walk for guessing if beeing perceived helpful VS annoying you know :) Thanks as said. |
1148931
to
195447f
Compare
3.7 is eol 3.8 is oldest still in support 3.10 is newest libtorrent provides wheels for.
Thanks for the work identifying the build issues, I prefer that we identify and resolve each issue separately to properly understand the issue that is being fixed, as such I referenced your changes in new commits. Just to note that the versions specified in requirements are usually there as a project minimum/maximum rather than for CI/CD and shouldn't be removed without checking the commit history. There are likely some improvements to how we handle the dependencies for project, packaging and CI/CD. Also until we specifically drop support for Python 3.7 I still want the runners for it. The total number of Python versions to test against should also be small so we don't have to wait too long for them all to finish. There could be an improvement where we run a single Python version for the PR/tests and then run against all versions once merged to develop branch. |
@cas-- Thank you for making the required changes. I am sorry though I might have not been clear maybe? Or I just didn't notice a reference to it in your commits. However the change for pillow is not just for compatibility with python versions but there is also missing header files for compiling from source.(and part of why move to wheel) So maybe a comment in a commit mentioning zlib source headers missing from pillow source code for compiling pillow from source? idk. Again sorry either way if you did or didn't. And big thanks for making the changes! |
Ah yeah I did find the other Pillow issue which is isolated to Windows 32-bit builds which as you say is due to missing wheels and trying build from source: 6c9b058 |
The latest Pillow 10 does not support Py3.7 therefore wheels are no longer available and we need to specify previous major version. Older versions of setuptools do not correctly determine the Twisted requirement for zope.interface>5 on Python 3.7 so ensure latest installed. For the CD builds we don't want any surprises so keep the setuptools version pinned. Refs: https://pillow.readthedocs.io/en/stable/installation.html Closes: deluge-torrent#433
The latest Pillow 10 does not support Py3.7 therefore wheels are no longer available and we need to specify previous major version. Older versions of setuptools do not correctly determine the Twisted requirement for zope.interface>5 on Python 3.7 so ensure latest installed. For the CD builds we don't want any surprises so keep the setuptools version pinned. Refs: https://pillow.readthedocs.io/en/stable/installation.html Closes: deluge-torrent#433
No description provided.