From 9fc26aaa47a75725f781e096a2816736af912a20 Mon Sep 17 00:00:00 2001 From: Jer Miller Date: Wed, 19 Aug 2026 17:39:56 -0600 Subject: [PATCH 1/5] fix: guard the printf format attribute so MSVC can build MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit src/common.hpp declares rfdetr_logf with __attribute__((format(printf, 2, 3))). MSVC has no __attribute__, so the declaration fails to parse (C3646 / C2059) and the definition in common.cpp then reads as a redefinition (C2084). The whole tree stops on the first translation unit, so rf-detr.cpp does not build on Windows at all today. Wrap it in RFDETR_ATTRIBUTE_FORMAT with the same three cases ggml.h already handles for GGML_ATTRIBUTE_FORMAT: empty on non-GNUC, gnu_printf on MinGW, printf elsewhere. No change on gcc/clang — the attribute is still applied and -Wformat still fires on a mismatched call. --- src/common.hpp | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/common.hpp b/src/common.hpp index dc1a5a5..c8718e6 100644 --- a/src/common.hpp +++ b/src/common.hpp @@ -9,8 +9,19 @@ * in tests without needing to include this header. */ void rfdetr_internal_log(rfdetr_log_level lvl, const char* msg); +/* Format-string checking where the compiler offers it. MSVC does not + * understand __attribute__, and MinGW's printf is the gnu_printf dialect — + * the same three cases ggml.h handles with GGML_ATTRIBUTE_FORMAT. */ +#ifndef __GNUC__ +# define RFDETR_ATTRIBUTE_FORMAT(...) +#elif defined(__MINGW32__) && !defined(__clang__) +# define RFDETR_ATTRIBUTE_FORMAT(...) __attribute__((format(gnu_printf, __VA_ARGS__))) +#else +# define RFDETR_ATTRIBUTE_FORMAT(...) __attribute__((format(printf, __VA_ARGS__))) +#endif + /* printf-style wrapper. Builds the string then dispatches. */ void rfdetr_logf(rfdetr_log_level lvl, const char* fmt, ...) - __attribute__((format(printf, 2, 3))); + RFDETR_ATTRIBUTE_FORMAT(2, 3); #endif From d1a5f85fdf1a892ecbde9d55fbc1569ae63b2197 Mon Sep 17 00:00:00 2001 From: Jer Miller Date: Wed, 19 Aug 2026 17:52:55 -0600 Subject: [PATCH 2/5] fix: give the bicubic kernel constant static storage for MSVC MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit bicubic_resample_patch_grid declares `constexpr float A` as an automatic and then names it from a captureless lambda. gcc and clang accept that — A is never odr-used, only read as a constant — but MSVC rejects it with C3493 ('cannot be implicitly captured because no default capture mode has been specified'), and the two follow-on errors at line 159 are that failure cascading through `kernel`. Making A `static constexpr` gives it static storage duration, so no capture is needed and every compiler accepts it. Same constant, same value, no behaviour change. --- src/model_loader.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/model_loader.cpp b/src/model_loader.cpp index 0326fcf..5c237d8 100644 --- a/src/model_loader.cpp +++ b/src/model_loader.cpp @@ -116,7 +116,10 @@ bool get_bool(gguf_context* g, const char* key, bool& out) { * with A = -0.5 for antialias=True. */ void bicubic_resample_patch_grid(const float* src, int src_side, int dim, float* dst, int dst_side) { - constexpr float A = -0.5f; // antialias=True path uses Keys, not Catmull-Rom + /* `static` so the captureless lambda below can name it: MSVC rejects an + * automatic constexpr there (C3493) where gcc/clang accept it. Static + * storage duration needs no capture, and is portable. */ + static constexpr float A = -0.5f; // antialias=True path uses Keys, not Catmull-Rom auto kernel = [](float x) -> float { const float ax = std::fabs(x); if (ax < 1.0f) { From 46218c98f85ab1715e1ca68ad2ba1be49dde1719 Mon Sep 17 00:00:00 2001 From: Jer Miller Date: Wed, 19 Aug 2026 20:14:50 -0600 Subject: [PATCH 3/5] fix: create the masks dir with std::filesystem so the CLI builds on MSVC MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit examples/cli/main.cpp calls POSIX ::mkdir, which MSVC does not provide — it has _mkdir in — so the CLI is the last thing blocking a Windows build once the library compiles. Replace the stat-probe-then-mkdir pair with std::filesystem::create_ directories, which is already available (the project is C++17) and is a no-op when the directory exists, so the probe is redundant. The two remaining ::stat calls are left alone: MSVC does provide stat. One behaviour difference worth naming: create_directories also creates missing parents, where ::mkdir created only the leaf. --masks some/new/dir now works instead of failing. The error message is unchanged. --- examples/cli/main.cpp | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/examples/cli/main.cpp b/examples/cli/main.cpp index 4253f78..abf9dde 100644 --- a/examples/cli/main.cpp +++ b/examples/cli/main.cpp @@ -11,7 +11,9 @@ #include #include #include +#include #include +#include #include #include #include @@ -131,13 +133,15 @@ static int cmd_detect(const rfdetr_cli::DetectArgs& a) { /* 7. Optional per-detection mask PNGs (seg models only). */ if (!a.masks_dir.empty()) { - /* Create the masks directory if it doesn't exist. */ - struct stat st_buf; - if (::stat(a.masks_dir.c_str(), &st_buf) != 0) { - if (::mkdir(a.masks_dir.c_str(), 0755) != 0) { - std::fprintf(stderr, "failed to create masks dir '%s'\n", - a.masks_dir.c_str()); - } + /* Create the masks directory if it doesn't exist. std::filesystem + * rather than stat + ::mkdir: MSVC has no POSIX mkdir (only _mkdir, + * in ), and create_directories already succeeds silently + * when the directory is there, so the stat probe goes with it. */ + std::error_code mkdir_ec; + std::filesystem::create_directories(a.masks_dir, mkdir_ec); + if (mkdir_ec) { + std::fprintf(stderr, "failed to create masks dir '%s'\n", + a.masks_dir.c_str()); } size_t n_written = 0; for (size_t i = 0; i < n; ++i) { From ebcc519115a1cb23a8c406addfe3ca95afe600a9 Mon Sep 17 00:00:00 2001 From: Jer Miller Date: Thu, 20 Aug 2026 09:30:03 -0600 Subject: [PATCH 4/5] test: skip the POSIX-only CLI integration test on Windows test_cli_integration drives rfdetr-cli as a child process and includes , which MSVC does not have, so enabling RFDETR_BUILD_TESTS on Windows fails the build outright on that one file. Guard its registration with NOT WIN32. Everything else under tests/ is portable, so the remaining 25 tests still build and run under MSVC, which is what makes a Windows CI job worth having. On Linux the test is registered and run exactly as before. --- tests/CMakeLists.txt | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 05a5bd7..8dc07c3 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -38,7 +38,10 @@ rfdetr_add_test(test_visualize) rfdetr_add_test(test_cli_smoke) -if(TARGET rfdetr-cli) +# test_cli_integration drives the CLI as a child process through , +# so it is POSIX-only. Everything else in this directory is portable; excluding +# just this one keeps the rest of the suite running under MSVC. +if(TARGET rfdetr-cli AND NOT WIN32) rfdetr_add_test(test_cli_integration) target_compile_definitions(test_cli_integration PRIVATE RFDETR_CLI_BINARY="$") From 9742e72848fcbcb686653183499de83cecc4be59 Mon Sep 17 00:00:00 2001 From: Jer Miller Date: Thu, 20 Aug 2026 09:26:46 -0600 Subject: [PATCH 5/5] ci: build on Windows with MSVC so the portability fixes stay fixed Everything in ci.yml is ubuntu-only, so nothing has ever compiled this tree with MSVC. That is why three portability breaks reached main unnoticed: an __attribute__ on a declaration, an automatic constexpr named from a captureless lambda, and a POSIX-only ::mkdir. All three build clean on gcc and stop MSVC on the first translation unit. Add one windows-2022 job in the same shape as the ubuntu build job - configure, compile, ctest, usage banner. CPU only and no model downloads: this guards the compile, not the detection numbers, which the existing ubuntu smoke-test job already covers. The ggml patch script is invoked explicitly before configure because the CMake configure-time hook shells out to bash, which is not on PATH in the MSVC environment. --- .github/workflows/ci.yml | 54 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 94e22e0..cf4e080 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -75,6 +75,60 @@ jobs: retention-days: 1 if-no-files-found: error + # ----------------------------------------------------------------------- + # build-windows: the same build under MSVC. + # + # Everything else in this file is ubuntu-only, so nothing here has ever + # compiled with MSVC, and portability breaks land silently: __attribute__, + # naming an automatic constexpr from a captureless lambda, and POSIX-only + # calls all build clean on gcc and stop MSVC on the first file. + # + # CPU only, no model downloads — this guards the compile, not the numbers, + # which the ubuntu smoke-test job already covers. GGML_NATIVE=ON matches the + # ubuntu build job: CI only has to run on its own runner. + # ----------------------------------------------------------------------- + build-windows: + name: Build & unit tests (MSVC) + runs-on: windows-2022 + timeout-minutes: 30 + steps: + - name: Checkout (with submodules) + uses: actions/checkout@v4 + with: + submodules: recursive + + # The CMake configure-time hook shells out to bash, which is not on PATH + # in the MSVC environment; run it explicitly first. + - name: Apply ggml patches (Git Bash) + shell: bash + run: bash scripts/apply_ggml_patches.sh + + - name: MSVC environment + uses: ilammy/msvc-dev-cmd@v1 + + - name: Configure CMake + shell: bash + run: | + cmake -B build -G Ninja \ + -DCMAKE_BUILD_TYPE=Release \ + -DRFDETR_BUILD_TESTS=ON \ + -DRFDETR_BUILD_CLI=ON \ + -DGGML_NATIVE=ON + + - name: Build + shell: bash + run: cmake --build build -j + + - name: Run unit tests + shell: bash + run: ctest --test-dir build --output-on-failure + + - name: Usage banner + shell: bash + run: | + out=$(./build/bin/rfdetr-cli.exe 2>&1 || true) + grep -qi usage <<<"$out" + # ----------------------------------------------------------------------- # smoke-test: download nano (all 4 quants) + base-f16 from HF, run detect # on a committed test image, compare JSON output against committed refs.