Conversation
Updated Windows build configuration for cibuildwheel to include scipy-openblas and adjust environment variables for LAPACK libraries.
Removed unnecessary steps for macOS and Windows in CI workflow.
Updated the cibuildwheel installation step to include build dependencies.
There was a problem hiding this comment.
Pull request overview
This pull request migrates the Windows build configuration from using vcpkg-managed OpenBLAS to using the scipy-openblas Python package. The changes simplify the CI/CD pipeline by removing platform-specific environment variable overrides from the GitHub Actions workflow and consolidating all configuration into pyproject.toml.
Changes:
- Removed the
readmefield from pyproject.toml project metadata - Replaced vcpkg-based OpenBLAS installation on Windows with scipy-openblas package
- Removed platform-specific environment variables and caching logic from GitHub Actions workflow
- Updated Windows configuration to use scipy-openblas paths for include and library directories
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| pyproject.toml | Removed readme field, replaced Windows vcpkg configuration with scipy-openblas integration, including dynamic path extraction via Python commands |
| .github/workflows/cibuildwheel.yml | Removed macOS uv installation, vcpkg caching, and all platform-specific environment variable overrides from the build step |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
pyproject.toml
Outdated
| before-build = "pip install scipy-openblas delvewheel" | ||
|
|
||
| # 2. Extract paths from scipy-openblas and set them for the compiler | ||
| environment = { EASYSBA_LAPACK_LIBS="openblas", INCLUDE="$(python -c \"import scipy_openblas; print(scipy_openblas.get_include_dir())\");$INCLUDE", LIB="$(python -c \"import scipy_openblas; print(scipy_openblas.get_lib_dir())\");$LIB" } |
There was a problem hiding this comment.
The shell command substitution syntax $(...) used in the environment variables will not work correctly on Windows. On Windows, cibuildwheel executes commands in PowerShell or cmd.exe, which use different syntax for command substitution. PowerShell uses $(...) but it needs to be properly escaped in TOML, and the semicolon separator should be a semicolon for Windows paths (which is correct here). However, the $() syntax in TOML environment variable values is not processed by the shell before being set as environment variables. Consider using CIBW_BEFORE_BUILD_WINDOWS to set these variables dynamically, or use a script file that cibuildwheel can execute.
| before-build = "pip install delvewheel" | ||
| repair-wheel-command = "delvewheel repair --add-path C:\\vcpkg\\installed\\x64-windows\\bin -w {dest_dir} {wheel}" | ||
| # 1. Install scipy-openblas (the lib) and delvewheel (to bundle the DLL) | ||
| before-build = "pip install scipy-openblas delvewheel" |
There was a problem hiding this comment.
The before-build step installs third-party packages scipy-openblas and delvewheel via pip without any version pinning, creating a supply-chain risk in the automated build pipeline. If either package (or one of their transitive dependencies) is compromised, an attacker could execute code during the wheel build and inject malicious payloads into the produced artifacts. To mitigate this, pin these dependencies to immutable versions or hashes (and ideally verify integrity) so that build-time code execution cannot be silently altered by upstream changes.
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
Refactor Windows environment setup for cibuildwheel.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
[WIP] WIP: Address feedback on Scipy openblas pull request
No description provided.