GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-95991: Add some infrastructure for testing Limited API in _testcapi by encukou · Pull Request #95992 · python/cpython

Conversation

@encukou

@encukou encukou commented Aug 15, 2022 •
edited by bedevere-bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member
  • Limited API needs to be enabled per source file
  • Some builds don't support Limited API, so Limited API tests must be skipped on those builds
    (currently this is Py_TRACE_REFS, but that may change.)
  • Py_LIMITED_API must be defined before <Python.h> is included.

This puts the hoop-jumping in testcapi/parts.h, so individual
test files can be relatively simple. (Currently that's only
vectorcall_limited.c, imagine more.)

  • Issue: Test Limited API in _testcapi module #95991

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@encukou
pythongh-95991: Add some infrastructure for testing Limited API in _t…
368e434 
…estcapi

- Limited API needs to be enabled per source file
- Some builds don't support Limited API, so Limited API tests must be skipped on those builds
  (currently this is `Py_TRACE_REFS`, but that may change.)
- `Py_LIMITED_API` must be defined before `<Python.h>` is included.

This puts the hoop-jumping in `testcapi/parts.h`, so individual
test files can be relatively simple. (Currently that's only
`vectorcall_limited.c`, imagine more.)
@bedevere-bot bedevere-bot added the awaiting core review label Aug 15, 2022
@encukou

encukou commented Aug 15, 2022

Copy link
Copy Markdown
Member Author

I'm not sure about best practices for test/support. @vstinner or @erlend-aasland, any comments on the @requires_limited_api decorator?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@encukou encukou added the skip news label Aug 15, 2022
@encukou

encukou commented Aug 15, 2022

Copy link
Copy Markdown
Member Author

