
This issue trackerhas been migrated toGitHub, and is currentlyread-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.
Created on2019-05-19 13:02 byerikjanss, last changed2022-04-11 14:59 byadmin. This issue is nowclosed.
| Pull Requests | |||
|---|---|---|---|
| URL | Status | Linked | Edit |
| PR 13421 | merged | erikjanss,2019-05-19 13:32 | |
| PR 13471 | merged | erikjanss,2019-05-22 07:22 | |
| Messages (14) | |||
|---|---|---|---|
| msg342845 -(view) | Author: Erik Janssens (erikjanss)* | Date: 2019-05-19 13:02 | |
inModules/main.c STATUS_CONTROL_C_EXIT is included conditionally if compiling when _MSC_VER is defined. Later on STATUS_CONTROL_C_EXIT is used if MS_WINDOWS is defined.This breaks compilation of main.c with any non MSC compiler while compiling for MS Windows.This appears to have been introduced inGH-12123 forbpo-36142 | |||
| msg342848 -(view) | Author: Erik Janssens (erikjanss)* | Date: 2019-05-19 13:36 | |
According to the Microsoft documentation athttps://docs.microsoft.com/en-us/windows-hardware/drivers/kernel/using-ntstatus-valuessystem-supplied status codes are defined in ntstatus.h. and the NTSTATUS type is defined in ntdef.hPR 13421 includes both ntstatus.h and ntdef.h to be able to use STATUS_CONTROL_C_EXIT when compiling for windows. | |||
| msg342932 -(view) | Author: Steve Dower (steve.dower)*![]() | Date: 2019-05-20 16:52 | |
Is including just "winnt.h" sufficient?It's very hard to tell which Windows headers are "public" vs "internal", but winternl.h is certainly one of the internal ones (according to the big warning comment at the top of the file) | |||
| msg342934 -(view) | Author: Erik Janssens (erikjanss)* | Date: 2019-05-20 17:01 | |
including "winnt.h" gives me compilation problems with other undefined types.however, simply including "windows.h" instead of both includes compiles just fine.so maybe that is sufficient ?? | |||
| msg342935 -(view) | Author: Steve Dower (steve.dower)*![]() | Date: 2019-05-20 17:11 | |
Some people say "windows.h" is the only one you're ever supposed to include, so if that works best, let's go with that :) | |||
| msg342936 -(view) | Author: Erik Janssens (erikjanss)* | Date: 2019-05-20 17:13 | |
ok, thank you for the advice, I'll keep it in mind and adapt the PR ! | |||
| msg342943 -(view) | Author: Erik Janssens (erikjanss)* | Date: 2019-05-20 18:49 | |
PR has been changed to include "windows.h" ... | |||
| msg342977 -(view) | Author: Eryk Sun (eryksun)*![]() | Date: 2019-05-21 05:08 | |
"crtdbg.h" doesn't provide STATUS_CONTROL_C_EXIT, but it should be fine to remove it anyway. I think it was left behind by accident in 2007. It was added to support a PYTHONNOERRORWINDOW environment variable, but then this idea was dropped in favor of extending the msvcrt module:*https://grokbase.com/t/python/python-3000/078wkax0sd/buildbots*https://github.com/python/cpython/commit/945362cf971fb2e10f8f2a8e71e8ff10516ebe4c#diff-75445bdc3b6b3dd20b005698fa165444*https://github.com/python/cpython/commit/3dc33d18452de871cff98914dda81ff00b4d00f6#diff-75445bdc3b6b3dd20b005698fa165444I presume STATUS_CONTROL_C_EXIT gets included from "winnt.h" -> "Windows.h" -> "Include/internal/pycore_condvar.h" -> "Include/internal/pycore_gil.h" -> "Include/internal/pycore_pystate.h".If [STATUS_]CONTROL_C_EXIT isn't defined, I suggest defining WIN32_LEAN_AND_MEAN before including "Windows.h". This reduces the number of included headers from about 350 down to about 200. Also, to stay strictly within the Windows API, we might want to use CONTROL_C_EXIT (from [min]winbase.h) instead of STATUS_CONTROL_C_EXIT (from "winnt.h"). | |||
| msg342998 -(view) | Author: STINNER Victor (vstinner)*![]() | Date: 2019-05-21 10:11 | |
New changeset925af1d99b69bf3e229411022ad840c5a0cfdcf8 by Victor Stinner (Erik Janssens) in branch 'master':bpo-36965: Fix includes in main.c on Windows with non-MSC compilers (GH-13421)https://github.com/python/cpython/commit/925af1d99b69bf3e229411022ad840c5a0cfdcf8 | |||
| msg343009 -(view) | Author: STINNER Victor (vstinner)*![]() | Date: 2019-05-21 10:57 | |
eryksun commented there, but I prefer to discuss here:https://github.com/python/cpython/commit/925af1d99b69bf3e229411022ad840c5a0cfdcf8#commitcomment-33617265""Windows.h" was already being included, as I mentioned on the issue tracker, because we certainly were not getting STATUS_CONTROL_C_EXIT from "crtdbg.h", a header that was left in this file accidentally about 12 years ago. If it's included explicitly here, also define WIN32_LEAN_AND_MEAN to cut the number of included headers by about a half."I prefer to explicitly include windows.h, it doesn't hurt :-)WIN32_LEAN_AND_MEAN is defined byInclude/internal/pycore_condvar.h which is indirectly included by pycore_pystate.h:#include "pycore_gil.h" /* _gil_runtime_state */pycore_gil.h:#include "pycore_condvar.h"By the way, WIN32_LEAN_AND_MEAN caused me issues while working onbpo-36728:https://twitter.com/VictorStinner/status/1127884878027079680I managed to workaround the issue: commitd5d9e81ce9a7efc5bc14a5c21398d1ef6f626884Extract of (fixed) posixmodule.c:---...#include "Python.h"#ifdef MS_WINDOWS /* include <windows.h> early to avoid conflict with pycore_condvar.h: #define WIN32_LEAN_AND_MEAN #include <windows.h> FSCTL_GET_REPARSE_POINT is not exported with WIN32_LEAN_AND_MEAN. */# include <windows.h>#endif#include "pycore_ceval.h" /* _PyEval_ReInitThreads() */#include "pycore_pystate.h" /* _PyRuntime */...--- | |||
| msg343013 -(view) | Author: STINNER Victor (vstinner)*![]() | Date: 2019-05-21 11:17 | |
> WIN32_LEAN_AND_MEAN is defined byInclude/internal/pycore_condvar.h (...)It would be nice to get ride of #include <windows.h> in Python headers. The <windows.h> was introduced by this commit:commit2ebc5ce42a8a9e047e790aefbf9a94811569b2b6Author: Eric Snow <ericsnowcurrently@gmail.com>Date: Thu Sep 7 23:51:28 2017 -0600bpo-30860: Consolidate stateful runtime globals. (#3397) * group the (stateful) runtime globals into various topical structs * consolidate the topical structs under a single top-level _PyRuntimeState struct * add a check-c-globals.py script that helps identify runtime globals Other globals are excluded (see globals.txt and check-c-globals.py).The current problem is that we need the HANDLE type which comes from <windows.h> to get the full structure:typedef struct _PyCOND_T{ HANDLE sem; int waiting; /* to allow PyCOND_SIGNAL to be a no-op */} PyCOND_T;I tried to avoid "HANDLE" using:typedef struct _PyCOND_T{ void* sem; int waiting; /* to allow PyCOND_SIGNAL to be a no-op */} PyCOND_T;... but then pycore_condvar.h compilation fails on "typedef CRITICAL_SECTION PyMUTEX_T;": CRITICAL_SECTION type is not defined :-(By the way, Python still uses _PY_EMULATED_WIN_CV by default, whereas we want to drop Vista support:bpo-32592. | |||
| msg343018 -(view) | Author: Eryk Sun (eryksun)*![]() | Date: 2019-05-21 11:47 | |
> FSCTL_GET_REPARSE_POINT is not exported with WIN32_LEAN_AND_MEANYou can explicitly include "winioctl.h" after "windows.h". Getting it from "windows.h" is indirect via "winscard.h" (smart card services), which will be skipped if either WIN32_LEAN_AND_MEAN or NOCRYPT is defined. | |||
| msg343172 -(view) | Author: STINNER Victor (vstinner)*![]() | Date: 2019-05-22 11:04 | |
New changeset791e5fcbab9e444b62d13d08707cbbbeb9406297 by Victor Stinner (Erik Janssens) in branch '3.7':bpo-36965: Fix includes in main.c on Windows with non-MSC compilers (GH-13421) (GH-13471)https://github.com/python/cpython/commit/791e5fcbab9e444b62d13d08707cbbbeb9406297 | |||
| msg343173 -(view) | Author: STINNER Victor (vstinner)*![]() | Date: 2019-05-22 11:05 | |
The initial issue is fixed in 3.7 and master branches. If you want to work on related issue like WIN32_LEAN_AND_MEAN, please open a separated issue. | |||
| History | |||
|---|---|---|---|
| Date | User | Action | Args |
| 2022-04-11 14:59:15 | admin | set | github: 81146 |
| 2019-05-22 11:05:10 | vstinner | set | status: open -> closed resolution: fixed messages: +msg343173 stage: patch review -> resolved |
| 2019-05-22 11:04:09 | vstinner | set | messages: +msg343172 |
| 2019-05-22 07:22:06 | erikjanss | set | pull_requests: +pull_request13401 |
| 2019-05-21 11:47:43 | eryksun | set | messages: +msg343018 |
| 2019-05-21 11:17:41 | vstinner | set | messages: +msg343013 |
| 2019-05-21 10:57:02 | vstinner | set | messages: +msg343009 |
| 2019-05-21 10:11:15 | vstinner | set | messages: +msg342998 |
| 2019-05-21 05:08:18 | eryksun | set | nosy: +eryksun messages: +msg342977 |
| 2019-05-20 18:49:32 | erikjanss | set | messages: +msg342943 |
| 2019-05-20 17:13:24 | erikjanss | set | messages: +msg342936 |
| 2019-05-20 17:11:39 | steve.dower | set | messages: +msg342935 |
| 2019-05-20 17:01:33 | erikjanss | set | messages: +msg342934 |
| 2019-05-20 16:52:15 | steve.dower | set | messages: +msg342932 |
| 2019-05-19 13:36:01 | erikjanss | set | messages: +msg342848 |
| 2019-05-19 13:32:12 | erikjanss | set | keywords: +patch stage: patch review pull_requests: +pull_request13330 |
| 2019-05-19 13:06:27 | SilentGhost | set | nosy: +vstinner |
| 2019-05-19 13:02:26 | erikjanss | create | |