Skip to content

Commit 49124e4

Browse files
build: add public API dumps with binary-compatibility-validator (#843)
Applies kotlinx binary-compatibility-validator to android-core and android-kit-base and commits the generated API dumps. `./gradlew apiCheck` now runs in the Unit Tests job and fails when the compiled public surface differs from the committed dump, so every change to the surface is visible in the pull request diff. A small classifier marks changes to frozen contracts: any class outside com.mparticle.internal, plus the internal classes that kits compile against or that android-core/proguard.pro names. Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
1 parent eb2b44b commit 49124e4

9 files changed

Lines changed: 4269 additions & 0 deletions

File tree

‎.github/workflows/pull-request.yml‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,20 @@ jobs:
9494
uses: gradle/actions/setup-gradle@3f5f9adaf7d9fecd50b5935e54106014257a94e6 # v6.4.0
9595
- name: "Run Unit Tests"
9696
run: ./gradlew test
97+
- name: "Check the public API dumps"
98+
run: ./gradlew apiCheck
99+
- name: "Fetch the base branch"
100+
if: github.event_name == 'pull_request'
101+
env:
102+
BASE_REF: ${{ github.base_ref }}
103+
run: git fetch --no-tags --depth=1 origin "$BASE_REF"
104+
- name: "Classify public API changes"
105+
if: >
106+
github.event_name == 'pull_request' &&
107+
!contains(github.event.pull_request.labels.*.name, 'api-change-approved')
108+
env:
109+
BASE_REF: ${{ github.base_ref }}
110+
run: python3 scripts/check_api_dump.py --base "origin/$BASE_REF"
97111
- name: "Print Android Unit Tests Report"
98112
uses: asadmansr/android-test-report-action@384cd31388782f4106dc4a1b37eea2ff02e0aad7 #v1.2.0
99113
if: always()

‎AGENTS.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,10 @@ JDK 17 — `gradle.properties` sets `JAVA_VERSION` and every CI job installs Zul
4646
`JAVA_HOME` section still tells you to install Java 11; ignore that part of it.
4747

4848
- Core + tooling unit tests — `./gradlew test`
49+
- Public API dumps — `./gradlew apiCheck` fails when the compiled surface of `android-core` or
50+
`android-kit-base` differs from `*/api/*.api`; regenerate with `./gradlew apiDump` after an
51+
intentional change and explain the diff in the PR. `scripts/check_api_dump.py --base origin/main`
52+
tells you whether a changed class is a frozen contract (see `scripts/api-frozen-internals.txt`).
4953
- Android lint — `./gradlew lint`; Kotlin lint — `./gradlew ktlintCheck`
5054
- Instrumented tests — `./gradlew :android-core:cAT :android-kit-base:cAT --stacktrace`, needs an
5155
API 28 emulator

‎android-core/api/android-core.api‎

Lines changed: 3527 additions & 0 deletions
Large diffs are not rendered by default.

‎android-core/build.gradle‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,12 @@
11
apply plugin: 'com.android.library'
22
apply plugin: 'mparticle.android.library.publish'
33
apply plugin: 'kotlin-android'
4+
apply plugin: 'org.jetbrains.kotlinx.binary-compatibility-validator'
5+
6+
apiValidation {
7+
ignoredClasses.add('com.mparticle.BuildConfig')
8+
nonPublicMarkers.add('androidx.annotation.RestrictTo')
9+
}
410

511
mparticleMavenPublish {
612
artifactId.set('android-core')

‎android-kit-base/api/android-kit-base.api‎

Lines changed: 492 additions & 0 deletions
Large diffs are not rendered by default.

‎android-kit-base/build.gradle‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,12 @@
11
apply plugin: 'com.android.library'
22
apply plugin: 'mparticle.android.library.publish'
33
apply plugin: 'kotlin-android'
4+
apply plugin: 'org.jetbrains.kotlinx.binary-compatibility-validator'
5+
6+
apiValidation {
7+
ignoredClasses.add('com.mparticle.kits.BuildConfig')
8+
nonPublicMarkers.add('androidx.annotation.RestrictTo')
9+
}
410

511
mparticleMavenPublish {
612
artifactId.set('android-kit-base')

‎build.gradle‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ plugins {
1717
id "mparticle.android.library.publish" apply false
1818
id "org.sonarqube" version "7.5.0.8588"
1919
id "org.jlleitschuh.gradle.ktlint" version "14.2.0"
20+
id "org.jetbrains.kotlinx.binary-compatibility-validator" version "0.18.2" apply false
2021
}
2122

2223
sonarqube {

‎scripts/api-frozen-internals.txt‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
# Classes inside com.mparticle.internal that are contracts anyway.
2+
#
3+
# Everything outside com.mparticle.internal is frozen automatically: those
4+
# packages are the documented customer API and the kit-author API. The classes
5+
# below live in an internal package by name but are compiled against from
6+
# outside this repository (kits import them), are resolved by name at runtime,
7+
# or are named explicitly in android-core/proguard.pro, so
8+
# scripts/check_api_dump.py treats a change to them as a failure rather than a
9+
# reviewable internal diff.
10+
#
11+
# One fully qualified class name or glob per line; a nested class matches its
12+
# outer class entry. Adding an entry is always safe. Removing one requires an
13+
# audit of kits/, android-kit-base/, testutils/ and android-core/proguard.pro.
14+
15+
# Imported by kits
16+
com.mparticle.internal.Logger
17+
com.mparticle.internal.MPUtility
18+
com.mparticle.internal.KitManager
19+
com.mparticle.internal.CoreCallbacks
20+
com.mparticle.internal.ReportingManager
21+
com.mparticle.internal.JsonReportingMessage
22+
com.mparticle.internal.KitsLoadedCallback
23+
com.mparticle.internal.SideloadedKit
24+
com.mparticle.internal.Constants
25+
26+
# Resolved by name at runtime
27+
com.mparticle.internal.MParticleJSInterface
28+
29+
# Named in android-core/proguard.pro
30+
com.mparticle.internal.ConfigManager
31+
com.mparticle.internal.InternalSession
32+
com.mparticle.internal.PushRegistrationHelper
33+
com.mparticle.internal.listeners.GraphListener
34+
com.mparticle.internal.listeners.InternalListenerManager

‎scripts/check_api_dump.py‎

Lines changed: 185 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,185 @@
1+
#!/usr/bin/env python3
2+
"""Classify changes to the committed public API dumps.
3+
4+
The binary-compatibility-validator plugin writes one ``.api`` file per published
5+
module (``android-core/api/android-core.api`` and
6+
``android-kit-base/api/android-kit-base.api``). ``./gradlew apiCheck`` fails
7+
when the compiled surface differs from the committed dump, and ``apiDump``
8+
regenerates it. That keeps every change to the public surface visible in the
9+
pull request diff, but it does not say whether a change is acceptable.
10+
11+
This script compares the dumps on the current tree with the dumps on a base
12+
revision and classifies every changed class:
13+
14+
* **frozen** -- a class outside ``com.mparticle.internal``, or an internal
15+
class listed in ``scripts/api-frozen-internals.txt``. These are the
16+
documented customer API and the contract kit authors compile against.
17+
Any change fails with exit code 2.
18+
* **reviewable** -- any other class inside ``com.mparticle.internal``. The
19+
change is reported and the script exits 0; the reviewer decides.
20+
21+
Usage::
22+
23+
scripts/check_api_dump.py --base origin/main
24+
25+
Exit codes: 0 no frozen change, 2 frozen change, 1 usage or tooling error.
26+
"""
27+
28+
from __future__ import annotations
29+
30+
import argparse
31+
import fnmatch
32+
import os
33+
import re
34+
import subprocess
35+
import sys
36+
from dataclasses import dataclass, field
37+
from pathlib import Path
38+
39+
REPO_ROOT = Path(__file__).resolve().parent.parent
40+
DUMPS = (
41+
Path("android-core/api/android-core.api"),
42+
Path("android-kit-base/api/android-kit-base.api"),
43+
)
44+
FROZEN_LIST = REPO_ROOT / "scripts" / "api-frozen-internals.txt"
45+
INTERNAL_PREFIX = "com.mparticle.internal."
46+
47+
_HEADER = re.compile(r"^(?P<modifiers>[^{]*?)\bclass (?P<name>\S+)(?P<rest>[^{]*)\{\s*$")
48+
49+
50+
@dataclass
51+
class ClassEntry:
52+
header: str
53+
members: set[str] = field(default_factory=set)
54+
55+
56+
def parse_dump(text: str) -> dict[str, ClassEntry]:
57+
"""Parse a BCV dump into ``{jvm_class_name: ClassEntry}``."""
58+
classes: dict[str, ClassEntry] = {}
59+
current: ClassEntry | None = None
60+
for raw in text.splitlines():
61+
line = raw.rstrip()
62+
if not line:
63+
continue
64+
if line.startswith(("\t", " ")):
65+
if current is not None:
66+
current.members.add(line.strip())
67+
continue
68+
if line == "}":
69+
current = None
70+
continue
71+
match = _HEADER.match(line)
72+
if match:
73+
current = ClassEntry(header=line.strip())
74+
classes[match.group("name")] = current
75+
return classes
76+
77+
78+
def jvm_to_dotted(name: str) -> str:
79+
return name.replace("/", ".")
80+
81+
82+
def outer_class(dotted: str) -> str:
83+
return dotted.split("$", 1)[0]
84+
85+
86+
def load_frozen_patterns(path: Path) -> list[str]:
87+
patterns: list[str] = []
88+
if not path.exists():
89+
return patterns
90+
for raw in path.read_text(encoding="utf-8").splitlines():
91+
entry = raw.split("#", 1)[0].strip()
92+
if entry:
93+
patterns.append(entry)
94+
return patterns
95+
96+
97+
def is_frozen(dotted: str, frozen_patterns: list[str]) -> bool:
98+
if not dotted.startswith(INTERNAL_PREFIX):
99+
return True
100+
outer = outer_class(dotted)
101+
for pattern in frozen_patterns:
102+
if fnmatch.fnmatchcase(dotted, pattern) or fnmatch.fnmatchcase(outer, pattern):
103+
return True
104+
return False
105+
106+
107+
def git_show(base: str, path: Path) -> str | None:
108+
try:
109+
return subprocess.run(
110+
["git", "show", f"{base}:{path.as_posix()}"],
111+
cwd=REPO_ROOT,
112+
check=True,
113+
capture_output=True,
114+
text=True,
115+
).stdout
116+
except subprocess.CalledProcessError:
117+
return None
118+
119+
120+
def describe_change(old: ClassEntry | None, new: ClassEntry | None) -> list[str]:
121+
lines: list[str] = []
122+
if old is None and new is not None:
123+
lines.append(" + class added")
124+
return lines
125+
if new is None and old is not None:
126+
lines.append(" - class removed")
127+
return lines
128+
assert old is not None and new is not None
129+
if old.header != new.header:
130+
lines.append(f" ~ declaration: {old.header} -> {new.header}")
131+
for member in sorted(old.members - new.members):
132+
lines.append(f" - {member}")
133+
for member in sorted(new.members - old.members):
134+
lines.append(f" + {member}")
135+
return lines
136+
137+
138+
def main(argv: list[str]) -> int:
139+
parser = argparse.ArgumentParser(description=__doc__.split("\n\n")[0])
140+
parser.add_argument("--base", required=True, help="git revision holding the baseline dumps, e.g. origin/main")
141+
args = parser.parse_args(argv)
142+
143+
frozen_patterns = load_frozen_patterns(FROZEN_LIST)
144+
frozen_hits: list[str] = []
145+
reviewable_hits: list[str] = []
146+
147+
for dump in DUMPS:
148+
current_path = REPO_ROOT / dump
149+
if not current_path.exists():
150+
print(f"error: {dump} is missing; run ./gradlew apiDump", file=sys.stderr)
151+
return 1
152+
old_text = git_show(args.base, dump)
153+
if old_text is None:
154+
print(f"note: {dump} does not exist at {args.base}; nothing to compare against (baseline creation)")
155+
continue
156+
old = parse_dump(old_text)
157+
new = parse_dump(current_path.read_text(encoding="utf-8"))
158+
159+
for jvm_name in sorted(set(old) | set(new)):
160+
before, after = old.get(jvm_name), new.get(jvm_name)
161+
if before is not None and after is not None and before.header == after.header and before.members == after.members:
162+
continue
163+
dotted = jvm_to_dotted(jvm_name)
164+
block = [f"{dump}: {dotted}"] + describe_change(before, after)
165+
(frozen_hits if is_frozen(dotted, frozen_patterns) else reviewable_hits).append("\n".join(block))
166+
167+
if not frozen_hits and not reviewable_hits:
168+
print(f"api dumps unchanged against {args.base}")
169+
return 0
170+
171+
if reviewable_hits:
172+
print("Internal implementation surface changed (reviewable; explain in the PR description):")
173+
print("\n".join(reviewable_hits))
174+
print()
175+
if frozen_hits:
176+
print("FROZEN API CHANGED. These classes are customer or kit-author contracts.")
177+
print("Fix the change, or label the pull request 'api-change-approved' after API review.")
178+
print("\n".join(frozen_hits))
179+
return 2
180+
return 0
181+
182+
183+
if __name__ == "__main__":
184+
os.chdir(REPO_ROOT)
185+
sys.exit(main(sys.argv[1:]))

0 commit comments

Comments
 (0)