cryptography icon indicating copy to clipboard operation
cryptography copied to clipboard

Free threaded support

Open ngoldbaum opened this issue 9 months ago • 2 comments

Towards fixing #12489.

Adds free-threaded CI using a hacky workaround to avoid building against the version of cffi on PyPI.

xref #12590

ngoldbaum avatar Mar 04 '25 23:03 ngoldbaum

I guess because I'm not using the tests nox session I don't get coverage reports on CI "for free".

ngoldbaum avatar Mar 04 '25 23:03 ngoldbaum

Great

On Thu, Mar 6, 2025 at 5:03 PM Nathan Goldbaum @.***> wrote:

@ngoldbaum commented on this pull request.


In src/rust/src/padding.rs:

@@ -110,6 +110,62 @@ impl PKCS7PaddingContext { } }

+#[pyo3::pyclass]

I'll cherrypick the Rust ports and make them their own PRs and keep the free-threading-specific stuff in this PR.

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you commented.Message ID: @.***>

-- All that is necessary for evil to succeed is for good people to do nothing.

alex avatar Mar 06 '25 22:03 alex

@alex it looks like all the wheel builds are successful except for win32 and armv7. See these actions runs on my fork:

win32: https://github.com/ngoldbaum/cryptography/actions/runs/16601346439/job/46961605807?pr=2 armv7: https://github.com/ngoldbaum/cryptography/actions/runs/16601346439/job/46961606173?pr=2

Not sure what to do about these. Do you have any ideas?

I also disabled some manylinux2014 builds due to the issue that should be fixed by https://github.com/python-cffi/cffi/pull/184.

I also wasn't able to run ppc64le on my fork so let's see if there are any issues there.

Are you OK with me marking all the rust PyModules with gil_used = false in this PR? We're relying on upstream guarantees from PyO3 in all the rust code.

ngoldbaum avatar Jul 29 '25 17:07 ngoldbaum

IDK what's going on with the win32 one. The armv7 one seems to suggest the build is building an aarch64 wheel, which is obviously just wrong.

And yes, going to gil_used=False is fine, we don't rely on it.

alex avatar Jul 29 '25 22:07 alex

it occurs to me that the gil_used=false should be good to split off from this and land now

alex avatar Jul 30 '25 01:07 alex

it occurs to me that the gil_used=false should be good to split off from this and land now

I'll put this in along with the new CFFI version constraints in pyproject.toml. We can sort out the win32 build issues later but it would be nice to at least get things in a state with no build errors or unnecessary warnings.

EDIT: actually on second thought let's try to sort out the build issues first...

ngoldbaum avatar Jul 31 '25 15:07 ngoldbaum

At this point I think we're just waiting for the cffi release! Thanks for your work here!

alex avatar Jul 31 '25 16:07 alex

At this point I think we're just waiting for the cffi release! Thanks for your work here!

While the remaining build issues here aren't very fun I've been telling people that working on the CFFI reverse dependencies is the programming equivalent of peeling plastic off of new electronics: so satisfying to fix stuff that's been broken for a year or so!

When you say "waiting for a CFFI release" does that mean you're not comfortable doing a cryptography release until there's a CFFI 2.0.0 final release? I think as long as there's an explicitly dependency on the beta release in cryptography's pyproject.toml, everything should be fine downstream. Unless I'm missing something.

ngoldbaum avatar Jul 31 '25 16:07 ngoldbaum

OK win32 is fixed. That came down to uv not selecting free-threaded python unless you explicitly opt into it. I added --python where appropriate.

I tried building the armv7 wheel manually in a docker container on my Mac, but frustratingly the build succeeds there and produces an armv7 wheel:

