Integrating scikit-core-build with vcpkg #4310

ktsitsi · 2025-11-13T15:30:34Z

Issue and/or context:

Combining the vcpkg + scikit-core-build + cibuildwheel build system

Changes:

Build System Migration - pyproject.toml and CMakeLists.txt updates
vcpkg Integration
incorporate with cibuildwheel. Fixes to python-packaging.yml, python-remote-storage.yml, and ci-r-python-interop.yml
Version Management - Simplified setuptools_scm configuration

Notes for Reviewer:

codecov · 2025-11-13T21:33:50Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.84%. Comparing base (dc42c79) to head (4c845ed).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4310      +/-   ##
==========================================
- Coverage   89.23%   86.84%   -2.39%     
==========================================
  Files          62      137      +75     
  Lines        7087    20706   +13619     
  Branches        0       15      +15     
==========================================
+ Hits         6324    17982   +11658     
- Misses        763     2724    +1961

Flag	Coverage Δ
python	`89.20% <ø> (-0.03%)`	⬇️
r	`85.61% <ø> (?)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Components	Coverage Δ
python_api	`89.20% <ø> (-0.03%)`	⬇️
libtiledbsoma	`77.24% <ø> (∅)`

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

apis/python/scripts/setup_build_env.ps1

jp-dark

Thanks for working on this! I have a couple small initial comments while I continue to experiment locally and dig into all the changes

.github/workflows/ci-r-python-interop.yml

.github/workflows/test-python.yml

apis/python/CMakeLists.txt

teo-tsirpanis

We can simplify the vcpkg integration and tiledbsoma's build system by eliminating the superbuild and following the Core's approaches.

teo-tsirpanis · 2025-11-21T16:23:40Z

apis/python/scripts/setup_build_env.sh

+    # Bootstrap vcpkg if needed
+    if [ ! -f "${VCPKG_ROOT}/vcpkg" ] && [ ! -f "${VCPKG_ROOT}/vcpkg.exe" ]; then
+        echo "Bootstrapping vcpkg..."
+        "${VCPKG_ROOT}/bootstrap-vcpkg.sh" || {
+            echo "Error: Failed to bootstrap vcpkg"
+            exit 1
+        }
+    fi


Vcpkg will do this by itself if needed.

teo-tsirpanis · 2025-11-21T16:24:47Z

apis/python/scripts/setup_build_env.sh

+    # Copy custom triplets if they exist in the Python package
+    if [ -d "${PYTHON_DIR}/vcpkg_triplets" ] && [ -d "${VCPKG_ROOT}/triplets" ]; then
+        echo "Installing custom vcpkg triplets..."
+        cp -f "${PYTHON_DIR}/vcpkg_triplets/"*-release.cmake "${VCPKG_ROOT}/triplets/" 2>/dev/null || true
+    fi


We can set VCPKG_OVERLAY_TRIPLETS when configuring. Modifying the vcpkg directory is not a good idea in general.

teo-tsirpanis · 2025-11-21T16:49:46Z

apis/python/CMakeLists.txt

@@ -0,0 +1,332 @@
+cmake_minimum_required(VERSION 3.21)
+cmake_policy(SET CMP0148 NEW) 


Why does the policy need to be set to NEW? It doesn't change anything if we don't use a deprecated find module, does it?

teo-tsirpanis · 2025-11-21T16:50:19Z

apis/python/CMakeLists.txt

+# Set macOS architecture before project() to avoid vcpkg warnings
+if(APPLE AND NOT DEFINED CMAKE_OSX_ARCHITECTURES)
+    # Detect system architecture
+    execute_process(
+        COMMAND uname -m
+        OUTPUT_VARIABLE DETECTED_ARCH
+        OUTPUT_STRIP_TRAILING_WHITESPACE
+    )
+    set(CMAKE_OSX_ARCHITECTURES "${DETECTED_ARCH}" CACHE STRING "macOS architecture")
+    message(STATUS "Auto-detected macOS architecture: ${CMAKE_OSX_ARCHITECTURES}")
+endif()


This should be done automatically; what warnings does vcpkg emit?

apis/python/CMakeLists.txt

teo-tsirpanis · 2025-11-21T17:26:26Z

.github/workflows/python-packaging.yml

+      - name: Create custom vcpkg triplets for release builds
+        run: |
+          mkdir -p vcpkg_triplets
+          
+          # x64-linux-release
+          cat > vcpkg_triplets/x64-linux-release.cmake << 'EOF'
+          set(VCPKG_TARGET_ARCHITECTURE x64)
+          set(VCPKG_CRT_LINKAGE dynamic)
+          set(VCPKG_LIBRARY_LINKAGE dynamic)
+          set(VCPKG_BUILD_TYPE release)
+          set(VCPKG_CMAKE_SYSTEM_NAME Linux)
+          EOF
+          
+          # arm64-linux-release
+          cat > vcpkg_triplets/arm64-linux-release.cmake << 'EOF'
+          set(VCPKG_TARGET_ARCHITECTURE arm64)
+          set(VCPKG_CRT_LINKAGE dynamic)
+          set(VCPKG_LIBRARY_LINKAGE dynamic)
+          set(VCPKG_BUILD_TYPE release)
+          set(VCPKG_CMAKE_SYSTEM_NAME Linux)
+          EOF
+          
+          # x64-osx-release
+          cat > vcpkg_triplets/x64-osx-release.cmake << 'EOF'
+          set(VCPKG_TARGET_ARCHITECTURE x64)
+          set(VCPKG_CRT_LINKAGE dynamic)
+          set(VCPKG_LIBRARY_LINKAGE dynamic)
+          set(VCPKG_BUILD_TYPE release)
+          set(VCPKG_CMAKE_SYSTEM_NAME Darwin)
+          set(VCPKG_OSX_ARCHITECTURES x86_64)
+          set(VCPKG_OSX_DEPLOYMENT_TARGET "13.0")
+          EOF
+          
+          # arm64-osx-release
+          cat > vcpkg_triplets/arm64-osx-release.cmake << 'EOF'
+          set(VCPKG_TARGET_ARCHITECTURE arm64)
+          set(VCPKG_CRT_LINKAGE dynamic)
+          set(VCPKG_LIBRARY_LINKAGE dynamic)
+          set(VCPKG_BUILD_TYPE release)
+          set(VCPKG_CMAKE_SYSTEM_NAME Darwin)
+          set(VCPKG_OSX_ARCHITECTURES arm64)
+          set(VCPKG_OSX_DEPLOYMENT_TARGET "13.0")
+          EOF


Remove it? We already have triplet files checked in, and they are better being placed there.

teo-tsirpanis · 2025-11-21T17:28:59Z

.github/workflows/test-python.yml

+      - name: Checkout vcpkg
+        uses: actions/checkout@v4
+        with:
+          repository: microsoft/vcpkg
+          path: vcpkg


Better let CMake clone vcpkg, per my other comments.

teo-tsirpanis · 2025-11-21T17:29:19Z

.github/workflows/python-remote-storage.yml

+      - name: Bootstrap vcpkg
+        run: |
+          if [ ! -d vcpkg ]; then
+            git clone --depth 1 https://github.com/microsoft/vcpkg.git vcpkg


Better let CMake clone vcpkg, per my other comments.

teo-tsirpanis · 2025-11-21T17:36:10Z

apis/python/CMakeLists.txt

+    CXX_STANDARD 20
+    CXX_STANDARD_REQUIRED ON


Suggested change

CXX_STANDARD 20

CXX_STANDARD_REQUIRED ON

We already set the C++ version globally in lines 19-20.

teo-tsirpanis · 2025-11-21T17:51:06Z

apis/python/pyproject.toml

+# Install system dependencies required by vcpkg (runs once per container)
+before-all = [
+    "yum install -y zip unzip tar curl ninja-build perl-IPC-Cmd autoconf automake libtool pkg-config",
+]


This is something the user (or CI) should do explicitly. Besides the side-effects of a system-wide operation, it might require root.

ktsitsi · 2025-11-21T19:55:54Z

Thank you @teo-tsirpanis for your comments!

Copying data from [PR #4310](#4310) Co-Authored-By: Konstantinos Tsitsimpikos <[email protected]>

ktsitsi force-pushed the kt/scikit-core-build-vcpkg-integration branch 11 times, most recently from eb1a067 to 305d935 Compare November 13, 2025 21:11

ktsitsi force-pushed the kt/scikit-core-build-vcpkg-integration branch 18 times, most recently from 375513b to ac8bca5 Compare November 18, 2025 05:59

ktsitsi added 12 commits November 18, 2025 14:10

Forward passing and debugging

4143a36

Fix with config-settings

cde1885

Fix repair wheel

c216700

Fix macos system var

db867b3

Fix repairwheel

c08c5c9

Add root to find the version from the git tags

b5ec25d

Remove double instance of libtiledb

e943132

Fix version vol2

b048f15

Fix weird dir in linux wheels

a415d85

Fix ci interop and remote storage jobs

c97534a

Fix version

45d7d45

Fix test version

1d2e1f0

ktsitsi force-pushed the kt/scikit-core-build-vcpkg-integration branch from d973812 to 1d2e1f0 Compare November 18, 2025 19:10

ktsitsi added 3 commits November 18, 2025 16:07

Fix after rebase

2b417fa

Minor miscs

ddfce0f

Remove old setup.py

c821608

ktsitsi requested review from bkmartinjr, jp-dark and kounelisagis November 19, 2025 21:39

jp-dark reviewed Nov 19, 2025

View reviewed changes

apis/python/scripts/setup_build_env.ps1 Outdated Show resolved Hide resolved

jp-dark requested a review from XanthosXanthopoulos November 20, 2025 15:26

Remove windows script

dbb8fba

jp-dark reviewed Nov 20, 2025

View reviewed changes

ktsitsi force-pushed the kt/scikit-core-build-vcpkg-integration branch from b916f6d to f1370f3 Compare November 21, 2025 16:11

PR comments/fixes

4c845ed

ktsitsi force-pushed the kt/scikit-core-build-vcpkg-integration branch from f1370f3 to 4c845ed Compare November 21, 2025 16:18

ktsitsi requested a review from teo-tsirpanis November 21, 2025 16:19

teo-tsirpanis suggested changes Nov 21, 2025

View reviewed changes

jp-dark added a commit that referenced this pull request Dec 1, 2025

(WIP) Add vcpkg triplets to python packaging

36d216b

Copying data from [PR #4310](#4310) Co-Authored-By: Konstantinos Tsitsimpikos <[email protected]>

		@@ -0,0 +1,332 @@
		cmake_minimum_required(VERSION 3.21)
		cmake_policy(SET CMP0148 NEW)

Integrating scikit-core-build with vcpkg #4310

Are you sure you want to change the base?

Integrating scikit-core-build with vcpkg #4310

Conversation

ktsitsi commented Nov 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Nov 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

jp-dark left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

teo-tsirpanis left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ktsitsi commented Nov 21, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ktsitsi commented Nov 13, 2025 •

edited

Loading

codecov bot commented Nov 13, 2025 •

edited

Loading