Skip to content

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 <[email protected]>
Signed-off-by: Henry Schreiner <[email protected]>
Signed-off-by: Henry Schreiner <[email protected]>

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.
@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 🙏

mayeut and others added 5 commits February 19, 2024 07:28
Co-authored-by: Jean-Christophe Fillion-Robin <[email protected]>
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 <[email protected]>
Co-authored-by: Jean-Christophe Fillion-Robin <[email protected]>

@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
@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 <[email protected]>
Reported-by: Shizhi Tang <[email protected]>
Co-authored-by: Fabio Luporini <[email protected]>
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 <[email protected]>
Reported-by: Shizhi Tang <[email protected]>
Co-authored-by: Fabio Luporini <[email protected]>
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.

3 participants