Skip to content

[WIP] cffi init_once (1.4+) support in the openssl backend#2537

Closed
reaperhulk wants to merge 2 commits into
pyca:masterfrom
reaperhulk:cffi-init-once
Closed

[WIP] cffi init_once (1.4+) support in the openssl backend#2537
reaperhulk wants to merge 2 commits into
pyca:masterfrom
reaperhulk:cffi-init-once

Conversation

@reaperhulk

Copy link
Copy Markdown
Member

cffi added init_once support in version 1.4. This can alleviate a race for us around the registration of the osrandom engine, but since we don't want to bump our PyPy minimum version beyond 2.6 we need to support both code paths. I am far from confident that the current PR represents the right approach, but it should serve as a starting point to figure out what we want to do.

@mention-bot

Copy link
Copy Markdown

By analyzing the blame information on this pull request, we identified @glyph, @alex and @public to be potential reviewers

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Simplifies to cffi.__version_info__ >= (1, 4, 0)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can we namespace these tags a bit more.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

e.g. cryptography.

@alex

alex commented Dec 21, 2015

Copy link
Copy Markdown
Member

Can we do this without moving any of the code around? Ideally just altering the callsites of these functions?

@codecov-io

Copy link
Copy Markdown

Current coverage is 99.95%

Merging #2537 into master will decrease coverage by -0.03% as of c202cb3

@@            master   #2537   diff @@
======================================
  Files          123     123       
  Stmts        12960   12966     +6
  Branches      1415    1417     +2
  Methods          0       0       
======================================
+ Hit          12958   12960     +2
- Partial          2       4     +2
- Missed           0       2     +2

Review entire Coverage Diff as of c202cb3

Powered by Codecov. Updated on successful CI builds.

@alex

alex commented Dec 21, 2015

Copy link
Copy Markdown
Member

Ooof, coverage is a mess here because PyPy is the one platform we don't have coverage on :-(

@reaperhulk

Copy link
Copy Markdown
Member Author

The coverage issue here is potentially resolved by merging #2463, although it will significantly increase our CI time. I don't really want to put this in the twelfth release though so we can wait on this one.

@reaperhulk reaperhulk added this to the Thirteenth Release milestone Jan 2, 2016
@reaperhulk

Copy link
Copy Markdown
Member Author

I'm unwilling to land a change like this at the moment. Mostly because until we have static callbacks sorted out I'm gunshy about making another major change. Kicking this out of the thirteenth release.

@reaperhulk reaperhulk removed this from the Thirteenth Release milestone Mar 14, 2016
@reaperhulk reaperhulk closed this Jun 4, 2016
@reaperhulk reaperhulk deleted the cffi-init-once branch June 27, 2017 06:04
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Aug 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Development

Successfully merging this pull request may close these issues.

4 participants