Merge TASK-007: CI test for public-header hygiene

Author: etr

.github/workflows/verify-build.yml

@@ -253,6 +253,24 @@ jobs:
             debug: debug
             coverage: nocoverage
             shell: bash
+          # TASK-007: dedicated header-hygiene gate. Runs `make check-hygiene`
+          # (preprocesses <httpserver.hpp> against the staged install and greps
+          # for forbidden backend headers). Surfaces this gate as its own named
+          # GitHub Actions check so reviewers see header-hygiene status
+          # independently of the broader `make check` log. Until M5 lands the
+          # check is informational (HEADER_HYGIENE_STRICT defaults to "no");
+          # TASK-020 flips it to strict.
+          - test-group: extra
+            os: ubuntu-latest
+            os-type: ubuntu
+            build-type: header-hygiene
+            compiler-family: gcc
+            c-compiler: gcc-14
+            cc-compiler: g++-14
+            debug: nodebug
+            coverage: nocoverage
+            linking: dynamic
+            shell: bash
           - test-group: basic
             os: windows-latest
             os-type: windows
@@ -632,7 +650,18 @@ jobs:
       run: |
         cd build ;
         make check;
-      if: ${{ matrix.build-type != 'iwyu' && matrix.compiler-family != 'arm-cross' }}
+      if: ${{ matrix.build-type != 'iwyu' && matrix.compiler-family != 'arm-cross' && matrix.build-type != 'header-hygiene' }}
+
+    - name: Run header-hygiene check
+      # TASK-007: dedicated public-header hygiene gate. Runs the
+      # preprocessor-grep target (Layer 2) against a staged install and
+      # reports any forbidden backend headers reaching <httpserver.hpp>.
+      # Currently informational (HEADER_HYGIENE_STRICT=no) -- TASK-020
+      # flips this to strict when M5 closes the umbrella.
+      run: |
+        cd build
+        make check-hygiene
+      if: ${{ matrix.build-type == 'header-hygiene' }}
 
     - name: Print tests results
       shell: bash

Makefile.am

@@ -40,7 +40,8 @@ endif
 
 EXTRA_DIST = libhttpserver.pc.in $(DX_CONFIG) scripts/extract-release-notes.sh scripts/validate-version.sh \
 	test/headers/consumer_direct.cpp test/headers/consumer_detail.cpp test/headers/consumer_umbrella.cpp \
-	test/headers/consumer_post_umbrella.cpp
+	test/headers/consumer_post_umbrella.cpp \
+	test/headers/consumer_umbrella_no_backend.cpp
 
 # ---------------------------------------------------------------------------
 # Header-hygiene checks (TASK-002)
@@ -131,45 +132,160 @@ CHECK_INSTALL_STAGE = $(abs_top_builddir)/.install-stage
 
 check-install-layout:
 	@echo "=== check-install-layout: staged install must hide details/ and *_impl.hpp ==="
-	@rm -rf $(CHECK_INSTALL_STAGE)
-	@$(MAKE) $(AM_MAKEFLAGS) install DESTDIR=$(CHECK_INSTALL_STAGE) >check-install.log 2>&1 || { \
-	    echo "FAIL: staged install failed"; \
-	    cat check-install.log; \
-	    rm -f check-install.log; \
+	@if test "$(CHECK_INSTALL_SHARED)" != "yes"; then \
 	    rm -rf $(CHECK_INSTALL_STAGE); \
-	    exit 1; \
-	}
-	@rm -f check-install.log
+	    $(MAKE) $(AM_MAKEFLAGS) install DESTDIR=$(CHECK_INSTALL_STAGE) >check-install.log 2>&1 || { \
+	        echo "FAIL: staged install failed"; \
+	        cat check-install.log; \
+	        rm -f check-install.log; \
+	        rm -rf $(CHECK_INSTALL_STAGE); \
+	        exit 1; \
+	    }; \
+	    rm -f check-install.log; \
+	fi
 	@leaked_details=`find $(CHECK_INSTALL_STAGE) -type d -name details 2>/dev/null`; \
 	if test -n "$$leaked_details"; then \
 	    echo "FAIL: details/ directory leaked into install:"; \
 	    echo "$$leaked_details"; \
-	    rm -rf $(CHECK_INSTALL_STAGE); \
+	    if test "$(CHECK_INSTALL_SHARED)" != "yes"; then rm -rf $(CHECK_INSTALL_STAGE); fi; \
 	    exit 1; \
 	fi
 	@leaked_impl=`find $(CHECK_INSTALL_STAGE) -name '*_impl.hpp' 2>/dev/null`; \
 	if test -n "$$leaked_impl"; then \
 	    echo "FAIL: *_impl.hpp file leaked into install:"; \
 	    echo "$$leaked_impl"; \
