diff --git a/.github/workflows/tests-on-pr.yml b/.github/workflows/tests-on-pr.yml index 7ff7499..01c19e3 100644 --- a/.github/workflows/tests-on-pr.yml +++ b/.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 diff --git a/CMakeLists.txt b/CMakeLists.txt index 1573731..b632fdc 100644 --- a/CMakeLists.txt +++ b/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") diff --git a/cmake/LibdiffpyWindows.cmake b/cmake/LibdiffpyWindows.cmake new file mode 100644 index 0000000..ab1a5b9 --- /dev/null +++ b/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) diff --git a/cmake/windows-runtimepath.cpp.in b/cmake/windows-runtimepath.cpp.in new file mode 100644 index 0000000..c1818b1 --- /dev/null +++ b/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 +#include +#include +#include +#include +#include +#include +#include + +#include + +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(&moduleDirectory), &module)) + { + throw system_error(GetLastError(), system_category(), + "Cannot locate the libdiffpy module"); + } + vector filename(256); + for (;;) + { + DWORD count = GetModuleFileNameW(module, filename.data(), + static_cast(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 diff --git a/news/windows-embedded-core.rst b/news/windows-embedded-core.rst new file mode 100644 index 0000000..c7a1e7a --- /dev/null +++ b/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. diff --git a/src/extensions/srreal_pickling.hpp b/src/extensions/srreal_pickling.hpp index 0a79abf..61bb3a2 100644 --- a/src/extensions/srreal_pickling.hpp +++ b/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(state[1]); diff --git a/src/extensions/wrap_OverlapCalculator.cpp b/src/extensions/wrap_OverlapCalculator.cpp index f1ac6b1..ee01f0f 100644 --- a/src/extensions/wrap_OverlapCalculator.cpp +++ b/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(obj); diff --git a/src/extensions/wrap_PDFCalculators.cpp b/src/extensions/wrap_PDFCalculators.cpp index 0190859..9235c71 100644 --- a/src/extensions/wrap_PDFCalculators.cpp +++ b/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(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(obj); diff --git a/tests/test_runtime_data.py b/tests/test_runtime_data.py new file mode 100644 index 0000000..357a83b --- /dev/null +++ b/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