Skip to content
This repository was archived by the owner on May 1, 2026. It is now read-only.

ci: lint + cross-platform build + sanitizers + ARM64 monitor#6

Merged
justinjoy merged 7 commits intomainfrom
ci/workflows
Apr 30, 2026
Merged

ci: lint + cross-platform build + sanitizers + ARM64 monitor#6
justinjoy merged 7 commits intomainfrom
ci/workflows

Conversation

@justinjoy
Copy link
Copy Markdown
Contributor

Summary

Adds the four GitHub Actions workflows the team agreed on, modelled
on ../wirelog/.github/workflows:

File Trigger Style
lint-pr.yml workflow_call hard gates: gst-indent → clang-tidy
lint-main.yml workflow_call non-blocking monitor (parallel)
ci-pr.yml pull_request → main lint → build matrix → sanitizers
ci-main.yml push → main full matrix + ARM64 + footprint

PR build matrix: ubuntu-latest × {gcc, clang} + macos-latest × clang + windows-latest × msvc.
The main-monitor matrix adds ubuntu-24.04-arm × gcc so the NEON SIMD path actually gets exercised; x86_64 lanes only ever build the SSE2 kernel.

What gates a PR

  1. gst-indent must produce zero working-tree diff (mirrors the local pre-commit hook).
  2. clang-tidy 18 must report zero warnings/errors on libksuid/*.{c,h}.
  3. Build + meson test must pass on all four PR matrix platforms.
  4. DESTDIR install check on the Linux GCC lane asserts that LICENSE, LICENSE.MIT, NOTICE, libksuid.pc, and libksuid/ksuid.h all land in the staging tree (catches the silent license-drop bug the meta-review flagged).
  5. ASan + UBSan sanitizer test pass on Linux GCC/Clang and macOS Clang.

TSan is intentionally omitted — libksuid is built around _Thread_local + <stdatomic.h> with no pthread, and TSan does not instrument C11 <threads.h> cleanly.

What main-monitor reports

Same checks as the PR gate, plus:

  • An ARM64 lane (ubuntu-24.04-arm) that exercises NEON.
  • A footprint table (stripped release-build sizes of libksuid.so.* / libksuid.a / ksuid-gen) emitted into the step summary on every push, so any regression against the README's 18 KB claim is visible.

All ci-main jobs run with continue-on-error: true so a flaky runner never blocks a merge that already landed.

Test plan

This PR is its own dogfood: merging it requires the workflows it adds to pass on this PR.

  • lint-pr.yml: gst-indent green, clang-tidy 18 green
  • ci-pr.yml: build matrix green on Linux GCC, Linux Clang, macOS Clang, Windows MSVC
  • ci-pr.yml: DESTDIR install asserts pass on Linux GCC
  • ci-pr.yml: sanitizer phase green on Linux GCC, Linux Clang, macOS Clang

Adapts the wirelog .github/workflows layout to libksuid. Four files,
two pairs of "PR (hard gate) vs main (non-blocking monitor)".

  .github/workflows/lint-pr.yml      hard-gated, called by ci-pr
  .github/workflows/lint-main.yml    non-blocking, called by ci-main
  .github/workflows/ci-pr.yml        on pull_request -> main
  .github/workflows/ci-main.yml      on push -> main

Lint phase enforces what the pre-commit hook (hooks/pre-commit.hook)
already runs locally:

  - gst-indent: GNU indent under tools/gst-indent must produce no
    working-tree diff. Cheap, runs first, fails fast.
  - clang-tidy 18: any warning/error on libksuid/*.{c,h} blocks the
    PR. Driven from compile_commands.json that meson generates.

Build/test matrix on PR:

  ubuntu-latest x { gcc, clang }
  macos-latest  x clang
  windows-latest x msvc (cl.exe via vcvars64)

The Linux GCC lane additionally runs DESTDIR meson install and asserts
that LICENSE / LICENSE.MIT / NOTICE / libksuid.pc / libksuid/ksuid.h
all land in the staging tree -- catching the kind of silent
install-data drop the Critic flagged on the original meson.build.

Sanitizers (ASan + UBSan) run as Phase 3 after the build matrix
clears, on Linux GCC/Clang and macOS Clang. TSan is intentionally
omitted: libksuid is built around _Thread_local + stdatomic with no
pthread, and TSan does not instrument C11 <threads.h> cleanly.

The ci-main.yml monitor adds an ubuntu-24.04-arm lane so the NEON
SIMD path actually gets exercised under CI -- the x86_64 lanes only
build the SSE2 kernel. It also emits a footprint table to the step
summary (stripped sizes of libksuid.so.* / libksuid.a / ksuid-gen)
so any size regression shows up against the README's 18 KB claim.

All ci-main jobs run with continue-on-error: true so a flaky runner
never blocks a merge that already landed.
Wirelog's workflow templates pinned LLVM 18 because that was the
current stable when wirelog adopted them. As of 2026-04-30 the apt.
llvm.org noble repo publishes packages for LLVM 17, 18, 19, 20, 21,
and 22, and LLVM 22's Release index is dated 2026-04-03 -- a four-
week-old stable. Two-year-old static-analysis output is not what we
want to gate PRs on.

Updates both lint-pr.yml (hard gate) and lint-main.yml (monitor):

  - apt source line: llvm-toolchain-noble-18 -> -22
  - packages:        clang-tidy-18 / clang-tools-18 -> -22
  - tool invocation: run-clang-tidy-18 -> run-clang-tidy-22
  - human-facing names + comments: "LLVM 18" / "clang-tidy 18" ->
    "LLVM 22" / "clang-tidy 22"

No behaviour change beyond the version bump itself; the regex that
restricts findings to libksuid/*.{c,h} and the workflow_call
plumbing into ci-pr.yml / ci-main.yml are untouched. If LLVM 22
introduces new default checks that flag legitimate-but-known
patterns we will add targeted // NOLINTNEXTLINE comments alongside
the fix, rather than disable checks globally.
GNU indent 2.2.13 exits with 64 (sysexits EX_USAGE) when invoked
with --version, even though it prints the version banner first
("GNU indent 2.2.13"). This is a long-standing quirk of the
upstream code that ships unchanged in every Linux distro I can
test -- including the indent_2.2.13-4build1_amd64.deb that
ubuntu-latest installs.

GitHub Actions runs every step under `bash -e {0}`, so the
non-zero exit terminated the "Install GNU indent" step before the
"Run gst-indent (hard gate)" step ever started. Result: every PR
checks failed at lint phase 1a with no useful diagnostic, and the
workflow looked broken when the formatter itself was working
fine.

Fix: append `|| true` to the diagnostic version probe in both
lint-pr.yml and lint-main.yml. The version banner still lands in
the step log; we just no longer take an avoidable failure on a
purely informational invocation.
meson 1.11.x on the local toolchain accepts File.full_path(), but
the meson the GitHub-hosted ubuntu-latest installs from apt is older
and rejects it:

  tests/meson.build:32:36: ERROR: Unknown method "full_path" in
  object <[FileHolder] holds [File]: <File: tests/test_cli.sh
  (not built)>> of type FileHolder.

This took down the build matrix at the meson setup step on every
PR matrix lane.

Fix: pass the File and the ksuid_gen Executable directly into
test() args without calling .full_path(). meson resolves File
objects and build targets to absolute paths when they appear in
args -- the explicit conversion was unnecessary and breaks under
older meson. Verified locally that test_cli still passes after the
change.
clang-tidy 22 (vs the 18 we previously pinned) ships with no
implicit default check selection -- without an explicit Checks list
or a .clang-tidy file the tool emits

  No checks enabled.
  Unable to run clang-tidy.

and exits 1, which took down the lint phase on every PR matrix lane.

This commit lands a project-root .clang-tidy with a moderate, well-
known-low-noise selection (clang-analyzer + bugprone + cert +
performance + portability) and the small set of exclusions a C
codebase always wants:

  -bugprone-easily-swappable-parameters
      Notorious false-positive on otherwise-fine APIs.
  -cert-err33-c, -cert-msc24-c, -cert-msc33-c
      Flag every fprintf / strtok / asctime call without offering
      actionable advice on a CSPRNG-backed library.
  -clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling
      Recommends C11 Annex K _s functions (fprintf_s, memcpy_s,
      ...) that are not implemented on Linux glibc, MUSL, macOS,
      or Windows MSVC without an opt-in flag. 409 of the 412
      warnings clang-tidy 22 produced for libksuid were this
      single check; almost zero were actionable.

After excluding the noise three real findings remained, all fixed
in this commit:

  libksuid/base62_sse2.c:30,56  -- cast-align on _mm_loadu_si128
                                   and _mm_storeu_si128. These are
                                   the SSE2 *unaligned* load/store
                                   intrinsics by spec. Suppressed
                                   with NOLINTNEXTLINE +
                                   one-line rationale.
  examples/ksuid-gen.c:150      -- bugprone-switch-missing-default-
                                   case on the FMT_* dispatch.
                                   Added an explicit default that
                                   documents why no other format
                                   value ever reaches print_one.

Verified locally with clang-tidy 22.1.3: 0 findings. The lint hard
gate's grep regex now finds nothing on the local run, so CI should
clear the same way.
Lint phase cleared on the previous push; the build matrix exposed
four distinct platform-specific failures, all addressed here:

[1] Ubuntu GCC + Ubuntu Clang -- DESTDIR install verification

13/13 tests passed but the workflow's hard-coded test paths
(staging/usr/local/lib/pkgconfig/libksuid.pc) miss Debian's
multiarch layout where everything under ${libdir} expands to
lib/x86_64-linux-gnu/. Switch to a `find ... -print | grep -q .`
check that succeeds against either flat or multiarch trees.

[2] macOS Clang -- <threads.h> fatal: 'threads.h' file not found

Apple Clang's libc has never shipped C11 <threads.h> through at
least Xcode 16. The threaded-isolation test for the per-thread
CSPRNG was the only consumer; the rest of the library uses
_Thread_local + <stdatomic.h> directly. Detect <threads.h> in
tests/meson.build via cc.has_header() and only register
test_rand_tls when the header is present. Falling back to pthread
just for one test is not worth the dependency.

[3] Windows MSVC -- C atomic support is not enabled

vcruntime_c11_stdatomic.h(12) emits #error "C atomic support is
not enabled" without /experimental:c11atomics. Microsoft still
gates C11 atomics behind that flag in MSVC 19.44 (current). Add
it to common_args under cc.get_argument_syntax() == 'msvc',
alongside split-out POSIX feature macros and visibility flags
that only mean anything on the gcc-style command line. The
warning probes (-Wshadow et al) are similarly skipped on MSVC
where they would just emit D9002 noise.

[4] Windows MSVC -- clock_gettime undefined

This was supposed to land in commit 45be111 alongside the
KSUID_GETPID() macro, but the Edit that replaced
clock_gettime(CLOCK_REALTIME, ...) with timespec_get(TIME_UTC)
silently failed at the time and the file kept the original call.
Re-apply the same fix:
  - rand_tls.c ksuid_now_seconds returns -1 on clock failure
    (fail-closed; forces reseed via the should_reseed predicate).
  - seed_pid widened from `long` to int64_t per the same commit's
    rationale.
  - struct field comments updated to mention TIME_UTC instead of
    CLOCK_REALTIME.

All 13 tests still pass on Linux GCC after the meson.build
restructuring; clang-tidy 22 still reports zero findings.
Last build-matrix holdout. The ksuid-gen CLI included <unistd.h> for
POSIX getopt(3); MSVC has neither the header nor the function, so

  examples/ksuid-gen.c(25): fatal error C1083: Cannot open include
  file: 'unistd.h': No such file or directory

took down the windows-latest / msvc lane while every other matrix
slot now passes (Ubuntu GCC, Ubuntu Clang, macOS Clang).

Replace the getopt loop with a hand-rolled option parser. The CLI
exposes only four flags (-n N, -f FMT, -v, -h) with values supplied
as separate tokens -- the same shape upstream Go ksuid uses and the
exact set tests/test_cli.sh exercises -- so a six-case while loop is
shorter than any portable getopt shim. The header drops -unistd.h-
entirely, so MSVC, every libc on Linux, and Apple Clang all build
the same translation unit.

The new parser intentionally does NOT support combined short
options (-vh) or attached values (-n4). Both are unidiomatic for
this CLI and not required by the integration test; if a future
flag wants either spelling, that is the time to vendor a real
getopt implementation.

Verified locally: 13/13 tests still pass, including test_cli's
full coverage of -f inspect / -f raw / -f payload / -v / parse-
mode / size-error / unknown-format paths.
@justinjoy justinjoy merged commit 16edeb0 into main Apr 30, 2026
9 checks passed
@justinjoy justinjoy deleted the ci/workflows branch April 30, 2026 04:57
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant