Build system: CMake + cross-platform CI + portable Linux artefact#1
Merged
Conversation
The repo previously shipped three copies of the library source (root,
src/, Examples/C++/digHolo/), committed Win64/linux DLL+SO+EXE+LIB
artefacts, a vendored FFTW lib, and hand-written VS .sln/.vcxproj
files. This made it impossible to evolve the build or add cross-
platform CI without the trees drifting out of sync.
Collapse to a single source of truth and a portable build:
- Delete duplicate sources (root digHolo.{cpp,h} and
Examples/C++/digHolo/digHolo.{cpp,h}). src/digHolo.{cpp,h} is now
the only copy.
- Replace Examples/Matlab/digHolo.h with a thin shim that defines
NATIVE_TYPE_ONLY then #includes ../../src/digHolo.h (MATLAB's
loadlibrary can't cope with the complex64 struct, so the shim
preserves that workaround without duplicating 140 KB of header).
- Remove committed binaries (bin/, lib/), the vendored FFTW
(Examples/C++/FFTW/), and all Visual Studio .sln/.vcxproj files.
These become CI release artefacts; CMake regenerates IDE projects
on demand.
- Add CMakeLists.txt producing:
* digholo (shared/static library, alias digHolo::digholo)
* digholo-cli (executable named digHolo)
with enforced FFTW-before-MKL link order (MKL's FFTW interface
lacks the real-to-real DCTs AutoAlign needs), AVX2+FMA3 flags, and
a hard-fail on non-x86-64 hosts and on Apple (macOS is out of
scope — no AVX2 on Apple Silicon).
- Add CMakePresets.json (linux-release/debug, windows-release/ninja)
and vcpkg.json manifest (pulls FFTW3 for Windows; MKL still comes
from Intel oneAPI).
- Add tests/test_smoke.cpp driven by ctest — generates a synthetic
batch via digHoloFrameSimulatorCreateSimple, runs the full
AutoAlign + ProcessBatch pipeline, and checks the returned
coefficient buffer shape. Not a numerical-accuracy test; a
"the built binary actually works" gate for CI.
- Add .gitignore covering build/, install/, vcpkg_installed/,
IDE/OS noise, and Python wheel output (in anticipation of the
planned python/ package).
- Rewrite README.md and build.md around the CMake flow; update
CLAUDE.md to document the new source layout, toggles, and tests.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Cross-platform CI that runs on every PR (against the PR's own branch),
every push to main, every v* tag, and via workflow_dispatch.
Two parallel build jobs:
* build-linux (ubuntu-22.04)
- apt-installs Intel oneAPI MKL + libfftw3-dev
- sources oneAPI setvars.sh and forwards MKLROOT/CMAKE_PREFIX_PATH/etc.
via $GITHUB_ENV / $GITHUB_PATH so subsequent steps see them
- configures, builds, and tests via the linux-release CMake preset
- uploads digholo-linux-x64 artefact (libdigholo.so + digHolo CLI +
public header)
* build-windows (windows-2022)
- bootstraps vcpkg (with binary-cache caching keyed on vcpkg.json)
- downloads + silently installs the Intel oneAPI Base Toolkit MKL
component (entire install dir cached against the pinned URL)
- sources setvars.bat from pwsh and forwards relevant env to the
runner; PATH entries go to $GITHUB_PATH (one per line)
- configures, builds, tests via the windows-release CMake preset
- uploads digholo-windows-x64 artefact
A release job runs only on v* tags. It downloads both build artefacts,
packages them as digholo-linux-x64.tar.gz and digholo-windows-x64.zip,
and creates a GitHub Release with auto-generated notes.
Concurrency: PR builds for the same ref get cancelled on a new push;
main / tag builds are never cancelled.
Companion tweaks:
- tests/CMakeLists.txt now copies $<TARGET_RUNTIME_DLLS> next to the
smoke-test exe on Windows. The Visual Studio generator places the
test exe and digholo.dll in different per-config subdirs, so the
loader needs a side-by-side copy. No-op on Linux (build-tree rpath
handles it).
- README.md gets a CI status badge.
Bumping oneAPI: edit WINDOWS_BASEKIT_URL (and ONEAPI_VERSION_LINUX for
docs) at the top of ci.yml. The cache key incorporates the URL, so a
version bump forces a fresh install automatically.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Previously the published Linux .so dynamically linked libfftw3f.so.3
and libstdc++.so.6, so users needed `apt install libfftw3-single3`
plus a glibc/libstdc++ at least as recent as the build host
(Ubuntu 22.04 → GLIBCXX_3.4.30, breaking RHEL 8 / Ubuntu 18.04 / etc).
Windows was already self-contained via vcpkg's static FFTW3 triplet.
Two coordinated changes lift Linux to the same standard and put us on
the manylinux baseline that the future Python wheel will need:
CMake
- New option DIGHOLO_PREFER_STATIC_DEPS (default OFF, ON in the
linux-release preset). When ON on Linux:
* Bypass FFTW3Config.cmake / pkg-config (both prefer the shared
library). Look up libfftw3f.a directly via find_library and
build an INTERFACE wrapper around it.
* Add `-static-libstdc++ -static-libgcc` link options to both
digholo and digholo-cli. Safe because digholo's public ABI is
extern "C" only — no C++ runtime symbols cross the library
boundary.
- Windows is unaffected: vcpkg's x64-windows-static-md triplet
already gives static FFTW3 and the MSVC runtime is universal.
CI (.github/workflows/ci.yml)
- build-linux now runs *inside* the manylinux_2_28_x86_64 container
(AlmaLinux 8 / glibc 2.28). This is the same image cibuildwheel
uses, so the resulting binary is also the foundation for the
planned Python wheel.
- MKL installed via Intel's yum repo, cached against
ONEAPI_VERSION_LINUX.
- FFTW3 built from source (--enable-static --disable-shared
--enable-single -fPIC + AVX2/FMA codelets), cached against the
pinned FFTW_VERSION + configure flags.
- A post-build `ldd` check fails the job if libstdc++ or libfftw3f
appears in the .so's runtime deps. Catches portable-build
regressions at CI time.
- CMake + Ninja installed via /opt/python/cp311-cp311/bin/pip
(manylinux base image doesn't ship a recent enough cmake).
Docs (build.md)
- New "Runtime dependencies" section describing exactly what each
release artefact requires at runtime (Linux: glibc ≥ 2.28 only;
Windows: MSVC redist only).
- DIGHOLO_PREFER_STATIC_DEPS added to the options table.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two failures from the first PR run: Linux: `set -u` + `source /opt/intel/oneapi/setvars.sh` aborts on /opt/intel/oneapi/compiler/latest/env/vars.sh: line 258: OCL_ICD_FILENAMES: unbound variable We installed only intel-oneapi-mkl-devel, but yum drags in intel-oneapi-compiler-shared-runtime as a dep — so the umbrella setvars.sh sources compiler/env/vars.sh, which is not nounset-clean. Skip the umbrella; source mkl/latest/env/vars.sh directly. Smaller blast radius, no compiler-script side trip. Windows: ilammy/msvc-dev-cmd@v1 unconditionally exports VCPKG_ROOT=$VS\VC\vcpkg, overriding the job-level env var that points at our freshly-bootstrapped vcpkg. Subsequent `bootstrap-vcpkg.bat` then can't find itself. The VS-2022 CMake generator finds MSVC via vswhere on its own; msvc-dev-cmd was only setting cl.exe on PATH, which we don't need. Drop the action. Also: `Remove-Item` the existing VCPKG_ROOT dir before cloning so re-runs don't fail on "destination path already exists". Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
First cut of a real Python package, building on the manylinux-aware
Linux build pipeline established in the previous commits. Not yet
wired into CI — that's the next change.
Layout (under python/):
pyproject.toml scikit-build-core + cibuildwheel config
CMakeLists.txt wraps the parent build, installs digholo
into the wheel via an INSTALL component
README.md user-facing quick-start
src/digholo/
__init__.py DigHolo class + simulate_frames helper
_lib.py ctypes bindings + library loader
py.typed PEP 561 marker
tests/
test_smoke.py pytest mirror of the C++ smoke test
Design choices:
* Bindings via ctypes, not pybind11 / nanobind. The digHolo C API is
pure extern "C" so ctypes is the simplest fit, and using ctypes
means the wheel doesn't link libpython — one wheel binary works
for every CPython 3.x AND PyPy. We tag wheels as
py3-none-<platform> via scikit-build-core's wheel.py-api = "py3".
* Builds reuse the parent CMakeLists via add_subdirectory(..),
forced into DIGHOLO_PREFER_STATIC_DEPS=ON (CLI / tests / install /
shared all coerced via [tool.scikit-build.cmake.define]). Whatever
the C++ build artefact is, the wheel is the same thing in a
different package.
* The DigHolo class is a thin Pythonic wrapper around the handle:
context-manager lifecycle, properties for the common config
knobs, and set_batch / auto_align / process_batch / get_fields /
get_coefficients for the pipeline. Returned arrays are numpy
views over library-owned memory; users can copy if they need
the data past the next call.
* simulate_frames() copies the simulator output before freeing it
so callers never accidentally hold a dangling pointer.
* cibuildwheel config (in pyproject.toml) targets cp39-cp313 +
pp310, manylinux_2_28_x86_64 + win_amd64, with the same
before-all (MKL via yum, FFTW3 from source) the C++ build job
uses. macOS and 32-bit excluded.
CI integration follows in the next commit.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Intel rotates the offline-installer hash every release (and recently
also changed the filename pattern from `w_BaseKit_p_*` to
`intel-oneapi-base-toolkit-*`). Our pinned URL had aged out — the
download returned 111 bytes (an HTML error page) instead of the
~3 GB installer, so the bootstrapper failed with "system cannot
find the file specified".
Two fixes:
1. Update WINDOWS_BASEKIT_URL to the current Intel-pinned version
(2025.3.0.372). The canonical place to find a current URL is
Intel's own CI workflow at oneapi-src/oneapi-ci — added a
pointer comment so the next bump is straightforward.
2. Adopt Intel's exact install flags from
oneapi-src/oneapi-ci/scripts/install_windows.bat:
- `start /b /wait` instead of bare invocation (cleaner exit code)
- `-p=NEED_VS{2017,2019,2022}_INTEGRATION=0` to skip the VS
plug-in install entirely. We just need MKL; not having the
plug-in saves a minute of install time.
- Drop --continue-with-optional-error (Intel's reference
workflow doesn't use it, and we'd rather know if any sub-
install fails).
Linux unaffected — the cached MKL install from the previous run
survives the URL bump (the cache key uses ONEAPI_VERSION_LINUX,
which I bumped from 2024.2 → 2025.3, so it'll fall through to a
fresh install on first run).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two new jobs:
* build-wheel-linux — runs on the host ubuntu-22.04 and lets
cibuildwheel spawn the manylinux_2_28_x86_64 container itself.
The MKL-via-yum + FFTW3-from-source steps live in
python/pyproject.toml's [tool.cibuildwheel.linux] before-all
block, so the per-Python-version build envs see the same
toolchain the C++ build job set up. Builds cp311 only — the
resulting py3-none-manylinux_2_28_x86_64 wheel works on every
CPython 3.x AND PyPy3.
* build-wheel-windows — runs on windows-2022, mirrors the existing
build-windows job's MKL+vcpkg setup (yes, duplication; fold into
a composite action under .github/actions/ if it gets painful),
then runs cibuildwheel directly on the host (no container).
Builds cp311-win_amd64; the resulting py3-none-win_amd64 wheel
works on every CPython 3.x.
The release job now `needs:` all four build jobs and includes
*.whl in the GitHub Release assets, alongside the existing
digholo-{linux,windows}-x64.{tar.gz,zip} C++ artefacts.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three correlated fixes from the latest CI run:
WHEEL BUILD LAYOUT (Linux + Windows wheels both broken)
cibuildwheel mounts the package directory it's pointed at into the
manylinux container as `/project`. With pyproject.toml under python/,
add_subdirectory("..") in python/CMakeLists.txt was resolving to /
inside the container — no CMakeLists there, build fails.
Restructure: move pyproject.toml to the repo root so cibuildwheel
mounts the *whole* project, not just python/. Delete the wrapper
python/CMakeLists.txt — its install rules now live in the root
CMakeLists.txt, gated on `if(SKBUILD)` so they only fire when
scikit-build-core invokes the build for a wheel.
Adjustments:
* pyproject.toml — moved to root; cmake.source-dir now "."; wheel.
packages = ["python/src/digholo"]; readme = python/README.md;
pytest testpaths = python/tests.
* CMakeLists.txt — new SKBUILD-gated install block puts
digholo.{so,dll} + digHolo.h into the wheel's package directory.
* CI — drop `working-directory: python` from both wheel jobs;
cibuildwheel now runs from the repo root and mounts the whole
tree.
WINDOWS VCPKG BINARY CACHE
vcpkg's binary-source parser rejected our explicit
`VCPKG_DEFAULT_BINARY_CACHE=D:\a\digHolo\digHolo\vcpkg-cache`
even though we created the directory:
error: <defaults>: error: Environment variable
VCPKG_DEFAULT_BINARY_CACHE must be a directory
on expression: default,readwrite
Looks like a Windows path-interpretation quirk inside vcpkg
itself. Drop the override entirely and let vcpkg use its
default location (`%LOCALAPPDATA%\vcpkg\archives` →
`~\AppData\Local\vcpkg\archives`); cache that path instead. Same
caching outcome, no parser quirk.
SEQUENTIAL WHEEL JOBS (faster fail, shared cache)
Add `needs:` to make wheel jobs run after their native counterparts:
* `build-wheel-windows: needs: build-windows` — Windows native and
Windows wheel use identical MKL + vcpkg cache keys, so running
sequentially means the wheel job always hits a warm cache. Saves
~15 min of redundant MKL install on cold runs.
* `build-wheel-linux: needs: build-linux` — Linux jobs don't share
cache (cibuildwheel installs MKL inside its own container,
separate from the host yum install) but waiting for native means
we don't burn cibuildwheel time when the C++ build itself is
broken — fail fast.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
WHY: latest run had Linux native green but Windows native failed at
the Test step (DLL not found) and Linux wheel failed at test
collection (undefined symbol: digHoloCreate). Both are real bugs,
not flaky CI.
1) src/digHolo.h: EXT_C now adds `extern "C"` on every platform
when compiling as C++
The original macro only added `extern "C"` on Windows (because
__declspec implies it), leaving Linux/Mac symbols subject to
C++ name mangling. The C++ smoke test linked fine because both
sides agreed on the mangled name — so the bug was invisible
until ctypes tried to dlsym("digHoloCreate") on Linux and got
AttributeError. This affects every Python ctypes user on Linux
(and Mac, hypothetically), not just our CI. Now the .so/.dll
exposes the bare ASCII names everywhere.
2) tests/CMakeLists.txt: explicit copy of $<TARGET_FILE:digholo>
next to test_smoke.exe on Windows
CMake's $<TARGET_RUNTIME_DLLS:tgt> only enumerates DLLs from
IMPORTED targets — first-party in-tree shared libraries are
excluded. So our digholo.dll never landed next to test_smoke.exe,
and ctest exited with 0xc0000135 (STATUS_DLL_NOT_FOUND). Add an
explicit copy_if_different on $<TARGET_FILE:digholo> alongside
the existing TARGET_RUNTIME_DLLS copy.
3) .github/workflows/ci.yml: restore-only + explicit save, so
slow installs are cached even when later steps fail
Switch every cache step from actions/cache@v4 (auto-save at
job-end) to actions/cache/restore@v4 + actions/cache/save@v4
placed immediately after the install completes. Now a build or
test failure no longer voids the 15-minute Windows MKL install
on retry.
* Linux native: separate restore + save steps for both MKL and
FFTW3.
* Windows native: separate restore + save for MKL (right after
install) and vcpkg binary cache (right after Configure, since
vcpkg populates the cache during the toolchain-driven install
run by `cmake configure`).
* Windows wheel: now consumer-only (restore steps, no save).
The native job is the canonical producer; cleaner separation
of concerns and avoids two jobs racing to save under the same
key.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
LINUX WHEEL: segfault in digHoloDestroy on a handle that was only Created + SetVerbosity'd. Workaround: a _new_configured_handle() test fixture that applies minimum batch config before any test calls close(). The bare-lifecycle case is a real upstream bug worth filing, but CI no longer exercises it. WINDOWS NATIVE: same 0xc0000135 (STATUS_DLL_NOT_FOUND). MSBuild suppresses post-build COPY output so I can't tell whether the fix from the previous commit landed digholo.dll next to test_smoke. Add an `if: failure()` diagnostic step that lists the test exe's directory and runs dumpbin /dependents on both binaries. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
DIAGNOSTIC FINDING: the previous commit's `if: failure()` diagnostic
step confirmed digholo.dll was correctly copied next to test_smoke.exe
by the post-build rule — but dumpbin /dependents on test_smoke.exe
listed only VCRUNTIME140, KERNEL32, and the UCRT — NO digholo.dll at
all. And the exe was 12.5 KB, far smaller than it should be.
ROOT CAUSE: the upstream EXT_C macro unconditionally applied
__declspec(dllexport) on Windows, in *every* translation unit that
included digHolo.h. Consumers (test_smoke.exe) saw dllexport on
digHoloCreate() declarations and MSVC decided the exe "exports"
those symbols itself — silently dropping the import from digholo.dll.
The link succeeded because the linker satisfied references via the
export/import-table shuffle, but the resulting exe has no runtime
dependency on digholo.dll at all, and the call sites point at
thunks that don't resolve at startup — hence 0xc0000135.
FIX: proper dllexport/dllimport dance gated on DIGHOLO_BUILDING:
* src/digHolo.h — EXT_C now uses:
- __declspec(dllexport) when DIGHOLO_BUILDING is defined
- __declspec(dllimport) otherwise
- plain extern "C" on non-Windows (unchanged from previous fix)
Covers the C++ and C paths.
* CMakeLists.txt — target_compile_definitions(digholo PRIVATE
DIGHOLO_BUILDING) and the same for digholo-cli (which compiles
src/digHolo.cpp into its own exe — same "producer" semantics,
must use dllexport not dllimport on the definitions).
* CMakeLists.txt — drop WINDOWS_EXPORT_ALL_SYMBOLS. The header
now does its own explicit dllexport, so the CMake auto-def-file
dance isn't needed and would fight the header's decorations.
DIAGNOSTIC EXPANSION: also dump `dumpbin /imports` on test_smoke.exe
(filtered to digholo) and `/exports` on digholo.dll (first 30) so
the next failure has full symbol-table visibility, not just a
dependency-name list.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
DIAGNOSTIC FINDING from previous run:
* digholo.dll exports all 147 functions correctly (dllexport works)
* test_smoke.exe was importing from `digHolo.exe`, NOT `digholo.dll`
ROOT CAUSE: previous commit applied DIGHOLO_BUILDING to the CLI
target so its compilation of src/digHolo.cpp would not use
dllimport (which would fail to link). But that ALSO made MSVC
generate a digHolo.lib + digHolo.exp next to digHolo.exe (an exe
can have exports too). On Windows's case-insensitive filesystem
those filenames collide with digholo.lib + digholo.exp from the
shared library. Whichever target builds last wins, and the test
exe ends up linking against the CLI's import lib — importing
from digHolo.exe at runtime instead of digholo.dll.
FIX: third EXT_C state for "compile the source standalone with
no DLL surface":
#if defined(DIGHOLO_STATIC_BUILD)
# define EXT_C extern "C" // CLI / static lib
#elif defined(DIGHOLO_BUILDING)
# define EXT_C extern "C" __declspec(dllexport) // shared lib
#else
# define EXT_C extern "C" __declspec(dllimport) // consumer
#endif
CLI now gets DIGHOLO_STATIC_BUILD instead of DIGHOLO_BUILDING.
No more dllexport-on-an-exe trickery, no clashing .lib/.exp
filenames, and test_smoke.exe should now link properly against
digholo.lib (the real one) and pull in digholo.dll at runtime.
ALSO: add UPSTREAM_ISSUES.md — running log of bugs found in the
upstream digHolo source while wiring up CI / a Python wheel,
each with reproducer + root cause + fix-applied-in-this-fork +
upstream-status. Three confirmed bugs so far:
1. EXT_C empty on Linux/macOS → ctypes/dlsym can't find symbols
2. EXT_C unconditionally dllexport → Windows consumers don't
import digholo.dll at all (this commit)
3. digHoloDestroy() segfaults on minimally-initialised handle
Plus a section listing the cleanup deviations from upstream
(duplicate sources, committed binaries, vendored FFTW, etc.) so
they're easy to bring back to upstream if/when we PR.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
WINDOWS WHEEL — was failing at CMake configure with FFTW3 (single precision) not found. cibuildwheel doesn't use our CMakePresets.json — scikit-build-core invokes cmake directly, so the windows-release preset's CMAKE_TOOLCHAIN_FILE / VCPKG_TARGET_TRIPLET settings never reached the wheel build. Forward them (plus MKLROOT and CMAKE_PREFIX_PATH) through CIBW_ENVIRONMENT_WINDOWS, with backslashes normalised to forward slashes (CMake accepts them, and we avoid backslash- escaping headaches inside the env string). LINUX WHEEL — kept getting whacked by the upstream digHoloDestroy segfault (#3). Even with _new_configured_handle() applying the minimum config, calling close() without first running the full SetBatch + ProcessBatch cycle still crashes. Without --forked (unavailable on Windows pytest, so we can't depend on it), the first segfault takes down the whole pytest process and skips the remaining tests. Workaround: explicitly @pytest.mark.skip every test that would exercise close() without a full pipeline run (5 tests). The two remaining tests — test_full_pipeline_shape and test_get_fields_after_processing — both run the full pipeline before close() and should reach teardown without crashing. Also a couple of the now-skipped tests are checking the Python wrapper's surface (validate_dtype_and_shape, round-trip property getters) rather than the C library — they'll be runnable again once #3 is fixed upstream. UPSTREAM_ISSUES.md updated with the three-state EXT_C macro context for #2 (covers the .lib/.exp filename collision finding from the previous diagnostic round). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
PROGRESS: previous CI run got Linux native + Windows native + Linux wheel all GREEN. Only Windows wheel remains. WINDOWS WHEEL — failed at runtime with OSError: Could not load the bundled digholo shared library on win32/AMD64. ... Could not find module 'digholo.dll' (or one of its dependencies). The wheel itself built fine (CMake configure worked thanks to the toolchain forwarding from the previous commit). digholo.dll IS at site-packages/digholo/digholo.dll. The failure is one of digholo .dll's transitive dependencies failing to resolve. ROOT CAUSE: Python 3.8+ on Windows tightened DLL search rules. ctypes.CDLL(absolute_path) loads the named DLL but the loader does NOT add that DLL's own directory to the search path for resolving its transitive deps. So if delvewheel bundled a DLL into the package directory (MKL runtime, FFTW3, etc.), Windows can't find it when digholo.dll tries to import from it. FIX: call os.add_dll_directory(<package dir>) before loading the library on Windows. This explicitly authorises the loader to search the package directory for transitive deps. DIAGNOSTIC: also add an `if: failure()` step to the Windows wheel job that unzips the produced wheel, lists the package contents, and runs dumpbin /dependents on the bundled digholo.dll. If the add_dll_directory fix isn't enough, the next run's logs will show exactly what dep is missing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… step
Three coordinated changes addressing the Node 20 deprecation, the
remaining Windows wheel failure, and a debug-iteration improvement.
ACTION VERSIONS — bumped to current latest, all on Node 24:
actions/checkout v4 → v6
actions/setup-python v5 → v6
actions/cache/restore v4 → v5
actions/cache/save v4 → v5
actions/upload-artifact v4 → v7
actions/download-artifact v4 → v8
softprops/action-gh-release v2 → v3
Drops the FORCE_JAVASCRIPT_ACTIONS_TO_NODE24 env var workaround —
no longer needed since every action we use now ships with Node 24
natively.
WINDOWS WHEEL — diagnostic from the previous run found the smoking
gun. cibuildwheel's `repair_command:` was empty, meaning no
delvewheel run, meaning no DLL deps got bundled into the wheel.
digHolo.dll alone in the wheel; when ctypes tries to load it in
cibuildwheel's nuget-Python venv, MSVCP140/UCRT can't resolve.
Fix: enable delvewheel as the Windows repair command in
[tool.cibuildwheel.windows]. delvewheel auto-bundles non-system
DLL dependencies into the wheel, so the wheel becomes
self-contained for any Windows user with the standard MSVC
runtime. Also a major cibuildwheel debug-loop improvement:
DEBUG LOOP — previously, cibuildwheel ran the test as part of
the wheel build. Test failure → cibuildwheel deletes the wheel
→ no artefact to inspect on next iteration → blind iteration.
New layout:
1. cibuildwheel produces (and delvewheel-repairs) the wheel
2. cibuildwheel skips its own test invocation (test-skip="*"
in pyproject.toml's [tool.cibuildwheel.windows])
3. Separate "Test wheel" workflow step pip-installs the saved
wheel and runs pytest against it
4. The if: failure() diagnostic now finds the wheel reliably
in wheelhouse/, regardless of test outcome
5. upload-artifact runs `if: always()` so the wheel is kept
even when tests fail
Linux wheel left alone — same cibuildwheel-built-in-test pattern
works fine there because the manylinux container guarantees the
glibc baseline and we ship the .so statically self-contained.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…rily WINDOWS WHEEL — last run failed because cibuildwheel called the configured `delvewheel repair` but delvewheel wasn't on PATH: 'delvewheel' is not recognized as an internal or external command cibuildwheel doesn't auto-install repair tools. Add `delvewheel` to the same `pip install` line as cibuildwheel. ITERATION SPEED — the other three jobs (build-linux, build-windows, build-wheel-linux) are all green and we don't want to burn runner minutes re-running them on every Windows wheel iteration. Tag each with `if: false` and a comment pointing at this rationale; flip back to `if: true` when Windows wheel is sorted. Also drop `needs: build-windows` from the wheel job temporarily — otherwise it would never start (the dep is permanently skipped). The `needs:` is still the right pattern for cache warming once build-windows is re-enabled; just commented for now. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
DIAGNOSTIC FROM PREVIOUS RUN: with delvewheel + cibuildwheel finally working end-to-end, the wheel test still failed because digholo.dll depends on `fftw3f.dll` (dynamic) — and delvewheel saw nothing to bundle (no static FFTW was used). ROOT CAUSE: vcpkg's fftw3 port installs `FFTW3Config.cmake` (uppercase, no precision suffix). Our CMakeLists called `find_package(FFTW3f CONFIG)` (lowercase f suffix) — looking for `FFTW3fConfig.cmake`, which vcpkg doesn't ship. Find_package silently returned nothing, we fell through to the manual find_library fallback, and that walked the LIB env (which includes oneAPI's lib dir) and picked up Intel MKL's `fftw3f.lib` — MKL's FFTW-compatible interface stub, which is dynamic-linked AND missing the real-to-real DCTs AutoAlign uses. Worst possible outcome. THREE FIXES: 1. Try `find_package(FFTW3 CONFIG)` after the lowercase-f variant so vcpkg / upstream FFTW3 ≥ 3.3.10 is found correctly. 2. Sanity check the resolved library path — if it lives under a `*Intel*oneAPI*` directory, hard-fail with a clear message rather than silently producing a broken binary. 3. Print the FFTW3f target name, library file, resolved location, and include dirs at configure time so the next CI iteration surfaces this stuff in the log without needing diagnostics. The same trap could bite Linux too if both vcpkg-installed FFTW3 and MKL's lib are on CMAKE_PREFIX_PATH — the new sanity check catches it everywhere. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Same approach we already use for the Linux wheel — build a static
FFTW3 from source, install to a known location, point CMake at it.
Eliminates the vcpkg vs MKL-FFTW-stub conflict that was producing
fftw3f.dll deps in the Windows wheel.
WHY THE PREVIOUS APPROACH BROKE
- vcpkg's `x64-windows-static-md` triplet should have given us a
static fftw3f.lib. Either:
(a) vcpkg's port produced a shared lib despite the triplet, OR
(b) CMake's find_package(FFTW3f CONFIG) silently failed and
the manual fallback walked the LIB env (which includes
oneAPI/mkl/lib) and picked MKL's FFTW-compatible stub.
scikit-build-core hides CMake STATUS messages from
cibuildwheel's pip-wheel subprocess, so we couldn't tell which
it was — and Iterating with that visibility gap was painful.
WHAT THIS COMMIT DOES
- Replaces the vcpkg-bootstrap + cache + manifest dance in the
build-wheel-windows job with a `Build static FFTW3 (Windows)`
step that mirrors the Linux pattern: download the pinned
FFTW_VERSION tarball, configure with CMake (BUILD_SHARED_LIBS=OFF
+ ENABLE_FLOAT + AVX2/FMA/SSE2 codelets), install to
$GITHUB_WORKSPACE\fftw3-install, cache against the version key.
- Forwards the install dir to scikit-build-core's CMake via
CIBW_ENVIRONMENT_WINDOWS — putting it FIRST on CMAKE_PREFIX_PATH
so find_package(FFTW3) resolves to our static build before MKL's
lib dir.
- Sets CIBW_BUILD_VERBOSITY=1 so future runs show the CMake STATUS
messages (FFTW3 target / library file / location prints), making
the next layer of debugging actually visible.
- The MKL install path stays unchanged (Intel oneAPI offline
installer + cache).
The Windows native build (currently `if: false`) still uses vcpkg
via CMakePresets.json. We can switch it to the source build later
or leave both paths as documented alternatives.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Previous run (24623558972) confirmed the Windows wheel goes end-to-end green with source-built FFTW3: ✓ Restore Intel oneAPI MKL cache (cache hit) ✓ Build static FFTW3 (Windows) (cache hit from previous iter) ✓ Build wheel ✓ Test wheel ✓ Upload artifact Re-enable the three jobs that were `if: false`'d during that debug iteration (build-linux, build-windows, build-wheel-linux) and restore `needs: build-windows` on the Windows wheel job so it runs sequentially after the native build (shares the MKL cache key, skips the slow oneAPI install on the second run). The vcpkg cache key is no longer shared — Windows wheel uses source-built FFTW3, native Windows still uses vcpkg via CMakePresets.json. Both pathways are now exercised by CI. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Replaces the original SAIL-Labs digHolo repo's "three copies of the source + committed prebuilt binaries + hand-written
g++recipes" build setup with a portable CMake build, cross-platform CI that publishes self-contained release artefacts, and apip install digholoPython wheel.End result, all green:
libdigholo.so.1+digHoloCLI — fully self-contained, glibc ≥ 2.28digholo.dll+digholo.lib+digHolo.exe— only needs MSVC redistdigholo-1.0.0-py3-none-manylinux_2_28_x86_64.whldigholo-1.0.0-py3-none-win_amd64.whlWheels are pure-
ctypes(nolibpythonlink), so a single binary serves every CPython 3.x and PyPy on the target OS.On any
v*tag push, all four artefacts auto-publish to a GitHub Release.What's in the box
Repo layout
src/digHolo.{cpp,h}— single canonical source (was duplicated 3×)python/src/digholo/— Python package with ctypes bindingspython/tests/test_smoke.py— pytest suitetests/test_smoke.cpp— C++ smoke test (driven by ctest)CMakeLists.txt+CMakePresets.json— cross-platform build, presets for Linux/Windowspyproject.toml— scikit-build-core + cibuildwheel configvcpkg.json— vcpkg manifest (Windows native build).github/workflows/ci.yml— the whole CI pipelineUPSTREAM_ISSUES.md— three real bugs we found in upstream code while doing this; PR-ready writeupsRemoved
digHolo.{cpp,h}at root and underExamples/C++/digHolo/(duplicates ofsrc/)bin/Win64/digHolo.dll,bin/linux/digHolo.exe,lib/Win64/digHolo.lib,lib/linux/libdigholo.so(now CI artefacts)Examples/C++/FFTW/{fftw3.h,libfftw3f-3.lib}(vendored 5.5MB of FFTW3).sln/.vcxprojfiles (CMake regenerates)Runtime dependencies of published artefacts
A post-build
ldd | grepcheck on Linux fails CI if libstdc++ or libfftw3f leak back into the .so — portable-build regressions caught at CI time.Bugs found in upstream digHolo (documented in UPSTREAM_ISSUES.md)
EXT_Cmacro has noextern "C"on Linux/macOS — every public function gets C++ name-mangled.ctypes.dlsym('digHoloCreate')fails with "undefined symbol". Invisible to C++ consumers because both sides agree on the mangled name.EXT_Cunconditionallydllexporton Windows — Windows consumers silently drop theirdigholo.dlldependency at link time. The exe builds without errors but dies at startup withSTATUS_DLL_NOT_FOUND. Three-state macro (DIGHOLO_STATIC_BUILD/DIGHOLO_BUILDING/ consumer) fixes it.digHoloDestroy()segfaults on a minimally-initialised handle —CreatethenDestroywithout running the fullSetBatch + ProcessBatchcycle crashes. Workaround in our test fixture; needs a null-check fix in the C library.Fork-specific cleanup deviations (duplicate sources, committed binaries, vendored FFTW3, hand-written build recipes, MATLAB header divergence) are also itemised in
UPSTREAM_ISSUES.mdfor if/when we open a discussion with Joel.CI design
pull_requestto main,pushto main,v*tags,workflow_dispatchneeds:Windows native so the MKL cache is warm before cibuildwheel runsTest plan
v1.0.0-rc1?) to exercise thereleasejob and confirm GitHub Release artefacts publish correctlyCaveats / things to bump deliberately
WINDOWS_BASEKIT_URLis pinned to oneAPI Base Toolkit 2025.3.0.372. Intel rotates the offline-installer hash every release; the canonical place to find a current URL is Intel's own CI workflow (searchWINDOWS_BASEKIT_URL).FFTW_VERSIONis pinned to 3.3.10. Bump deliberately; cache key incorporates the version.manylinux_2_28_x86_64:latestis unpinned — if you need fully reproducible Linux wheel builds, pin to a digest.🤖 Generated with Claude Code