-	    rm -rf $(CHECK_INSTALL_STAGE); \
+	    if test "$(CHECK_INSTALL_SHARED)" != "yes"; then rm -rf $(CHECK_INSTALL_STAGE); fi; \
 	    exit 1; \
 	fi
 	@umbrella_count=`find $(CHECK_INSTALL_STAGE) -name 'httpserver.hpp' | wc -l | tr -d ' '`; \
 	if test "$$umbrella_count" != "1"; then \
 	    echo "FAIL: expected exactly 1 installed httpserver.hpp, got $$umbrella_count"; \
-	    rm -rf $(CHECK_INSTALL_STAGE); \
+	    if test "$(CHECK_INSTALL_SHARED)" != "yes"; then rm -rf $(CHECK_INSTALL_STAGE); fi; \
 	    exit 1; \
 	fi
-	@rm -rf $(CHECK_INSTALL_STAGE)
+	@if test "$(CHECK_INSTALL_SHARED)" != "yes"; then rm -rf $(CHECK_INSTALL_STAGE); fi
 	@echo "  PASS: staged install layout is clean"
 
-check-local: check-headers check-install-layout
+# ---------------------------------------------------------------------------
+# Header-hygiene preprocessor gate (TASK-007).
+#
+# This is the preprocessor-grep half of the TASK-007 enforcement (the
+# compile-time half lives as `header_hygiene` in test/Makefile.am).
+#
+# Procedure:
+#   1. Stage `make install DESTDIR=$(CHECK_HYGIENE_STAGE)` to get a
+#      pristine public include tree -- exactly what packagers and
+#      downstream consumers see.
+#   2. Preprocess test/headers/consumer_umbrella_no_backend.cpp using
+#      ONLY -I$(CHECK_HYGIENE_STAGE)$(includedir) plus $(CPPFLAGS) (so
+#      e.g. /opt/homebrew/include is on the search path -- the grep
+#      below NEEDS to resolve <microhttpd.h> if the umbrella pulls it
+#      in, otherwise we couldn't detect the leak).
+#   3. Grep the cpp output for `# <line> "<file>"` line markers that
+#      name any forbidden backend header. The line-marker filter
+#      avoids false positives from substrings in code or comments.
+#
+# HEADER_HYGIENE_STRICT controls whether a leak is fatal:
+#   - "no"  (default until M5): leaks are reported as EXPECTED-FAIL
+#           and exit 0. This keeps `make check` green during M2-M5
+#           while making M2-M5 progress visible in CI logs.
+#   - "yes" (TASK-020 close-out): leaks are fatal. Set this from the
+#           command line (`make check-hygiene HEADER_HYGIENE_STRICT=yes`)
+#           or flip the default below.
+#
+# Cross-reference: keep HEADER_HYGIENE_FORBIDDEN in sync with the
+# #ifdef ladder in test/unit/header_hygiene_test.cpp.
+# ---------------------------------------------------------------------------
+
+HEADER_HYGIENE_FORBIDDEN = microhttpd\.h|pthread\.h|gnutls/gnutls\.h|sys/socket\.h|sys/uio\.h
+CHECK_HYGIENE_STAGE      = $(abs_top_builddir)/.hygiene-stage
+CHECK_HYGIENE_CXX        = $(CXX) -std=c++20 -E -I$(CHECK_HYGIENE_STAGE)$(includedir) $(CPPFLAGS)
+HEADER_HYGIENE_STRICT   ?= no
+
+# Sentinel file: only re-run the staged install when headers have changed.
+# This is an mtime gate used exclusively for standalone `make check-hygiene`
+# invocations — it avoids paying a full `make install` cost on every
+# repeated standalone run.  When check-local drives check-hygiene it sets
+# CHECK_HYGIENE_SHARED=yes and passes CHECK_HYGIENE_STAGE pointing at its
+# own pre-built shared stage, so this stamp target is bypassed entirely.
+HYGIENE_STAMP = $(CHECK_HYGIENE_STAGE)/.hygiene-stamp
 
