Quote glob env values in wheels.yml#171
Merged
Merged
Conversation
Unquoted `*` in CIBW_BUILD and CIBW_SKIP caused YAML parse errors.
Eigen's FetchContent was configuring its entire test suite (CHOLMOD, UMFPACK, CUDA, Qt, Boost, etc.) causing slow builds and failures in isolated CI environments like manylinux.
astomodynamics
force-pushed
the
fix/quote-wheels-env-globs
branch
from
07bb160 to
8c0a7b3
Compare
Eigen's unsupported/test/ creates a custom target named "autodiff" which collides with the real autodiff library when both are added via FetchContent_MakeAvailable. Since Eigen is header-only, use FetchContent_Populate to download without add_subdirectory, then create the Eigen3::Eigen target manually.
FetchContent_Populate is deprecated in CMake 4.x (CMP0169), so switch back to FetchContent_MakeAvailable. Use EXCLUDE_FROM_ALL to limit Eigen's scope and BUILD_TESTING=OFF to prevent Eigen's unsupported/test from creating an "autodiff" custom target that collides with the real autodiff library. Set Eigen3_DIR to the build tree so autodiff's find_package(Eigen3) can locate the generated Eigen3Config.cmake.
- Add _USE_MATH_DEFINES for MSVC so M_PI is defined in <cmath> - Remove macos-13 from build matrix (deprecated x86 runner)
There was a problem hiding this comment.
Pull request overview
This PR updates CI wheel-build workflows to avoid YAML parsing issues with glob patterns, and also adjusts build configuration around Eigen/autodiff and MSVC compilation.
Changes:
- Quote
CIBW_BUILD/CIBW_SKIPvalues in the wheel build workflow to prevent YAML interpreting*as special syntax. - Remove
macos-13from the OS matrices in wheel build and publish workflows. - Modify CMake Eigen FetchContent settings (disable Eigen tests/docs, set
Eigen3_DIR, addEXCLUDE_FROM_ALL) and add an MSVC compile definition.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
CMakeLists.txt |
Adjusts Eigen FetchContent configuration and adds an MSVC compile definition. |
.github/workflows/wheels.yml |
Quotes cibuildwheel glob env values and updates the OS matrix. |
.github/workflows/publish.yml |
Updates the OS matrix used for building artifacts prior to publish. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| endif() | ||
| # Point Eigen3_DIR at the build tree so autodiff's find_package(Eigen3) | ||
| # finds the generated Eigen3Config.cmake | ||
| set(Eigen3_DIR "${eigen_BINARY_DIR}" CACHE PATH "" FORCE) |
There was a problem hiding this comment.
When Eigen3 isn't found, this block fetches Eigen and sets Eigen3_DIR, but it never re-runs find_package(Eigen3) (nor does it recreate the Eigen3::Eigen target). Later the project links against Eigen3::Eigen, so configuration can fail unless the FetchContent'd Eigen happens to define that target. Consider re-invoking find_package(Eigen3 CONFIG REQUIRED) after setting Eigen3_DIR (or restoring the fallback creation of Eigen3::Eigen) to guarantee the target exists.
Suggested change
| set(Eigen3_DIR "${eigen_BINARY_DIR}" CACHE PATH "" FORCE) | |
| set(Eigen3_DIR "${eigen_BINARY_DIR}" CACHE PATH "" FORCE) | |
| find_package(Eigen3 3.4 CONFIG REQUIRED NO_MODULE) |
Eigen's full configure probes for Fortran, Qt4, CUDA, OpenMP etc. adding ~30s+ per wheel on Windows. Since Eigen is header-only, use SOURCE_SUBDIR pointing to a nonexistent path so FetchContent downloads the source without running add_subdirectory. Create the Eigen3::Eigen target manually and write a minimal Eigen3Config.cmake for autodiff.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
CIBW_BUILDandCIBW_SKIPenv values inwheels.ymlto fix YAML parse error*characters were interpreted as YAML special syntax, causing workflow validation to fail on line 61