build(windows): package and validate PyPI wheels#1118
Conversation
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c05b40b7eb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| before-build = "python -m pip install delvewheel==1.13.0 && python ./tools/cibuildwheel_before_build.py" | ||
| repair-wheel-command = "python {project}/tools/repair_windows_wheel.py --wheel {wheel} --dest-dir {dest_dir}" | ||
| test-command = "python {project}/tools/validate_windows_wheel.py" |
There was a problem hiding this comment.
These Windows hooks are currently unreachable: a repository-wide workflow search shows that .github/workflows/release_pypi.yml still runs only Ubuntu and macOS, while .github/workflows/release_pypi_cuda.yml runs only Ubuntu. Consequently neither new Pixi wheel environment is activated, no win_amd64 wheel is built or published, and this repair/validation configuration never executes; add Windows release jobs that enter wheel-cpu and wheel-cuda and invoke cibuildwheel.
Useful? React with 👍 / 👎.
| if is_cuda: | ||
| command.extend(["--exclude", ";".join(CUDA_DLL_PATTERNS)]) |
There was a problem hiding this comment.
When a cytnx-cuda Windows wheel is repaired, this removes every CUDA/cuTENSOR DLL from the wheel, but tools/prepare_cuda_release.py declares all corresponding runtime dependencies only under sys_platform == 'linux'. A clean Windows pip install cytnx-cuda therefore installs none of the distributions that cytnx/__init__.py searches, so the extension cannot resolve its excluded DLLs; add Windows dependency markers for the runtime packages before externalizing them.
Useful? React with 👍 / 👎.
| import os,sys,warnings | ||
|
|
||
|
|
||
| def _distribution_directory(distribution_name, relative_path): |
There was a problem hiding this comment.
The newly added _distribution_directory begins a broader set of new leading-underscore identifiers here and in both Windows wheel tools, including _has_distribution, _register_windows_dll_directories, _wheel_distribution_name, _installed_distribution, and _clear_development_roots. The repository contract strictly prohibits leading underscores in new code, so rename these helpers and the new module-level handle accordingly. CLAUDE.mdL62-L64
Useful? React with 👍 / 👎.
Add isolated OpenBLAS wheel environments, relocatable extension installs, Windows DLL discovery, and delvewheel repair plus clean-install validation while leaving CUDA runtimes in their declared PyPI distributions. Co-Authored-By: OpenAI Codex <codex@openai.com>
CMake classifies MODULE targets as LIBRARY artifacts on every platform. Keep the wheel install destination relative while using the portable artifact kind so Windows configuration succeeds. Co-Authored-By: OpenAI Codex <codex@openai.com>
Conda-forge BLAS and LAPACK DLLs contain export forwarders to the literal openblas.dll basename. Tell delvewheel not to mangle that DLL so clean wheel imports can resolve the forwarded symbols. Co-Authored-By: OpenAI Codex <codex@openai.com>
c05b40b to
2d71696
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2d7169602b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # compiler runtimes found in the activated Pixi prefix. The repair helper keeps | ||
| # CUDA/cuTENSOR DLLs external for cytnx-cuda wheels because those are supplied | ||
| # by the wheel's declared NVIDIA PyPI dependencies. | ||
| before-build = "python -m pip install delvewheel==1.13.0 && python ./tools/cibuildwheel_before_build.py" |
There was a problem hiding this comment.
When cibuildwheel is run from either new Windows wheel environment, this hook never invokes tools/prepare_windows_import_libraries.py; Pixi activation does not automatically execute the existing prepare-windows/prepare-cuda tasks. The repository's Windows preparation script documents that conda-forge ARPACK lacks an MSVC import library and that the NVIDIA math wheels also omit several required .lib files, so the CPU build cannot link ARPACK and the CUDA build additionally cannot link its CUDA libraries. Chain the preparation script into the wheel build, using --cuda for the CUDA variant, before CMake configures.
Useful? React with 👍 / 👎.
1 participant
Stack
#1109 → #1110 → #1116 → #1117 → #1118 → #1119 → #1113Depends on #1117 and contains the Windows wheel packaging/runtime layer.
Summary
wheel-cpuandwheel-cudaenvironments;MODULEtarget through CMake's portableLIBRARYartifact rule into the relocatablecytnxpackage;os.add_dll_directory, since Windows has no ELF-style RUNPATH;Linux and macOS
CMake classifies
MODULEtargets asLIBRARYartifacts on every platform, so the single relative install rule preserves the existing Linux/macOS behavior while also installing the Windows.pyd. The previous full stack passed both macOS wheel jobs and both Ubuntu architectures; fresh checks are running for this smaller layer.Validation
wheel-cpudoctor and import-library layout checks pass;MODULEinstall correction;cytnx.cp311-win_amd64.pydunder the relativecytnxpackage with componentlibraries;Part of #1114.