From 27b83cddca2c0d359473bca5d6f34bd8ad72dc4e Mon Sep 17 00:00:00 2001
From: Dario Gabriel Lipicar <lipigl@gmail.com>
Date: Fri, 31 Jul 2026 18:16:29 -0300
Subject: [PATCH] fix(headers): a failing logos-cpp-generator must fail the
 build
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

buildHeaders.nix swallowed the generator's exit status:

    logos-cpp-generator ... || {
      echo "Warning: ... may be expected if the module has no public API"
      touch ./generated_headers/.no-api
    }

so every real failure — a plugin the generator cannot dlopen (SDK/ABI
skew), an --api-style the pinned SDK rejects, a missing plugin file —
produced an EMPTY header set and exit 0. `nix build .#headers-<style>`
looked green with an include/ holding nothing but a `.generated`
placeholder, and the first symptom was an unresolved symbol when a
downstream module linked, far from the cause.

The escape hatch was aimed at a case that does not exist in this mode:
the generator introspects the plugin's QMetaObject, so a module with no
Q_INVOKABLE method still yields a wrapper class (with zero methods) and
exits 0. Measured against logos-cpp-sdk's generator:

    Q_INVOKABLE present     -> exit 0, <name>_api.{h,cpp}
    no Q_INVOKABLE at all    -> exit 0, <name>_api.{h,cpp} (no methods)
    plugin fails to dlopen   -> exit 3, nothing written
    retired/unknown api-style-> exit 1, nothing written
    plugin file missing      -> exit 2, nothing written

So "no public API" is signalled by SUCCESS, not by a status, and every
non-zero status is a build error. buildPlugin.nix already treats the
generator's status as fatal; buildHeaders.nix was the outlier.

The generator call moves to lib/generate-module-headers.sh, which:
  * fails the build on any non-zero status, echoing the status, the
    plugin, the api style and the likely causes (pin mismatch first);
  * fails a zero status that produced no <module>_api.{h,cpp} pair, so
    an empty include/ can never be installed;
  * reports a missing generator instead of a bare "command not found".
The install phase's "no headers -> empty include/" branch becomes an
error for the same reason.

tests/test-header-generator-guard.nix drives that script directly (so
the guard under test is the guard the build runs) across: no-public-API
success, generator failure, rejected api style, exit-0-with-no-output,
half-written output, missing generator, and argument forwarding
(--module-only / --api-style / --events-from).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---
 flake.nix                             |   5 +
 lib/buildHeaders.nix                  |  37 ++---
 lib/generate-module-headers.sh        | 116 +++++++++++++++
 tests/test-header-generator-guard.nix | 197 ++++++++++++++++++++++++++
 4 files changed, 339 insertions(+), 16 deletions(-)
 create mode 100755 lib/generate-module-headers.sh
 create mode 100644 tests/test-header-generator-guard.nix

diff --git a/flake.nix b/flake.nix
index f639a4a..4f38fee 100644
--- a/flake.nix
+++ b/flake.nix
@@ -66,6 +66,11 @@
           inherit pkgs;
           backendCommon = rawLib.common;
         };
+        # A failing logos-cpp-generator must fail the headers build; a module
+        # with no public API must not.
+        header-generator-guard = import ./tests/test-header-generator-guard.nix {
+          inherit pkgs;
+        };
       });
 
       # Dev shell for working on the backend itself
diff --git a/lib/buildHeaders.nix b/lib/buildHeaders.nix
index d89d527..c2c80ca 100644
--- a/lib/buildHeaders.nix
+++ b/lib/buildHeaders.nix
@@ -84,18 +84,19 @@
       # buildPlugin.nix's installPhase from `logos_events:` blocks in
       # the impl header). Absent for handcrafted Qt modules — that's
       # fine, we skip the flag in that case.
-      EVENTS_FROM_FLAG=""
+      EVENTS_SIDECAR=""
       if [ -f "${lib}/share/logos/${config.name}.lidl" ]; then
-        EVENTS_FROM_FLAG="--events-from ${lib}/share/logos/${config.name}.lidl"
-        echo "Using events sidecar: ${lib}/share/logos/${config.name}.lidl"
+        EVENTS_SIDECAR="${lib}/share/logos/${config.name}.lidl"
+        echo "Using events sidecar: $EVENTS_SIDECAR"
       fi
 
