From 31d0ebfa51aa2401664593c3b015c5550bbcfdab Mon Sep 17 00:00:00 2001 From: Gabriel Cruz <8129788+gmelodie@users.noreply.github.com> Date: Fri, 22 May 2026 11:43:37 -0300 Subject: [PATCH] chore(ci): extend cpp-e2e to OS matrix (#38) --- .github/workflows/ci.yml | 36 +++++++++++++--- examples/timer/cpp_bindings/CMakeLists.txt | 43 +++++++++++++++++++- ffi.nimble | 28 +++++++++---- ffi/codegen/templates/cpp/CMakeLists.txt.tpl | 43 +++++++++++++++++++- tests/e2e/cpp/CMakeLists.txt | 22 +++++++--- tests/e2e/cpp/test_timer_e2e.cpp | 4 +- 6 files changed, 150 insertions(+), 26 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d36234d..3aa6db2 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -69,15 +69,25 @@ jobs: nimble-version: ${{ needs.versions.outputs.nimble }} cpp-e2e: - name: C++ E2E + # Codegen output doesn't vary with mm, so we matrix over OS and Nim only. + # Windows runs MSVC by default and may surface codegen tweaks needed in + # the generated CMake (e.g. /EHsc, dllexport) — track follow-ups as bugs + # if they appear. `fail-fast: false` keeps Linux/macOS results visible. needs: versions - # Codegen output doesn't vary with mm and CMake/FetchContent is most - # reliable on Linux, so we don't matrix over OS or mm here — just Nim. strategy: fail-fast: false matrix: + os: [ubuntu-22.04, macos-15, windows-latest] nim-version: ${{ fromJSON(needs.versions.outputs.nim-versions) }} - runs-on: ubuntu-22.04 + include: + - os: ubuntu-22.04 + label: Linux + - os: macos-15 + label: macOS + - os: windows-latest + label: Windows + runs-on: ${{ matrix.os }} + name: C++ E2E · ${{ matrix.label }} · Nim ${{ matrix.nim-version }} env: NIMBLE_VERSION: ${{ needs.versions.outputs.nimble }} steps: @@ -90,7 +100,11 @@ jobs: repo-token: ${{ secrets.GITHUB_TOKEN }} - name: Install Nimble ${{ env.NIMBLE_VERSION }} + shell: bash run: | + if [ "$RUNNER_OS" == "Windows" ]; then + export PATH="$GITHUB_WORKSPACE/.nim_runtime/bin:$PATH" + fi cd /tmp && nimble install "nimble@${{ env.NIMBLE_VERSION }}" -y echo "$HOME/.nimble/bin" >> $GITHUB_PATH @@ -108,7 +122,12 @@ jobs: - name: Install nimble deps if: steps.cache-nimbledeps.outputs.cache-hit != 'true' - run: nimble setup --localdeps -y + shell: bash + run: | + if [ "$RUNNER_OS" == "Windows" ]; then + export PATH="$GITHUB_WORKSPACE/.nim_runtime/bin:$HOME/.nimble/bin:$PATH" + fi + nimble setup --localdeps -y - name: Cache CMake FetchContent (GoogleTest) uses: actions/cache@v4 @@ -117,7 +136,12 @@ jobs: key: ${{ runner.os }}-cpp-e2e-deps-${{ hashFiles('tests/e2e/cpp/CMakeLists.txt') }} - name: Run C++ e2e tests - run: nimble test_cpp_e2e -y + shell: bash + run: | + if [ "$RUNNER_OS" == "Windows" ]; then + export PATH="$GITHUB_WORKSPACE/.nim_runtime/bin:$HOME/.nimble/bin:$PATH" + fi + nimble test_cpp_e2e -y tests-asan-ubsan: name: Tests · ASan+UBSan+LSan diff --git a/examples/timer/cpp_bindings/CMakeLists.txt b/examples/timer/cpp_bindings/CMakeLists.txt index 07eae57..c08e11e 100644 --- a/examples/timer/cpp_bindings/CMakeLists.txt +++ b/examples/timer/cpp_bindings/CMakeLists.txt @@ -28,10 +28,22 @@ if(CMAKE_SYSTEM_NAME STREQUAL "Darwin") set(NIM_LIB_FILE "${REPO_ROOT}/libmy_timer.dylib") elseif(CMAKE_SYSTEM_NAME STREQUAL "Windows") set(NIM_LIB_FILE "${REPO_ROOT}/my_timer.dll") + # MSVC consumers link against the `.lib` import library, not the DLL. + # MinGW's ld emits one when asked via `--out-implib`; the resulting COFF + # archive is readable by MSVC's link.exe. + set(NIM_IMPLIB_FILE "${REPO_ROOT}/my_timer.lib") else() set(NIM_LIB_FILE "${REPO_ROOT}/libmy_timer.so") endif() +# On Windows the default Nim toolchain (mingw gcc) doesn't emit an import +# library unless told to. Without it, MSVC consumers can't resolve any +# symbol exported by the DLL at link time. +set(NIM_IMPLIB_PASSL "") +if(CMAKE_SYSTEM_NAME STREQUAL "Windows") + set(NIM_IMPLIB_PASSL "--passL:-Wl,--out-implib,${NIM_IMPLIB_FILE}") +endif() + add_custom_command( OUTPUT "${NIM_LIB_FILE}" COMMAND "${NIM_EXECUTABLE}" c @@ -40,19 +52,39 @@ add_custom_command( --app:lib --noMain "--nimMainPrefix:libmy_timer" + ${NIM_IMPLIB_PASSL} "-o:${NIM_LIB_FILE}" "${NIM_SRC}" WORKING_DIRECTORY "${REPO_ROOT}" DEPENDS "${NIM_SRC}" + BYPRODUCTS "${NIM_IMPLIB_FILE}" COMMENT "Compiling Nim library libmy_timer" VERBATIM ) add_custom_target(nim_lib ALL DEPENDS "${NIM_LIB_FILE}") -add_library(my_timer SHARED IMPORTED GLOBAL) -set_target_properties(my_timer PROPERTIES IMPORTED_LOCATION "${NIM_LIB_FILE}") +# On Windows, an `IMPORTED SHARED` target needs IMPORTED_IMPLIB pointing at +# the `.lib` import library so MSVC's `link.exe` can resolve symbols. The +# Visual Studio multi-config generator did not pick up `IMPORTED_IMPLIB` — +# nor per-config `IMPORTED_IMPLIB_` variants — and emitted +# `my_timer-NOTFOUND.obj` into every link line. Side-step the IMPORTED +# machinery on Windows by exposing the import library through a plain +# INTERFACE library that links the `.lib` by path. +if(CMAKE_SYSTEM_NAME STREQUAL "Windows") + add_library(my_timer INTERFACE) + target_link_libraries(my_timer INTERFACE "${NIM_IMPLIB_FILE}") +else() + add_library(my_timer SHARED IMPORTED GLOBAL) + set_target_properties(my_timer PROPERTIES IMPORTED_LOCATION "${NIM_LIB_FILE}") +endif() add_dependencies(my_timer nim_lib) +# Absolute path to the runtime library (DLL/dylib/so). Exposed via the cache +# so consumers in other directories (e.g. tests/e2e/cpp) can stage the DLL +# next to their executable on Windows. +set(my_timer_RUNTIME_LIB "${NIM_LIB_FILE}" CACHE INTERNAL + "Absolute path to the my_timer runtime library") + # ── TinyCBOR (vendored at ffi/codegen/templates/cpp/vendor/tinycbor) ───────── set(TINYCBOR_SRC_DIR "${REPO_ROOT}/ffi/codegen/templates/cpp/vendor") add_library(tinycbor STATIC @@ -77,4 +109,11 @@ if(EXISTS "${CMAKE_CURRENT_SOURCE_DIR}/main.cpp") add_executable(example main.cpp) target_link_libraries(example PRIVATE my_timer_headers) add_dependencies(example nim_lib) + if(CMAKE_SYSTEM_NAME STREQUAL "Windows") + add_custom_command(TARGET example POST_BUILD + COMMAND "${CMAKE_COMMAND}" -E copy_if_different + "${my_timer_RUNTIME_LIB}" + "$" + COMMENT "Staging my_timer.dll next to example.exe") + endif() endif() diff --git a/ffi.nimble b/ffi.nimble index 089a97a..c3ad2e8 100644 --- a/ffi.nimble +++ b/ffi.nimble @@ -33,6 +33,18 @@ proc discoverUnitTests(): seq[string] = let unitTests = discoverUnitTests() +proc runOrQuit(cmd: string) = + # Workaround for newer nimble (shipping with Nim 2.2.10+) printing the + # OSError from a failed `exec` but exiting 0, which causes CI to report + # green on actual build/test failures. Echo the command first so the log + # makes clear which step failed. + try: + exec cmd + except OSError as e: + echo "command failed: ", cmd + echo "error: ", e.msg + quit(QuitFailure) + proc sanFlags(san: string): string = # Each --passC / --passL adds one literal flag to the C compiler / linker # invocation — avoids any quoting ambiguity that arises from putting @@ -80,10 +92,10 @@ task test_serial, "Run CBOR codec unit tests": task test_cpp_e2e, "Build and run the C++ end-to-end tests for the timer example": # Regenerate the C++ bindings so the suite always runs against fresh codegen. - exec "nimble genbindings_cpp" - exec "cmake -S tests/e2e/cpp -B tests/e2e/cpp/build" - exec "cmake --build tests/e2e/cpp/build" - exec "ctest --test-dir tests/e2e/cpp/build --output-on-failure" + runOrQuit "nimble genbindings_cpp" + runOrQuit "cmake -S tests/e2e/cpp -B tests/e2e/cpp/build" + runOrQuit "cmake --build tests/e2e/cpp/build" + runOrQuit "ctest --test-dir tests/e2e/cpp/build --output-on-failure" task test_sanitized, "Run all unit tests under a sanitizer (NIM_FFI_SAN) and mm (NIM_FFI_MM)": let san = getEnv("NIM_FFI_SAN", "none") @@ -107,12 +119,12 @@ task test_sanitized, "Run all unit tests under a sanitizer (NIM_FFI_SAN) and mm task test_cpp_e2e_sanitized, "Build and run the C++ e2e tests with a sanitizer (NIM_FFI_SAN) and mm (NIM_FFI_MM)": let mm = getEnv("NIM_FFI_MM", "orc") let san = getEnv("NIM_FFI_SAN", "none") - exec "nimble genbindings_cpp" - exec "cmake -S tests/e2e/cpp -B tests/e2e/cpp/build" & + runOrQuit "nimble genbindings_cpp" + runOrQuit "cmake -S tests/e2e/cpp -B tests/e2e/cpp/build" & " -DNIM_FFI_MM=" & mm & " -DNIM_FFI_SANITIZER=" & san - exec "cmake --build tests/e2e/cpp/build -j" - exec "ctest --test-dir tests/e2e/cpp/build --output-on-failure" + runOrQuit "cmake --build tests/e2e/cpp/build -j" + runOrQuit "ctest --test-dir tests/e2e/cpp/build --output-on-failure" task genbindings_example, "Generate Rust bindings for the timer example": exec "nim c " & nimFlagsOrc & " --app:lib --noMain --nimMainPrefix:libmy_timer -d:ffiGenBindings -o:/dev/null examples/timer/timer.nim" diff --git a/ffi/codegen/templates/cpp/CMakeLists.txt.tpl b/ffi/codegen/templates/cpp/CMakeLists.txt.tpl index 3cfa449..5604b0a 100644 --- a/ffi/codegen/templates/cpp/CMakeLists.txt.tpl +++ b/ffi/codegen/templates/cpp/CMakeLists.txt.tpl @@ -28,10 +28,22 @@ if(CMAKE_SYSTEM_NAME STREQUAL "Darwin") set(NIM_LIB_FILE "${REPO_ROOT}/lib{{LIB}}.dylib") elseif(CMAKE_SYSTEM_NAME STREQUAL "Windows") set(NIM_LIB_FILE "${REPO_ROOT}/{{LIB}}.dll") + # MSVC consumers link against the `.lib` import library, not the DLL. + # MinGW's ld emits one when asked via `--out-implib`; the resulting COFF + # archive is readable by MSVC's link.exe. + set(NIM_IMPLIB_FILE "${REPO_ROOT}/{{LIB}}.lib") else() set(NIM_LIB_FILE "${REPO_ROOT}/lib{{LIB}}.so") endif() +# On Windows the default Nim toolchain (mingw gcc) doesn't emit an import +# library unless told to. Without it, MSVC consumers can't resolve any +# symbol exported by the DLL at link time. +set(NIM_IMPLIB_PASSL "") +if(CMAKE_SYSTEM_NAME STREQUAL "Windows") + set(NIM_IMPLIB_PASSL "--passL:-Wl,--out-implib,${NIM_IMPLIB_FILE}") +endif() + add_custom_command( OUTPUT "${NIM_LIB_FILE}" COMMAND "${NIM_EXECUTABLE}" c @@ -40,19 +52,39 @@ add_custom_command( --app:lib --noMain "--nimMainPrefix:lib{{LIB}}" + ${NIM_IMPLIB_PASSL} "-o:${NIM_LIB_FILE}" "${NIM_SRC}" WORKING_DIRECTORY "${REPO_ROOT}" DEPENDS "${NIM_SRC}" + BYPRODUCTS "${NIM_IMPLIB_FILE}" COMMENT "Compiling Nim library lib{{LIB}}" VERBATIM ) add_custom_target(nim_lib ALL DEPENDS "${NIM_LIB_FILE}") -add_library({{LIB}} SHARED IMPORTED GLOBAL) -set_target_properties({{LIB}} PROPERTIES IMPORTED_LOCATION "${NIM_LIB_FILE}") +# On Windows, an `IMPORTED SHARED` target needs IMPORTED_IMPLIB pointing at +# the `.lib` import library so MSVC's `link.exe` can resolve symbols. The +# Visual Studio multi-config generator did not pick up `IMPORTED_IMPLIB` — +# nor per-config `IMPORTED_IMPLIB_` variants — and emitted +# `{{LIB}}-NOTFOUND.obj` into every link line. Side-step the IMPORTED +# machinery on Windows by exposing the import library through a plain +# INTERFACE library that links the `.lib` by path. +if(CMAKE_SYSTEM_NAME STREQUAL "Windows") + add_library({{LIB}} INTERFACE) + target_link_libraries({{LIB}} INTERFACE "${NIM_IMPLIB_FILE}") +else() + add_library({{LIB}} SHARED IMPORTED GLOBAL) + set_target_properties({{LIB}} PROPERTIES IMPORTED_LOCATION "${NIM_LIB_FILE}") +endif() add_dependencies({{LIB}} nim_lib) +# Absolute path to the runtime library (DLL/dylib/so). Exposed via the cache +# so consumers in other directories (e.g. tests/e2e/cpp) can stage the DLL +# next to their executable on Windows. +set({{LIB}}_RUNTIME_LIB "${NIM_LIB_FILE}" CACHE INTERNAL + "Absolute path to the {{LIB}} runtime library") + # ── TinyCBOR (vendored at ffi/codegen/templates/cpp/vendor/tinycbor) ───────── set(TINYCBOR_SRC_DIR "${REPO_ROOT}/ffi/codegen/templates/cpp/vendor") add_library(tinycbor STATIC @@ -77,4 +109,11 @@ if(EXISTS "${CMAKE_CURRENT_SOURCE_DIR}/main.cpp") add_executable(example main.cpp) target_link_libraries(example PRIVATE {{LIB}}_headers) add_dependencies(example nim_lib) + if(CMAKE_SYSTEM_NAME STREQUAL "Windows") + add_custom_command(TARGET example POST_BUILD + COMMAND "${CMAKE_COMMAND}" -E copy_if_different + "${{{LIB}}_RUNTIME_LIB}" + "$" + COMMENT "Staging {{LIB}}.dll next to example.exe") + endif() endif() diff --git a/tests/e2e/cpp/CMakeLists.txt b/tests/e2e/cpp/CMakeLists.txt index 7b68a3d..3c9e731 100644 --- a/tests/e2e/cpp/CMakeLists.txt +++ b/tests/e2e/cpp/CMakeLists.txt @@ -36,12 +36,22 @@ endif() # The Nim-built shared library has install_name `@rpath/libmy_timer.dylib` # (set by `declareLibrary` on macOS for portability). The test binary must # therefore know where to find that dylib at load time — embed the build-tree -# directory of the IMPORTED `my_timer` target as an rpath. -get_target_property(_my_timer_loc my_timer IMPORTED_LOCATION) -get_filename_component(_my_timer_dir "${_my_timer_loc}" DIRECTORY) -set_target_properties(timer_e2e_tests PROPERTIES - BUILD_RPATH "${_my_timer_dir}" - INSTALL_RPATH "${_my_timer_dir}") +# directory of the IMPORTED `my_timer` target as an rpath. On Windows the +# `my_timer` target is a plain INTERFACE library (no IMPORTED_LOCATION) and +# we stage the DLL next to the exe via POST_BUILD instead. +if(NOT CMAKE_SYSTEM_NAME STREQUAL "Windows") + get_target_property(_my_timer_loc my_timer IMPORTED_LOCATION) + get_filename_component(_my_timer_dir "${_my_timer_loc}" DIRECTORY) + set_target_properties(timer_e2e_tests PROPERTIES + BUILD_RPATH "${_my_timer_dir}" + INSTALL_RPATH "${_my_timer_dir}") +else() + add_custom_command(TARGET timer_e2e_tests POST_BUILD + COMMAND "${CMAKE_COMMAND}" -E copy_if_different + "${my_timer_RUNTIME_LIB}" + "$" + COMMENT "Staging my_timer.dll next to timer_e2e_tests.exe") +endif() # Per-sanitizer runtime options: halt and exit non-zero on any report so # ctest fails the job. The matching ASAN_OPTIONS / UBSAN_OPTIONS / diff --git a/tests/e2e/cpp/test_timer_e2e.cpp b/tests/e2e/cpp/test_timer_e2e.cpp index ffed1ec..d223ea7 100644 --- a/tests/e2e/cpp/test_timer_e2e.cpp +++ b/tests/e2e/cpp/test_timer_e2e.cpp @@ -152,10 +152,10 @@ TEST(TimerE2E, ThreadedHammer) { auto own = makeCtx("hammer-t" + std::to_string(t)); for (int i = 0; i < kIters; ++i) { if ((i & 1) == 0) { - const auto r = shared.echo(EchoRequest{"s", 0}); + const auto r = shared->echo(EchoRequest{"s", 0}); if (r.echoed != "s") ++errors; } else { - auto f = own.echoAsync(EchoRequest{"a", 1}); + auto f = own->echoAsync(EchoRequest{"a", 1}); if (f.get().echoed != "a") ++errors; } }