-.PHONY: check-headers check-install-layout
+$(HYGIENE_STAMP): $(wildcard $(top_srcdir)/src/httpserver/*.hpp)
+	@rm -rf $(CHECK_HYGIENE_STAGE)
+	@$(MAKE) $(AM_MAKEFLAGS) install DESTDIR=$(CHECK_HYGIENE_STAGE) >check-hygiene-install.log 2>&1 || { \
+	    echo "FAIL: staged install failed"; cat check-hygiene-install.log; \
+	    rm -f check-hygiene-install.log; rm -rf $(CHECK_HYGIENE_STAGE); exit 1; }
+	@rm -f check-hygiene-install.log
+	@touch $(HYGIENE_STAMP)
+
+check-hygiene:
+	@echo "=== check-hygiene: <httpserver.hpp> must not transitively include backend headers ==="
+	@if test "$(CHECK_HYGIENE_SHARED)" != "yes"; then \
+	    $(MAKE) $(AM_MAKEFLAGS) $(HYGIENE_STAMP); \
+	else \
+	    if ! test -d "$(CHECK_HYGIENE_STAGE)"; then \
+	        echo "FAIL: CHECK_HYGIENE_SHARED=yes but stage dir '$(CHECK_HYGIENE_STAGE)' does not exist."; \
+	        echo "  Always pair CHECK_HYGIENE_SHARED=yes with CHECK_HYGIENE_STAGE=<valid-dir>."; \
+	        exit 1; \
+	    fi; \
+	fi
+	@status=0; \
+	if ! $(CHECK_HYGIENE_CXX) $(top_srcdir)/test/headers/consumer_umbrella_no_backend.cpp >check-hygiene.i 2>check-hygiene.err; then \
+	    if test "$(HEADER_HYGIENE_STRICT)" = "yes"; then \
+	        echo "FAIL: preprocessor failed"; cat check-hygiene.err; \
+	        status=1; \
+	    else \
+	        echo "EXPECTED-FAIL (informational until M5): preprocessor failed against staged install."; \
+	        echo "  This is expected while M2-M5 are in flight (e.g. webserver.hpp still"; \
+	        echo "  references private detail headers that aren't shipped)."; \
+	        echo "  Tail of preprocessor diagnostics:"; \
+	        sed 's/^/    /' check-hygiene.err | tail -10; \
+	    fi; \
+	else \
+	    leaks=`grep -hE '^# [0-9]+ "[^"]*/($(HEADER_HYGIENE_FORBIDDEN))"' check-hygiene.i | awk '{print $$3}' | sort -u`; \
+	    if test -n "$$leaks"; then \
+	        if test "$(HEADER_HYGIENE_STRICT)" = "yes"; then \
+	            echo "FAIL: forbidden headers leaked through <httpserver.hpp>:"; \
+	            echo "$$leaks"; \
+	            status=1; \
+	        else \
+	            echo "EXPECTED-FAIL (informational until M5): forbidden headers currently leak through <httpserver.hpp>:"; \
+	            echo "$$leaks"; \
+	        fi; \
+	    else \
+	        echo "  PASS: no forbidden headers reached the consumer TU"; \
+	    fi; \
+	fi; \
+	rm -f check-hygiene.i check-hygiene.err; \
+	exit $$status
+
+# check-local runs check-install-layout and check-hygiene against a single
+# shared staged install to avoid paying two full `make install` costs on
+# every `make check`. Both sub-checks can still be invoked standalone (they
+# will do their own install when CHECK_*_SHARED is not set).
+check-local: check-headers
+	@echo "=== Shared staged install for check-install-layout and check-hygiene ==="
+	@rm -rf $(abs_top_builddir)/.shared-check-stage
+	@$(MAKE) $(AM_MAKEFLAGS) install DESTDIR=$(abs_top_builddir)/.shared-check-stage >check-shared-install.log 2>&1 || { \
+	    echo "FAIL: shared staged install failed"; cat check-shared-install.log; \
+	    rm -f check-shared-install.log; rm -rf $(abs_top_builddir)/.shared-check-stage; exit 1; }
+	@rm -f check-shared-install.log
+	@$(MAKE) $(AM_MAKEFLAGS) check-install-layout \
+	    CHECK_INSTALL_STAGE=$(abs_top_builddir)/.shared-check-stage \
+	    CHECK_INSTALL_SHARED=yes
+	@$(MAKE) $(AM_MAKEFLAGS) check-hygiene \
+	    CHECK_HYGIENE_STAGE=$(abs_top_builddir)/.shared-check-stage \
+	    CHECK_HYGIENE_SHARED=yes
+	@rm -rf $(abs_top_builddir)/.shared-check-stage
+
+.PHONY: check-headers check-install-layout check-hygiene
 
 MOSTLYCLEANFILES = $(DX_CLEANFILES) *.gcda *.gcno *.gcov
 DISTCLEANFILES = DIST_REVISION
 
+clean-local:
+	rm -rf $(CHECK_HYGIENE_STAGE) $(abs_top_builddir)/.shared-check-stage $(CHECK_INSTALL_STAGE)
+
 pkgconfigdir = $(libdir)/pkgconfig
 pkgconfig_DATA = libhttpserver.pc
 

specs/tasks/M1-foundation/TASK-007.md

@@ -0,0 +1,51 @@
+### TASK-007: CI test for public-header hygiene
+
+**Milestone:** M1 - Foundation
+**Component:** CI / Test infrastructure
+**Estimate:** S
+
+**Goal:**
+Lock in the "no backend headers leak through `<httpserver.hpp>`" invariant with a CI gate so a future commit can't silently regress it.
+
+**Action Items:**
+- [x] Add a test program `test/header_hygiene.cpp` containing only `#include <httpserver.hpp>` and `int main(){}`. *(Implemented as `test/unit/header_hygiene_test.cpp` for test-tree symmetry; `test/headers/consumer_umbrella_no_backend.cpp` is the parallel source consumed by the preprocessor-grep target.)*
+- [x] In `Makefile.am`, build it without `-I` flags pointing at libmicrohttpd / pthread / gnutls headers (use only the installed-header path). *(Per-target `header_hygiene_CPPFLAGS = -I$(top_srcdir)/src $(CPPFLAGS)` overrides `AM_CPPFLAGS`, dropping `-DHTTPSERVER_COMPILATION` and `-I$(top_srcdir)/src/httpserver/`. The preprocessor-grep target uses ONLY the staged `DESTDIR` install include path.)*
+- [x] Run `g++ -E test/header_hygiene.cpp -I<install-prefix>/include` and `grep -E 'microhttpd\.h|pthread\.h|gnutls/gnutls\.h|sys/socket\.h|sys/uio\.h'` — expect zero matches. *(See `check-hygiene` in top-level `Makefile.am`. Today the grep finds matches; that's the EXPECTED-FAIL state until M5.)*
+- [x] Wire the check into `make check` (or a dedicated `make hygiene` target invoked by CI). *(Both: the runtime sentinel `header_hygiene` runs as part of `make check` (XFAIL until M5); the preprocessor-grep `check-hygiene` runs via `check-local` and also stands alone as a target for CI.)*
+- [x] Add a CI job that fails if any of the forbidden headers appear in the preprocessed output. *(Added `header-hygiene` matrix entry in `.github/workflows/verify-build.yml` running `make check-hygiene`. Currently informational; flips to fatal at TASK-020 by setting `HEADER_HYGIENE_STRICT=yes`.)*
+
+**Dependencies:**
+- Blocked by: TASK-002
+- Blocks: None (informational gate; will fail until M2-M5 land, that's expected and intended)
+
+**Acceptance Criteria:**
+- `grep -lE 'microhttpd\.h|pthread\.h|gnutls\.h|sys/socket\.h' src/httpserver/*.hpp` returns no results once M2-M5 land (PRD §3.1 acceptance).
+- The hygiene test is invoked by `make check` and fails loudly when violated.
+- Typecheck passes.
+
+**Related Requirements:** PRD-HDR-REQ-001..003
+**Related Decisions:** §9 testing item 1
+
+**Status:** Done (informational gate landed; full enforcement at TASK-020)
+
+---
+
+**Implementation Notes (TASK-007 close-out):**
+
+- **Strategy:** Option (c) from the plan -- "implement the test machinery now, mark it XFAIL until M5." Rejected (a) "leave `make check` red" (would block every PR for weeks); rejected (b) "narrow the grep to today's leaks" (encodes a binary invariant as a moving target, four chances to forget).
+- **Two layers of enforcement, both wired into `make check`:**
+  - *Layer 1 (compile-time sentinel):* `test/unit/header_hygiene_test.cpp` includes `<httpserver.hpp>` then checks well-known include-guard macros (`MHD_VERSION`, `_PTHREAD_H{,_}`, `GNUTLS_GNUTLS_H`, `_SYS_SOCKET_H{,_}`, `_SYS_UIO_H{,_}`). At runtime it prints the leaked headers and exits 1. Marked `XFAIL_TESTS` in `test/Makefile.am` so `make check` stays green.
+  - *Layer 2 (preprocessor grep):* `make check-hygiene` in the top-level `Makefile.am` stages `make install DESTDIR=$(CHECK_HYGIENE_STAGE)` and preprocesses `test/headers/consumer_umbrella_no_backend.cpp` against ONLY the staged include path, then greps cpp line markers for forbidden headers. Default `HEADER_HYGIENE_STRICT=no` makes it informational; flipping to `yes` makes it fatal.
+- **CI:** dedicated `header-hygiene` matrix entry in `.github/workflows/verify-build.yml` invokes `make check-hygiene` so the gate surfaces as its own GitHub Actions check.
+- **`<sys/uio.h>` rationale:** PRD-HDR-REQ-001..003 don't name `<sys/uio.h>` directly, but TASK-004 introduced `iovec_entry` specifically to avoid exposing it. Listing it here is a hardening assertion that TASK-004's intent isn't regressed.
+- **Why preprocessor-grep currently fails ahead of leak detection:** the staged install does not ship `details/` headers (per TASK-002); `webserver.hpp` still references `httpserver/details/http_endpoint.hpp` until TASK-014's PIMPL split. The `check-hygiene` recipe treats this preprocessor failure as EXPECTED-FAIL in informational mode, with diagnostics so M2-M5 progress remains visible.
+
+**M5 close-out (TASK-020 owner: zero ambiguity):**
+
+When TASK-020 makes `<httpserver.hpp>` clean of backend headers:
+
+1. Run `make check-hygiene HEADER_HYGIENE_STRICT=yes` from the build dir -- confirm exit 0 and `PASS: no forbidden headers reached the consumer TU`.
+2. Run `make check` -- expect Automake to report `XPASS: header_hygiene` (treated as a hard error by default), confirming the sentinel now passes.
+3. In `test/Makefile.am`, delete the line `XFAIL_TESTS = header_hygiene` and the comment block above it. Re-run `make check` -- expect `PASS: header_hygiene` and overall green.
+4. In `Makefile.am`, change `HEADER_HYGIENE_STRICT ?= no` to `HEADER_HYGIENE_STRICT ?= yes` (or remove the conditional and inline the strict path). Re-run `make check` to confirm `check-hygiene` is green.
+5. Mark this task `Status: Done (full enforcement)` and tick the M5 acceptance criterion (`grep -lE '...' src/httpserver/*.hpp` returns no results).

specs/tasks/M3-request/TASK-020.md

@@ -0,0 +1,31 @@
+### TASK-020: Final public-header backend-include sweep
+
+**Milestone:** M3 - Webserver internal & Request Refactor
+**Component:** Public headers (sweep)
+**Estimate:** S
+
+**Goal:**
+Verify and lock the "no backend headers in public surface" invariant after PIMPL splits and accessor refactors land, removing any straggler includes that survived earlier tasks.
+
+**Action Items:**
+- [ ] `grep -lE 'microhttpd\.h|pthread\.h|gnutls/gnutls\.h|sys/socket\.h|sys/uio\.h' src/httpserver/*.hpp`. Each file that turns up: route the include into the corresponding `details/*_impl.hpp` or `.cpp` file.
+- [ ] Verify after the sweep that the grep returns zero results.
+- [ ] Ensure the hygiene CI test from TASK-007 now passes. **Specifically:**
+  - [ ] In `test/Makefile.am`, delete the line `XFAIL_TESTS = header_hygiene` (and the explanatory comment block above it). After this edit, `make check` should report `PASS: header_hygiene` -- not `XFAIL` and not `XPASS`.
+  - [ ] In `Makefile.am`, change `HEADER_HYGIENE_STRICT ?= no` to `HEADER_HYGIENE_STRICT ?= yes` (or remove the conditional and inline the strict-mode path). Verify `make check-hygiene` exits 0 with `PASS: no forbidden headers reached the consumer TU`.
+  - [ ] Run `make check-hygiene HEADER_HYGIENE_STRICT=yes` from the build dir as a final smoke check.
+
+**Dependencies:**
+- Blocked by: TASK-014, TASK-015, TASK-019
+- Blocks: None (gating outcome that the rest of the project relies on)
+
+**Acceptance Criteria:**
+- `grep -lE 'microhttpd\.h|pthread\.h|gnutls\.h|sys/socket\.h' src/httpserver/*.hpp` returns no results (PRD §3.1 acceptance).
+- A test program containing only `#include <httpserver.hpp>` and `int main(){}` compiles without `-I` to libmicrohttpd / pthread / gnutls (PRD §3.1 acceptance).
+- TASK-007's hygiene test (red until now) goes green.
+- Typecheck passes.
+
+**Related Requirements:** PRD-HDR-REQ-001, PRD-HDR-REQ-002, PRD-HDR-REQ-003
+**Related Decisions:** §2.2, §5.5
+
+**Status:** Not Started