Skip to content
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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

gh-111650: Generate pyconfig.h on Windows #112179

Closed
wants to merge 4 commits into from
Closed

gh-111650: Generate pyconfig.h on Windows #112179

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6585ab1
gh-111650: Generate pyconfig.h on Windows
colesbury Nov 16, 2023
dbe0ddd
Fix test_sysconfig.py
colesbury Nov 16, 2023
25c6028
Add blurb
colesbury Nov 16, 2023
b654d10
Rename Py_NOGIL to Py_GIL_DISABLED
colesbury Nov 20, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ Programs/_testembed
PC/python_nt*.h
PC/pythonnt_rc*.h
Modules/python.exp
PC/pyconfig.h
PC/*/*.exp
PC/*/*.lib
PC/*/*.bsc
Expand Down
7 changes: 4 additions & 3 deletions Lib/test/test_sysconfig.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -472,9 +472,10 @@ def test_srcdir(self):
# should be a full source checkout.
Python_h = os.path.join(srcdir, 'Include', 'Python.h')
self.assertTrue(os.path.exists(Python_h), Python_h)
# <srcdir>/PC/pyconfig.h always exists even if unused on POSIX.
pyconfig_h = os.path.join(srcdir, 'PC', 'pyconfig.h')
self.assertTrue(os.path.exists(pyconfig_h), pyconfig_h)
if os.name == 'nt':
# <srcdir>/PC/pyconfig.h only exists on Windows.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Honestly, it's probably a better long-term move to get rid of PC/pyconfig.h completely and generate it in Include where the POSIX one would be.

Eventually (though I suspect it's "likely never") we'll get a combined build system. At that point, it'll be much more sensible to have a coherent set of include files.

The code that tries to include header files in installers already assumes it can pick it up directly from a repository checkout (Tools/msi/dev and PC/layout, IIRC). That will need additional logic to pick it up from somewhere else - I'd suggest the build directory $(Py_OutDir) along with every other generated file that we include in the distribution.

pyconfig_h = os.path.join(srcdir, 'PC', 'pyconfig.h')
self.assertTrue(os.path.exists(pyconfig_h), pyconfig_h)
pyconfig_h_in = os.path.join(srcdir, 'pyconfig.h.in')
self.assertTrue(os.path.exists(pyconfig_h_in), pyconfig_h_in)
elif os.name == 'posix':
Expand Down
2 changes: 2 additions & 0 deletions Misc/NEWS.d/next/Windows/2023-11-16-16-19-51.gh-issue-111650.zdWe-n.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
On Windows, ``PC\pyconfig.h`` is now generated from ``PC\pyconfig.h.in``
during the build process.
5 changes: 4 additions & 1 deletion PC/pyconfig.h → PC/pyconfig.h.in
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#ifndef Py_CONFIG_H
#define Py_CONFIG_H

/* pyconfig.h. NOT Generated automatically by configure.
/* pyconfig.h.in.

This is a manually maintained version used for the Watcom,
Borland and Microsoft Visual C++ compilers. It is a
Expand Down Expand Up @@ -710,6 +710,9 @@ Py_NO_ENABLE_SHARED to find out. Also support MS_NO_COREDLL for b/w compat */
/* Define to 1 if you have the `erfc' function. */
#define HAVE_ERFC 1

/* Define if you want to disable the GIL */
#undef Py_GIL_DISABLED

// netdb.h functions (provided by winsock.h)
#define HAVE_GETHOSTNAME 1
#define HAVE_GETHOSTBYADDR 1
Expand Down
50 changes: 50 additions & 0 deletions PCbuild/generate_pyconfig.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
#
# Generates pyconfig.h from PC\pyconfig.h.in
#

param (
[string[]]$define,
[string]$File
)

$definedValues = @{}

foreach ($arg in $define) {
$parts = $arg -split '='

if ($parts.Count -eq 1) {
$key = $parts[0]
$definedValues[$key] = ""
} elseif ($parts.Count -eq 2) {
$key = $parts[0]
$value = $parts[1]
$definedValues[$key] = $value
} else {
Write-Host "Invalid argument: $arg"
exit 1
}
}

$cpythonRoot = Split-Path $PSScriptRoot -Parent
$pyconfigPath = Join-Path $cpythonRoot "PC\pyconfig.h.in"

