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.

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-95991: Add some infrastructure for testing Limited API in _testcapi #95992

Merged
merged 5 commits into from Aug 17, 2022
Merged

gh-95991: Add some infrastructure for testing Limited API in _testcapi #95992

merged 5 commits into from Aug 17, 2022

Conversation

encukou
Copy link
Member

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

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

@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.)
@encukou
Copy link
Member Author

encukou commented Aug 15, 2022

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

@encukou
Copy link
Member Author

encukou commented Aug 15, 2022

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
erlend-aasland previously approved these changes Aug 15, 2022
View changes
Copy link
Contributor

@erlend-aasland erlend-aasland left a comment

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

Lib/test/support/__init__.py Outdated Show resolved Hide resolved
Modules/_testcapi/parts.h Show resolved Hide resolved
encukou and others added 2 commits Aug 16, 2022
@encukou @erlend-aasland
Update Lib/test/support/__init__.py
4b946d4 
Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com>
@encukou
Document requires_limited_api
b3b50c0
@encukou encukou mentioned this pull request Aug 16, 2022
Merged
@encukou
Copy link
Member Author

encukou commented Aug 16, 2022

This happens to move #undef NDEBUG and so it should fix assert warnings reported in #96017

@encukou encukou added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Aug 16, 2022
@bedevere-bot
Copy link

bedevere-bot commented Aug 16, 2022

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

If you want to schedule another build, you need to add the "🔨 test-with-buildbots" label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Aug 16, 2022
@encukou
Copy link
Member Author

encukou commented Aug 16, 2022

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.

@erlend-aasland erlend-aasland dismissed their stale review Aug 16, 2022

Windows failures need to be resolved

@tiran tiran mentioned this pull request Aug 16, 2022
Open
@encukou encukou mentioned this pull request Aug 16, 2022
Merged
@encukou
Copy link
Member Author

encukou commented Aug 16, 2022 •
edited

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

@zooba
Copy link
Member

zooba commented Aug 16, 2022

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 encukou requested a review from a team as a code owner Aug 16, 2022
@encukou encukou added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Aug 16, 2022
@bedevere-bot
Copy link

bedevere-bot commented Aug 16, 2022

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

If you want to schedule another build, you need to add the "🔨 test-with-buildbots" label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Aug 16, 2022
@encukou encukou mentioned this pull request Aug 17, 2022
Merged
@encukou
Copy link
Member Author

encukou commented Aug 17, 2022

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

@encukou encukou merged commit 0f2b469 into python:main Aug 17, 2022
78 of 82 checks passed
@encukou encukou deleted the skip-limited-tests-for-tracerefs branch Aug 17, 2022
@vstinner
Copy link
Member

vstinner commented Aug 17, 2022

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.

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

@erlend-aasland erlend-aasland

@python/windows-team windows-team

Assignees
No one assigned
Labels
skip news
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@encukou @bedevere-bot @zooba @vstinner @erlend-aasland