Skip to content

Comments

GH-145110: fix targets "Clean" and "CleanAll" in case of Windows PGO builds#145111

Open
chris-eibl wants to merge 1 commit intopython:mainfrom
chris-eibl:fix_pgo_clean
Open

GH-145110: fix targets "Clean" and "CleanAll" in case of Windows PGO builds#145111
chris-eibl wants to merge 1 commit intopython:mainfrom
chris-eibl:fix_pgo_clean

Conversation

@chris-eibl
Copy link
Member

@chris-eibl chris-eibl commented Feb 22, 2026

In case of Clean or CleanAll, the

  • the pgo_job must not be invoked
  • and the target must not be switched to Build


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

if /I "%target%"=="Clean" set clean=true
Copy link
Member Author

Choose a reason for hiding this comment

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

This is a simple poor man's or, since Windows batch files do not feature that. Using /I here for a case insensitive compare, because msbuild accepts it case insensitive as well.

@chris-eibl

This comment was marked as outdated.

@bedevere-bot

This comment was marked as outdated.

@chris-eibl
Copy link
Member Author

!buildbot AMD64 Windows PGO NoGIL

@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @chris-eibl for commit f0a4c99 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F145111%2Fmerge

The command will test the builders whose names match following regular expression: AMD64 Windows PGO NoGIL

The builders matched are:

  • AMD64 Windows PGO NoGIL Tailcall PR
  • AMD64 Windows PGO NoGIL PR

call "%dir%\..\python.bat" %pgo_job%
@echo off
call :Kill
set target=Build
Copy link
Member Author

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).

@chris-eibl
Copy link
Member Author

The builders matched are:

  • AMD64 Windows PGO NoGIL Tailcall PR
  • AMD64 Windows PGO NoGIL PR

Build logs look good now, but test_peg_generator failed on AMD64 Windows PGO NoGIL Tailcall PR. This looks unrelated to me, triggering again ...

@chris-eibl
Copy link
Member Author

!buildbot AMD64 Windows PGO NoGIL Tailcall

@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @chris-eibl for commit f0a4c99 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F145111%2Fmerge

The command will test the builders whose names match following regular expression: AMD64 Windows PGO NoGIL Tailcall

The builders matched are:

  • AMD64 Windows PGO NoGIL Tailcall PR

@chris-eibl
Copy link
Member Author

Ups, failed again. Lots of

fatal error C1083: Cannot open compiler generated file: '': Invalid argument

errors which hint that we are hitting a path limit here.

test_peg_generator runs fine for me locally.

@itamaro enabling long paths or just limiting the folder where the builds run by a tad will hopefully fix this.

This seems slightly too long

/FoC:\Users\Administrator\buildarea\pull_request.itamaro-win64-srv-22-aws.nogil.tailcall.pgo\build\build\test_python_14836æ\tmp_hqf4_wt\Users\Administrator\buildarea\pull_request.itamaro-win64-srv-22-aws.nogil.tailcall.pgo\build\Parser\tokenizer\string_tokenizer.obj

The non-pr version ran fine just few hours ago. The pull_request part in the folder name vs the non-pr versions seems to bring us over the default 260 character path limit.

Most probably enabling long paths on your build bot will help?

@itamaro
Copy link
Contributor

itamaro commented Feb 22, 2026

Most probably enabling long paths on your build bot will help?

I thought long paths are already enabled... I'll take another look when I get a chance.

Is the build script checking and logging that? That would be a useful diagnostic to have

@itamaro
Copy link
Contributor

itamaro commented Feb 22, 2026

I thought long paths are already enabled... I'll take another look when I get a chance.

@chris-eibl the worker provisioning script includes this:

New-ItemProperty -Path "HKLM:\SYSTEM\CurrentControlSet\Control\FileSystem" -Name "LongPathsEnabled" -Value 1 -PropertyType DWORD -Force

@itamaro
Copy link
Contributor

itamaro commented Feb 22, 2026

"individual applications must declare long path awareness in their manifest to actually use paths beyond 260 characters." - maybe this is the issue?

@chris-eibl
Copy link
Member Author

Yupp, might be. Long paths is still not the "norm". A quick search hints e.g.
https://developercommunity.visualstudio.com/t/11031940

From $work I at least know that during linking we frequently have troubles.

And for the fatal error C1083: a quick search hinted the path problem, so that was my best bet ...

@chris-eibl
Copy link
Member Author

Another hint which makes me think of long path problems is, that we're slightly longer than 260 characters in case of the PR worker, and the "regular" one just passed before. Likewise does a local build for me.

When I looked into the <my_python_repo>build\test_python_<random> folder during building, it was far less "deep".

In the build bot case, it seems to replicate the Users\Administrator\buildarea\<and_so_on> part in there.

AFAICT, this does not happen for me locally ...

@chris-eibl
Copy link
Member Author

chris-eibl commented Feb 22, 2026

AFAICT, this does not happen for me locally ...

The build bot uses Tools\buildbot\test.bat, whereas I usually use PCbuild\rt.bat.
Seems it does this "replication" of the repo root path in the first case.
Couldn't dig in deeper, yet ...

Update: please scratch that, this "replication" happens for me in both cases ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants