Skip to content
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

Merged
merged 28 commits into from
Mar 1, 2024

Conversation

mayeut
Copy link
Contributor

@mayeut mayeut commented Feb 18, 2024

Depending on wether or not we want to still support "python<3.7"

close #312

mayeut and others added 21 commits November 16, 2022 21:08
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.
pyproject.toml Show resolved Hide resolved
@jcfr
Copy link
Contributor

jcfr commented Feb 18, 2024

Depending on whether or not we want to still support "python<3.7"

Testing that the wheels works on Python 2.7 and 3.5 makes sense. Thanks for working on this 🙏

tests/test_cmake.py Outdated Show resolved Hide resolved
mayeut and others added 5 commits February 19, 2024 07:28
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)
Comment on lines 22 to 32
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
Copy link
Contributor

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
```
@jcfr
Copy link
Contributor

jcfr commented Feb 25, 2024

@mayeut @henryiii I suggest we move forward with the integration using Squash & Merge specifying the following commit message:

chore(build): move to scikitbuild-core (#455)

* Maintain support for testing wheels for Python 2.7 & 3.5 (on Windows)
  by adding the `test_old_pythons` GitHub workflow job.

* Drop support for building sdist on Python 2.7 and 3.6

* Add support for editable install with Python >= 3.4 using the default "redirect" mode.

* Add support for building and testing Python 3.x wheels using Python 3.12.

* Update GitHub workflow macOS runner from `macos-11` to `macos-14`.

Co-authored-by: Henry Schreiner <HenrySchreinerIII@gmail.com>
Co-authored-by: Jean-Christophe Fillion-Robin <jchris.fillionr@kitware.com>

@henryiii
Copy link
Contributor

henryiii commented Mar 1, 2024

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.

@henryiii henryiii merged commit 5f068d5 into scikit-build:master Mar 1, 2024
25 checks passed
@mayeut mayeut deleted the scikit-build-core-2 branch March 2, 2024 11:47
jcfr added a commit to jcfr/cmake-python-distributions that referenced this pull request Mar 26, 2024
…on < 3.10

Fixes a regression introduced in 5f068d5 ("chore(build): move to
scikitbuild-core (scikit-build#455)", 2024-03-01)
jcfr added a commit to jcfr/cmake-python-distributions that referenced this pull request Mar 26, 2024
…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>
jcfr added a commit that referenced this pull request Mar 26, 2024
…on < 3.10

Fixes a regression introduced in 5f068d5 ("chore(build): move to
scikitbuild-core (#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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants