Skip to content

gh-99108: Refactor _sha256 & _sha512 into _sha2.#101924

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.

Sign up for GitHub

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

Jump to bottom
Merged
gpshead merged 13 commits into from
Feb 16, 2023
Merged

gh-99108: Refactor _sha256 & _sha512 into _sha2. #101924

gpshead merged 13 commits into from
Feb 16, 2023

Conversation

@gpshead
Copy link
Member

@gpsheadgpshead commented Feb 15, 2023•
edited
Loading

This merges their code. They're backed by the same single HACL* static library, having them be a single module simplifies maintenance.

This should unbreak the wasm enscripten builds that currently fail due to linking in --whole-archive mode and the HACL* library appearing twice.

Long unnoticed error fixed: _sha512.SHA384Type was doubly assigned and was actually SHA512Type. Nobody depends on those internal names.

This is a followup to PRs #101707 and #101917.

@gpshead
pythongh-99108: Refactor _sha256 & _sha512 into _sha2.
fea4cdf
This merges their code. They're backed by the same single HACL* star static library, having them be a single module simplifies maintenance. This should unbreak the wasm enscripten builds that currently fail due to linking in --whole-archive mode and the HACL* library appearing twice. Long unnoticed error fixed: `_sha512.SHA384Type` was doubly assigned and was actually SHA512Type. Nobody depends on those internal names.
@bedevere-botbedevere-bot mentioned this pull request Feb 15, 2023
Closed
@gpsheadgpshead self-assigned this Feb 15, 2023
@gpsheadgpshead added extension-modules C modules in the Modules dir build The build process and cross-build labels Feb 15, 2023
@gpsheadgpshead added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Feb 15, 2023
@bedevere-bot
Copy link

bedevere-bot commented Feb 15, 2023

🤖 New build scheduled with the buildbot fleet by @gpshead for commit 4c6a4fb 🤖

If you want to schedule another build, you need to add the :hammer: test-with-buildbots label again.

@bedevere-botbedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Feb 15, 2023
@gpsheadgpshead marked this pull request as ready for review February 15, 2023 03:04
erlend-aasland
erlend-aasland reviewed Feb 15, 2023
View reviewed changes
@erlend-aasland
Copy link
Contributor

erlend-aasland commented Feb 15, 2023

BTW, I'm pretty sure the refleaks are related to #101908.

msprotz
msprotz reviewed Feb 15, 2023
View reviewed changes
@gpsheadgpshead added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Feb 16, 2023
@bedevere-bot
Copy link

bedevere-bot commented Feb 16, 2023

🤖 New build scheduled with the buildbot fleet by @gpshead for commit fb6454d 🤖

If you want to schedule another build, you need to add the :hammer: test-with-buildbots label again.

@bedevere-botbedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Feb 16, 2023
@gpsheadgpshead added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Feb 16, 2023
@bedevere-bot
Copy link

bedevere-bot commented Feb 16, 2023

🤖 New build scheduled with the buildbot fleet by @gpshead for commit 32b8f91 🤖

If you want to schedule another build, you need to add the :hammer: test-with-buildbots label again.

@bedevere-botbedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Feb 16, 2023
@gpsheadgpshead added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Feb 16, 2023
@bedevere-bot
Copy link

bedevere-bot commented Feb 16, 2023

🤖 New build scheduled with the buildbot fleet by @gpshead for commit b67918d 🤖

If you want to schedule another build, you need to add the :hammer: test-with-buildbots label again.

@bedevere-botbedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Feb 16, 2023
@gpsheadgpshead merged commit 0b13575 into python:mainFeb 16, 2023
@gpsheadgpshead deleted the build/sha2 branch February 16, 2023 06:08
@asottileasottile mentioned this pull request Feb 16, 2023
Closed
picnixz added a commit that referenced this pull request Mar 3, 2025
@picnixz
gh-130149: refactor tests for HMAC (#130150)
8f11af4
Since we plan to introduce a built-in implementation for HMAC based on HACL*, it becomes important for the HMAC tests to be flexible enough to avoid code duplication. In addition to the new layout based on mixin classes, we extend test coverage by also testing the `__repr__` of HMAC objects and the HMAC one-shot functions. We also fix the import to `_sha256` which, since gh-101924, resulted in some tests being skipped as the module is no more available (its content was moved to the `_sha2` module).
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@erlend-aaslanderlend-aaslanderlend-aasland left review comments

@tirantiranAwaiting requested review from tiran

@corona10corona10Awaiting requested review from corona10corona10 is a code owner

+1 more reviewer

@msprotzmsprotzmsprotz left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@gpsheadgpshead

Labels

buildThe build process and cross-buildextension-modulesC modules in the Modules dir

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@gpshead@bedevere-bot@erlend-aasland@msprotz