Migrate from scikit-build to scikit-build-core #2084
+132
−92
Conversation
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
This commit addresses issue OpenChemistry#1996 by migrating the build system from the legacy scikit-build to the modern scikit-build-core. Changes made: - Updated pyproject.toml to use scikit-build-core>=0.3.3 build backend - Moved all project metadata from setup.py to pyproject.toml - Simplified setup.py to minimal stub for backward compatibility - Updated GitHub Actions workflow to use latest PyPI publish action - Added environment variable support for wheel building - Maintained all existing CMake configuration and functionality Benefits: - Modern Python build standards compliance - Better integration with Python tooling ecosystem - Simplified single-file configuration - Improved wheel building capabilities - Enhanced environment variable support All existing functionality is preserved while gaining the benefits of the modern build system. CI workflows continue to work with minimal changes. Tested with successful wheel building and package import verification. Signed-off-by: [email protected] <[email protected]>
ghutchis
reviewed
Jul 6, 2025
Member
ghutchis
left a comment
ghutchis
left a comment
There was a problem hiding this comment.
Overall looks good, thanks a lot.
| [build-system] | ||
| requires = ["setuptools>=40.8.0", "wheel", "scikit-build", "pybind11", "ninja"] | ||
| build-backend = "setuptools.build_meta:__legacy__" | ||
| requires = ["scikit-build-core>=0.3.3", "pybind11", "ninja", "setuptools_scm"] |
Member
There was a problem hiding this comment.
There's actually a comment from the builds that ninja is not needed?
Member
|
Looks like the macOS builds are failing at the repair stage: Maybe we need to update |
Member
|
Actually looking deeper into the mac build logs, it seems like it can't find the Avogadro Not sure why the various libraries aren't found with this change (but were found previously) |
Remove install component restriction that was preventing shared libraries from being included in wheels. The 'python' component restriction was excluding the main Avogadro libraries (libAvogadroCore, libAvogadroCalc, etc.) that the Python extensions depend on. This should resolve the dylib not found errors on macOS builds. Signed-off-by: [email protected] <[email protected]>
|
This pull request has been mentioned on Avogadro Discussion. There might be relevant details there: https://discuss.avogadro.cc/t/july-2025-development-update/7032/1 |
- Add custom CIBW_REPAIR_WHEEL_COMMAND_MACOS to ensure dylib files are properly included in wheels before delocate-wheel runs - Update pyproject.toml with better wheel packaging configuration - Add explicit Python version builds (cp39-* through cp312-*) - Increase build verbosity for better debugging - Update cibuildwheel to v2.16.5 for better scikit-build-core support This addresses the DelocationError where libAvogadroCore.1.dylib and other required libraries were not found during wheel repair. Signed-off-by: [email protected] <[email protected]>
- Enhanced CIBW_REPAIR_WHEEL_COMMAND_MACOS with better debugging output
- Search for dylib files in multiple locations ({project} and {project}/build)
- Added verbose output to help identify where dylib files are located
- Set install.components = [] to ensure all CMake targets are installed
- More robust wheel unpacking and repacking process
This should help identify why libAvogadroCore.1.dylib and other libraries
are not being found during the delocate step.
Signed-off-by: [email protected] <[email protected]>
rrsathe
force-pushed
the
migrate-to-scikit-build-core
branch
from
0189f38 to
442bd48
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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
This PR implements the migration from
scikit-buildtoscikit-build-coreas requested in #1996, modernizing the build system to align with current Python packaging standards.Motivation
The legacy
scikit-buildhas been superseded byscikit-build-core, which provides better performance, improved tooling integration, and follows modern Python packaging conventions. This migration ensures the project remains compatible with the evolving Python ecosystem.Changes
Build System Migration
setuptools.build_meta:__legacy__toscikit_build_core.buildscikit-buildtoscikit-build-core>=0.3.3pyproject.tomlfollowing PEP 621 standardsProject Metadata Modernization
setup.pyto declarativepyproject.tomlformatsetuptools_scmBackward Compatibility
setup.pystub for legacy compatibilityCI/CD Improvements
pypa/gh-action-pypi-publish@release/v1Technical Details
CMake Configuration
All existing CMake defines are preserved:
Environment Variables
PYTHON_WHEEL_BUILD: Detected automatically during wheel buildsEXTRA_CMAKE_ARGS: Support for additional CMake argumentsTesting
avogadro,avogadro.core,avogadro.io)Benefits
Breaking Changes
None. This is a behind-the-scenes migration that maintains full API and functionality compatibility.
Checklist
Closes #1996