Windows/MSVC support: CI harness + portability fixes (Phase 0 + 1a) - #5
Merged
Conversation
First step toward native Windows/MSVC support (neml2 issue #414). The
critical path is nmhit: it fails to build with MSVC today, which blocks
every neml2 route on Windows (including pure Python, since the factory
parses HIT via the nmhit bindings).
Phase 0 -- Windows dev harness:
- Add a non-blocking `windows-latest` cell to the `python` and `cpp` CI
jobs (continue-on-error while we iterate the build to green; the
required Linux/macOS cells still gate merges). The generated
parser/lexer MSVC errors can only be surfaced by compiling on Windows,
so this job IS the iteration loop for the remaining fixes.
- `cpp` job runs under bash on every OS and passes an explicit config so
the multi-config Visual Studio generator (CMake's Windows default)
builds/tests correctly.
Phase 1a -- free/known MSVC fixes:
- Guard the `-Wall -Wextra -Wno-unused-parameter` warning flags behind
the GCC/Clang compiler id; give MSVC `/W3`. Passing the GCC/Clang
flags to cl.exe was the originally reported blocker.
- abi_compat.cpp: move `#include <cxxabi.h>` (nonexistent on MSVC) inside
the existing `__GLIBCXX__` guard, so the whole shim compiles to nothing
on MSVC/libc++.
- Define YY_NO_UNISTD_H target-wide on Windows so the flex-generated
Lexer.h does not pull in the POSIX <unistd.h>; define
_CRT_SECURE_NO_WARNINGS to silence C4996 on std::getenv.
Remaining generated-code MSVC errors (C2664/C2208/C2062/C2513 in
Lexer.cpp/Parser.cpp) are Phase 1b -- iterated on the Windows CI cell.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Surfaced by the Windows CI cells. The Flex/Bison-generated parser/lexer
compiled cleanly under MSVC; all failures were in hand-written code and
the build config:
- Node.h/Node.cpp: rename the `_inline` member to `_is_inline`. `_inline`
is a reserved keyword in the MSVC dialect (an alias for `inline`), so a
member of that name parsed as `inline` and produced the reported
C2062/C2513/C2208/C2059/C2222/C3536 cascade.
- Node.cpp wrap_in_sections(): take the filename as std::filesystem::path
instead of const std::string&. Callers hold a path (_fname) and forward
it straight to _set_location (which also takes a path); path->string is
implicit only where path::string_type is std::string (POSIX), but is
std::wstring on Windows, so the string parameter failed to convert
(C2664). path end-to-end removes both conversions.
- CMakeLists: make the parser-regeneration gate multi-config-aware. The
old `NOT CMAKE_BUILD_TYPE` branch pulled in FLEX/BISON on the Windows
wheel build, because scikit-build-core drives the multi-config Visual
Studio generator where CMAKE_BUILD_TYPE is empty by design. Guard on
GENERATOR_IS_MULTI_CONFIG so multi-config generators always use the
committed generated/ sources (no flex/bison), same as Release.
Linux Release + Debug builds and tests unaffected (verified locally).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…(Phase 1b, round 2) The Windows cells reduced to a single error: Parser.h(260) referenced the deleted copy constructor of the move-only semantic value std::unique_ptr<nmhit::Node> (C2280). Root cause: MSVC reports __cplusplus == 199711L even under /std:c++17 unless /Zc:__cplusplus is passed, so the Bison-generated header took its pre-C++11 branch, which copies the semantic value instead of moving it. Add /Zc:__cplusplus to the MSVC compile options so __cplusplus reports the true standard and Bison selects the C++11 (move) path. Compiler-scoped genexpr, so GCC/Clang builds are unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… test env (Phase 1b, round 3)
The nmhit library now compiles under MSVC (round 2). Remaining failures:
- _nmhit binding (C2280, deleted unique_ptr copy): the binding forced
Node's copy/move traits to false via an ILLEGAL std::is_move_constructible
specialization (C5285), which newer MSVC ignores -- so nanobind
instantiated Node's copy ctor and tried to copy the move-only
vector<unique_ptr<Node>> _children. Fix at the root: delete Node's copy
constructor and copy assignment (they were a vestigial protected
`= default` that nothing uses -- every clone() rebuilds from fields).
With the copy ops deleted, std::is_copy_constructible /
is_move_constructible report false on every compiler, so both the
nanobind::detail::is_copy_constructible AND the illegal
std::is_move_constructible specializations are removed. Verified the
GCC miscomputation that motivated them is gone (Linux build + 27 pytest
+ C++ tests all pass without the workarounds).
- test_hit.cpp used POSIX setenv(): add a _putenv_s/setenv portable shim.
- Move /Zc:__cplusplus from the nmhit target to a project-wide MSVC flag
so the library, the Python module, and the tests all report the true
__cplusplus.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
hugary1995
added a commit
that referenced
this pull request
Jul 9, 2026
Now that nmhit builds and tests clean on Windows/MSVC (PR #5), make it a first-class published platform: - release.yml: add windows-latest to the wheel matrix (CIBW_ARCHS_WINDOWS = AMD64), so cp39-cp314 Windows wheels are built and published to PyPI alongside Linux/macOS. - ci.yml: drop continue-on-error from the Windows python/cpp cells -- they now gate merges like every other cell. The `package` job also builds + tests a Windows wheel, pinned to the same cibuildwheel as release.yml so the packaging check exercises the exact published-wheel build path. - pyproject.toml: add [tool.cibuildwheel.windows] archs = AMD64. - Bump 0.3.4 -> 0.3.5 (pyproject + CMakeLists) for the release. - README: correct the published-platform list (Linux x86_64 -- the workflow never built aarch64 -- macOS x86_64/arm64, and now Windows AMD64). Co-Authored-By: Claude Opus 4.8 (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.
Goal
First step toward native Windows/MSVC support, driven by neml2 issue #414. nmhit is the critical path — it fails to build with MSVC today, and that blocks every NEML2 route on Windows, including pure Python (the factory parses HIT via the nmhit bindings).
Strategy
This PR (Phase 0 + 1a)
Phase 0 — Windows dev harness
windows-latestcell added to thepythonandcppjobs (continue-on-errorwhile iterating; the required Linux/macOS cells still gate merges). The generated parser/lexer MSVC errors can only be surfaced by compiling on Windows, so this cell is the iteration loop for Phase 1b.cppjob now runs under bash on every OS and passes an explicit config (Windows defaults to the multi-config Visual Studio generator).Phase 1a — free/known MSVC fixes
-Wall -Wextra -Wno-unused-parameterbehind the GCC/Clang compiler id; MSVC gets/W3. (Passing the GCC/Clang flags tocl.exewas the originally reported blocker.)abi_compat.cpp: move#include <cxxabi.h>(absent on MSVC) inside the existing__GLIBCXX__guard → the shim compiles to nothing on MSVC/libc++.YY_NO_UNISTD_Htarget-wide on Windows (flex-generatedLexer.hotherwise pulls in POSIX<unistd.h>); define_CRT_SECURE_NO_WARNINGS(C4996 onstd::getenv).Next (not in this PR)
Phase 1b — clear the residual generated-code MSVC errors (
C2664/C2208/C2062/C2513inLexer.cpp/Parser.cpp), iterated on the Windows CI cell; likely a regen with a newer MSVC-clean Flex/Bison or targeted shims.Phase 2 — Windows wheels (
release.yml+ cibuildwheelAMD64), then flip the Windows CI cells to required.Linux/macOS build + tests verified locally.
🤖 Generated with Claude Code