classification
Title: crypt.h should be in _cryptmodule.c, not in public header
Type: Stage: resolved
Components: C API Versions: Python 3.11, Python 3.10, Python 3.9, Python 3.8, Python 3.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Arfrever, geofft, miss-islington, thesamesam, vstinner
Priority: normal Keywords: patch

Created on 2021-07-27 21:23 by geofft, last changed 2021-09-30 21:54 by vstinner. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 27394 merged geofft, 2021-07-27 21:32
PR 28636 merged miss-islington, 2021-09-29 20:24
PR 28638 merged miss-islington, 2021-09-29 20:25
Messages (8)
msg398321 - (view) Author: Geoffrey Thomas (geofft) * Date: 2021-07-27 21:23
In #32635, it was discovered that _cryptmodule.c was missing a dependency on crypt.h, which caused it to segfault when it was missing the proper prototype for crypt. This was fixed by adding an #include <crypt.h> to Python.h.

This include doesn't need to be in the public header; it only needs to be in _cryptmodule.c. Removing it from the public header is helpful for packagers, because it means that the libpython-dev (or whatever) package doesn't need a dependency on libcrypt-dev, only on the libcrypt runtime library.
msg398328 - (view) Author: miss-islington (miss-islington) Date: 2021-07-27 22:58
New changeset 196998e220d6ca030e5a1c8ad63fcaed8e049a98 by Geoffrey Thomas in branch 'main':
closes bpo-44751: Move crypt.h include from public header to _cryptmodule (GH-27394)
https://github.com/python/cpython/commit/196998e220d6ca030e5a1c8ad63fcaed8e049a98
msg402894 - (view) Author: Arfrever Frehtes Taifersar Arahesis (Arfrever) * (Python triager) Date: 2021-09-29 17:41
Could you backport this fix to at least 3.9 and 3.10 branches?
This is fix for regression which was previously backported to 2.7 and 3.6 branches.
msg402909 - (view) Author: miss-islington (miss-islington) Date: 2021-09-29 20:48
New changeset 9626ac8b7421aa5fc04a30359521d24f40105141 by Miss Islington (bot) in branch '3.9':
closes bpo-44751: Move crypt.h include from public header to _cryptmodule (GH-27394)
https://github.com/python/cpython/commit/9626ac8b7421aa5fc04a30359521d24f40105141
msg402919 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-09-29 23:02
New changeset 80285ecc8deaa2b0e7351bf4be863d1a0ad3c188 by Miss Islington (bot) in branch '3.10':
closes bpo-44751: Move crypt.h include from public header to _cryptmodule (GH-27394) (GH-28636)
https://github.com/python/cpython/commit/80285ecc8deaa2b0e7351bf4be863d1a0ad3c188
msg402920 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-09-29 23:04
> Could you backport this fix to at least 3.9 and 3.10 branches?

Done.

> This is fix for regression which was previously backported to 2.7 and 3.6 branches.

I'm not sure what you mean.

In Python 2.7 and Python 3.6, <crypt.h> was already included by Include/Python.h:

#ifdef HAVE_CRYPT_H
#include <crypt.h>
#endif
msg402986 - (view) Author: Arfrever Frehtes Taifersar Arahesis (Arfrever) * (Python triager) Date: 2021-09-30 21:02
> > This is fix for regression which was previously backported to 2.7 and 3.6 branches.
> 
> I'm not sure what you mean.
> 
> In Python 2.7 and Python 3.6, <crypt.h> was already included by Include/Python.h:

See issue #32635, where Victor Stinner backported some commit (with problematic location of '#include <crypt.h>') to 2.7 and 3.6 branches (which was released in >=2.7.15 and >=3.6).
msg402990 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-09-30 21:54
> See issue #32635, where Victor Stinner backported some commit (with problematic location of '#include <crypt.h>') to 2.7 and 3.6 branches (which was released in >=2.7.15 and >=3.6).

crypt.h was added to Python.h by: https://github.com/python/cpython/pull/5284/files

And this change was then backported to 2.7 and 3.6. Ok, now I understand.

Well, it's now fixed in all branches accepting bugfixes.
History
Date User Action Args
2021-09-30 21:54:16vstinnersetmessages: + msg402990
2021-09-30 21:02:41Arfreversetmessages: + msg402986
2021-09-29 23:04:11vstinnersetmessages: + msg402920
2021-09-29 23:02:15vstinnersetnosy: + vstinner
messages: + msg402919
2021-09-29 20:48:04miss-islingtonsetmessages: + msg402909
2021-09-29 20:25:08miss-islingtonsetpull_requests: + pull_request27007
2021-09-29 20:24:35miss-islingtonsetpull_requests: + pull_request27005
2021-09-29 17:49:33thesamesamsetnosy: + thesamesam
2021-09-29 17:41:21Arfreversetnosy: + Arfrever
messages: + msg402894
2021-07-27 22:58:40miss-islingtonsetstatus: open -> closed

nosy: + miss-islington
messages: + msg398328

resolution: fixed
stage: patch review -> resolved
2021-07-27 21:32:14geofftsetkeywords: + patch
stage: patch review
pull_requests: + pull_request25927
2021-07-27 21:23:37geofftcreate