Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

gh-131876: extract_hashlib helpers into a separate directory#136995

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged

Conversation

@picnixz
Copy link
Member

@picnixzpicnixz commentedJul 22, 2025
edited by bedevere-appbot
Loading

@picnixzpicnixz changed the titlegh-135341: extract_hashlib helpers into a separate directorygh-131876: extract_hashlib helpers into a separate directoryJul 22, 2025
@picnixzpicnixzforce-pushed therefactor/hashlib/extract-helpers-135341 branch from93fdbd3 to59b0cdaCompareJuly 22, 2025 13:23
@picnixzpicnixz added the 🔨 test-with-buildbotsTest PR w/ buildbots; report in status section labelJul 22, 2025
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by@picnixz for commit59b0cda 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F136995%2Fmerge

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

@bedevere-botbedevere-bot removed the 🔨 test-with-buildbotsTest PR w/ buildbots; report in status section labelJul 22, 2025
@picnixzpicnixz marked this pull request as ready for reviewJuly 25, 2025 14:00
@picnixz
Copy link
MemberAuthor

picnixz commentedJul 27, 2025
edited
Loading

I'll first merge the fortication PR, then merge this one where the macros are rewritten are pure functions when possible. I actually wanted to check whether using an exported function vs a static inline function could change a lot the performance (if possible I don't want to make the code significantly slower because I used non-static inline functions).

@gpshead
Copy link
Member

for these code paths Idoubt inline even matters. in general these days I assume the compiler will figure that out for static things, at least in pgo and lto builds.

@picnixz
Copy link
MemberAuthor

picnixz commentedJul 28, 2025
edited
Loading

for these code paths I doubt inline even matters. in general these days I assume the compiler will figure that out for static things, at least in pgo and lto builds.

Actually, I was also a bit worried about the exported symbol. I also try to see if I can just live withextern and not withPyAPI_FUNC actually and if everything links correctly, then I'll be able to move the function definitions into a .c file instead.

Otherwise, I'll usestatic inline for both instead of usingPyAPI_FUNC.

EDIT: everything works well withextern

@picnixz
Copy link
MemberAuthor

Hum. Windows. Ok I don't know how to correct the project so usingPyAPI_FUNC should be enough. I think we should find a better way to manage cryptographic modules on Windows as I feel it's confusing (I don't really know what to exactly do...)

@picnixz
Copy link
MemberAuthor

!buildbot FIPS only

@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by@picnixz for commita0fa849 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F136995%2Fmerge

The command will test the builders whose names match following regular expression:FIPS only

The builders matched are:

  • AMD64 RHEL8 FIPS Only Blake2 Builtin Hash PR
  • AMD64 CentOS9 FIPS Only Blake2 Builtin Hash PR

@picnixzpicnixz marked this pull request as ready for reviewJuly 28, 2025 09:17
@picnixzpicnixz merged commit45138d3 intopython:mainJul 28, 2025
45 checks passed
@picnixzpicnixz deleted the refactor/hashlib/extract-helpers-135341 branchJuly 28, 2025 09:28
@picnixz
Copy link
MemberAuthor

I don't think I can backport this as I think it's an ABI change?

@freakboy3742
Copy link
Contributor

Not sure why this hasn't flagged, but this seems to have caused a big problem for the Emscripten buildbot:https://buildbot.python.org/#/builders/1810/builds/234

@hoodmane
Copy link
Contributor

Problem is we pass libhashlib.a to the linker twice which wasm-ld doesn't like.

@zooba
Copy link
Member

There shouldn't be any need forPyAPI_FUNC in internal headers - it's for public exported API (that needs to be used by apps loadingpython315.dll).

This seems like it should be a fully internal change, just refactoring sources?

gpshead reacted with thumbs up emoji

@picnixz
Copy link
MemberAuthor

Oh so it did cause an issue. I actually tried using extern but the Windows build failed so I wasn't sure about it.

@picnixz
Copy link
MemberAuthor

picnixz commentedAug 1, 2025
edited
Loading

I will take care of this by making everything static inline just to prevent polluting main for a moment. Or I'll directly try to see how I can correctly use extern (I think the problem lies with the MSVC projects where I get duplicated symbols)

@zooba could you help me here setting up the projects?

@picnixz
Copy link
MemberAuthor

picnixz commentedAug 1, 2025
edited
Loading

I tried something but I can only rely on the CI to test the build:#137301. I think the issue is that we have a hashlib project but we don't have HACL* projects. Is it sufficient for me to actually only specify the files to compile in the hashlib project (I don't know if it's possible to disable hashlib on Windows)? would they be picked up by the "parent" project correctly? EDIT: well apparently not. Let's discuss what I should do on the new PR.

picnixz added a commit to picnixz/cpython that referenced this pull requestAug 1, 2025
gpshead pushed a commit that referenced this pull requestAug 1, 2025
…ate directory (#136995) (#137307)Revert "gh-131876: extract `_hashlib` helpers into a separate directory (#136995)"This reverts commit45138d3.
@bedevere-bot
Copy link

⚠️⚠️⚠️ Buildbot failure⚠️⚠️⚠️

Hi! The buildbotAMD64 CentOS9 NoGIL Refleaks 3.x (tier-1) has failed when building commitfe0e921.

What do you need to do:

  1. Don't panic.
  2. Checkthe buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. Go to the page of the buildbot that failed (https://buildbot.python.org/#/builders/1610/builds/1839) and take a look at the build logs.
  4. Check if the failure is related to this commit (fe0e921) or if it is a false positive.
  5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/#/builders/1610/builds/1839

Failed tests:

  • test_free_threading

Summary of the results of the build (if available):

==

Click to see traceback logs
remote:Enumerating objects: 7, done.remote:Counting objects:  14% (1/7)remote:Counting objects:  28% (2/7)remote:Counting objects:  42% (3/7)remote:Counting objects:  57% (4/7)remote:Counting objects:  71% (5/7)remote:Counting objects:  85% (6/7)remote:Counting objects: 100% (7/7)remote:Counting objects: 100% (7/7), done.remote:Compressing objects:  14% (1/7)remote:Compressing objects:  28% (2/7)remote:Compressing objects:  42% (3/7)remote:Compressing objects:  57% (4/7)remote:Compressing objects:  71% (5/7)remote:Compressing objects:  85% (6/7)remote:Compressing objects: 100% (7/7)remote:Compressing objects: 100% (7/7), done.remote:Total 7 (delta 0), reused 0 (delta 0), pack-reused 0 (from 0)From https://github.com/python/cpython * branch                    main       -> FETCH_HEADNote:switching to 'fe0e921817a7f96c62c91085884ab910859328ce'.You are in 'detached HEAD' state. You can look around, make experimentalchanges and commit them, and you can discard any commits you make in thisstate without impacting any branches by switching back to a branch.If you want to create a new branch to retain commits you create, you maydo so (now or later) by using -c with the switch command. Example:  git switch -c <new-branch-name>Or undo this operation with:  git switch -Turn off this advice by setting config variable advice.detachedHead to falseHEAD is now at fe0e921817a gh-131876: Revert "gh-131876: extract `_hashlib` helpers into a separate directory (#136995) (#137307)Switched to and reset branch 'main'configure:WARNING: no system libmpdec found; falling back to pure-Python version for the decimal modulemake:*** [Makefile:2486: buildbottest] Error 2

picnixz added a commit to picnixz/cpython that referenced this pull requestAug 2, 2025
…python#136995)The `Modules/hashlib.h` helper file is now removed and split into multiple files:* `Modules/_hashlib/hashlib_buffer.[ch]` -- Utilities for getting a buffer view and handling buffer inputs.* `Modules/_hashlib/hashlib_fetch.h` -- Utilities used when fetching a message digest from a digest-like identifier.  Currently, this file only contains common error messages as the fetching API is not yet implemented.* `Modules/_hashlib/hashlib_mutex.h` -- Utilities for managing the lock on cryptographic hash objects.
picnixz added a commit to picnixz/cpython that referenced this pull requestAug 2, 2025
…python#136995)The `Modules/hashlib.h` helper file is now removed and split into multiple files:* `Modules/_hashlib/hashlib_buffer.[ch]` -- Utilities for getting a buffer view and handling buffer inputs.* `Modules/_hashlib/hashlib_fetch.h` -- Utilities used when fetching a message digest from a digest-like identifier.  Currently, this file only contains common error messages as the fetching API is not yet implemented.* `Modules/_hashlib/hashlib_mutex.h` -- Utilities for managing the lock on cryptographic hash objects.
picnixz added a commit to picnixz/cpython that referenced this pull requestAug 2, 2025
…python#136995)The `Modules/hashlib.h` helper file is now removed and split into multiple files:* `Modules/_hashlib/hashlib_buffer.[ch]` -- Utilities for getting a buffer view and handling buffer inputs.* `Modules/_hashlib/hashlib_fetch.h` -- Utilities used when fetching a message digest from a digest-like identifier.  Currently, this file only contains common error messages as the fetching API is not yet implemented.* `Modules/_hashlib/hashlib_mutex.h` -- Utilities for managing the lock on cryptographic hash objects.
picnixz added a commit to picnixz/cpython that referenced this pull requestAug 2, 2025
…python#136995)The `Modules/hashlib.h` helper file is now removed and split into multiple files:* `Modules/_hashlib/hashlib_buffer.h` -- Utilities for getting a buffer view and handling buffer inputs.* `Modules/_hashlib/hashlib_fetch.h` -- Utilities used when fetching a message digest from a digest-like identifier.  Currently, this file only contains common error messages as the fetching API is not yet implemented.* `Modules/_hashlib/hashlib_mutex.h` -- Utilities for managing the lock on cryptographic hash objects.
Agent-Hellboy pushed a commit to Agent-Hellboy/cpython that referenced this pull requestAug 19, 2025
…python#136995)The `Modules/hashlib.h` helper file is now removed and split into multiple files:* `Modules/_hashlib/hashlib_buffer.[ch]` -- Utilities for getting a buffer view and handling buffer inputs.* `Modules/_hashlib/hashlib_fetch.h` -- Utilities used when fetching a message digest from a digest-like identifier.  Currently, this file only contains common error messages as the fetching API is not yet implemented.* `Modules/_hashlib/hashlib_mutex.h` -- Utilities for managing the lock on cryptographic hash objects.
Agent-Hellboy pushed a commit to Agent-Hellboy/cpython that referenced this pull requestAug 19, 2025
…into a separate directory (python#136995) (python#137307)Revert "pythongh-131876: extract `_hashlib` helpers into a separate directory (python#136995)"This reverts commit45138d3.
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@gpsheadgpsheadgpshead approved these changes

@tirantiranAwaiting requested review from tiran

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@picnixz@bedevere-bot@gpshead@freakboy3742@hoodmane@zooba

[8]ページ先頭

©2009-2025 Movatter.jp