- Notifications
You must be signed in to change notification settings - Fork 75
Project updates for python 3.12 compatibility#194
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
Uh oh!
There was an error while loading. Please reload this page.
Conversation
JacobCallahan commented Oct 3, 2023 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
JacobCallahan commented Oct 3, 2023
This should resolve the first (and hopefully last) error we're seeing for #193 |
SafeConfigParser has been deprecated since Python 3.2 and was removed in Python 3.12 entirely. Per the release notes [1] it is recommended to use the ConfigParser class now. Additionally, the readfp method has been removed in favor of read_file. [1] - https://docs.python.org/3.12/whatsnew/3.12.html#configparser
JacobCallahan commented Oct 3, 2023
Also, would it be possible to add python 3.11 and 3.12 to the circleci runs? |
JacobCallahan commented Oct 3, 2023
CI failure related to pypa/setuptools#2497 |
ogajduse commented Oct 9, 2023
@pkittenis Are you still around? Could we get your attention on this PR? |
JacobCallahan commented Oct 9, 2023
One option here for moving away from versioneer is what @kdart-brt did in his fork. |
pbrezina commented Nov 3, 2023
@enkore@pkittenis May I kindly ask you to review and merge this pull request? |
pbrezina commented Nov 7, 2023
@enkore@pkittenis If the project is dead, I would like to talk to you about giving me access to the organization. I don't have time to fully maintain it, but I can keep it functional and I would try to find some contributors. This is a great project, it would be a pity if we can't keep improving it. |
653909f to 6db0fc7CompareThis included a number of changes related to the CPython API. I additionally bumped the supported python versions in CI.
JacobCallahan commented Nov 10, 2023
Steps forward in latest commit, but still issues to resolve. It now builds locally in python 3.12.0 $ pip install . Processing /home/jake/Programming/ssh2-python Installing build dependencies ... done Getting requirements to build wheel ... done Preparing metadata (pyproject.toml) ... done Building wheels for collected packages: ssh2-python Building wheel for ssh2-python (pyproject.toml) ... done Created wheel for ssh2-python: filename=ssh2_python-1.0.0+2.g75176e3.dirty-cp312-cp312-linux_x86_64.whl size=786650 sha256=f130033efc5a35c3e46382234f3033cc5b4f96187f7a5647257b6a9467c8f0c1 Stored in directory: /home/jake/.cache/pip/wheels/2c/19/89/55dc2bd0f7a333b550bcc8bbe940c10f7bde79d2959852e6de Successfully built ssh2-python Installing collected packages: ssh2-python Successfully installed ssh2-python-1.0.0+2.g75176e3.dirtyHowever, there appears to be an issue still with the package itself. In [1]: fromssh2importchannel---------------------------------------------------------------------------ValueErrorTraceback (mostrecentcalllast) CellIn[1], line1---->1fromssh2importchannelFile~/Programming/ssh2-python/ssh2/channel.pyx:1, ininitssh2.channel() ---->1# This file is part of ssh2-python.2# Copyright (C) 2017-2020 Panos Kittenis3#File~/Programming/ssh2-python/ssh2/session.pyx:1, ininitssh2.session() ---->1# This file is part of ssh2-python.2# Copyright (C) 2017-2020 Panos Kittenis3#ValueError: builtins.boolsizechanged, mayindicatebinaryincompatibility. Expected32fromCheader, got24fromPyObjectAdditionally, the |
enkore commented Nov 10, 2023
@pbrezina@JacobCallahan I can only push to non-master branches in this org. |
JacobCallahan commented Nov 10, 2023
@enkore do you know of a way to contact @pkittenis? Thanks! |
| key: depsv3-{{.Branch }}.{{arch }}-PY<< parameters.python_ver >> | ||
| #- python/load-cache: # This command is unavailable in the orb | ||
| # dependency-file: requirements_dev.txt | ||
| # key: depsv3-{{.Branch }}.{{arch }}-PY<< parameters.python_ver >> |
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.
See ParallelSSH/ssh-python@97cfd5c
iirc this is now automated and doesn't have to happen explicitly
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.
That is what I was thinking, so can remove it in follow-up commits.
enkore commented Nov 10, 2023
I don't |
JacobCallahan commented Nov 10, 2023
@enkore bah, ok. Any help with this would definitely be appreciated! |
enkore commented Nov 10, 2023
I've sent a mail to the mail addresses listed on Github, the website and PyPI (they're all different) |
enkore commented Nov 11, 2023 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
Regarding the issue here #194 (comment) The .c files need to be re-cythonized with Cython >= 0.29.34 (cython/cython@43ce558). Cython 3 is the future of course, but that migration might be a bit more involved (mostly for setup.py, I started doing this for ssh-python) |
enkore commented Nov 11, 2023
I created a |
JacobCallahan commented Nov 13, 2023
@enkore the latest commit is working locally in 3.12 and passing CI in 3.8 and 3.10. However, the appveyor build is failing with the incorrect version error. circleci: 1.0.0+4 |
020da1a to abe89a4Compareenkore commented Nov 13, 2023 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
The cause for the appveyor build failure is that there is an extra script involved in the build to generate the version number (https://github.com/ParallelSSH/ssh2-python/blob/master/ci/appveyor/fix_version.py), which doesn't generate a PEP440 compliant version number, which is rejected by newer setuptools. I made this change over in ssh-python to work around that: ParallelSSH/ssh-python@3a7b14e |
JacobCallahan commented Nov 13, 2023 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
@enkore ahh yeah i was digging in and noticed that, but took a bit of a different route edit: And Appveyor is now green! |
.circleci/config.yml Outdated
| workflows: | ||
| version: 2.1 | ||
| #version: 2.1 - default? |
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.
2 should work
.circleci/config.yml Outdated
| default: "3.6" | ||
| default: "3.11" | ||
| docker: | ||
| - image: circleci/python:<< parameters.python_ver >> |
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.
cimg/... instead of circleci/... should fix the 3.10+ build failures
Constrained the Cython version to be less than 3.0, but greater than 0.29.34. Updated the setup.py to account for the updated cythonize parameters. Also made the appveyor version PEP440 compatible
JacobCallahan commented Nov 13, 2023
@enkore another update: we're now running into issues with the base test class setup trying to perform a handshake for the session. None of the key files have been touched in 6 years, and I'm able to reproduce these failures locally. Another note: I update my local Cython to 3.0.5 and rebuilt. Everything built and installed as now with the versions pinned in this PR, so we are likely good to unbound the upper limit for Cython. (tests still failing though) |
JacobCallahan commented Nov 14, 2023
@enkore would it be useful to update the vendored libssh2? I see it's been about 1.5 years since the last update. |
pletnes commented Nov 15, 2023
Excited to see some activity here, this project is useful to me. Is there work being done on windows binary builds, which are missing from some releases? Is there anything I could do to help? |
JacobCallahan commented Nov 15, 2023
@pletnes if we can get it released via this project, it should be able to use the existing build pipeline to deliver windows binaries. |
enkore commented Nov 16, 2023
Regarding how we can continue with the projects I created a dedicated discussion ticket here: ParallelSSH/parallel-ssh#386 |
Rebuilt the project with the latest version of Cython and unbound the upper limit of Cython. This was a big jump, but now it a good time to make it.
This commit adds all relevant files from the libssh2 repo, as of commit 631e773, with some additional attribution and licenses.
JacobCallahan commented Nov 22, 2023
Huge scope expansion with the latest two commits. I've both updated the latest compiled filed with Cython 3.0.5 as well as updated the vendored libssh2 for this repo. However, even with these updates, we're still seeing the issues with key exchanges. Locally, password-based auth workflows are working flawlessly as exercised by my own project's tests. |
enkore commented Nov 23, 2023
Thank you very much! |
This comment was marked as spam.
This comment was marked as spam.
enkore commented Nov 23, 2023 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
Yes, looks like libssh2 is being built without support for the newer rsa-sha2 schemes: Which is kinda strange because both the SHA1 and SHA2 variants should either all be present or entirely absent in the OpenSSL backend: #ifdefOPENSSL_NO_RSA# defineLIBSSH2_RSA 0 # defineLIBSSH2_RSA_SHA1 0 # defineLIBSSH2_RSA_SHA2 0 #else# defineLIBSSH2_RSA 1 # defineLIBSSH2_RSA_SHA1 1 # defineLIBSSH2_RSA_SHA2 1 #endif |
enkore commented Nov 23, 2023
Oh so you see we have two nested copies of the libssh2 source tree and end up building the older, inner one. Whoopsie. |
| @@ -13,6 +13,7 @@ | |||
| try: | |||
| from Cython.Distutils.extension import Extension | |||
| from Cython.Distutils import build_ext | |||
| from Cython.Build import cythonize | |||
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.
cythonize should not be used here. Need to commit pre-built cython sources (the .c files) directly or the source files will get re-generated on each build which may not work depending on the system doing the building.
pkittenis commented Nov 29, 2023
Should also update the https://github.com/ParallelSSH/ssh2-python/blob/master/ci/docker/manylinux/libssh2-1.10.0.tar.gz source file (used for manylinux builds) if updating libssh2 sources. The build failures look related to which libssh2 is being used for building on the CI. Once those are resolved can be merged. |
JacobCallahan commented Dec 1, 2023
@pkittenis is this something you're wanting to take on, or would you like me to? If the latter, can you provide some documentation of what you'd like done? Ideally, we could use some project documentation for how to handle situations like this in the future. |

SafeConfigParser has been deprecated since Python 3.2 and was removed
in Python 3.12 entirely.
Per the release notes [1] it is recommended to use the ConfigParser
class now.
Additionally, the readfp method has been removed in favor of read_file.
Unbind the upper limit of Cython dependency and rebuild with Cython 3.0.5.
Update the vendored version of libssh2.
[1] - https://docs.python.org/3.12/whatsnew/3.12.html#configparser