I also plan to add some instructions for the devguide, mentioning the structure for a Modules/testcapi/*.c file:

#define Py_LIMITED_API 0x03...
#include "parts.h"
#ifdef LIMITED_API_AVAILABLE
(all content goes here)
#endif

and @requires_limited_api for any Python parts.
erlend-aasland reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@encukou
Remove repeated #undef NDEBU
df49186
erlend-aasland
erlend-aasland previously approved these changes Aug 15, 2022
View reviewed changes

@erlend-aasland erlend-aasland left a comment

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Looks good! You might want to document the test decorator in Doc/library/test.rst.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Lib/test/support/__init__.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Modules/_testcapi/parts.h Resolved

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot added awaiting merge and removed awaiting core review labels Aug 15, 2022
encukou and others added 2 commits August 16, 2022 09:47
@encukou @erlend-aasland
Update Lib/test/support/__init__.py
4b946d4 
Co-authored-by: Erlend E. Aasland <[email protected]>
@encukou
Document requires_limited_api
b3b50c0
@encukou encukou mentioned this pull request Aug 16, 2022
gh-96017: Fix some compiler warnings (GH-96018) #96018
Merged
@encukou

encukou commented Aug 16, 2022

Copy link
Copy Markdown
Member Author

This happens to move #undef NDEBUG and so it should fix assert warnings reported in #96017
erlend-aasland reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@encukou encukou added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Aug 16, 2022
@bedevere-bot

bedevere-bot commented Aug 16, 2022

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @encukou for commit b3b50c0 🤖

If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Aug 16, 2022
@encukou

encukou commented Aug 16, 2022

Copy link
Copy Markdown
Member Author

I don't understand the failure on the Windows buildbot:

     6>PrepareForBuild:
         Creating directory "C:\Workspace\buildarea\pull_request.linaro-win-arm64.nondebug\build\PCbuild\obj\312arm64_Release\_testcapi\_testcapi.tlog\".
       InitializeBuildStatus:
         Creating "C:\Workspace\buildarea\pull_request.linaro-win-arm64.nondebug\build\PCbuild\obj\312arm64_Release\_testcapi\_testcapi.tlog\unsuccessfulbuild" because "AlwaysCreate" was specified.
       ClCompile:
         C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\MSVC\14.31.31103\bin\HostX86\arm64\CL.exe /c /I"C:\Workspace\buildarea\pull_request.linaro-win-arm64.nondebug\build\Include" /I"C:\Workspace\buildarea\pull_request.linaro-win-arm64.nondebug\build\Include\internal" /I"C:\Workspace\buildarea\pull_request.linaro-win-arm64.nondebug\build\PC" /I"C:\Workspace\buildarea\pull_request.linaro-win-arm64.nondebug\build\PCbuild\obj\312arm64_Release\_testcapi\\" /Zi /nologo /W3 /WX- /diagnostics:column /MP5 /O2 /Oi /Oy- /GL /D WIN32 /D "PY3_DLLNAME=L\"python3\"" /D _WIN32 /D NDEBUG /D _ARM64_WINAPI_PARTITION_DESKTOP_SDK_AVAILABLE=1 /D _WINDLL /GF /Gm- /MD /GS /Gy /fp:precise /Zc:wchar_t /Zc:forScope /Zc:inline /Fo"C:\Workspace\buildarea\pull_request.linaro-win-arm64.nondebug\build\PCbuild\obj\312arm64_Release\_testcapi\\" /Fd"C:\Workspace\buildarea\pull_request.linaro-win-arm64.nondebug\build\PCbuild\obj\312arm64_Release\_testcapi\vc143.pdb" /external:W3 /Gd /TC /analyze- /FC /errorReport:queue /utf-8 ..\Modules\_testcapimodule.c ..\Modules\_testcapi\vectorcall.c ..\Modules\_testcapi\vectorcall_limited.c ..\Modules\_testcapi\heaptype.c ..\Modules\_testcapi\unicode.c
         _testcapimodule.c
         vectorcall.c
         vectorcall_limited.c
         heaptype.c
         unicode.c
       ResourceCompile:
         C:\Program Files (x86)\Windows Kits\10\bin\10.0.19041.0\x86\rc.exe /D "ORIGINAL_FILENAME=\\\"_testcapi.pyd\\\"" /D FIELD3=100 /D NDEBUG /l"0x0409" /I"C:\Workspace\buildarea\pull_request.linaro-win-arm64.nondebug\build\PC" /I"C:\Workspace\buildarea\pull_request.linaro-win-arm64.nondebug\build\Include" /I"C:\Workspace\buildarea\pull_request.linaro-win-arm64.nondebug\build\PCbuild\obj\312arm64_Release\_testcapi\\" /nologo /fo"C:\Workspace\buildarea\pull_request.linaro-win-arm64.nondebug\build\PCbuild\obj\312arm64_Release\_testcapi\python_nt.res" ..\PC\python_nt.rc 
       Link:
         C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\MSVC\14.31.31103\bin\HostX86\arm64\link.exe /ERRORREPORT:QUEUE /OUT:"C:\Workspace\buildarea\pull_request.linaro-win-arm64.nondebug\build\PCbuild\arm64\_testcapi.pyd" /INCREMENTAL:NO /NOLOGO /LIBPATH:"C:\Workspace\buildarea\pull_request.linaro-win-arm64.nondebug\build\PCbuild\arm64\\" advapi32.lib shell32.lib ole32.lib oleaut32.lib kernel32.lib user32.lib gdi32.lib winspool.lib comdlg32.lib advapi32.lib shell32.lib ole32.lib oleaut32.lib uuid.lib odbc32.lib odbccp32.lib /NODEFAULTLIB:LIBC /MANIFEST:NO /DEBUG /PDB:"C:\Workspace\buildarea\pull_request.linaro-win-arm64.nondebug\build\PCbuild\arm64\_testcapi.pdb" /SUBSYSTEM:WINDOWS /LTCG /LTCGOUT:"C:\Workspace\buildarea\pull_request.linaro-win-arm64.nondebug\build\PCbuild\obj\312arm64_Release\_testcapi\_testcapi.iobj" /TLBID:1 /DYNAMICBASE /NXCOMPAT /IMPLIB:"C:\Workspace\buildarea\pull_request.linaro-win-arm64.nondebug\build\PCbuild\arm64\_testcapi.lib" /MACHINE:ARM64 /OPT:REF,NOICF /DLL "C:\Workspace\buildarea\pull_request.linaro-win-arm64.nondebug\build\PCbuild\obj\312arm64_Release\_testcapi\python_nt.res"
         "C:\Workspace\buildarea\pull_request.linaro-win-arm64.nondebug\build\PCbuild\obj\312arm64_Release\_testcapi\_testcapimodule.obj"
         "C:\Workspace\buildarea\pull_request.linaro-win-arm64.nondebug\build\PCbuild\obj\312arm64_Release\_testcapi\vectorcall.obj"
         "C:\Workspace\buildarea\pull_request.linaro-win-arm64.nondebug\build\PCbuild\obj\312arm64_Release\_testcapi\vectorcall_limited.obj"
         "C:\Workspace\buildarea\pull_request.linaro-win-arm64.nondebug\build\PCbuild\obj\312arm64_Release\_testcapi\heaptype.obj"
         "C:\Workspace\buildarea\pull_request.linaro-win-arm64.nondebug\build\PCbuild\obj\312arm64_Release\_testcapi\unicode.obj"
         "C:\Workspace\buildarea\pull_request.linaro-win-arm64.nondebug\build\PCbuild\arm64\python312.lib"
     6>LINK : fatal error LNK1104: cannot open file 'python3.lib' [C:\Workspace\buildarea\pull_request.linaro-win-arm64.nondebug\build\PCbuild\_testcapi.vcxproj]
     6>Done Building Project "C:\Workspace\buildarea\pull_request.linaro-win-arm64.nondebug\build\PCbuild\_testcapi.vcxproj" (Build target(s)) -- FAILED.
     1>Done Building Project "C:\Workspace\buildarea\pull_request.linaro-win-arm64.nondebug\build\PCbuild\pcbuild.proj" (Build target(s)) -- FAILED.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@erlend-aasland
erlend-aasland dismissed their stale review August 16, 2022 08:54

Windows failures need to be resolved

@tiran tiran mentioned this pull request Aug 16, 2022
Compiler warnings in main / 3.11 #96017
Closed
@encukou encukou mentioned this pull request Aug 16, 2022
Support metaclasses in HPy. hpyproject/hpy#335
Merged
@encukou

encukou commented Aug 16, 2022 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member Author

@python/windows-team, do you know what's wrong here, by any chance?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@zooba

zooba commented Aug 16, 2022

Copy link
Copy Markdown
Member

It's probably getting built before the limited API DLL (python3.dll). We need to force a dependency into the project file to ensure they are built in the right order.

Add the below to _testcapi.vcxproj underneath the existing <ProjectReference Include="pythoncore.vcxproj"> that's in there:

    <ProjectReference Include="python3dll.vcxproj">
      <Project>{885d4898-d08d-4091-9c40-c700cfe3fc5a}</Project>
      <ReferenceOutputAssembly>false</ReferenceOutputAssembly>
    </ProjectReference>
encukou reacted with heart emoji erlend-aasland reacted with eyes emoji
All reactions
  • ❤️ 1 reaction
  • 👀 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@encukou
Force a dependency on python3.dll in the Windows build
ee59cb3
@encukou
encukou requested a review from a team as a code owner August 16, 2022 18:03
@encukou encukou added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Aug 16, 2022
@bedevere-bot

bedevere-bot commented Aug 16, 2022

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @encukou for commit ee59cb3 🤖

If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Aug 16, 2022
erlend-aasland
erlend-aasland approved these changes Aug 16, 2022
View reviewed changes
@encukou encukou mentioned this pull request Aug 17, 2022
gh-93649: Split heaptype tests from _testcapimodule.c #95386
Merged
@encukou

encukou commented Aug 17, 2022

Copy link
Copy Markdown
Member Author

Woohoo! That worked! Thanks for the wincantation, Steve, and thanks Erlend for the review!

The other buildbot failures are unrelated, and being handled in #96006 and #95898
erlend-aasland reacted with hooray emoji erlend-aasland reacted with rocket emoji
All reactions
  • 🎉 1 reaction
  • 🚀 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@encukou
encukou merged commit 0f2b469 into python:main Aug 17, 2022
@bedevere-bot bedevere-bot removed the awaiting merge label Aug 17, 2022
@encukou
encukou deleted the skip-limited-tests-for-tracerefs branch August 17, 2022 11:48
@vstinner

vstinner commented Aug 17, 2022

Copy link
Copy Markdown
Member

When Py_TRACE_REFS is defined, it was discussed once to move the double linked list of objects out of PyObject, to keep the ABI compatiiblity. It shouldn't be hard to store it somewhere else. But so far, nobody proposed a concrete implementation of this idea.
erlend-aasland reacted with eyes emoji
All reactions
  • 👀 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@erlend-aasland erlend-aasland erlend-aasland approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

skip news

Projects

None yet

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.

5 participants

@encukou @bedevere-bot @zooba @vstinner @erlend-aasland
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.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.