From 172b20374e4a976cea3058b54bb806eff2733524 Mon Sep 17 00:00:00 2001 From: Mohamed Ibrahim Date: Sun, 14 Jun 2026 15:36:51 +0100 Subject: [PATCH] Enforce :sharingan/:sharingan-noop public API parity in CI (#12) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The release-swap safety story (consumer compiles against :sharingan in debug, swaps :sharingan-noop in release) depends on the two independently-compiled modules exposing a signature-for-signature identical public contract. Until now nothing enforced that — a drifted default or a forgotten method would surface only as a consumer's release-build compile error. Add a `checkApiParity` Gradle task (root build) that compares the four committed BCV dumps (JVM .api + native .klib.api, both modules) after filtering out the symbols that legitimately exist in only the real module, and fails on any bidirectional divergence: - debug-only UI/platform classes the consumer never references directly (dev/sharingan/ui/**, internal/**, SharinganActivity, ComposableSingletons$*, and the native ui SharinganScreen composable); - the Compose-compiler $stable field / $stableprop synthetics that only the Compose-carrying real module emits — the gotcha that would false-fail a naive diff since they live inside otherwise-shared classes; - the klib `// Library unique name` header line, which differs by construction. After filtering, both modules' 19 JVM contract classes (+ native SharinganViewController/presentSharingan) must match exactly. The check is pure text processing over committed files — no compilation, no iOS link — so it runs in seconds with no Android/iOS SDK. Wiring: - registered in the `verification` group; runnable as `./gradlew checkApiParity` - hooked into the root `check` lifecycle - new `api-parity` CI job in api-check.yml on ubuntu (no macOS host needed) Verified: red→green demonstrated on both surfaces (dropping a contract symbol from a noop dump fails the check with a precise per-symbol diff; reverting passes). `./gradlew apiCheck` confirms the committed dumps are current. Docs: docs/api-parity.md explains the contract, the why, and every exclusion so contributors keep the modules in lockstep; README contributing section points to it. Implementation note: the comparison helpers are local functions inside the task action because Gradle Kotlin DSL silently drops top-level statements placed after top-level `fun` declarations. Co-Authored-By: Claude Opus 4.8 (1M context) --- .github/workflows/api-check.yml | 29 +++++- README.md | 7 ++ build.gradle.kts | 170 ++++++++++++++++++++++++++++++++ docs/api-parity.md | 132 +++++++++++++++++++++++++ 4 files changed, 335 insertions(+), 3 deletions(-) create mode 100644 docs/api-parity.md diff --git a/.github/workflows/api-check.yml b/.github/workflows/api-check.yml index a93ee3e..744f027 100644 --- a/.github/workflows/api-check.yml +++ b/.github/workflows/api-check.yml @@ -1,9 +1,14 @@ name: Public API check # Guards the committed public-API dumps (sharingan/api, sharingan-noop/api). -# apiCheck fails the build when the public surface drifts from the dumps, so an -# accidental signature change cannot slip in between releases — the "swap -# sharingan-noop in release" safety story depends on the API staying stable. +# +# Two complementary gates live here: +# - api-check : apiCheck fails when a module's surface drifts from its own +# committed dump (an accidental signature change between releases). +# - api-parity : checkApiParity fails when :sharingan and :sharingan-noop drift +# from EACH OTHER, i.e. they no longer expose an identical public contract +# (issue #12). This is what makes the "swap sharingan-noop in release" safety +# story enforceable rather than dependent on human discipline. # Regenerate intentional changes locally with `./gradlew apiDump` and commit. on: pull_request: @@ -28,3 +33,21 @@ jobs: - name: Check public API against committed dumps run: ./gradlew apiCheck + + api-parity: + # Cross-module parity is a pure text comparison of the four committed dumps — + # no Android/iOS SDK, no compilation, no Kotlin/Native link — so it runs on + # cheap ubuntu for fast, independent feedback (it does NOT need the macOS host + # the api-check job above requires). + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + + - name: Set up JDK 17 + uses: actions/setup-java@v5 + with: + distribution: temurin + java-version: "17" + + - name: Assert :sharingan and :sharingan-noop expose an identical public contract + run: ./gradlew checkApiParity diff --git a/README.md b/README.md index 5ab0641..5a46980 100644 --- a/README.md +++ b/README.md @@ -97,6 +97,13 @@ git clone https://github.com/mibrahimdev/Sharingan && cd Sharingan ./gradlew publishToMavenLocal ``` +Contributors: `:sharingan` and `:sharingan-noop` must expose an **identical +public API** so the debug→release swap is safe. `./gradlew checkApiParity` (run in +CI) enforces this and fails on any drift — see +[`docs/api-parity.md`](docs/api-parity.md) for the contract and why a few debug-only +symbols are excluded. After an intentional API change, run `./gradlew apiDump` and +commit the regenerated dumps. + Maintainers: cutting a Maven Central release is a two-step, stage-then-manually-release flow — see [`docs/RELEASING.md`](docs/RELEASING.md). diff --git a/build.gradle.kts b/build.gradle.kts index 8e8d19e..a7fbea9 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -24,3 +24,173 @@ apiValidation { enabled = true } } + +// --------------------------------------------------------------------------- +// Cross-module API-parity gate (issue #12). +// +// The "swap :sharingan-noop in release" safety story depends on the two modules +// exposing a SIGNATURE-FOR-SIGNATURE identical public CONTRACT. They are compiled +// independently, so the only thing keeping them in lockstep is human discipline: +// a drifted default or a forgotten method would surface only as a *consumer's +// release-build compile error* — the worst place to find it. +// +// BCV (#11) already dumps each module's full public surface to api/*.api (JVM) and +// api/*.klib.api (native/iOS). This task compares those committed dumps and FAILS +// the build on any divergence. It is pure text processing over the four files — +// no compilation, no iOS link — so it runs in seconds and needs no Android/iOS SDK. +// +// It is NOT a whole-file diff: :sharingan legitimately carries debug-only symbols +// the noop must not (and cannot) expose. Those are filtered out before comparing — +// see the local helpers and docs/api-parity.md for the rationale behind each +// exclusion. (The helpers are local to `doLast` on purpose: Gradle Kotlin DSL drops +// top-level statements placed after top-level `fun` declarations, so the comparison +// logic lives inside the task action rather than as script-level functions.) +// --------------------------------------------------------------------------- + +tasks.register("checkApiParity") { + group = "verification" + description = + "Asserts :sharingan and :sharingan-noop expose an identical public API contract (issue #12)." + + val realApi = layout.projectDirectory.file("sharingan/api/sharingan.api").asFile + val noopApi = layout.projectDirectory.file("sharingan-noop/api/sharingan-noop.api").asFile + val realKlib = layout.projectDirectory.file("sharingan/api/sharingan.klib.api").asFile + val noopKlib = layout.projectDirectory.file("sharingan-noop/api/sharingan-noop.klib.api").asFile + + // Re-run only when one of the committed dumps changes. + inputs.files(realApi, noopApi, realKlib, noopKlib) + + doLast { + // Debug-only JVM classes the real module ships but a consumer never references + // directly, so :sharingan-noop correctly omits them. Excluded from parity: + // - dev/sharingan/ui/** : the in-app Compose viewer screens + // - dev/sharingan/internal/** : init ContentProvider + notification receiver + // - dev/sharingan/SharinganActivity : the Android viewer Activity + // - any ComposableSingletons$* : Compose-compiler lambda synthetics + fun isExcludedJvmClass(name: String): Boolean = + name.startsWith("dev/sharingan/ui/") || + name.startsWith("dev/sharingan/internal/") || + name == "dev/sharingan/SharinganActivity" || + name.contains("ComposableSingletons\$") + + // Reduce a JVM `.api` dump to the consumer-facing contract as a set of + // qualified signatures. Drops the excluded classes wholesale and strips the + // Compose-compiler `public static final field $stable I` line that only the + // real module carries (Compose ships only in :sharingan). Each surviving + // member is keyed by its enclosing class so an identical signature in a + // different class cannot mask a divergence. + fun jvmContractEntries(apiFile: File): Set { + val lines = apiFile.readLines() + val entries = linkedSetOf() + var i = 0 + while (i < lines.size) { + val line = lines[i] + // Class/interface headers sit at column 0; members are tab-indented + // and the block closes with a lone "}". Skip non-header lines. + if (line.isBlank() || line.first().isWhitespace() || line == "}") { + i++ + continue + } + val className = + if (" class " in line) line.substringAfter(" class ").substringBefore(" ") else "" + val block = mutableListOf(line) + var j = i + 1 + while (j < lines.size && lines[j] != "}") { + block.add(lines[j]) + j++ + } + i = j + 1 + if (className.isEmpty() || isExcludedJvmClass(className)) continue + // Keep the header verbatim (sans trailing " {") so a changed supertype is caught. + entries.add("CLASS " + line.removeSuffix(" {").trim()) + for (member in block.drop(1)) { + val m = member.trim() + if (m.isEmpty() || "\$stable" in m) continue + entries.add("$className :: $m") + } + } + return entries + } + + // Reduce a native `.klib.api` dump to the consumer-facing contract. Each BCV + // declaration carries a globally-unique signature-id trailing comment, so the + // trimmed line is unique and used as-is. Dropped: + // - the `//` header block, incl. `// Library unique name: <…:sharingan[-noop]>` + // which differs by module name and would always false-fail a raw diff; + // - `$stableprop` / `$stableprop_getter`: the native equivalent of the JVM + // `$stable` field, present only in the Compose-carrying real module; + // - `dev.sharingan.ui/SharinganScreen`: the top-level UI composable (ui/**). + fun nativeContractEntries(klibFile: File): Set { + val entries = linkedSetOf() + for (raw in klibFile.readLines()) { + val t = raw.trim() + if (t.isEmpty() || t == "}" || t.startsWith("//")) continue + if ("\$stableprop" in raw || "dev.sharingan.ui/" in raw) continue + entries.add(t) + } + return entries + } + + // Human-readable mismatch report for one surface, or null when the two + // contracts are identical. Compared both directions so a symbol the noop is + // MISSING and a symbol it EXPOSES EXTRA are both reported. + fun diffSurface( + surface: String, + realPath: String, + noopPath: String, + realEntries: Set, + noopEntries: Set, + ): String? { + val missingFromNoop = (realEntries - noopEntries).sorted() + val extraInNoop = (noopEntries - realEntries).sorted() + if (missingFromNoop.isEmpty() && extraInNoop.isEmpty()) return null + return buildString { + appendLine("API PARITY MISMATCH on the $surface surface") + appendLine(" :sharingan dump: $realPath") + appendLine(" :sharingan-noop dump: $noopPath") + if (missingFromNoop.isNotEmpty()) { + appendLine(" In :sharingan but MISSING from :sharingan-noop") + appendLine(" (consumer code compiled against debug would FAIL to compile after a release swap):") + missingFromNoop.forEach { appendLine(" - $it") } + } + if (extraInNoop.isNotEmpty()) { + appendLine(" In :sharingan-noop but NOT in :sharingan") + appendLine(" (the noop over-exposes surface the real module lacks — equally a contract break):") + extraInNoop.forEach { appendLine(" + $it") } + } + } + } + + val problems = listOfNotNull( + diffSurface( + "Android/JVM (.api)", + "sharingan/api/sharingan.api", + "sharingan-noop/api/sharingan-noop.api", + jvmContractEntries(realApi), + jvmContractEntries(noopApi), + ), + diffSurface( + "native/iOS (.klib.api)", + "sharingan/api/sharingan.klib.api", + "sharingan-noop/api/sharingan-noop.klib.api", + nativeContractEntries(realKlib), + nativeContractEntries(noopKlib), + ), + ) + if (problems.isNotEmpty()) { + throw GradleException( + problems.joinToString("\n\n") + "\n\n" + + "The release-swap safety story requires :sharingan and :sharingan-noop to\n" + + "expose a signature-for-signature identical public contract. Reconcile the\n" + + "lagging module (add/remove the symbol), run `./gradlew apiDump`, and commit\n" + + "the regenerated dumps. See docs/api-parity.md for the contract and rationale.", + ) + } + logger.lifecycle( + "API parity OK: :sharingan and :sharingan-noop expose an identical public contract (JVM + native).", + ) + } +} + +// Make the standard `check` lifecycle run the parity gate locally when present. +tasks.matching { it.name == "check" }.configureEach { dependsOn("checkApiParity") } diff --git a/docs/api-parity.md b/docs/api-parity.md new file mode 100644 index 0000000..c4a8409 --- /dev/null +++ b/docs/api-parity.md @@ -0,0 +1,132 @@ +# Cross-module API parity + +`:sharingan` (the real debug library) and `:sharingan-noop` (the inert release +replacement) **must expose a signature-for-signature identical public contract.** +This document explains why, how the `checkApiParity` gate enforces it, and — most +importantly for contributors — *why a handful of symbols are deliberately excluded +from the comparison* so you don't "fix" a false positive by breaking the contract. + +## Why parity matters + +The whole point of the two-module design is the **release swap**: a consumer +compiles against `:sharingan` in debug builds and substitutes `:sharingan-noop` in +release builds (typically `debugImplementation` / `releaseImplementation`). For +that swap to be safe, every symbol the consumer can reference in debug must also +exist — with an identical signature — in the noop. + +The two modules are compiled **independently** from separate sources. Nothing but +human discipline keeps them in lockstep. A drifted default value, a renamed +parameter, or a method added to one module and forgotten in the other does not +fail either module's own build. It surfaces only later, as a **consumer's +release-build compile error** — the worst possible place to discover it, on +someone else's machine, at release time. + +`checkApiParity` turns that latent, human-dependent invariant into an automated +gate that fails fast in CI on every PR. + +## How the check works + +[Binary Compatibility Validator](https://github.com/Kotlin/binary-compatibility-validator) +(BCV, wired up in #11) dumps each module's full public surface to committed files: + +| Surface | `:sharingan` | `:sharingan-noop` | +| ---------------- | -------------------------------- | ------------------------------------------ | +| Android / JVM | `sharingan/api/sharingan.api` | `sharingan-noop/api/sharingan-noop.api` | +| Native / iOS | `sharingan/api/sharingan.klib.api`| `sharingan-noop/api/sharingan-noop.klib.api`| + +The `checkApiParity` Gradle task (defined in the root `build.gradle.kts`) reads +those four committed dumps, reduces each to its **consumer-facing contract** (by +filtering — see below), and asserts the real and noop contracts are identical for +*both* surfaces. The comparison is **bidirectional**: it fails if the noop is +*missing* a symbol the real module has, *and* if the noop *exposes* a symbol the +real module lacks. Both are contract breaks. + +It is pure text processing over already-committed files — no compilation, no +Kotlin/Native link, no Android or iOS SDK — so it runs in a couple of seconds. + +```bash +./gradlew checkApiParity +``` + +In CI it runs as the `api-parity` job in +[`.github/workflows/api-check.yml`](../.github/workflows/api-check.yml), on a cheap +`ubuntu` runner (it needs no macOS host). + +## Why it is NOT a whole-file diff + +A naïve `diff` of the committed dumps would fail constantly, because `:sharingan` +legitimately carries symbols the noop neither has nor should have. The check +filters these out before comparing. **If you hit a parity failure, first confirm +it is a real contract change and not one of the categories below.** + +### 1. Debug-only UI / platform classes (real module only) + +`:sharingan` ships an in-app log viewer and the Android plumbing to launch it. +None of it is part of the API a consumer calls directly, so the noop correctly +omits it. Excluded from the JVM comparison: + +- `dev/sharingan/ui/**` — the Compose viewer screens (`SharinganScreenKt`, etc.). +- `dev/sharingan/internal/**` — `SharinganInitProvider` (the auto-init + `ContentProvider`) and `SharinganNotificationReceiver`. +- `dev/sharingan/SharinganActivity` — the Android Activity that hosts the viewer. +- any `ComposableSingletons$*` class — synthetic lambda holders generated by the + Compose compiler. + +On the native surface the only counterpart is the top-level +`dev.sharingan.ui/SharinganScreen` composable, which is excluded for the same +reason. (`SharinganViewController` and `presentSharingan` are **not** excluded — +those *are* the iOS entry points the noop must mirror, and it does.) + +### 2. The Compose `$stable` / `$stableprop` synthetics + +The Compose compiler adds stability metadata to every class it sees. Because +Compose ships **only in `:sharingan`**, the real dumps carry symbols the noop +never will, even for classes that are otherwise part of the shared contract +(e.g. `HttpEvent`): + +- JVM: a `public static final field $stable I` line inside each class. +- Native: top-level `…$stableprop` values and `…$stableprop_getter` functions. + +These are compiler bookkeeping, not API a consumer references, so they are +stripped before comparing. **This is the subtle one** — it lives *inside* classes +that are otherwise shared, so a naïve diff would false-fail on every event type. + +### 3. The klib header line + +Each `.klib.api` begins with a comment block including +`// Library unique name: ` vs `…:sharingan-noop`. +That line differs by construction. All `//` header comments are dropped before +comparing. + +## The contract that MUST stay in lockstep + +After filtering, both modules expose the same 19 JVM contract classes (plus the +native top-level `SharinganViewController` / `presentSharingan` functions): + +`Sharingan`, `HttpLogger` (+`Companion`), `MqttLogger`, `BleLogger`, +`SharinganStore` (+`Companion`), `SharinganEvent` (+`DefaultImpls`), `HttpEvent`, +`MqttEvent`, `BleEvent`, `TimingPhase`, `BleOperation`, `MqttDirection`, +`SharinganExport`, `SharinganAndroidKt`, `ktor.SharinganKtorConfig`, +`ktor.SharinganKtorKt`. + +Anything a consumer can reach lives here, and any change to it must be mirrored in +**both** modules. + +## What to do when `checkApiParity` fails + +1. Read the failure. It lists each diverging symbol, qualified by class, under + "MISSING from :sharingan-noop" (real has it, noop doesn't) or "NOT in + :sharingan" (noop over-exposes it). +2. Decide which module is correct. Usually you changed `:sharingan`'s public API + and forgot to make the same change in `:sharingan-noop` (or vice versa). Make + the matching change to the lagging module's source. +3. Regenerate the dumps and commit them: + ```bash + ./gradlew apiDump + ``` +4. Re-run `./gradlew checkApiParity` to confirm it passes. + +If the failure is genuinely one of the excluded categories above leaking through +(e.g. a new debug-only package), update the filter in the `checkApiParity` task +*and* document the new exclusion here — never widen the noop to match a debug-only +symbol.