$header = "/* pyconfig.h. Generated from PC\pyconfig.h.in by generate_pyconfig.ps1. */"

$lines = Get-Content -Path $pyconfigPath
$lines = @($header) + $lines

foreach ($i in 0..($lines.Length - 1)) {
if ($lines[$i] -match "^#undef (\w+)$") {
$key = $Matches[1]
if ($definedValues.ContainsKey($key)) {
$value = $definedValues[$key]
$lines[$i] = "#define $key $value".TrimEnd()
} else {
$lines[$i] = "/* #undef $key */"
}
}
}
Comment on lines +36 to +46
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
foreach ($i in 0..($lines.Length - 1)) {
if ($lines[$i] -match "^#undef (\w+)$") {
$key = $Matches[1]
if ($definedValues.ContainsKey($key)) {
$value = $definedValues[$key]
$lines[$i] = "#define $key $value".TrimEnd()
} else {
$lines[$i] = "/* #undef $key */"
}
}
}
$lines = $lines | %{
if ($_ -match "^#undef (\w+)$") {
$key = $Matches[1];
if ($definedValues.ContainsKey($key)) {
"#define $key ${definedValues[$key]}".TrimEnd()
} else {
"/* #undef $key */"
}
} else {
$_
}
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, that was a bit of a self-indulgent rewrite that didn't actually make it shorter, simpler, or probably faster 😆

But if you want, we already assume that a copy of Python is available for bootstrapping on Windows (and I believe we require 3.10), so you could make this a Python script. IIRC, $(HostPython) has the location - it should be used elsewhere in regen.targets if you need examples.


$ParentDir = Split-Path -Path $File -Parent
New-Item -ItemType Directory -Force -Path $ParentDir | Out-Null
Set-Content -Path $File -Value $lines
7 changes: 7 additions & 0 deletions PCbuild/pcbuild.proj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
<Import Project="$(VCTargetsPath)\Microsoft.Cpp.Default.props"/>
<Import Project="$(VCTargetsPath)\Microsoft.Cpp.props"/>
<Import Project="$(VCTargetsPath)\Microsoft.Cpp.targets"/>
<Import Project="python.props" />
<PropertyGroup Label="Globals">
<ProjectGuid>{CC9B93A2-439D-4058-9D29-6DCF43774405}</ProjectGuid>
<Platform Condition="'$(Platform)' == ''">Win32</Platform>
Expand All @@ -14,6 +15,7 @@
<IncludeSSL Condition="'$(IncludeSSL)' == ''">true</IncludeSSL>
<IncludeTkinter Condition="'$(IncludeTkinter)' == ''">true</IncludeTkinter>
<IncludeUwp Condition="'$(IncludeUwp)' == ''">false</IncludeUwp>
<PyConfigArgs Condition="'$(DisableGil)' == 'true'">Py_GIL_DISABLED=1,$(PyConfigArgs)</PyConfigArgs>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Semicolon separated is the usual for MSBuild files. I know I'll think this is wrong every time I see it - can we maybe $(PyConfigArgs.Replace(`;`, `,`)) later if it's really needed?

</PropertyGroup>

<ItemDefinitionGroup>
Expand Down Expand Up @@ -94,6 +96,11 @@
<Projects2 Include="venvlauncher.vcxproj;venvwlauncher.vcxproj" />
</ItemGroup>

<Target Name="PreBuild" BeforeTargets="Build">
<!-- Stick a _PLACEHOLDER=1 after $(PyConfigArgs) to handle both trailing commas and empty $(PyConfigArgs) -->
<Exec Command="powershell.exe $(PySourcePath)PCbuild\generate_pyconfig.ps1 -File $(PySourcePath)PC\pyconfig.h -define $(PyConfigArgs)_PLACEHOLDER=1" />
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would rather have this happen in pythoncore.vcxproj if possible, or _freeze_module.vcxproj if necessary (or both). That way we don't have to rely on building through this file to make sure it happens.

It probably also belongs in the regen.targets file, and can then be referenced from the others.

</Target>

<Target Name="Build">
<MSBuild Projects="@(FreezeProjects)"
Properties="Configuration=%(Configuration);Platform=%(Platform);%(Properties)"
Expand Down
Loading