pyOpenSSL

Merge lp://staging/~stepankk/pyopenssl/bug-845445 into lp://staging/~exarkun/pyopenssl/trunk

bug-845445
Merge into trunk

Proposed by stepankk on 2012-01-19

Status:

Work in progress

Proposed branch:

lp://staging/~stepankk/pyopenssl/bug-845445

Merge into:

lp://staging/~exarkun/pyopenssl/trunk

Diff against target:

105 lines (+13/-3)

3 files modified

OpenSSL/ssl/connection.c (+6/-0)
OpenSSL/ssl/context.c (+4/-0)
setup.py (+3/-3)

To merge this branch:

bzr merge lp://staging/~stepankk/pyopenssl/bug-845445

Undecided

New

Link a bug report

Reviewer	Review Type	Date Requested	Status
Jean-Paul Calderone		2012-01-19	Needs Fixing on 2012-01-19
Review via email: mp+89242@code.staging.launchpad.net

Description of the change

Excluded some parts of C code according to OpenSSL version.

Revision history for this message

Jean-Paul Calderone (exarkun) wrote on 2012-01-19:

Thanks. It looks like this branch does fix the build problems. A few tweaks are probably in order:

  1. The setup.py changes look superfluous. Can you drop them?
  2. 8 unit tests fail with the branch. Unsurprisingly, these are the tests for the APIs being disabled. Can you change the test suite to skip these tests if the version of OpenSSL in use is too old to have them?
  3. Is a version check in the implementation the ideal way to do this? Another approach would be to check if the API symbols themselves are defined. I don't feel too strongly about this either way, I mostly want to see if you considered it or if you have any thoughts on the alternate approach.

Thanks again.

review: Needs Fixing

Revision history for this message

Keshav Kini (keshav-kini) wrote on 2012-08-08:

See also a patch on the actual bug #845445 page, which checks whether the API symbols themselves are defined, but doesn't skip any unit tests.