From 9169bad6db853cadc7099ea44fe504307f2c6ba1 Mon Sep 17 00:00:00 2001 From: Revar Desmera Date: Sat, 25 Jul 2026 19:25:22 -0700 Subject: [PATCH 1/4] Add Windows to CI matrix; update parser submodule to its own Windows-CI merge Updates the openscad_cpp_parser submodule pointer to the just-merged PR #2 (BelfrySCAD/openscad_cpp_parser@35d9957), which added and fixed its own Windows CI (MSYS2-sourced bison/flex, /Zc:__cplusplus for Bison's move-only-variant codegen, a Windows-only test fixture bug). This repo's own CI matrix gains windows-latest, sourcing bison/flex from MSYS2 the same way. Two of this repo's own build files needed the identical MSVC fixes already applied in the submodule: - src/CMakeLists.txt: -Wall/-Wextra gated behind a non-MSVC generator expression (MSVC's cl.exe doesn't understand GCC-style warning flags). - CMakeLists.txt (root): /Zc:__cplusplus for MSVC, since this project's own targets (and its FetchContent deps -- nlohmann_json, Manifold, Boost.Polygon) are a separate directory scope from the submodule's own copy of the same fix and don't inherit it. fail-fast: false added to the matrix so a Windows-specific failure during this initial rollout doesn't cancel the already-established Linux/macOS jobs mid-run. Manifold/Boost.Polygon's own Windows buildability under MSVC is not yet verified in this project's specific FetchContent-pinned configuration -- expect further MSVC-specific fixes from the actual CI run, same iterative pattern used to land the submodule's own Windows CI. --- .github/workflows/ci.yml | 23 +++++++++++++++++++---- CMakeLists.txt | 11 +++++++++++ external/openscad_cpp_parser | 2 +- src/CMakeLists.txt | 4 +++- 4 files changed, 34 insertions(+), 6 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index af9bbf7..a27aaa5 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -8,8 +8,9 @@ on: jobs: test: strategy: + fail-fast: false matrix: - os: [ubuntu-latest, macos-latest] + os: [ubuntu-latest, macos-latest, windows-latest] runs-on: ${{ matrix.os }} steps: - uses: actions/checkout@v7 @@ -32,11 +33,25 @@ jobs: brew untap aws/tap || true brew install bison flex + - name: Install bison/flex (Windows) + if: runner.os == 'Windows' + # windows-latest ships MSYS2 pre-installed at C:\msys64 (not on + # PATH). Its bison (3.8.2+) satisfies the submodule grammar's + # %require "3.8" -- the Chocolatey winflexbison3 package is stuck + # on bison 3.7.4 and fails that check. The submodule's own + # CMakeLists.txt WIN32 block points BISON_EXECUTABLE/ + # FLEX_EXECUTABLE straight at the MSYS2 binaries. + run: C:\msys64\usr\bin\bash.exe -lc "pacman -Sy --noconfirm bison flex" + - name: Configure run: cmake -S . -B build -DCMAKE_BUILD_TYPE=Release + # --config/-C Release: a no-op on the single-config generators + # Linux/macOS use (CMAKE_BUILD_TYPE already picked the config), but + # required on Windows's default multi-config Visual Studio + # generator, which ignores CMAKE_BUILD_TYPE entirely. - name: Build - run: cmake --build build -j + run: cmake --build build --config Release -j # Bytecode VM on (the default as of the Phase 5 rollout decision -- # see evaluator.hpp's own bytecodeVmEnabled() comment) and off @@ -44,9 +59,9 @@ jobs: # the interpreted path fails CI, not just whichever happens to be # default at the time. - name: Test (bytecode VM on, the default) - run: ctest --test-dir build --output-on-failure + run: ctest --test-dir build --output-on-failure -C Release - name: Test (bytecode VM off) - run: ctest --test-dir build --output-on-failure + run: ctest --test-dir build --output-on-failure -C Release env: OSCAD_BYTECODE_VM: "0" diff --git a/CMakeLists.txt b/CMakeLists.txt index d8afced..c8712ad 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -5,6 +5,17 @@ set(CMAKE_CXX_STANDARD 20) set(CMAKE_CXX_STANDARD_REQUIRED ON) set(CMAKE_EXPORT_COMPILE_COMMANDS ON) +# ponytail: MSVC always reports __cplusplus as 199711L regardless of the +# real /std: level unless this flag is passed (a long-standing legacy- +# compatibility quirk) -- set it at the outer-project level too (the +# submodule's own CMakeLists.txt sets it for its own targets, but that +# add_compile_options call is directory-scoped and doesn't reach this +# project's own src/tools/tests targets). Several dependencies here +# (nlohmann_json, Manifold, Boost.Polygon) branch on __cplusplus. +if(MSVC) + add_compile_options(/Zc:__cplusplus) +endif() + add_subdirectory(external/openscad_cpp_parser) # openscad_cpp_parser's own src/CMakeLists.txt exposes its public headers via diff --git a/external/openscad_cpp_parser b/external/openscad_cpp_parser index 45589a2..35d9957 160000 --- a/external/openscad_cpp_parser +++ b/external/openscad_cpp_parser @@ -1 +1 @@ -Subproject commit 45589a2af7fb115398f8b467353505ee7a963f05 +Subproject commit 35d9957e8e7e06838f3d2b102058b56e4bab57e6 diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 6b2e7c2..6e74693 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -48,4 +48,6 @@ target_include_directories(openscad_cpp_evaluator PUBLIC target_include_directories(openscad_cpp_evaluator PRIVATE ${stb_SOURCE_DIR} ${CMAKE_BINARY_DIR}/generated) target_link_libraries(openscad_cpp_evaluator PUBLIC openscad_cpp_parser manifold Boost::polygon) -target_compile_options(openscad_cpp_evaluator PRIVATE -Wall -Wextra) +target_compile_options(openscad_cpp_evaluator PRIVATE + $<$>:-Wall;-Wextra> +) From 304018ddc3b5c94fa8189af25cf9611a82cce830 Mon Sep 17 00:00:00 2001 From: Revar Desmera Date: Sat, 25 Jul 2026 19:34:51 -0700 Subject: [PATCH 2/4] Fix Windows/MSVC compile errors surfaced by the first Windows CI run Two genuine bugs, both from this being the evaluator's first-ever MSVC build: - scope_trail.hpp used std::out_of_range without #include -- masked on Clang/GCC by transitive coverage through some other standard header, same class of gap as this session's earlier Linux-build header sweep. - M_PI (used in src/builtins/roof.cpp and src/import/svg_import.cpp) is a POSIX extension Unix libcs define unconditionally; MSVC only defines it when _USE_MATH_DEFINES is set before 's first include. Fixed as a single MSVC-only compile definition in the root CMakeLists.txt rather than patching each call site, so a future M_PI use anywhere in this project doesn't reintroduce the same gap. (The svg_import.cpp "const object must be initialized"/ "no matching std::max overload" errors in the same CI log were cascading fallout from the undeclared M_PI, not separate bugs -- confirmed by inspecting the actual flagged expressions.) Verified locally on macOS/Clang first: clean rebuild, full 1190-test suite passes unchanged. --- CMakeLists.txt | 6 ++++++ include/openscad_cpp_evaluator/scope_trail.hpp | 1 + 2 files changed, 7 insertions(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index c8712ad..c253ef3 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -14,6 +14,12 @@ set(CMAKE_EXPORT_COMPILE_COMMANDS ON) # (nlohmann_json, Manifold, Boost.Polygon) branch on __cplusplus. if(MSVC) add_compile_options(/Zc:__cplusplus) + # MSVC's only defines M_PI et al. when this is set before the + # first include -- a command-line define guarantees that regardless of + # each translation unit's own include order. Unix libcs define these + # unconditionally (a POSIX extension our code already relies on), so no + # equivalent is needed there. + add_compile_definitions(_USE_MATH_DEFINES) endif() add_subdirectory(external/openscad_cpp_parser) diff --git a/include/openscad_cpp_evaluator/scope_trail.hpp b/include/openscad_cpp_evaluator/scope_trail.hpp index 7329994..a0d4af6 100644 --- a/include/openscad_cpp_evaluator/scope_trail.hpp +++ b/include/openscad_cpp_evaluator/scope_trail.hpp @@ -1,6 +1,7 @@ #pragma once #include +#include #include #include #include From 3296b87478a89d1a48656d3dcbef2030c2d495ea Mon Sep 17 00:00:00 2001 From: Revar Desmera Date: Sat, 25 Jul 2026 19:47:27 -0700 Subject: [PATCH 3/4] Force Manifold to build static: fixes Windows DLL-not-found test failures Manifold's own CMakeLists.txt defaults BUILD_SHARED_LIBS ON, building manifold.dll/manifoldc.dll under build/_deps/manifold-build/ instead of static libs. Harmless on Linux/macOS -- CMake gives build-tree executables an automatic rpath to a build-tree .so/.dylib -- but Windows has no rpath equivalent, so every test/CLI executable failed to even launch (STATUS_DLL_NOT_FOUND, surfaced as CMake's gtest_discover_tests failing at build time with exit code 0xc0000135) since the DLLs live in a different directory than the .exe files and nothing copies them over or adds that directory to PATH. Forcing BUILD_SHARED_LIBS off before fetching Manifold (matching the existing MANIFOLD_TEST override right above it) is simpler and more robust than a Windows-only DLL-copy CI step, and fixes this for good across every executable this project builds (CLI, tests, examples), not just the one gtest_discover_tests happened to catch first. Verified locally on macOS/Clang: clean rebuild produces no .dylib/.so anywhere in the build tree (confirms the static-forcing actually took effect), full 1190-test suite passes unchanged. --- CMakeLists.txt | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index c253ef3..4b3c9a3 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -41,6 +41,17 @@ include(FetchContent) # shows single-threaded Boolean ops are a bottleneck. MANIFOLD_CROSS_SECTION # stays at its own default (ON) -- 2D CrossSection support is needed from # Phase 3 onward. +# +# BUILD_SHARED_LIBS off: Manifold's own CMakeLists.txt defaults this ON, +# which builds manifold.dll/manifoldc.dll instead of static libs. Harmless +# on Linux/macOS (CMake gives build-tree executables an automatic rpath to +# find a build-tree .so/.dylib), but Windows has no rpath equivalent -- a +# DLL must be either next to the .exe or on PATH, neither of which is true +# for a FetchContent dependency built into its own build/_deps/ tree, so +# every test/CLI executable failed to even start (STATUS_DLL_NOT_FOUND). +# Forcing this off, matching the existing MANIFOLD_TEST override pattern, +# fixes Windows and is a no-op everywhere else this project already worked. +set(BUILD_SHARED_LIBS OFF CACHE BOOL "" FORCE) set(MANIFOLD_TEST OFF CACHE BOOL "" FORCE) FetchContent_Declare( manifold From 2ce82a2d151ed1e75533ab1f8dd5aef1b40023b2 Mon Sep 17 00:00:00 2001 From: Revar Desmera Date: Sat, 25 Jul 2026 20:01:28 -0700 Subject: [PATCH 4/4] Fix two Windows-only test failures: hardcoded /tmp/, unclosed ifstream Two genuine test-portability bugs, both invisible until this repo's first Windows CI run: - test_export.cpp hardcoded "/tmp/..." paths (a Unix-only directory) for four fixtures, unlike every other test file here, which already uses std::filesystem::temp_directory_path() (test_cli.cpp, test_dxf_svg_import.cpp, test_surface.cpp, test_zip_stored.cpp, test_use.cpp, test_import_export.cpp). Brought it in line with that existing pattern rather than inventing a new one. - test_cli.cpp's CliDebugRepl.SetOverridesVariableOnResume opened an ifstream on the exported output file to verify its contents, then called std::filesystem::remove() on that same file later in the same scope while the ifstream was still open (its destructor doesn't run until the end of the TEST body). POSIX allows deleting an open file (unlink just removes the directory entry); Windows enforces an exclusive lock and the remove() throws. Fixed with an explicit in.close() right after the last read. Verified locally on macOS/Clang: full rebuild, full 1190-test suite passes unchanged. --- tests/test_cli.cpp | 1 + tests/test_export.cpp | 13 +++++++++---- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/tests/test_cli.cpp b/tests/test_cli.cpp index 575e515..1b40cc7 100644 --- a/tests/test_cli.cpp +++ b/tests/test_cli.cpp @@ -238,6 +238,7 @@ TEST(CliDebugRepl, SetOverridesVariableOnResume) { double v; while (ls >> v) maxCoord = std::max(maxCoord, std::fabs(v)); } + in.close(); // Windows can't remove() a file while it's still open. EXPECT_DOUBLE_EQ(maxCoord, 2.0); // would be 10.0 without the override std::filesystem::remove(src); std::filesystem::remove(out); diff --git a/tests/test_export.cpp b/tests/test_export.cpp index 80565d9..b3d28f5 100644 --- a/tests/test_export.cpp +++ b/tests/test_export.cpp @@ -7,6 +7,7 @@ #include #include #include +#include #include #include @@ -15,6 +16,10 @@ using namespace oscadeval::test; namespace { +std::filesystem::path tempPath(const std::string& name) { + return std::filesystem::temp_directory_path() / ("oscad_eval_test_" + name); +} + std::vector evalToBodies(const std::string& code) { auto ast = parseSrc(code); auto scope = oscad::buildScopes(ast); @@ -55,7 +60,7 @@ StlSummary readStl(const std::string& path) { TEST(ExportStl, WritesCorrectTriangleCountAndHeader) { std::vector bodies = evalToBodies("cube(2);"); - const std::string path = "/tmp/oscad_eval_test_cube.stl"; + const std::string path = tempPath("cube.stl").string(); writeStl(path, bodies); StlSummary s = readStl(path); @@ -67,7 +72,7 @@ TEST(ExportStl, WritesCorrectTriangleCountAndHeader) { TEST(ExportStl, TranslatedCubeBoundsMatch) { std::vector bodies = evalToBodies("translate([1,0,0]) cube(2);"); - const std::string path = "/tmp/oscad_eval_test_translate.stl"; + const std::string path = tempPath("translate.stl").string(); writeStl(path, bodies); StlSummary s = readStl(path); @@ -78,7 +83,7 @@ TEST(ExportStl, TranslatedCubeBoundsMatch) { TEST(ExportStl, NoGeometryThrows) { std::vector empty; - EXPECT_THROW(writeStl("/tmp/oscad_eval_test_empty.stl", empty), std::runtime_error); + EXPECT_THROW(writeStl(tempPath("empty.stl").string(), empty), std::runtime_error); } // -- toRenderableBodies (Phase 6) ------------------------------------------ @@ -101,7 +106,7 @@ TEST(ToRenderableBodies, ThinExtrudesABareTopLevel2dShapeSoItCanExportAsStl) { manifold::Box bbox = renderable[0].body->BoundingBox(); EXPECT_NEAR(bbox.max.z - bbox.min.z, 1e-3, 1e-9); - const std::string path = "/tmp/oscad_eval_test_flat_preview.stl"; + const std::string path = tempPath("flat_preview.stl").string(); writeStl(path, renderable); StlSummary s = readStl(path); EXPECT_GT(s.triangleCount, 0u);