Skip to content

Conversation

@zasexton
Copy link
Contributor

@zasexton zasexton commented Apr 24, 2025

Current situation

The solver has first-class CI/CD for Linux and macOS only. On Windows the build breaks because

  • C++ sources rely on <dlfcn.h> and POSIX path semantics.
  • CMakeLists lack WIN32 logic, so libraries aren’t produced with the correct suffixes.
  • GitHub Actions matrix does not include Windows runners.

Issue #171 tracks these gaps and the discussion around adding Windows wheels/MSI installers.

This PR introduces 14 commits that

  • wrap the POSIX dl* API with a Windows shim in LPNSolverInterface.h
  • add & inclusion plus minor clang-format fixes
  • extend CMake with if (WIN32) branches (MinGW toolchain & -Wl flags)
  • create a custom build_ext subclass to copy the compiled .pyd/.dll into the Python wheel
  • augment the CI pipeline (tests.yml) with a full Windows job (build → pytest → interface binaries → CPack MSI)

Release Notes

  • New: Official Windows (≥ Win10) build, test and packaging workflow on GitHub Actions.

  • New: Auto-generated Windows installer (.msi) and Python wheel containing pysvzerod.pyd.

  • New: Portable dynamic-loading layer replaces direct <dlfcn.h> usage; no code changes needed in client projects.

  • Change: setup.py now uses CustomCMakeBuild to place binaries correctly; downstream pip install . behaviour is unchanged.

  • Migration: No breaking API changes. maintained custom CMake flags.

  • Documentation

  • TODO: Add a “Building on Windows” subsection to docs/developer_guide.md with MinGW & MSVC instructions.

  • TODO: Update README install table to list the Windows wheel and MSI artifacts.

Testing

  • CI matrix now covers ubuntu-22.04, macos-latest, windows-latest; all stages (build, unit tests, interface tests, coverage) must pass.
  • Two new interface tests run on Windows against the generated .dll.
  • Coverage upload remains Linux-only to keep run-time short.

Code of Conduct & Contributing Guidelines

@zasexton zasexton requested review from emilinmathew, ktbolt and mrp089 and removed request for emilinmathew April 24, 2025 19:19
@zasexton zasexton linked an issue Apr 24, 2025 that may be closed by this pull request
1 task
@zasexton zasexton added the enhancement New feature or request label Apr 24, 2025
@zasexton zasexton self-assigned this Apr 24, 2025
@zasexton zasexton marked this pull request as ready for review April 24, 2025 20:52
@zasexton
Copy link
Contributor Author

Right now the installer generated for windows is actually the default exe file from cpack and not an msi.

Copy link
Contributor

@ktbolt ktbolt left a comment

Choose a reason for hiding this comment

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

@zasexton I assume all of these changes compile on the latest Ubuntu and MacOS ?

@zasexton
Copy link
Contributor Author

@zasexton I assume all of these changes compile on the latest Ubuntu and MacOS ?

yes, I've added the windows build and installer generation to the same test.yml GitHub Action workflow and we are able to generate installers across all platforms in the matrix.os strategy and pass all tests as well as all interface tests for all platforms.

@zasexton
Copy link
Contributor Author

This is currently the most involved change to the source code where I am adding a Windows shim to provide a workaround for the interface testing:

#if defined(_WIN32) || defined(_WIN64)
/* ---------- Windows implementation ---------- */
#include <windows.h>
#ifdef interface
#undef interface
#endif
using dl_handle_t = HMODULE;
// Define windows flags to emulate <dlfcn.h>
#ifndef RTLD_LAZY
#define RTLD_LAZY 0
#define RTLD_NOW 0
#define RTLD_GLOBAL 0
#define RTLD_LOCAL 0
#endif
inline dl_handle_t dlopen(const char* file, int /*flags*/)
{
/* LoadLibraryA allows UTF-8 compatible narrow strings under MSVC ≥ 2015 */
return ::LoadLibraryA(file);
}
inline void* dlsym(dl_handle_t handle, const char* symbol)
{
return reinterpret_cast<void*>(::GetProcAddress(handle, symbol));
}
inline int dlclose(dl_handle_t handle)
{
return ::FreeLibrary(handle) ? 0 : 1; // 0 = success, POSIX-style
}
/* Store the last error message in a local static buffer and return a C-string
* (roughly mimicking the POSIX API).
*/
inline const char* dlerror()
{
static char buf[256];
DWORD code = ::GetLastError();
if (code == 0) return nullptr;
::FormatMessageA(FORMAT_MESSAGE_FROM_SYSTEM |
FORMAT_MESSAGE_IGNORE_INSERTS,
nullptr,
code,
MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT),
buf, sizeof(buf), nullptr);
return buf;
}
#else
/* ---------- POSIX / Unix-like ---------- */
#include <dlfcn.h>
using dl_handle_t = void*;
#endif

I think the solution ends up being good enough to include here since we can avoid pulling in another library for windows dynamic loading and can instead rely on native <windows.h> dynamic loading libraries.

Copy link
Contributor

@ktbolt ktbolt left a comment

Choose a reason for hiding this comment

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

A good enough solution for now.

@ktbolt ktbolt merged commit 5395527 into SimVascular:master Apr 25, 2025
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Windows Builds & Installers

3 participants