Conversation
…sock.h VolumeRover2 fails to compile on Windows with sockaddr/fd_set/timeval "struct type redefinition": on MSVC <windows.h> transitively includes the legacy <winsock.h> (which declares those structs), and when a TU reaches windows.h before the XmlRPC layer's <winsock2.h> (XmlRpcSocket.cpp / XmlRpcDispatch.cpp), winsock2.h re-declares them. The build system defined no guard project-wide (only -DNOMINMAX). Define WIN32_LEAN_AND_MEAN in the existing if(WIN32) block. It excludes winsock 1.1 from <windows.h> so <winsock2.h> is the sole definer regardless of include order. Crucially it does this WITHOUT defining _WINSOCKAPI_ — unlike a bare -D_WINSOCKAPI_, which fixes VolumeRover2 but trips Boost.Asio's "WinSock.h has already been included" guard (asio checks _WINSOCKAPI_ && !_WINSOCK2API_) and breaks the CVC library's asio TUs. WIN32_LEAN_AND_MEAN is in fact Boost.Asio's own recommended macro. It leaves GDI/USER/KERNEL intact; only Cryptography/DDE/RPC/Shell/Sockets headers are excluded from <windows.h> (included explicitly where needed). Scoped to if(WIN32), so the passing macOS/Linux builds are untouched.
transfix
force-pushed
the
fix/win-winsock-redefinition
branch
from
September 22, 2026 21:45
791429e to
0127d7d
Compare
transfix
changed the title
The Windows CI pulled glew/fftw3/gsl/log4cplus/pthreads/boost from vcpkg, and
vcpkg's log4cplus does not compile with MSVC 14.44 (log4cplus/helpers/
win32fstream.h — handle_/file_size not found). The project already has a proven
cvcpkg flow for exactly these deps (cvc-requirements.yaml + the cvcpkg-install
action, used by publish-cvcpkg.yml), and the cvcpkg log4cplus (2.1.2+cvc)
compiles clean. So drop vcpkg entirely and build the Windows job the same way
publish-cvcpkg.yml does:
* Qt6, Boost, GLEW, FFTW3, GSL, log4cplus, ImageMagick and pthreads4w now come
from the cvcpkg prefix (cvc-requirements.yaml) — no vcpkg install, no vcpkg
toolchain file, no install-qt-action. CMAKE_PREFIX_PATH points at the cvcpkg
prefix and that is the whole story.
* Windows builds Release only: the catalog ships Release (shared) bundles, and
a Debug app linked against Release deps is an MSVC CRT mismatch — the same
reason publish-cvcpkg.yml is Release-only on Windows.
Two source fixes the cvcpkg path needs:
* -DBOOST_ALL_NO_LIB — Boost's MSVC auto-link otherwise requests the
vcpkg-mangled name libboost_thread-vc143-mt-x64-1_86.lib, which the cvcpkg
boost does not use (LNK1104). With auto-link off, linking uses the explicit
Boost::* imported targets that point at cvcpkg's libs.
* -DWIN32_LEAN_AND_MEAN — keep the earlier winsock fix: it stops <windows.h>
pulling the legacy <winsock.h> (sockaddr/fd_set/timeval redefinition vs the
XmlRPC layer's <winsock2.h>) without defining _WINSOCKAPI_ (which would trip
Boost.Asio's guard).
Linux (apt) and macOS (brew) are unchanged — they never used vcpkg.
transfix
changed the title
transfix
added a commit
to cy-pca/cvcpkg
that referenced
this pull request
Sep 23, 2026
log4cplus on MSVC builds one character variant per config: UNICODE (wchar_t) ->
log4cplusU.lib, narrow (char) -> log4cplus.lib. The recipe shipped only the
MSVC-default UNICODE lib, but consumers are split:
* Qt apps that keep UNICODE want the wide log4cplusU.lib.
* The CVC ecosystem uses the char API — VolumeRover's log4cplus_compat.h
undefs UNICODE, and libcvc's FindLog4cplus searches for "log4cplus", never
"log4cplusU" — so it needs the narrow log4cplus.lib and otherwise gets
LNK2001 on the narrow basic_string<char> symbols (Logger::getInstance,
get_macro_body_oss, macro_forced_log).
Build BOTH variants into the one prefix, from separate build dirs, so either
camp links the lib it needs and UNICODE support is preserved. The narrow pass
runs last so the exported cmake config target is log4cplus::log4cplus (the
recipe's declared cmake_packages target); log4cplusU.lib stays on disk for
wide consumers to link by name. package.files already globs lib/log4cplus*, so
both libs are captured. Non-Windows already builds narrow via build.sh.
cvc_revision 6 -> 7 to ship the rebuilt bundle (published variants are
immutable). Unblocks the VolumeRover Windows build against cvcpkg deps
(transfix/volrover#47) without dropping UNICODE support.
The Windows package bundled runtime DLLs two ways: $<TARGET_RUNTIME_DLLS> (deps reached through IMPORTED targets) and the Qt deploy helper (Qt + plugins). With the vcpkg toolchain gone, deps found through a Find-MODULE — which expose only a .lib path, no IMPORTED target — are invisible to $<TARGET_RUNTIME_DLLS>, most notably log4cplus (via libcvc's FindLog4cplus). vcpkg's toolchain applocal- deployed everything; the cvcpkg prefix has no equivalent, so the installed app would be missing those DLLs. Copy the cvcpkg dependency prefix's runtime DLLs into the package as a fallback so it stays self-contained (CMAKE_PREFIX_PATH is that prefix on the cvcpkg CI).
transfix
added a commit
to cy-pca/cvcpkg
that referenced
this pull request
Sep 24, 2026
…ows (#82) log4cplus on MSVC builds one character variant per config: UNICODE (wchar_t) -> log4cplusU.lib, narrow (char) -> log4cplus.lib. The recipe shipped only the MSVC-default UNICODE lib, but consumers are split: * Qt apps that keep UNICODE want the wide log4cplusU.lib. * The CVC ecosystem uses the char API — VolumeRover's log4cplus_compat.h undefs UNICODE, and libcvc's FindLog4cplus searches for "log4cplus", never "log4cplusU" — so it needs the narrow log4cplus.lib and otherwise gets LNK2001 on the narrow basic_string<char> symbols (Logger::getInstance, get_macro_body_oss, macro_forced_log). Build BOTH variants into the one prefix, from separate build dirs, so either camp links the lib it needs and UNICODE support is preserved. The narrow pass runs last so the exported cmake config target is log4cplus::log4cplus (the recipe's declared cmake_packages target); log4cplusU.lib stays on disk for wide consumers to link by name. package.files already globs lib/log4cplus*, so both libs are captured. Non-Windows already builds narrow via build.sh. cvc_revision 6 -> 7 to ship the rebuilt bundle (published variants are immutable). Unblocks the VolumeRover Windows build against cvcpkg deps (transfix/volrover#47) without dropping UNICODE support.
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.
Problem
The Windows CI pulled
glew/fftw3/gsl/log4cplus/pthreads/boostfrom vcpkg, and vcpkg's log4cplus does not compile with MSVC 14.44 —log4cplus/helpers/win32fstream.hfails (handle_/file_sizenot found, syntax errors). Before that,<windows.h>pulled the legacy<winsock.h>, colliding with the XmlRPC layer's<winsock2.h>.The project already has a proven cvcpkg flow for exactly these deps —
cvc-requirements.yaml+ thecvcpkg-installaction, used bypublish-cvcpkg.yml— and the cvcpkg log4cplus (2.1.2+cvc) compiles clean (verified from the publish-cvcpkg Windows build: it gets all the way past log4cplus). vcpkg is the odd one out.Change
Drop vcpkg from the Windows CI and build it the way
publish-cvcpkg.ymlalready does:cvc-requirements.yaml). Novcpkg install, no vcpkg toolchain file, noinstall-qt-action—CMAKE_PREFIX_PATHat the cvcpkg prefix is the whole story.publish-cvcpkg.ymlis Release-only on Windows).Two source fixes the cvcpkg path needs (both
if(WIN32)):-DBOOST_ALL_NO_LIB— Boost's MSVC auto-link otherwise requests the vcpkg-mangledlibboost_thread-vc143-mt-x64-1_86.lib, which cvcpkg's boost doesn't use (LNK1104— this is exactly where the cvcpkg build currently stops). With auto-link off, linking uses the explicitBoost::*targets pointing at cvcpkg's libs.-DWIN32_LEAN_AND_MEAN— the winsock fix: stops<windows.h>pulling<winsock.h>without defining_WINSOCKAPI_(which would trip Boost.Asio's guard).Linux (apt) and macOS (brew) are unchanged — they never used vcpkg.
Note
This supersedes the winsock-only scope this PR started with. Packaging (
cpack/NSIS) may need a follow-up to bundle the cvcpkg runtime DLLs, but the build+link is the blocker this addresses.