root@18e5360863db:~/cryptography# mkdir tempwheelhouse root@18e5360863db:~/cryptography# OPENSSL_DIR="/opt/pyca/cryptography/openssl" OPENSSL_STATIC=1 uv build --python=/opt/python/cp314-cp314t/bin/python --require-hashes --build-constraint=.github/requirements/build-requirements.txt --sdist -o tempwheelhouse Building source distribution... Running maturin pep517 write-sdist --sdist-directory /root/cryptography/tempwheelhouse 🍹 Building a mixed python/rust project 🔗 Found pyo3 bindings with abi3 support 📡 Using build options locked from pyproject.toml 📦 Including files matching "CHANGELOG.rst" 📦 Including files matching "CONTRIBUTING.rst" 📦 Including files matching "docs//*" 📦 Including files matching "src/_cffi_src//.py" 📦 Including files matching "src/_cffi_src/**/.c" 📦 Including files matching "src/_cffi_src//*.h" 📦 Including files matching "Cargo.toml" 📦 Including files matching "Cargo.lock" 📦 Including files matching "src/rust//Cargo.toml" 📦 Including files matching "src/rust//Cargo.lock" 📦 Including files matching "src/rust//.rs" 📦 Including files matching "tests/**/.py" 📦 Built source distribution to /root/cryptography/tempwheelhouse/cryptography-46.0.0.dev1.tar.gz cryptography-46.0.0.dev1.tar.gz Successfully built tempwheelhouse/cryptography-46.0.0.dev1.tar.gz root@18e5360863db:~/cryptography# OPENSSL_DIR="/opt/pyca/cryptography/openssl" OPENSSL_STATIC=1 uv build --python=/opt/python/cp314-cp314t/bin/python --wheel --require-hashes --build-constraint=.github/requirements/build-requirements.txt tempwheelhouse/cryptography-46.0.0.dev1.tar.gz Building wheel from source distribution... Running maturin pep517 build-wheel -i /root/.cache/uv/builds-v0/.tmpbDRLqK/bin/python --compatibility off 🍹 Building a mixed python/rust project 🔗 Found pyo3 bindings with abi3 support 🐍 Found CPython 3.14t at /root/.cache/uv/builds-v0/.tmpbDRLqK/bin/python 📡 Using build options locked from pyproject.toml ⚠️ Warning: CPython 3.14t at /root/.cache/uv/builds-v0/.tmpbDRLqK/bin/python does not yet support abi3 so the build artifacts will be version-specific. Compiling target-lexicon v0.13.2 Compiling proc-macro2 v1.0.95 Compiling unicode-ident v1.0.18 Compiling shlex v1.3.0 Compiling once_cell v1.21.3 Compiling pkg-config v0.3.32 Compiling vcpkg v0.2.15 Compiling libc v0.2.174 Compiling autocfg v1.5.0 Compiling foreign-types-shared v0.1.1 Compiling openssl v0.10.73 Compiling foreign-types v0.3.2 Compiling cc v1.2.30 Compiling heck v0.5.0 Compiling cfg-if v1.0.1 Compiling bitflags v2.9.1 Compiling itoa v1.0.15 Compiling indoc v2.0.6 Compiling memoffset v0.9.1 Compiling unindent v0.2.4 Compiling cryptography-key-parsing v0.1.0 (/root/cryptography/dist/.tmpfGqU11/cryptography-46.0.0.dev1/src/rust/cryptography-key-parsing) Compiling cryptography-openssl v0.1.0 (/root/cryptography/dist/.tmpfGqU11/cryptography-46.0.0.dev1/src/rust/cryptography-openssl) Compiling base64 v0.22.1 Compiling self_cell v1.2.0 Compiling quote v1.0.40 Compiling pyo3-build-config v0.25.1 Compiling pem v3.0.5 Compiling syn v2.0.104 Compiling openssl-sys v0.9.109 Compiling cryptography-cffi v0.1.0 (/root/cryptography/dist/.tmpfGqU11/cryptography-46.0.0.dev1/src/rust/cryptography-cffi) Compiling pyo3-ffi v0.25.1 Compiling pyo3-macros-backend v0.25.1 Compiling pyo3 v0.25.1 Compiling cryptography-rust v0.1.0 (/root/cryptography/dist/.tmpfGqU11/cryptography-46.0.0.dev1/src/rust) Compiling openssl-macros v0.1.1 Compiling asn1_derive v0.22.0 Compiling asn1 v0.22.0 Compiling cryptography-x509 v0.1.0 (/root/cryptography/dist/.tmpfGqU11/cryptography-46.0.0.dev1/src/rust/cryptography-x509) warning: [email protected]: In file included from /opt/_internal/cpython-3.14.0rc1-nogil/include/python3.14t/Python.h:92, warning: [email protected]: from /root/cryptography/dist/.tmpfGqU11/cryptography-46.0.0.dev1/target/release/build/cryptography-cffi-723ec3746b75d810/out/_openssl.c:58: warning: [email protected]: /opt/_internal/cpython-3.14.0rc1-nogil/include/python3.14t/cpython/longintrepr.h: In function '_PyLong_CompactValue': warning: [email protected]: /opt/_internal/cpython-3.14.0rc1-nogil/include/python3.14t/cpython/longintrepr.h:135:12: warning: conversion to 'Py_ssize_t' {aka 'int'} from 'unsigned int' may change the sign of the result [-Wsign-conversion] warning: [email protected]: 135 | sign = 1 - (op->long_value.lv_tag & _PyLong_SIGN_MASK); warning: [email protected]: | ^ Compiling pyo3-macros v0.25.1 Compiling cryptography-crypto v0.1.0 (/root/cryptography/dist/.tmpfGqU11/cryptography-46.0.0.dev1/src/rust/cryptography-crypto) Compiling cryptography-x509-verification v0.1.0 (/root/cryptography/dist/.tmpfGqU11/cryptography-46.0.0.dev1/src/rust/cryptography-x509-verification) Compiling cryptography-keepalive v0.1.0 (/root/cryptography/dist/.tmpfGqU11/cryptography-46.0.0.dev1/src/rust/cryptography-keepalive) Finished release profile [optimized] target(s) in 4m 51s 📦 Including files matching "CHANGELOG.rst" 📦 Including files matching "CONTRIBUTING.rst" 📦 Including files matching "docs//*" 📦 Including files matching "tests//*.py" 📦 Built wheel for CPython 3.14t to /root/cryptography/dist/.tmpfGqU11/cryptography-46.0.0.dev1/target/wheels/cryptography-46.0.0.dev1-cp314-cp314t-linux_armv7l.whl /root/cryptography/dist/.tmpfGqU11/cryptography-46.0.0.dev1/target/wheels/cryptography-46.0.0.dev1-cp314-cp314t-linux_armv7l.whl Successfully built dist/cryptography-46.0.0.dev1-cp314-cp314t-linux_armv7l.whl

