bpo-1635741: Port hashlib modules to multiphase init (PEP 489) by koubaa · Pull Request #21818 · python/cpython (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation56 Commits15 Checks0 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
Bpo 1635741 hashes bpo-1635741: Port hashlib modules to multiphase init (PEP 489)
@shihai1991 @vstinner @corona10 FYI. The macOS test failure looks to be due to the following error which seems unrelated to my change:
module 'test.support' has no attribute 'EnvironmentVarGuard'
The failed test cases have been fixed in 490c542
Can you do some git rebase operation? MAYBE this operation can solve your probleam.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR is a little bit too large. Would you mind to restrict it to blake2 which is already complex enough?
Once this PR is merged, you can write a PR for sha3 which is complex. Then we can finish with remaining modules like md5 and sha1. Or publish all PR are once.
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.
Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.
@vstinner that's not a problem - I put each file in its own commit so I can easily submit separate PRs. I'll submit one for sha3, one for blake2, and one with the remaining ones. After breaking up and creating PRs for each I'll address your comments
@vstinner I'm going on vacation so I'll come back to address you comment next week.
The MacOS/Ubuntu CI has these test failures:
3 tests failed:
test_hashlib test_random test_statistics
Running locally (windows) those tests pass:
./PCBuild/amd64/python.exe -m test test_statistics test_hashlib test_random
0:00:00 Run tests sequentially
0:00:00 [1/3] test_statistics
0:00:08 load avg: 0.00 [2/3] test_hashlib
0:00:09 load avg: 0.00 [3/3] test_random
== Tests result: SUCCESS ==
I rebased on the latest master so I'm not sure what could be causing it.
**** EDIT ****
This was due to a use-after free which is now fixed.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks for the multiple updates. These PR wasn't easy!
| if (Py_IS_TYPE((PyObject*)self, &SHA512type)) { |
| if ( (newobj = newSHA512object())==NULL) |
| if (Py_IS_TYPE((PyObject*)self, st->sha512_type)) { |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Without defining_class, this test was wrong. So thanks for using defining_class :-)
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, but please fix the NEWS entry.
shihai1991 added a commit to shihai1991/cpython that referenced this pull request
- origin/master: (1373 commits) bpo-1635741: Port mashal module to multi-phase init (python#22149) bpo-1635741: Port _string module to multi-phase init (pythonGH-22148) bpo-1635741: Convert _sha256 types to heap types (pythonGH-22134) bpo-1635741: Port the termios to multi-phase init (PEP 489) (pythonGH-22139) bpo-41732: add iterator to memoryview (pythonGH-22119) bpo-40744: Drop support for SQLite pre 3.7.3 (pythonGH-20909) bpo-41316: Make tarfile follow specs for FNAME (pythonGH-21511) bpo-41720: Add "return NotImplemented" in turtle.Vec2D.rmul(). (pythonGH-22092) bpo-1635741 port _curses_panel to multi-phase init (PEP 489) (pythonGH-21986) bpo-1635741: Port _overlapped module to multi-phase init (pythonGH-22051) bpo-1635741: Port _opcode module to multi-phase init (PEP 489) (pythonGH-22050) bpo-1635741 port zlib module to multi-phase init (pythonGH-21995) [doc] Add link to Generic in typing (pythonGH-22125) bpo-41513: Expand comments and add references for a better understanding (pythonGH-22123) bpo-1635741: Port _sha1, _sha512, _md5 to multiphase init (pythonGH-21818) closes bpo-41723: Fix an error in the py_compile documentation. (pythonGH-22110) [doc] Fix padding in some typing definitions (pythonGH-22114) Fix documented Python version for venv --upgrade-deps (pythonGH-22113) bpo-40318: Migrate to SQLite3 trace v2 API (pythonGH-19581) bpo-41687: Fix sendfile implementation to work with Solaris (python#22040) ...
xzy3 pushed a commit to xzy3/cpython that referenced this pull request
Port the _sha1, _sha512, and _md5 extension modules to multi-phase initialization API (PEP 489).
