ci(winhdr): classify Winsock2/asio headers by dependency, not directory#2651
Merged
Conversation
The Windows header-compatibility check split headers into a WIN32_LEAN_AND_MEAN group and an unsanitized group using hard-coded directory globs (net/*, rpc/*) plus an explicit list. That is correct for today's tree but fragile: a future header that pulls in Winsock2/asio from outside those directories silently lands in the unsanitized group and fails with an opaque Winsock-vs-Winsock2 redefinition error, with the fix buried in this CI CMakeLists. Derive the lean set from the headers' own #include directives instead: seed on direct asio/winsock includes, then propagate along the rooted "glaze/..." include graph to a fixed point. The set now tracks real dependencies and stays correct as headers are added, moved, or renamed. On the current tree this reduces the lean group from 20 (by directory) to the 6 headers that actually reach Winsock2/asio; the 14 over-matched headers move to the unsanitized group, where they are additionally checked against the `small` macro. This also drops the load-bearing registry.hpp include-ordering hack: the non-standalone *_registry_impl.hpp fragments (which specialize templates declared only in their parent) are excluded from direct inclusion and exercised transitively through rpc/registry.hpp. Also documents why each header group is compiled as one aggregate translation unit rather than per-header or per-subdir: many Glaze headers are not standalone (they depend on declarations from an earlier include), so splitting would surface spurious failures unrelated to Windows macros. Real macro collisions remain pinpointed via the compiler's #include stack and the named macro-preservation #error checks.
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
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.
Follow-up to #2630. Makes the Windows header-compatibility check classify headers by their actual Winsock2/asio dependency instead of by hard-coded directory.
Problem
The check splits headers into a
WIN32_LEAN_AND_MEANgroup and an unsanitized group using directory globs (net/*,rpc/*) plus an explicit list. That's correct for today's tree but fragile: a future header that pulls in Winsock2/asio from outside those directories silently lands in the unsanitized group and fails with an opaqueWinsock.h-vs-Winsock2.hredefinition error, with the fix buried in this CICMakeLists.txt.Change
Derive the lean set from the headers' own
#includedirectives: seed on directasio/winsockincludes, then propagate along the rooted"glaze/..."include graph to a fixed point. The set now tracks real dependencies and stays correct as headers are added, moved, or renamed.On the current tree this reduces the lean group from 20 (by directory) to the 6 headers that actually reach Winsock2/asio:
The 14 over-matched headers move to the unsanitized group, where they're additionally checked against the
smallmacro (lean mode omits it) — a small coverage gain.This also drops the load-bearing
registry.hppinclude-ordering hack: the non-standalone*_registry_impl.hppfragments (which specialize templates declared only in their parent) are excluded from direct inclusion and exercised transitively throughrpc/registry.hpp.Aggregate-TU rationale (doc only)
Also adds a comment recording why each group is compiled as one aggregate translation unit rather than per-header/per-subdir: a local sweep found ~25 public headers are not standalone (they rely on declarations from an earlier include, e.g.
util/dump.hpp→string_literal,core/array_apply.hpp→size_t). Splitting would surface spurious failures unrelated to Windows macros. Real macro collisions are still pinpointed via the compiler's#includestack and the named macro-preservation#errorchecks.Verification
cmake -Pagainst the live tree: 240 unsanitized + 6 lean + 3 excluded fragments = 249; fragments appear in neither TU;net/http.hpp(#undef DELETE) sits safely in the unsanitized TU withDELETEcorrectly not asserted per-header.min/max/ERRORidentically; only Winsock differs). Thewindows-header-compatibilityworkflow runs on this PR and is the authoritative cross-check.