-
Notifications
You must be signed in to change notification settings - Fork 1.1k
test: pytest on windows (with python_binding enabled) #1796
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
Conversation
847cfc2 to
86064ad
Compare
This reverts commit 249841a5dcd956551b5d4ddbff2d46bd20e2afc1.
This reverts commit de6f9dc.
- Rename 'cpp_app_mproc_python' to shorter name - Resolve GetFullPathNameA() 'The filename or extension is too long' error on Windows
ec0263e to
b68f3b7
Compare
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.
This PR is being reviewed by Cursor Bugbot
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| Write-Output "[Ollama] Zip size (bytes): $zipSize" | ||
| if ($zipSize -lt 1000000000) { | ||
| Write-Error "[Ollama] Zip file seems too small (<1GB), aborting." | ||
| exit 1 |
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.
Bug: Ollama zip size check threshold is too large
The check $zipSize -lt 1000000000 (less than 1GB) will always fail and abort because the Ollama Windows CLI zip file is typically only ~100-300MB. The threshold of 1GB is far too large for a sanity check. This will cause the test-integration-python job to always fail when trying to validate the downloaded Ollama zip file. The threshold should likely be something like 1MB (1000000) or less to catch failed/partial downloads.
| shell: pwsh | ||
| run: | | ||
| Write-Output "Enabling Windows long path support..." | ||
| New-ItemProperty -Path "HKLM:\SYSTEM\CurrentControlSet\Control\FileSystem" -Name "LongPathsEnabled" -Value 1 -PropertyType DWORD -Force |
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.
Bug: Registry modification fails if property already exists
The New-ItemProperty -Force command will fail with "The property already exists" error if the LongPathsEnabled registry value is already present on the Windows runner. The -Force parameter only overrides read-only attributes, not existing properties. This could cause intermittent CI failures if GitHub updates their runner images to have this setting pre-configured. Using Set-ItemProperty instead would be idempotent and work regardless of whether the property exists.
Note
Enables Python integration on Windows by adding safe/compat DLL loading, Windows-friendly start/bootstrap scripts, CI jobs (incl. Ollama) and broad test/BUILD tweaks; also adjusts requirements and symbol handling.
ten_module_load_with_path_search()usingLoadLibraryAand use it inpython_addon_loaderto locatepython310.dllvia PATH.libten_runtime_pythonas.pydon Windows with global symbol visibility.os.add_dll_directoryforten_runtimeimport on Windows; export full API in__init__.py."Osn","O!n") for pointer-sized args.uvloopoptional on Windows.bootstrap.py/start.pyscripts that setPYTHONPATHand copy.dll→.pyd.Scripts/, call platform-specific bootstrap/start, and add DLL directories viaadd_dll_directory.cpp_app_mproc_python) and skip leak test on Windows; extend pytest ignores.Written by Cursor Bugbot for commit 307eb81. This will update automatically on new commits. Configure here.