-      logos-cpp-generator "$PLUGIN_FILE" --output-dir ./generated_headers \
-        --module-only --api-style ${apiStyle} $EVENTS_FROM_FLAG || {
-        echo "Warning: logos-cpp-generator failed, this may be expected if the module has no public API"
-        # Create a marker file to indicate attempt was made
-        touch ./generated_headers/.no-api
-      }
+      # The generator's exit status is FATAL — see the header comment of
+      # generate-module-headers.sh for why "the module has no public API" is
+      # not a reason to swallow it (that case exits 0 and generates a wrapper
+      # with zero methods). The script also refuses a zero exit that produced
+      # no wrapper, so this derivation can never install an empty include/.
+      bash ${./generate-module-headers.sh} \
+        "$PLUGIN_FILE" ./generated_headers "${apiStyle}" "$EVENTS_SIDECAR"
 
       runHook postBuild
     '';
@@ -109,15 +110,19 @@
       # Copy all generated files (.h and .cpp) to include/ if they exist.
       # Both are needed: downstream modules #include the _api.cpp files from
       # the umbrella logos_sdk.cpp generated by --general-only.
+      # buildPhase already guarantees the wrapper pair exists; this is the last
+      # line of defence, and it is an ERROR rather than an empty include/ —
+      # publishing "no headers" is what made an SDK-pin mismatch look like a
+      # successful build and blow up later in a downstream link.
       gen_count=$(find ./generated_headers -maxdepth 1 \( -name '*.h' -o -name '*.cpp' \) 2>/dev/null | wc -l)
-      if [ "$gen_count" -gt 0 ]; then
-        echo "Copying generated headers and API files..."
-        ls -la ./generated_headers
-        find ./generated_headers -maxdepth 1 \( -name '*.h' -o -name '*.cpp' \) -exec cp {} $out/include/ \;
-      else
-        echo "Warning: No generated headers found, creating empty include directory"
-        echo "# No headers generated by logos-cpp-generator" > $out/include/.generated
+      if [ "$gen_count" -eq 0 ]; then
+        echo "Error: no generated headers to install from ./generated_headers" >&2
+        ls -la ./generated_headers >&2 || true
+        exit 1
       fi
+      echo "Copying generated headers and API files..."
+      ls -la ./generated_headers
+      find ./generated_headers -maxdepth 1 \( -name '*.h' -o -name '*.cpp' \) -exec cp {} $out/include/ \;
 
       runHook postInstall
     '';
