Repository navigation
Py_GIL_DISABLED not defined in "free-threaded" Windows C-API extensions #111650
Description
Activity
Among other problems, this leads to test_peg_generator failing on the nogil Windows buildbot (example failure).
I was curious about how GHA passed the test, but I noticed that it was skipped from the GHA..
https://github.com/python/cpython/actions/runs/6695685454/job/181918354850:16:09 load avg: 10.69 [405/469/1] test_peg_generator skipped (resource denied) -- running (1): test_importlib (42.1 sec) test_peg_generator skipped -- Use of the 'cpu' resource not enabledReacted by Sam GrossI think we probably need to start modifying pyconfig.h at some stage in the build process. Eventually we should get an improved sysconfig [alternative] that can include the flag, but until then I don't think we want to make modifications in every build backend that's out there, nor do we want to force them to launch the target interpreter in order to get the right settings (which prevents cross-compilation, for example).
I'm not sure how best to do it though. Probably the ideal looks like:
- copy/patch pyconfig.h and write it to
$(IntDir)(not back into the source tree) - do rest of build referencing that directory instead of
PC\ - copy patched pyconfig.h into
$(OutDir) - make sure Tools/msi and PC/layout pull it from the output directory and not the sources
- make sure the build in release-tools does the same (though I think it should happen automatically with the previous changes)
We shouldn't have to patch anything in the stdlib that finds the header, I'm pretty sure they were only in distutils. And an install layout should have it end up in the
Includedirectory if Tools/msi and PC/layout are updated.So there's a lot of moving parts, plus we need to figure out the best way to template the header so it can be modified as part of the build. There's not really anything built into our build system for this yet.
Reacted by Sam GrossReacted by stonebig- copy/patch pyconfig.h and write it to
Hi @zooba, I started implementing this and ran into a few issues. Not having a
pyconfig.hinPCorIncludebreaks setuptools. The install layout can place the generatedpyconfig.hin the installedIncludedirectory, but you can also runpipdirectly from the CPython source directory without the installer. It seems like we'll run into fewer issues if we place the generatedpyconfig.hin thePCdirectory.- added 6 commits that reference this issue
on Nov 16, 2023 Left a few suggestions on the draft PR - in short:
- I prefer
Include\pyconfig.hoverPC\pyconfig.h, and I suspect/hope every other tool will, too. setuptoolsis definitely going to need an update of some kind here, at least for running within build directories (likely in code taken fromdistutils). Regular installs shouldn't have any issue. I think we should do the setuptools update in parallel, and not try to make their older versions work.- As neat as the PowerShell script is, we already assume a bootstrap Python is available, so it may as well be a Python script
- I prefer
POSIX places the generated
pyconfig.hin the root directory, not inInclude. Placing it inInclude/pyconfig.his would be a third location for tools to handle, breakssetuptools(and possibly other build tools), and does not get us any closer to unifying the builds.setuptools is definitely going to need an update of some kind here...
If we don't move
pyconfig.h, what update will setuptools need?As neat as the PowerShell script is, we already assume a bootstrap Python is available, so it may as well be a Python script
Sure - I'll convert the script to Python.
1 remaining item
I don't know how to implement what you are asking for and I've already spent a few days on this, so I'm going to defer this and come back to it later.
Reacted by Steve DowerI'll get it figured out. Chances are I'm going to have to do all the integration into the release process anyway, so I may as well go end-to-end.
- changed the title
[-]Py_NOGIL not defined in "free-threaded" Windows C-API extensions [/-][+]Py_GIL_DISABLED not defined in "free-threaded" Windows C-API extensions [/+]on Dec 6, 2023 Is there anything left to do here?
- added 2 commits that reference this issue
on Sep 27, 2024 - added a commit that references this issue
on Sep 30, 2024
Bug report
On Windows, the
Py_GIL_DISABLEDmacro is not defined when building C API extensions with the free-threaded build of CPython 3.13 (i.e.,--disable-gil). Now that biased reference counting (6dfb8fe) landed, this leads to incorrect/failed compilation of extensions on Windows 1. The problem does not occur on Linux/macOS and other./configurebased builds because that build system defines thePy_NOGILmacro as part ofpyconfig.h. On Windows,Py_GIL_DISABLEDinstead defined as a pre-processor definition inpyproject.props, but that's used by C-API extensions.Among other problems, this leads to
test_peg_generatorfailing on the nogil Windows buildbot (example failure).I can think of two possible approaches to fixing this:
Py_GIL_DISABLEDwhen invoking the compiler ifsysconfig.get_config_var("Py_NOGIL")isTrue.Py_GIL_DISABLEDdepending on if the build is invoked with--disable-gilor not.The first option seems a little more straightforward to me, but there is a risk that we might encounter this same issue across multiple extension build systems. The second option seems like it would more robustly address this issue.
Linked PRs
Footnotes
Modules built as part of the CPython build (i.e., standard library modules) still work ↩