Skip to content

Conversation

@AlexyPellegrini
Copy link
Collaborator

@AlexyPellegrini AlexyPellegrini commented Mar 25, 2024

This is the first batch of code to build the sdk wheel!

The URL of the SDK is composed from the target information, and no hash is checked for now, it was easier to test since my python versions mismatch between windows and linux :)

I removed the c++ code from the repo too.

I'm not sure how we are supposed to propagate the version using the dynamic metadata, for now it seems to always be v0.1.

Also I see that on "cmake-python-distributions" there is a script to update the version in different files, is this something that should be done for this repo too?

@jcfr
Copy link
Contributor

jcfr commented Mar 26, 2024

we are supposed to propagate the version using the dynamic metadata, for now it seems to always be v0.1

There are two aspects to consider:

  • The version of the SDK to download will be hardcoded in the CMakeLists.txt
  • The version version associated with the wheel will be "dynamically" set based on the git tag

@AlexyPellegrini AlexyPellegrini force-pushed the add-sdk-build branch 2 times, most recently from 3b56e25 to 674648a Compare March 26, 2024 13:50
@AlexyPellegrini
Copy link
Collaborator Author

Hmm it seems that pylint triggers on VTK files, should we disable it since we won't have any python code?

@AlexyPellegrini
Copy link
Collaborator Author

we are supposed to propagate the version using the dynamic metadata, for now it seems to always be v0.1

There are two aspects to consider:

  • The version of the SDK to download will be hardcoded in the CMakeLists.txt
  • The version version associated with the wheel will be "dynamically" set based on the git tag

Ok, so we will tag later I guess. For now the version I hardcoded is 9.2.5. If we hardcode the version maybe I can bring back the hashes of the archives. Can we use the python 3.8 archive version for any newerpython version, or should each version match exactly ?

@jcfr
Copy link
Contributor

jcfr commented Mar 27, 2024

a script [...] to update the version in different files, is this something that should be done for this repo too?

Definitely, we should look into adding a script called update_vtk_sdk_version.py that would be similar to update_cmake_version.py and also have a logic similar to the following:

vtk_version = "9.3.0"

for py_abi_tag in ["cp38-cp38", "cp39-cp39", "cp310-cp310", "cp311-cp311", "cp312-cp312"]:
    for platform_tag in ["macosx_10_10_x86_64", "macosx_11_0_arm64", "manylinux_2_17_x86_64.manylinux2014_x86_64", "win_amd64"]:
        url = f"https://vtk.org/files/wheel-sdks/vtk-wheel-sdk-{vtk_version}-{py_abi_tag}-{platform_tag}.tar.xz"

        # Download files
        # -> Adapt from https://github.com/girder/slicer_package_manager/blob/342b48d7b6fdf407731513c30f5dc147eca30d5b/tests/__init__.py#L324-L334

        # Compute checksum 
        # -> See "computeFileChecksum" at https://github.com/girder/slicer_package_manager/blob/342b48d7b6fdf407731513c30f5dc147eca30d5b/tests/__init__.py#L232-L254

@jcfr
Copy link
Contributor

jcfr commented Mar 27, 2024

Hmm it seems that pylint triggers on VTK files, should we disable it since we won't have any python code?

I ended up addressing the "root" cause by installing the content of the archive in a dedicated directory without relying on the automatic discovery of wheel packages.

@codecov
Copy link

codecov bot commented Mar 27, 2024

Welcome to Codecov 🎉

Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests.

Thanks for integrating Codecov - We've got you covered ☂️

@AlexyPellegrini
Copy link
Collaborator Author

AlexyPellegrini commented Mar 27, 2024

Thank you for making the required changes!

It seems that the mac runners fail because the platform tag (macosx_12_0...) is not the same as the one on the sdk repo. We probably need to the same thing as we do for linux.
There is an additional "cpN" in the windows url, I think that's because on WIndows the Python SOABI already contains "cpN" at the beginning.

I'm working on the script to generate the cmake urls file so we will get rid of the problem by changing the logic I think (since the URLs will be hardcoded)