diff --git a/lib/generate-module-headers.sh b/lib/generate-module-headers.sh
new file mode 100755
index 0000000..247ef31
--- /dev/null
+++ b/lib/generate-module-headers.sh
@@ -0,0 +1,116 @@
+#!/usr/bin/env bash
+#
+# Generate a module's SDK client wrapper by introspecting its built Qt plugin.
+#
+#   generate-module-headers.sh <plugin-file> <output-dir> <api-style> [events-sidecar]
+#
+# Called from lib/buildHeaders.nix. It lives in its own file (rather than
+# inline in the buildPhase) because the exit-status handling below is the
+# thing that keeps a BROKEN build from looking like a successful one, and
+# tests/test-header-generator-guard.nix drives THIS file — so the guard the
+# tests check is literally the guard the build runs.
+#
+# ---------------------------------------------------------------------------
+# Why every non-zero status is fatal here
+# ---------------------------------------------------------------------------
+# "The module has no public API" is a legitimate, non-failing case — but it is
+# NOT signalled by a non-zero exit. logos-cpp-generator introspects the
+# plugin's QMetaObject: a module that exposes no Q_INVOKABLE method still
+# yields a wrapper class (with zero methods) and exits 0. Measured against
+# logos-cpp-sdk's generator (plugin-introspection mode, the mode used here):
+#
+#   plugin with a Q_INVOKABLE method  -> exit 0, wrote <name>_api.{h,cpp}
+#   plugin with NO Q_INVOKABLE method -> exit 0, wrote <name>_api.{h,cpp}
+#   plugin that fails to dlopen       -> exit 3, wrote nothing   (SDK/ABI skew)
+#   unknown / retired --api-style     -> exit 1, wrote nothing   (SDK-pin skew)
+#   plugin file missing               -> exit 2, wrote nothing
+#   write failure in the output dir   -> exit 5/6
+#
+# So there is no "expected" failure to swallow. The historical
+# `|| { echo Warning...; }` turned an SDK-pin mismatch into a SILENTLY EMPTY
+# header set: `nix build .#headers-<style>` exited 0 with an empty include/,
+# and the first symptom was an unresolved symbol when some downstream module
+# linked — far from the cause. (buildPlugin.nix never swallowed the
+# generator's status; buildHeaders.nix was the outlier.)
+#
+# Belt and braces: a zero exit is additionally required to have PRODUCED the
+# wrapper pair, so a generator that ever "succeeds" without writing anything
+# fails the build here instead of shipping an empty include/.
+
+set -uo pipefail
+
+if [ "$#" -lt 3 ] || [ "$#" -gt 4 ]; then
+  echo "Usage: $0 <plugin-file> <output-dir> <api-style> [events-sidecar]" >&2
+  exit 64
+fi
+
+plugin_file="$1"
+output_dir="$2"
+api_style="$3"
+events_sidecar="${4:-}"
+
+if ! command -v logos-cpp-generator >/dev/null 2>&1; then
+  echo "Error: logos-cpp-generator is not on PATH." >&2
+  echo "       The headers derivation must have logos-cpp-sdk in nativeBuildInputs." >&2
+  exit 1
+fi
+
+mkdir -p "$output_dir"
+
+gen_args=("$plugin_file" --output-dir "$output_dir" --module-only --api-style "$api_style")
+
+# --events-from: typed `on<EventName>(callback)` accessors on the generated
+# wrapper come from the dep's LIDL sidecar. Handcrafted Qt modules publish
+# none — that is fine, the flag is simply omitted.
+if [ -n "$events_sidecar" ]; then
+  gen_args+=(--events-from "$events_sidecar")
+fi
+
+echo "Running: logos-cpp-generator ${gen_args[*]}"
+logos-cpp-generator "${gen_args[@]}"
+status=$?
+
+if [ "$status" -ne 0 ]; then
+  {
+    echo ""
+    echo "Error: logos-cpp-generator failed (exit status $status)."
+    echo "  plugin:    $plugin_file"
+    echo "  api-style: $api_style"
+    [ -n "$events_sidecar" ] && echo "  events:    $events_sidecar"
+    echo ""
+    echo "This is a build error, not the 'module has no public API' case: a"
+    echo "module without Q_INVOKABLE methods still generates a wrapper and"
+    echo "exits 0. Common causes of a non-zero status here:"
+    echo "  * the plugin cannot be dlopen'd by the generator (exit 3) — usually"
+    echo "    the module and logos-cpp-sdk are pinned to incompatible commits;"
+    echo "  * the SDK does not accept --api-style $api_style (exit 1) — again a"
+    echo "    pin mismatch between this backend and logos-cpp-sdk;"
+    echo "  * the built plugin is missing or unreadable (exit 2)."
+    echo "See the generator's own message above for which one it is."
+  } >&2
+  exit "$status"
+fi
+
+# Postcondition: a successful generator run wrote the per-module wrapper pair.
+shopt -s nullglob
+generated_headers=("$output_dir"/*_api.h)
+generated_sources=("$output_dir"/*_api.cpp)
+shopt -u nullglob
+
+if [ "${#generated_headers[@]}" -eq 0 ] || [ "${#generated_sources[@]}" -eq 0 ]; then
+  {
+    echo ""
+    echo "Error: logos-cpp-generator exited 0 but produced no client wrapper."
+    echo "  plugin:     $plugin_file"
+    echo "  api-style:  $api_style"
+    echo "  expected:   $output_dir/<module>_api.h and <module>_api.cpp"
+    echo "  contents of $output_dir:"
+    ls -la "$output_dir" >&2 || true
+    echo ""
+    echo "Refusing to install an empty header set — a downstream module would"
+    echo "fail much later, at link time, with a missing symbol."
+  } >&2
+  exit 1
+fi
+
+echo "Generated ${#generated_headers[@]} header(s) and ${#generated_sources[@]} source(s) in $output_dir"
diff --git a/tests/test-header-generator-guard.nix b/tests/test-header-generator-guard.nix
new file mode 100644
index 0000000..10428e0
--- /dev/null
+++ b/tests/test-header-generator-guard.nix
@@ -0,0 +1,197 @@
+# Regression test for lib/generate-module-headers.sh — the guard that decides
+# whether a logos-cpp-generator run is a build failure or a legitimate
+# "module has no public API" build.
+#
+# It drives the SAME script lib/buildHeaders.nix runs, so the guard under test
+# cannot drift from the guard the build uses. logos-cpp-generator itself is
+# stubbed: the stubs reproduce the statuses the real generator was measured to
+# produce in plugin-introspection mode (see the script's header comment), which
+# is exactly the contract this guard depends on.
+#
+# The bug this pins down: the generator's status used to be swallowed, so an
+# SDK-pin mismatch produced an empty header set and exit 0, and the failure
+# only surfaced later as a missing symbol in a downstream link.
+{ pkgs }:
+
+let
+  script = ../lib/generate-module-headers.sh;
+in
+pkgs.runCommand "logos-plugin-qt-header-generator-guard-test" {
+  nativeBuildInputs = [ pkgs.bash ];
+} ''
+  set -uo pipefail
+  # Failing runs are the point of this test — collect them, don't abort on them.
+  set +e
+
+  work="$PWD/work"
+  mkdir -p "$work"
+  failures=0
+
+  # Writes a stub `logos-cpp-generator` into its own bin dir and echoes the dir.
+  #   $1 = stub name, $2 = body
+  make_stub() {
+    local dir="$work/bin-$1"
+    mkdir -p "$dir"
+    printf '#!/usr/bin/env bash\n%s\n' "$2" > "$dir/logos-cpp-generator"
+    chmod +x "$dir/logos-cpp-generator"
+    echo "$dir"
+  }
+
+  # run <case> <stub-dir|""> <out-dir> [extra script args...]
+  # Records exit status in $status and combined output in $output.
+  run() {
+    local case="$1"; local bindir="$2"; local outdir="$3"; shift 3
+    rm -rf "$outdir"; mkdir -p "$outdir"
+    local path="$PATH"
+    [ -n "$bindir" ] && path="$bindir:$PATH"
+    output=$(PATH="$path" bash ${script} "$work/plugin.so" "$outdir" "$@" 2>&1)
+    status=$?
+    echo "--- case $case: exit $status"
+    echo "$output" | sed 's/^/    /'
+  }
+
+  check() {
+    if [ "$1" = "ok" ]; then
+      echo "  PASS: $2"
+    else
+      echo "  FAIL: $2"
+      failures=$((failures + 1))
+    fi
+  }
+
+  expect_status() {  # <expected> <actual> <what>
+    if [ "$2" = "$1" ]; then check ok "$3 (exit $2)"; else check no "$3: expected exit $1, got $2"; fi
+  }
+
+  expect_output() {  # <needle> <what>
+    case "$output" in
+      *"$1"*) check ok "$2" ;;
+      *)      check no "$2: output does not mention '$1'" ;;
+    esac
+  }
+
+  touch "$work/plugin.so"
+
+  ##########################################################################
+  # 1. A module with NO public API: the generator exits 0 and still writes a
+  #    wrapper (with zero methods). This must remain a successful build —
+  #    it is the case the historical `|| true` existed for.
+  ##########################################################################
+  noapi=$(make_stub noapi '
+    outdir=""
+    while [ $# -gt 0 ]; do case "$1" in --output-dir) outdir="$2"; shift 2;; *) shift;; esac; done
+    printf "class NoApi {};\n" > "$outdir/no_api_module_api.h"
+    printf "// no methods\n"   > "$outdir/no_api_module_api.cpp"
+    echo "Generated: $outdir/no_api_module_api.h and $outdir/no_api_module_api.cpp"
+    exit 0
+  ')
+  run "no-public-api" "$noapi" "$work/out1" qt
+  expect_status 0 "$status" "a module with no public API still builds"
+  if [ -f "$work/out1/no_api_module_api.h" ] && [ -f "$work/out1/no_api_module_api.cpp" ]; then
+    check ok "the wrapper pair is left in place for installPhase"
+  else
+    check no "the wrapper pair is left in place for installPhase"
+  fi
+
+  ##########################################################################
+  # 2. The generator genuinely fails (exit 3 = plugin could not be dlopen'd,
+  #    i.e. the mismatched-SDK-pin case). This must FAIL the build, loudly.
+  ##########################################################################
+  loadfail=$(make_stub loadfail '
+    echo "Failed to load plugin at /nix/store/xxx/lib/foo_plugin.so: undefined symbol" >&2
+    exit 3
+  ')
+  run "generator-failed" "$loadfail" "$work/out2" qt
+  expect_status 3 "$status" "an unloadable plugin fails the build"
+  expect_output "logos-cpp-generator failed" "the failure names the generator"
+  expect_output "pinned to incompatible commits" "the failure points at the likely cause"
+
+  ##########################################################################
+  # 3. The generator rejects the api style (exit 1 — e.g. a retired style
+  #    against a newer SDK). Must fail rather than yield empty headers.
+  ##########################################################################
+  badstyle=$(make_stub badstyle '
+    echo "Unknown --api-style value" >&2
+    exit 1
+  ')
+  run "bad-api-style" "$badstyle" "$work/out3" std
+  expect_status 1 "$status" "a rejected --api-style fails the build"
+  expect_output "api-style: std" "the failure reports the api style it used"
+
+  ##########################################################################
+  # 4. Exit 0 with nothing written: still a failure — an empty include/ is
+  #    what made the original bug invisible until a downstream link broke.
+  ##########################################################################
+  silent=$(make_stub silent 'exit 0')
+  run "silent-empty" "$silent" "$work/out4" qt
+  if [ "$status" -ne 0 ]; then check ok "a zero exit with no output fails the build (exit $status)";
+  else check no "a zero exit with no output fails the build: got exit 0"; fi
+  expect_output "produced no client wrapper" "the failure explains the empty output"
+
+  ##########################################################################
+  # 5. Half the pair written (header, no source) is also incomplete.
+  ##########################################################################
+  halfway=$(make_stub halfway '
+    outdir=""
+    while [ $# -gt 0 ]; do case "$1" in --output-dir) outdir="$2"; shift 2;; *) shift;; esac; done
+    printf "class Half {};\n" > "$outdir/half_api.h"
+    exit 0
+  ')
+  run "half-output" "$halfway" "$work/out5" qt
+  if [ "$status" -ne 0 ]; then check ok "a missing _api.cpp fails the build (exit $status)";
+  else check no "a missing _api.cpp fails the build: got exit 0"; fi
+
+  ##########################################################################
+  # 6. Generator absent from PATH: a clear message, not "command not found".
+  ##########################################################################
+  run "generator-missing" "" "$work/out6" qt
+  if [ "$status" -ne 0 ]; then check ok "a missing generator fails the build (exit $status)";
+  else check no "a missing generator fails the build: got exit 0"; fi
+  expect_output "not on PATH" "the failure says the generator is missing"
+
+  ##########################################################################
+  # 7. Argument forwarding: --module-only, the requested --api-style and the
+  #    events sidecar must reach the generator (an api-style regression is
+  #    how the original empty-header report was produced in the first place).
+  ##########################################################################
+  recorder=$(make_stub recorder '
+    outdir=""
+    args="$@"
+    while [ $# -gt 0 ]; do case "$1" in --output-dir) outdir="$2"; shift 2;; *) shift;; esac; done
+    echo "$args" > "$outdir/argv.txt"
+    printf "h\n" > "$outdir/rec_api.h"; printf "c\n" > "$outdir/rec_api.cpp"
+    exit 0
+  ')
+  run "argv-with-events" "$recorder" "$work/out7" lp "$work/rec.lidl"
+  expect_status 0 "$status" "the forwarding case builds"
+  argv=$(cat "$work/out7/argv.txt")
+  echo "    argv: $argv"
+  for needle in "--module-only" "--api-style lp" "--events-from $work/rec.lidl"; do
+    case "$argv" in
+      *"$needle"*) check ok "argv carries $needle" ;;
+      *)           check no "argv is missing $needle" ;;
+    esac
+  done
+
+  run "argv-without-events" "$recorder" "$work/out8" qt
+  expect_status 0 "$status" "the no-sidecar case builds"
+  argv=$(cat "$work/out8/argv.txt")
+  echo "    argv: $argv"
+  case "$argv" in
+    *--events-from*) check no "argv must not carry --events-from without a sidecar" ;;
+    *)               check ok "argv omits --events-from when there is no sidecar" ;;
+  esac
+  case "$argv" in
+    *"--api-style qt"*) check ok "argv carries --api-style qt" ;;
+    *)                  check no "argv is missing --api-style qt" ;;
+  esac
+
+  echo ""
+  if [ "$failures" -ne 0 ]; then
+    echo "FAILED: $failures assertion(s)"
+    exit 1
+  fi
+  echo "All header-generator guard assertions passed"
+  mkdir -p $out
+  echo "ok" > $out/result.txt
+''