so I still have no idea what's going wrong on the CI builds...

ngoldbaum avatar Jul 31 '25 16:07 ngoldbaum

That's correct that we don't want to ship a release until this is in a cffi release.

alex avatar Jul 31 '25 16:07 alex

I reported a bug against maturin: https://github.com/PyO3/maturin/issues/2701

ngoldbaum avatar Jul 31 '25 17:07 ngoldbaum

is there any chance it's an issue in the manylinux image, and that the 3.14-freethreaded binary in the armv7l image is accidentally aarch64?

alex avatar Jul 31 '25 17:07 alex

is there any chance it's an issue in the manylinux image, and that the 3.14-freethreaded binary in the armv7l image is accidentally aarch64?

looks like that's indeed what's happening: https://github.com/pypa/manylinux/issues/1825#issuecomment-3140852437

ngoldbaum avatar Jul 31 '25 19:07 ngoldbaum

Apologies for the noise here, I'm trying to debug the armv7 issue.

ngoldbaum avatar Aug 05 '25 22:08 ngoldbaum

No worries, make as much noise and use as much CI as you need :-)

On Tue, Aug 5, 2025 at 6:17 PM Nathan Goldbaum @.***> wrote:

ngoldbaum left a comment (pyca/cryptography#12555) https://github.com/pyca/cryptography/pull/12555#issuecomment-3156805689

Apologies for the noise here, I'm trying to debug the armv7 issue.

— Reply to this email directly, view it on GitHub https://github.com/pyca/cryptography/pull/12555#issuecomment-3156805689, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAAGBF6TGDZGLOO3NBPL4T3MEUODAVCNFSM6AAAAABYKW7YW2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTCNJWHAYDKNRYHE . You are receiving this because you were mentioned.Message ID: @.***>

-- All that is necessary for evil to succeed is for good people to do nothing.

alex avatar Aug 05 '25 22:08 alex

Looks like that fixed it! There's one unrelated failure. I'll go ahead and do a separate PR for the manylinux-entrypoint fix.

ngoldbaum avatar Aug 06 '25 01:08 ngoldbaum

OK, I think this should be ready whenever CFFI does a release. I'll try to rebase every now and then since this will bitrot quickly whenever the CI or wheel building config changes.

ngoldbaum avatar Aug 06 '25 02:08 ngoldbaum