@jcfr jcfr force-pushed the add-sdk-build branch 5 times, most recently from 8e1ac88 to 47488c9 Compare March 27, 2024 18:31
@jcfr jcfr linked an issue Mar 27, 2024 that may be closed by this pull request
7 tasks
@jcfr
Copy link
Contributor

jcfr commented Mar 27, 2024

Before moving forward with the integration, we should add a test checking that a simple project can be built against the generated wheel.

Add files like the following:

tests/test_find_package.py
tests/packages/find_package/CMakeLists.txt
tests/packages/find_package/pyproject.toml
tests/packages/find_package/src/vtk_simple/__init__.py

Consider borrowing some of the testing logic found here:

In this simple test, the CMakeLists.txt will be similar to this:

cmake_minimum_required(VERSION 3.15...3.26)

project(
  ${SKBUILD_PROJECT_NAME}
  LANGUAGES C
  VERSION ${SKBUILD_PROJECT_VERSION})

find_package(Python COMPONENTS Interpreter Development.Module)

set(expected_version "9.2.5")
find_package(VTK ${expected_version} REQUIRED)

@AlexyPellegrini AlexyPellegrini force-pushed the add-sdk-build branch 3 times, most recently from ea7aca1 to 85ca0b8 Compare March 28, 2024 15:08
@AlexyPellegrini
Copy link
Collaborator Author

I'm not sure how I am supposed to fix this error:
tests/test_find_package.py:10: error: Cannot find implementation or library stub for module named "virtualenv" [import-not-found]
I tried to add virtualenv as a test dependency, but it does not seem to fix the issue.

@AlexyPellegrini AlexyPellegrini force-pushed the add-sdk-build branch 8 times, most recently from 37784fc to 9a840d6 Compare March 29, 2024 16:00
@jcfr jcfr force-pushed the add-sdk-build branch 2 times, most recently from d6511e7 to 61a1344 Compare October 21, 2025 20:24
AlexyPellegrini and others added 7 commits October 21, 2025 16:36
- Explicitly set setuptools_scm to derive the version from the nearest tag
  name only:
    git describe --tags --abbrev=0 --match "*[0-9]*"
  This yields a clean tag (e.g., 9.5.0) without distance/hash,
  making wheels/tag-based releases deterministic.

- In CMake, compare it with VTK_VERSION and emit a warning if they differ.

Trade-off: by removing distance/hash, non-tag builds will look like the
nearest tag. This is intentional here to keep package versions aligned
with SDK tags.

Co-authored-by: Jean-Christophe Fillion-Robin <[email protected]>
This drops support for Python 3.8 and 3.9 as they are nearing or have
reached end-of-life, and adds support for Python 3.12 and 3.13:
- Python 3.8 is end-of-life since 2024-10-07
- Python 3.9 will be end-of-life at the end of 2025-10

See https://devguide.python.org/versions

Co-authored-by: Jean-Christophe Fillion-Robin <[email protected]>
This adds a test project that verifies the ability to use `find_package(VTK REQUIRED)`
in a scikit-build-core-based Python package.

- Adds `CMakeLists.txt` and `pyproject.toml` under `tests/packages/find_package`
- Creates a minimal Python package (`vtk_simple`) as the test subject
- Implements `test_find_package.py` to build the wheel using `pip wheel`
  in an isolated virtualenv with a custom wheelhouse if provided
- Install vtk-sdk package before using it

Co-authored-by: Jean-Christophe Fillion-Robin <[email protected]>
@jcfr jcfr force-pushed the add-sdk-build branch 3 times, most recently from c46bc96 to f84bd0a Compare October 21, 2025 21:32
@jcfr jcfr force-pushed the add-sdk-build branch 2 times, most recently from 412e940 to 872d254 Compare October 21, 2025 22:15
jcfr added 3 commits October 21, 2025 18:17
Since `Python_SOABI` is set to value like `cpython-XYZ-darwin`, instead
checks the value of `CMAKE_SYSTEM_PROCESSOR`.
@jcfr
Copy link
Contributor

jcfr commented Oct 21, 2025

@AlexyPellegrini Can you check that the wheels1 work as expected. Then we will merge & tag so that those are uploaded on PyPI.

