GeezerVison is showing github.com — gh-91048: Refactor _testexternalinspection and add Windows support by pablogsal · Pull Request #132852 · python/cpython
Conversation
pablogsal
commented
Apr 23, 2025
•
edited by bedevere-app
Bot
Loading
Uh oh!
There was an error while loading. Please reload this page.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
1st1 reacted with thumbs up emoji
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
|
Man, getting windows to work was NOT easy
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
This change broke FreeBSD buildbot:
--- Modules/_testexternalinspection.o ---
In file included from ./Modules/_testexternalinspection.c:20:
./Modules/../Python/remote_debug.h:695:15: error: call to undeclared function 'search_map_for_section'; ISO C99 and later do not support implicit function declarations [-Werror,-Wimplicit-function-declaration]
695 | address = search_map_for_section(handle, "PyRuntime", "libpython");
| ^
./Modules/_testexternalinspection.c:62:15: error: call to undeclared function 'search_map_for_section'; ISO C99 and later do not support implicit function declarations [-Werror,-Wimplicit-function-declaration]
62 | address = search_map_for_section(handle, "AsyncioDebug", "_asyncio.cpython");
| ^
2 errors generated.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
This change broke FreeBSD buildbot
I wrote #132945 to fix the build on FreeBSD.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
This was referenced Apr 25, 2025
|
Seems like this change broke JIT CI on main for Windows #132953
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Seems like this change broke JIT CI on main for Windows #132953
How? This should not affect the JIT in any way
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
======================================================================
ERROR: test_async_gather_remote_stack_trace (test.test_external_inspection.TestGetStackTrace.test_async_gather_remote_stack_trace)
----------------------------------------------------------------------
Traceback (most recent call last):
File "D:\a\cpython\cpython\Lib\test\test_external_inspection.py", line 314, in test_async_gather_remote_stack_trace
stack_trace = get_async_stack_trace(p.pid)
RuntimeError: Failed to read asyncio debug offsets
All look like that
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Seems like this change broke JIT CI on main for Windows #132953
How? This should not affect the JIT in any way
Yeah I'm confused as well. See what Steve said above about the tests failing though. I wonder if the JIT build lays out stuff differently in memory? No clue.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
pablogsal
commented
Apr 25, 2025
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
======================================================================
ERROR: test_async_gather_remote_stack_trace (test.test_external_inspection.TestGetStackTrace.test_async_gather_remote_stack_trace)
----------------------------------------------------------------------
Traceback (most recent call last):
File "D:\a\cpython\cpython\Lib\test\test_external_inspection.py", line 314, in test_async_gather_remote_stack_trace
stack_trace = get_async_stack_trace(p.pid)
RuntimeError: Failed to read asyncio debug offsets
All look like that
No, that looks like whatever the JIT build is doing is breaking this feature, which is different. This PR makes that test run on windows and is failing, ergo is not that this broke the JIT but the other way around.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
No, that looks like whatever the JIT build is doing is breaking this feature, which is different. This PR makes that test run on windows and is failing, ergo is not that this broke the JIT but the other way around
Yeah I believe the JIT broke the test internal inspection, but the JIT CI was passing before this, so if I'm being pedantic, the PR did add tests that broke the JIT CI :).
Pedantics aside, @chris-eibl is looking into it, and I'm looking too.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Pedantics aside, @chris-eibl is looking into it, and I'm looking too.
Hold on I am looking into this I think I know what's going on
|
chris-eibl and Fidget-Spinner reacted with thumbs up emoji
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
But JIT must do something different on Windows, because Linux is green?
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
But JIT must do something different on Windows, because Linux is green?
Windows is also green in the normal CI. My bet is that it has to be with release/vs not release and not related to the JIT
|
Fidget-Spinner reacted with thumbs up emoji
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
But JIT must do something different on Windows, because Linux is green?
Windows is also green in the normal CI. My bet is that it has to be with release/vs not release and not related to the JIT
I think you're right. On the commit, Azure pipelines (which uses release builds for Windows) fails, but not the normal debug builds.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
chris-eibl
commented
Apr 25, 2025
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
|
Can confirm on a local Windows release 64bit build, that the tests fail:
======================================================================
ERROR: test_asyncgen_remote_stack_trace (test.test_external_inspection.TestGetStackTrace.test_asyncgen_remote_stack_trace)
----------------------------------------------------------------------
Traceback (most recent call last):
File "e:\cpython_chris\Lib\test\test_external_inspection.py", line 246, in test_asyncgen_remote_stack_trace
stack_trace = get_async_stack_trace(p.pid)
RuntimeError: Failed to read asyncio debug offsets
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
pablogsal
commented
Apr 25, 2025
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
Can confirm on a local Windows release 64bit build, that the tests fail:
I know what the problem is, I am working on a fix
|
Fidget-Spinner reacted with heart emoji
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
As a takeaway: it would be good to get at least one release CI run on all PRs :)
|
Fidget-Spinner and chris-eibl reacted with thumbs up emoji
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
chris-eibl
commented
Apr 25, 2025
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
|
Yeah,
Dump of file amd64\_asyncio.pyd
File Type: DLL
Summary
2000 .data
1000 .pdata
6000 .rdata
1000 .reloc
1000 .rsrc
8000 .text
dumpbin shows no AsyncioD section.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Reviewers
zooba
zooba left review comments
ambv
Awaiting requested review from ambv
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assigned
Milestone
No milestone
Development
Successfully merging this pull request may close these issues.
Uh oh!
There was an error while loading. Please reload this page.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
|