Fix Windows builds of the embedded libdiffpy core

.github/workflows/tests-on-pr.yml

@@ -19,3 +19,34 @@ jobs:
         fi
     secrets:
       CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}
+
+  windows-tests:
+    runs-on: windows-latest
+    strategy:
+      fail-fast: false
+      matrix:
+        python-version: ["3.12", "3.13", "3.14"]
+    defaults:
+      run:
+        shell: bash -l {0}
+    steps:
+      - uses: actions/checkout@v6
+        with:
+          fetch-depth: 0
+      - uses: conda-incubator/setup-miniconda@v3
+        with:
+          activate-environment: test
+          channels: conda-forge
+          auto-activate-base: false
+          python-version: ${{ matrix.python-version }}
+      - name: Install build and test requirements
+        run: |
+          conda install --yes --file requirements/conda.txt
+          conda install --yes --file requirements/tests.txt
+          conda install --yes --file requirements/build.txt
+      - name: Build and install the wheel
+        run: python -m pip install . --no-deps
+      - name: Import the compiled extension
+        run: python -c "import diffpy.srreal.srreal_ext"
+      - name: Test the installed package
+        run: python -m pytest

CMakeLists.txt

@@ -36,6 +36,11 @@ file(GLOB LIBDIFFPY_SOURCES CONFIGURE_DEPENDS
 # ObjCryst conversion uses the public Python API, so it does not depend on
 # pyobjcryst's private static library, compiler, REAL precision, or nanobind ABI.
 list(FILTER LIBDIFFPY_SOURCES EXCLUDE REGEX "ObjCryst")
+if(WIN32)
+    include(cmake/LibdiffpyWindows.cmake)
+    list(FILTER LIBDIFFPY_SOURCES EXCLUDE REGEX "/runtimepath\\.cpp$")
+    list(APPEND LIBDIFFPY_SOURCES "${LIBDIFFPY_WINDOWS_RUNTIME}")
+endif()
 # Include every object: libdiffpy has static factory/serialization registrars
 # that an ordinary static archive link can otherwise discard.
 add_library(diffpy_core OBJECT ${LIBDIFFPY_SOURCES})
@@ -49,6 +54,10 @@ target_include_directories(diffpy_core PUBLIC
     "${LIBDIFFPY_SOURCE_DIR}/src"
 )
 target_compile_definitions(diffpy_core PRIVATE DIFFPYRUNTIMERELPATH=data)
+if(WIN32)
+    # The extension also includes libdiffpy headers and uses M_PI.
+    target_compile_definitions(diffpy_core PUBLIC _USE_MATH_DEFINES NOMINMAX)
+endif()
 target_link_libraries(diffpy_core PUBLIC Boost::serialization GSL::gsl GSL::gslcblas ${CMAKE_DL_LIBS})
 
 file(GLOB SRREAL_SOURCES CONFIGURE_DEPENDS "src/extensions/*.cpp")

cmake/LibdiffpyWindows.cmake

@@ -0,0 +1,44 @@
+# Backport Windows support for the pinned libdiffpy revision. Generate the
+# adapted files in the build tree so local source overrides remain untouched.
+# Remove these adaptations when the upstream revision includes the fixes.
+set(_diffpy_src "${LIBDIFFPY_SOURCE_DIR}/src/diffpy")
+set(_diffpy_inc "${CMAKE_CURRENT_BINARY_DIR}/include/diffpy")
+
+function(_diffpy_replace_file source destination before after)
+    set_property(DIRECTORY APPEND PROPERTY CMAKE_CONFIGURE_DEPENDS "${source}")
+    file(READ "${source}" contents)
+    string(FIND "${contents}" "${before}" position)
+    if(position EQUAL -1)
+        message(FATAL_ERROR "Recheck the libdiffpy Windows adaptation for ${source}.")
+    endif()
+    string(REPLACE "${before}" "${after}" contents "${contents}")
+    file(CONFIGURE OUTPUT "${destination}" CONTENT "${contents}" @ONLY)
+endfunction()
+
+# Modern MSVC supplies round, remainder and log2 in its standard library.
+_diffpy_replace_file("${_diffpy_src}/mathutils.ipp"
+    "${_diffpy_inc}/mathutils.ipp"
+    "#ifdef _MSC_VER"
+    "#if defined(_MSC_VER) && _MSC_VER < 1800")
+_diffpy_replace_file("${_diffpy_src}/srreal/StructureAdapter.hpp"
+    "${_diffpy_inc}/srreal/StructureAdapter.hpp"
+    "        virtual void customPQConfig(PairQuantity* pq) const\n            __attribute__ ((deprecated))"
+    "        [[deprecated]] virtual void customPQConfig(PairQuantity* pq) const")
+
+# Keep libdiffpy's LineReader implementation; replace only the POSIX path
+# discovery with native Windows module discovery and standard filesystem APIs.
+file(READ "${_diffpy_src}/runtimepath.cpp" _runtime_source)
+set_property(DIRECTORY APPEND PROPERTY CMAKE_CONFIGURE_DEPENDS
+    "${_diffpy_src}/runtimepath.cpp")
+set(_runtime_marker "// Implementation ------------------------------------------------------------")
+string(FIND "${_runtime_source}" "${_runtime_marker}" _runtime_start)
+if(_runtime_start EQUAL -1)
+    message(FATAL_ERROR "Recheck the libdiffpy Windows runtimepath adaptation.")
+endif()
+string(SUBSTRING "${_runtime_source}" ${_runtime_start} -1 _runtime_body)
+file(READ "${CMAKE_CURRENT_LIST_DIR}/windows-runtimepath.cpp.in" _runtime_windows)
+set_property(DIRECTORY APPEND PROPERTY CMAKE_CONFIGURE_DEPENDS
+    "${CMAKE_CURRENT_LIST_DIR}/windows-runtimepath.cpp.in")
+set(LIBDIFFPY_WINDOWS_RUNTIME "${CMAKE_CURRENT_BINARY_DIR}/runtimepath-windows.cpp")
+file(CONFIGURE OUTPUT "${LIBDIFFPY_WINDOWS_RUNTIME}"
+    CONTENT "${_runtime_windows}\n${_runtime_body}" @ONLY)

cmake/windows-runtimepath.cpp.in

@@ -0,0 +1,72 @@
+// Windows path discovery for the embedded libdiffpy core. LineReader below
+// this section is copied unchanged from the pinned libdiffpy source.
+#include <cassert>
+#include <cstdlib>
+#include <filesystem>
+#include <sstream>
+#include <stdexcept>
+#include <system_error>
+#include <vector>
+#include <windows.h>
+
+#include <diffpy/runtimepath.hpp>
+
+using namespace std;
+
+namespace {
+
+namespace fs = std::filesystem;
+
+fs::path moduleDirectory()
+{
+    HMODULE module = nullptr;
+    if (!GetModuleHandleExW(GET_MODULE_HANDLE_EX_FLAG_FROM_ADDRESS |
+                GET_MODULE_HANDLE_EX_FLAG_UNCHANGED_REFCOUNT,
+                reinterpret_cast<LPCWSTR>(&moduleDirectory), &module))
+    {
+        throw system_error(GetLastError(), system_category(),
+                "Cannot locate the libdiffpy module");
+    }
+    vector<wchar_t> filename(256);
+    for (;;)
+    {
+        DWORD count = GetModuleFileNameW(module, filename.data(),
+                static_cast<DWORD>(filename.size()));
+        if (!count)
+        {
+            throw system_error(GetLastError(), system_category(),
+                    "Cannot read the libdiffpy module path");
+        }
+        if (count < filename.size())
+        {
+            return fs::path(wstring(filename.data(), count)).parent_path();
+        }
+        filename.resize(filename.size() * 2);
+    }
+}
+
+const string& diffpyruntime()
+{
+    // Resolve relative to the extension module, never the working directory.
+    static const fs::path module_directory = moduleDirectory();
+    thread_local string result;
+    const char* override_path = getenv("DIFFPYRUNTIME");
+    fs::path directory;
+    if (override_path && *override_path)
+    {
+        directory = override_path;
+    }
+    else
+    {
+        directory = module_directory / "data";
+    }
+    if (!fs::is_directory(directory))
+    {
+        throw runtime_error("Directory '" + directory.string() +
+                "' does not exist.");
+    }
+    result = fs::canonical(directory).string();
+    return result;
+}
+
+}   // namespace

news/windows-embedded-core.rst

@@ -0,0 +1,7 @@
+**Fixed:**
+
+* Restore Windows builds of the embedded libdiffpy core with modern MSVC.
+  Enable mathematical constants, replace compiler-specific declarations,
+  disambiguate tuple restoration in calculator pickles, and locate packaged
+  runtime tables relative to the extension module while honoring
+  ``DIFFPYRUNTIME``. Add Windows CI for Python 3.12, 3.13, and 3.14.

src/extensions/srreal_pickling.hpp

@@ -455,7 +455,8 @@ class PairQuantityPickleSuite :
             using namespace diffpy::srreal;
             ensure_tuple_length(state, 2);
             // restore the state using boost serialization
-            nb::tuple state0(state[0]);
+            // Resolve the accessor explicitly to avoid MSVC overload ambiguity.
+            nb::tuple state0(nb::handle(state[0].ptr()));
             Super::setstate(obj, state0);
             // restore the structure object
             StructureAdapterPtr pstru = nb::cast<StructureAdapterPtr>(state[1]);

src/extensions/wrap_OverlapCalculator.cpp

@@ -229,7 +229,7 @@ class OverlapCalculatorPickleSuite :
         {
             ensure_tuple_length(state, 2);
             // restore the state using boost serialization
-            nb::tuple state0(state[0]);
+            nb::tuple state0(nb::handle(state[0].ptr()));
             Super::setstate(obj, state0);
             // restore component that was kept out of Boost serialization
             OverlapCalculator& calc = nb::cast<OverlapCalculator&>(obj);

src/extensions/wrap_PDFCalculators.cpp

@@ -492,7 +492,7 @@ class DebyePDFCalculatorPickleSuite :
         {
             ensure_tuple_length(state, 4);
             // restore the state using boost serialization
-            nb::tuple st0(state[0]);
+            nb::tuple st0(nb::handle(state[0].ptr()));
             Super::setstate(obj, st0);
             // restore components that were kept out of Boost serialization
             DebyePDFCalculator& calc = nb::cast<DebyePDFCalculator&>(obj);
@@ -571,7 +571,7 @@ class PDFCalculatorPickleSuite :
         {
             ensure_tuple_length(state, 6);
             // restore the state using boost serialization
-            nb::tuple st0(state[0]);
+            nb::tuple st0(nb::handle(state[0].ptr()));
             Super::setstate(obj, st0);
             // restore components that were kept out of Boost serialization
             PDFCalculator& calc = nb::cast<PDFCalculator&>(obj);

tests/test_runtime_data.py

@@ -0,0 +1,111 @@
+"""Exercise runtime table discovery in a fresh process and working
+directory."""
+
+import os
+import shutil
+import subprocess
+import sys
+from pathlib import Path
+
+import pytest
+
+from diffpy.srreal import srreal_ext
+
+
+@pytest.fixture(
+    params=[
+        (
+            "f0_WaasKirf.dat",
+            "from diffpy.srreal.scatteringfactortable import SFTXray; "
+            "assert 27 < SFTXray().lookup('Ni', 0) < 29",
+        ),
+        (
+            "nsftable.dat",
+            "from diffpy.srreal.scatteringfactortable import SFTNeutron; "
+            "assert 10 < SFTNeutron().lookup('Ni', 0) < 11",
+        ),
+        (
+            "bvparm2011sel.cif",
+            "from diffpy.srreal.srreal_ext import BVParametersTable; "
+            "p = BVParametersTable().lookup('Ti', 4, 'O', -2); "
+            "assert abs(p.Ro - 1.815) < 1e-12; "
+            "assert abs(p.B - 0.37) < 1e-12",
+        ),
+    ],
+    ids=["xray", "neutron", "bond-valence"],
+)
+def lookup(request):
+    return request.param
+
+
+@pytest.fixture
+def custom_tables(tmp_path):
+    source = Path(srreal_ext.__file__).parent / "data"
+    return shutil.copytree(source, tmp_path / "custom tables")
+
+
+def run_lookup(directory, code, override=None):
+    env = os.environ.copy()
+    env.pop("DIFFPYRUNTIME", None)
+    if override is not None:
+        env["DIFFPYRUNTIME"] = str(override)
+    return subprocess.run(
+        [sys.executable, "-c", code],
+        cwd=directory,
+        env=env,
+        capture_output=True,
+        text=True,
+        timeout=60,
+    )
+
+
+@pytest.mark.parametrize("override", [None, ""])
+def test_packaged_tables_from_unrelated_working_directory(
+    tmp_path, lookup, override
+):
+    _, code = lookup
+    result = run_lookup(tmp_path, code, override)
+    assert result.returncode == 0, result.stdout + result.stderr
+
+
+def test_runtime_override_with_spaces(tmp_path, lookup, custom_tables):
+    _, code = lookup
+    result = run_lookup(tmp_path, code, custom_tables)
+    assert result.returncode == 0, result.stdout + result.stderr
+
+
+@pytest.mark.parametrize("exists", [False, True])
+def test_runtime_override_does_not_fall_back_to_packaged_tables(
+    tmp_path, lookup, exists
+):
+    _, code = lookup
+    override = tmp_path / "custom tables"
+    if exists:
+        override.mkdir()
+    result = run_lookup(tmp_path, code, override)
+    assert result.returncode != 0
+    assert "RuntimeError" in result.stderr
+    assert "custom tables" in result.stderr
+
+
+def test_missing_runtime_table(tmp_path, lookup, custom_tables):
+    filename, code = lookup
+    (custom_tables / filename).unlink()
+    result = run_lookup(tmp_path, code, custom_tables)
+    assert result.returncode != 0
+    assert "RuntimeError" in result.stderr
+    assert filename in result.stderr
+
+
+def test_custom_table_contents_take_precedence(tmp_path, custom_tables):
+    table = custom_tables / "nsftable.dat"
+    contents = table.read_text()
+    assert "28-Ni,,,10.3(1)," in contents
+    table.write_text(contents.replace("28-Ni,,,10.3(1),", "28-Ni,,,12.345,"))
+    result = run_lookup(
+        tmp_path,
+        "from diffpy.srreal.scatteringfactortable import SFTNeutron; "
+        "assert abs(SFTNeutron().lookup('Ni', 0) - 12.345) < 1e-12",
+        custom_tables,
+    )
+    assert result.returncode == 0, result.stdout + result.stderr