cc: @finetjul

Footnotes

  1. https://github.com/Kitware/vtk-sdk-python-distributions/actions/runs/18699298440#summary-53324404983

@jcfr
Copy link
Contributor

jcfr commented Oct 21, 2025

Notes

Checksums:

I re-inroduced the use of checksums. This is critical to ensure integrity.

After locally downloading the wheels .. the following is helpful:

sha256sum vtk-wheel-sdk-9.5.0-cp3* | awk '{
  hash=$1; file=$2
  sub(/^.*\//, "", file)                       # strip any path
  sub(/^vtk-wheel-sdk-/, "", file)             # drop prefix
  sub(/\.tar\.xz$/, "", file)                  # drop suffix
  n=split(file, a, "-"); py=a[2]               # cp tag (e.g., cp38, cp39)
  if (NR>1 && py!=lastpy) print ""             # blank line between cp groups
  lastpy=py
  printf "set(sha256_%s\n    \"%s\")\n", file, hash
}'

CI & Runners:

  • Updated check-cibw to include all platforms
  • Switched to macos15-intel for x86_64, and macos-14 for arm64
  • Excluded Windows 32-bit builds

@jcfr jcfr requested a review from Copilot October 21, 2025 22:45
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds support for packaging the VTK SDK as a Python wheel by downloading pre-built VTK SDK archives, extracting them, and installing their contents along with CMake configuration files. The implementation removes the previous C++ example code and introduces new infrastructure for distributing VTK SDKs as first-class Python wheels with scikit-build-core integration.

Key changes:

  • Replaces C++ example code with VTK SDK packaging infrastructure
  • Adds CMake logic to download and install VTK SDK archives based on platform/Python version
  • Updates minimum Python version from 3.8 to 3.10 and adds Python 3.13 support
  • Introduces comprehensive testing using virtual environments to verify find_package(VTK) functionality

Reviewed Changes

Copilot reviewed 17 out of 18 changed files in this pull request and generated 11 comments.

Show a summary per file
File Description
CMakeLists.txt Implements VTK SDK download/extraction logic and CMake configuration file installation
cmake/vtk-sdk-urls.cmake Defines platform-specific URLs and SHA256 checksums for VTK SDK archives
cmake/vtk-config.cmake.in Template for CMake config file that redirects to extracted SDK
cmake/vtk-config-version.cmake.in Template for CMake version config file
src/vtk_sdk/cmake/init.py Entry point module for scikit-build-core cmake.prefix
src/vtk_sdk/init.py Updated module description from cmake-module to cmake.prefix
tests/test_find_package.py New test infrastructure for verifying VTK SDK can be found via CMake
tests/packages/find_package/* Test package that uses find_package(VTK) to validate SDK integration
pyproject.toml Updates Python version requirements, entry points, and test dependencies
.github/workflows/ci.yml Updates CI to test Python 3.10-3.13 and adds wheel building checks
.github/workflows/cd.yml Adds SDist testing and distribution checks

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

jcfr and others added 2 commits October 21, 2025 19:01
@Thibault-Pelletier
Copy link

Hi @jcfr,

Thx for working on this!
Just to clarify the goal of this MR, is it now to push VTK SDK versions from 9.2 to 9.5.2 on PyPI and let the VTK CI take over for versions 9.5.2+?

Will this repo be retired once the changes on VTK are cleanly integrated?

@jcfr
Copy link
Contributor

jcfr commented Oct 22, 2025

Indeed, once the corresponding capabilities are in upstream VTK, there would be no need to publish the vtk-sdk wheels from this project.

I was initially thinking we could publish also publish a custom scikit build backend along side the SDK but this should instead be done in a dedicated GitHub project.

Indeed, the goal being to simplify how project like the following are externally built by leveraging VTKExternalModule (a Slicer agnostic project):

https://github.com/Kitware/LookingGlassVTKModule

@jcfr jcfr merged commit d676beb into Kitware:main Oct 22, 2025
19 checks passed
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.

Add support for re-packaging and distributing existing VTK wheel SDKs

4 participants