-
Notifications
You must be signed in to change notification settings - Fork 31
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
chore(build): move to scikitbuild-core #455
Conversation
Update pyproject.toml Apply suggestions from code review Update pyproject.toml WIP: try branch of skbc Update pyproject.toml WIP: right before repro builds Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com> Update pyproject.toml
* master: (112 commits) docs: fix up docs (scikit-build#454) chore(deps): update pre-commit hooks (scikit-build#443) chore(deps): bump cmake from 3.28.1 to 3.28.3 (scikit-build#452) chore(deps): bump the actions group with 1 update (scikit-build#453) Update to CMake 3.28.3 (scikit-build#450) chore(ci): use 4 threads to compile on GHA (scikit-build#449) Update to OpenSSL 3.0.13 (scikit-build#448) chore(deps): bump the actions group with 1 update (scikit-build#447) chore(deps): update pre-commit hooks (scikit-build#441) ci: group dependabot updates (scikit-build#442) chore(deps): bump cmake from 3.27.9 to 3.28.1 (scikit-build#440) Update to CMake 3.28.1 (scikit-build#439) ci: support artifact v4 (scikit-build#438) chore(deps): bump actions/upload-artifact from 3 to 4 chore(deps): bump actions/download-artifact from 3 to 4 Update to CMake 3.28.0 chore(deps): update pre-commit hooks chore(deps): bump actions/setup-python from 4 to 5 (scikit-build#431) chore(deps): bump cmake from 3.27.7 to 3.27.9 (scikit-build#430) Update to CMake 3.27.9 (scikit-build#429) ...
Test command is already specified in pyproject.toml
From scikit-build#312 (comment) > If we build from SDist, I think it's already much slower, more likely > to fail, etc - it's really supposed to be a wheel distribution. So not > supporting building from SDist on <=3.6 doesn't seem too bad. And a user > can always either use Python 3.7+ to build a wheel then install it on > 2.7-3.6, or use an older version.
…ualenv The use of the `virtualenv` pytest fixture become obsolete following commit 280260d ("chore(ci): bump `test_sdist` python from 3.11 to 3.12 (scikit-build#423)", 2023-12-02)
Use `ubuntu-latest` for ubuntu builds, `macos-14` for macOS build.
Testing that the wheels works on Python 2.7 and 3.5 makes sense. Thanks for working on this 🙏 |
Co-authored-by: Jean-Christophe Fillion-Robin <jchris.fillionr@kitware.com>
See https://cmake.org/cmake/help/latest/policy/CMP0135.html This will avoid warnings like the following: CMake Warning (dev) at /path/to/cmake-3.28.1-linux-x86_64/share/cmake-3.28/Modules/ExternalProject.cmake:3198 (message): The DOWNLOAD_EXTRACT_TIMESTAMP option was not given and policy CMP0135 is not set. The policy's OLD behavior will be used. When using a URL download, the timestamps of extracted files should preferably be that of the time of extraction, otherwise code that depends on the extracted contents might not be rebuilt if the URL changes. The OLD behavior preserves the timestamps from the archive instead, but this is usually not what you want. Update your project to the NEW behavior or specify the DOWNLOAD_EXTRACT_TIMESTAMP option with a value of true to avoid this robustness issue. Adapted from dff357a ("ExternalProjectDependency: Set DOWNLOAD_EXTRACT_TIMESTAMP to 1 if CMake >= 3.24", 2023-07-29)
Function `cpd_ExternalProject_AlwaysConfigure` copied from `ExternalProjectDependency.cmake` (commontk/Artichoke@613e3739a)
… >= 3.4 Add support for editable install using Python 3.4 (first version with built-in pathlib support)
src/cmake/__init__.py
Outdated
cmake_executable_path = None | ||
for script in distribution("cmake").files: | ||
if str(script).startswith("cmake/data/bin/cmake"): | ||
if sys.version_info < (3, 6): | ||
# pre-3.6 behavior is strict | ||
resolved_script = script.locate().resolve() | ||
else: | ||
resolved_script = script.locate().resolve(strict=True) | ||
cmake_executable_path = resolved_script.parents[1] | ||
break | ||
CMAKE_DATA = cmake_executable_path if cmake_executable_path else None |
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 adds support for editable install adding a generic way of recovering the cmake/data
directory.
When support for Python < 3.7 will be dropped, the overall logic could be greatly simplified by directly recovering the path to ctest
, cmake
and cpack
using the distribution API.
@henryiii Did I miss anything ?
Fixes the following error: ``` tests\test_cmake.py:15: in <module> import cmake C:\hostedtoolcache\windows\Python\3.5.4\x64\lib\site-packages\cmake\__init__.py:37: in <module> assert os.path.exists(CMAKE_DATA) C:\hostedtoolcache\windows\Python\3.5.4\x64\lib\genericpath.py:19: in exists os.stat(path) E TypeError: argument should be string, bytes or integer, not WindowsPath ```
@mayeut @henryiii I suggest we move forward with the integration using
|
Decision was to have a followup (before release) requiring 3.7+, but we could revert that if someone really needs a Python 2 or 3.6 package of the old CMake. |
…on < 3.10 Fixes a regression introduced in 5f068d5 ("chore(build): move to scikitbuild-core (scikit-build#455)", 2024-03-01)
…on < 3.10 Fixes a regression introduced in 5f068d5 ("chore(build): move to scikitbuild-core (scikit-build#455)", 2024-03-01) Reported-by: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com> Reported-by: Shizhi Tang <rd0x01@gmail.com> Co-authored-by: Fabio Luporini <fabio@devitocodes.com>
Depending on wether or not we want to still support "python<3.7"
close #312