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

[DO NOT MERGE] smoke test cryptography with CFFI 1.16.0 pre-release #9653

Closed
wants to merge 5 commits into from

Conversation

nitzmahone
Copy link

  • testing cryptography with a hard dep on CFFI 1.16.0rc1 where possible - we dropped Python 3.7 support but added python_requires metadata accordingly, so 3.7 should still use CFFI==1.15.1.

@nitzmahone nitzmahone force-pushed the test_cffi_1.16.0rc1 branch 2 times, most recently from f24a25c to 2219365 Compare September 26, 2023 02:32
@nitzmahone
Copy link
Author

Welp, looks like there's going to be a CFFI 1.16.0rc2, and probably some new surrogate test coverage once I figure out what we screwed up with the shim module. ;) I'll update this to force rc2 once we get that sorted.

@nitzmahone nitzmahone marked this pull request as draft September 26, 2023 02:42
@alex
Copy link
Member

alex commented Sep 26, 2023

Sounds good, thanks for testing.

* corrected packaging issue
@nitzmahone
Copy link
Author

OK, rc2 is looking a lot better- all CI tests are green (CFFI's source layout masked a packaging failure, even though all tests run against the built wheels). The wheel builder jobs are still failing, but I assume that's maybe due to some isolation/caching going on for the deps? Please let me know if there's more I should look into on the CFFI end, but ATM I'm assuming cryptography is good to go if we release this week.

@alex
Copy link
Member

alex commented Sep 26, 2023

See: .github/requirements/build-requirements.{txt,in} for the wheel builder jobs.

@nitzmahone
Copy link
Author

@alex aha, thanks for the tip- that did it. All the non-PyPy wheel builder jobs are green against CFFI 1.16.0rc2, and since it seems to be a "non-user-serviceable" component of PyPy (I've asked at https://foss.heptapod.net/pypy/pypy/-/issues/4005 to be sure there's nothing more we can verify there upstream), I'm feeling pretty good about it.

I'll leave this PR open and validate any further RCs and/or the final bits as they're released.

Also: cryptography is the primary justification for our team to maintain CFFI's packaging on PyPI (since that keeps cryptography easily pip-installable for Ansible users). It looks like the project is steadily migrating to native PyO3 wrappers for the various C libraries linked under hazmat, which IIUC would no longer require CFFI once complete. Two questions:

  1. Am I understanding correctly the decreasing need for cffi?
  2. Is there a planned timeline for that migration to be complete? (I know, silly question of an open source project)

I know there's an awful lot of CFFI code stubs remaining- if the technical issues for an all-PyO3-no-CFFI cryptography have been solved, and it's merely an issue of people-power to rewrite all the bindings, I'd be interested to discuss that, as I'd much prefer to "write myself out of a job" maintaining CFFI if possible. 😉

@nitzmahone nitzmahone changed the title [DO NOT MERGE] smoke test cryptography with CFFI 1.16.0rc1 [DO NOT MERGE] smoke test cryptography with CFFI 1.16.0 pre-release Sep 26, 2023
@reaperhulk
Copy link
Member

We are slowly decreasing our usage of cffi, but pyOpenSSL uses our bindings layer and what we do there is still very much TBD.

Outside of the pyOpenSSL situation I'm not aware of any fundamental technical issues, but as we convert we run into many yaks that are in need of shaving. Frequently that's additions to rust-openssl, but occasionally it ranges farther afield into the PyO3 or the broader Rust ecosystem.

I don't think we can really even guess a date until we get to the point where we have a plan for pyOpenSSL, but @alex may have a different opinion 😄

@alex
Copy link
Member

alex commented Sep 26, 2023 via email

@nitzmahone
Copy link
Author

OK, sounds like we're on the hook for another year or two at least- do please let us know if you get to a point where it's purely a matter of cranking out binding adapters or something, because we could probably justify sending some capable hands your way for a bit if it meant eliminating CFFI from the picture.

Comment on lines 84 to 72
# via -r build-requirements.in
tomli==2.0.1 \
--hash=sha256:939de3e7a6161af0c887ef91b7d41a53e7c5a1ca976325f429cb46ea9bc30ecc \
--hash=sha256:de526c12914f0c550d15924c62d72abc48d6fe7364aa87328337a31007fe8a4f
# via setuptools-rust
# via -r build-requirements.in
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change here is what caused the test failures btw.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh, I ran the pip-compile under Python 3.11, so setuptools et al probably decided they didn't need an aftermarket TOML parser.

@alex
Copy link
Member

alex commented Sep 28, 2023

I think we're good. Thanks!

@alex alex closed this Sep 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants