Skip to content

gh-148200: fix warning on missing safe memzero() on CYGWIN#148202

Open
carlo-bramini wants to merge 3 commits intopython:mainfrom
carlo-bramini:fix-cygwin-warning-2
Open

gh-148200: fix warning on missing safe memzero() on CYGWIN#148202
carlo-bramini wants to merge 3 commits intopython:mainfrom
carlo-bramini:fix-cygwin-warning-2

Conversation

@carlo-bramini
Copy link
Copy Markdown

@carlo-bramini carlo-bramini commented Apr 7, 2026

@bedevere-app
Copy link
Copy Markdown

bedevere-app bot commented Apr 7, 2026

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

@bedevere-app
Copy link
Copy Markdown

bedevere-app bot commented Apr 7, 2026

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

@carlo-bramini
Copy link
Copy Markdown
Author

I have not added a news entry because this seems to me an extremely trivial change, however let me know if this could be required. Thank you very much.

@picnixz
Copy link
Copy Markdown
Member

picnixz commented Apr 9, 2026

For this, I think we should rather change upstream. I think we changed those values upstream instead (I don't want to diverge from HACL* sources in general).

cc @protz

@protz
Copy link
Copy Markdown
Contributor

protz commented Apr 9, 2026

Thanks I'll replicate the patch upstream

protz added a commit to hacl-star/hacl-star that referenced this pull request Apr 9, 2026
protz added a commit to hacl-star/hacl-star that referenced this pull request Apr 9, 2026
@picnixz
Copy link
Copy Markdown
Member

picnixz commented Apr 10, 2026

@carlo-bramini Could you update the PR so that it pulls the updated upstream instead (update refresh.sh to pull new sources)

@picnixz
Copy link
Copy Markdown
Member

picnixz commented Apr 10, 2026

Sorry, I meant you need to change the refresh.sh script in Modules/_hacl (or something like that) and set the commit hash that contains the upstream fix.

@python-cla-bot
Copy link
Copy Markdown

python-cla-bot bot commented Apr 10, 2026

All commit authors signed the Contributor License Agreement.

CLA signed

@carlo-bramini
Copy link
Copy Markdown
Author

The following commit authors need to sign the Contributor License Agreement:

* [30959007+carlo-bramini@users.noreply.github.com](mailto:30959007+carlo-bramini@users.noreply.github.com)

CLA not signed

Excuse me, I had already signed the CLA, what is this?

@carlo-bramini carlo-bramini force-pushed the fix-cygwin-warning-2 branch 2 times, most recently from 1e6383f to f099b51 Compare April 10, 2026 08:49
@picnixz
Copy link
Copy Markdown
Member

picnixz commented Apr 10, 2026

Excuse me, I had already signed the CLA, what is this?

This may happen if you push with a different account / git setting or sometimes web UI

@picnixz
Copy link
Copy Markdown
Member

picnixz commented Apr 10, 2026

FTR, you need to call the refresh.sh script (it seems the upstream commit was not pulled)

@picnixz
Copy link
Copy Markdown
Member

picnixz commented Apr 10, 2026

Oh and you actually need a local HACL* copy for that to work.

@carlo-bramini
Copy link
Copy Markdown
Author

carlo-bramini commented Apr 10, 2026

@picnixz @protz I have seen that there are several copies of Lib_Memzero0.c into the upstream of HACL.
The patch has been imported only into lib/c/Lib_Memzero0.c, but the refresh.sh script from the sources of Python is importing a file with the same name but from the dist\gcc-compatible directory.
As result, the latest patches are not imported into Python.
How does this thing need to be handled?

@protz
Copy link
Copy Markdown
Contributor

protz commented Apr 10, 2026

Upstream was just fixed via hacl-star/hacl-star#1070 which propagates the hand-written copy into the packaged distributions ("dist").

Sorry for the confusion -- I should've commented here that there were still a few more steps required to fully propagate the fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants