Skip to content

GH-145110: fix arguments Clean and CleanAll for argument -t of batch.bat in case of PGO builds on Windows #145111

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

Merged
zooba merged 1 commit into from
Feb 23, 2026
+16 −5
Merged

GH-145110: fix arguments Clean and CleanAll for argument -t of batch.bat in case of PGO builds on Windows #145111

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 2 additions & 0 deletions Misc/NEWS.d/next/Build/2026-02-22-13-35-20.gh-issue-145110.KgWofW.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Fix targets "Clean" and "CLeanAll" in case of PGO builds on Windows. Patch by
Chris Eibl.
19 changes: 14 additions & 5 deletions PCbuild/build.bat
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ setlocal
set platf=x64
set conf=Release
set target=Build
set clean=false
set dir=%~dp0
set parallel=/m
set verbose=/nologo /v:m /clp:summary
Expand Down Expand Up @@ -119,6 +120,9 @@ if "%UseJIT%" NEQ "true" set IncludeLLVM=false

if "%IncludeExternals%"=="true" call "%dir%get_externals.bat"

if /I "%target%"=="Clean" set clean=true
if /I "%target%"=="CleanAll" set clean=true

if "%do_pgo%" EQU "true" if "%platf%" EQU "x64" (
if "%PROCESSOR_ARCHITEW6432%" NEQ "AMD64" if "%PROCESSOR_ARCHITECTURE%" NEQ "AMD64" (
echo.ERROR: Cannot cross-compile with PGO
Expand Down Expand Up @@ -159,15 +163,20 @@ if "%do_pgo%"=="true" (
rem %VARS% are evaluated eagerly, which would lose the ERRORLEVEL
rem value if we didn't split it out here.
if "%do_pgo%"=="true" if ERRORLEVEL 1 exit /B %ERRORLEVEL%

rem In case of a PGO build, we switch the conf here to PGUpdate
rem to get it cleaned or built as well.
if "%do_pgo%"=="true" (
del /s "%dir%\*.pgc"
del /s "%dir%\..\Lib\*.pyc"
echo on
call "%dir%\..\python.bat" %pgo_job%
@echo off
call :Kill
set conf=PGUpdate
set target=Build
if "%clean%"=="false" (
echo on
call "%dir%\..\python.bat" %pgo_job%
@echo off
call :Kill
set target=Build
Copy link
Member Author

@chris-eibl chris-eibl Feb 22, 2026

Choose a reason for hiding this comment

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

I do not really know why the target is switched here. For Clean and CleanAll this is definitely wrong. This only leaves Rebuild which is still converted to Build here like before.

I'd happily remove it so that Rebuild does what is asked for, and a quick test shows that it works.
Git blame reveals it was introduced in this commit linked to https://bugs.python.org/issue28573, which is dealing about PATH getting to long and

[...] also fixes two other issues with my build script - the PGOOPTS need to come before CERTOPTS, as they are parsed by different scripts, and the debug build command fell off somewhere which resulted in the debug builds not being rebuilt for 3.6.0b4 - they are still the 3.6.0b3 debug build.

Yet, I am unsure and might be overlooking something, so I haven't touched it (yet).

Copy link
Member

@zooba zooba Feb 23, 2026

Choose a reason for hiding this comment

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

Unless I'm misremembering how this works (I almost never rely on this batch file anymore - real releases don't use it at all), Rebuild here would cause recompiling post-profile, which isn't necessary. All the PGO happens during link, so we should save about half the build time by not rebuilding things that don't need rebuilding.

We would want Rebuild during PGInstrument, since that's the first stage. But from then, it's already re-built, and so subsequent stages shouldn't Rebuild.

Copy link
Member Author

@chris-eibl chris-eibl Feb 23, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Ok, then let's keep it. Then I think this is fine to merge? I'll take care about the long path problem of the AMD64 Windows PGO NoGIL Tailcall PR builder with @itamaro in a discord discussion. I am close to 100% sure the regular builders will be green (like AMD64 Windows PGO NoGIL PR is for this PR as well).

)
)
goto :Build

Expand Down
Loading