Skip to content

feat(release): bake real interface version + contract hash into snapshots at freeze - #634

Merged
Ulrond merged 6 commits into
developfrom
feature/633-interface-version-freeze-wiring
Jul 5, 2026
Merged

feat(release): bake real interface version + contract hash into snapshots at freeze#634
Ulrond merged 6 commits into
developfrom
feature/633-interface-version-freeze-wiring

Conversation

@Ulrond

@Ulrond Ulrond commented Jun 23, 2026

Copy link
Copy Markdown
Collaborator

Refs #633 — consumer-side half of linux_binder_idl #32 / PR #33.

Problem

getInterfaceVersion() / getInterfaceHash() return 1 / notfrozen for every snapshot, so a client can't tell which interface version a service exposes. Snapshots are compile-only (their CMakeLists glob src/*.cpp), so this can only be fixed at freeze time.

Change (create_snapshot)

Before generating, it now:

  • derives the interface VERSION from the X.Y.Z.W digits — the int32 is the version with dots removed (base-10): 0.2.0.0 → 0200 → 200, 0.3.0.0 → 300, 0.1.0.1 → 101;
  • computes the contract hash (sha1 of sorted per-file .aidl sha1sums + the version_for_hashgen label), matching the toolchain's integrity check;
  • stamps current/ with version: <int> + .hash, generates (the version-aware generator emits them), copies into the snapshot, restores current/, and records the snapshot's .hash.

Proof (tested end-to-end, audiodecoder 0.2.0.0)

Snapshot emitted static const int32_t VERSION = 200; and HASH = "bd961e2c…", imports still resolved (generator decouples emit-version from resolution-version), library built. Encoding spot-check: 0.1.0.0→100, 0.2.0.0→200, 0.3.0.0→300, 0.1.0.1→101.

Notes

  • int32 can't carry a leading zero, so 0.2.0.0 reads back as 200 (zero-pad to 4 digits to display 0200). Assumes single-digit components (true for all current versions); a wider fixed-field scheme would be needed if a component reaches 10.
  • With today's generator the .hash already flows → HASH correct; the VERSION digit-encoding activates once linux_binder_idl #33 is merged and binder_sdk.version is bumped.
  • Follow-on (separate): retrofit the existing cohort snapshots (10 lack a .hash) by re-freezing.

…apshots (#633)

Module-local snapshots are compile-only (their CMakeLists glob src/*.cpp and
never regenerate), so getInterfaceVersion()/getInterfaceHash() can only be set
at freeze time. create_snapshot() now, before generating:
  - computes a monotonic ordinal = index of the version among the component's
    released versions (0.1.0.0->1, 0.2.0.0->2, ...)
  - computes the contract hash (sha1 of sorted per-file .aidl sha1sums +
    version_for_hashgen label), matching the toolchain's integrity check
  - stamps current/ with 'version: <ordinal>' + .hash, generates (the
    version-aware generator emits them), copies into the snapshot, restores
    current/, and records the snapshot's .hash

Verified end-to-end (audiodecoder, ordinal 2): the snapshot emits
VERSION = 2 and HASH = <real sha>, imports still resolve (the generator
decouples emit-version from resolution-version), and the library builds.

Graceful: with the current generator the .hash already flows (HASH correct,
VERSION stays 1); the ordinal takes effect once linux_binder_idl #33 is merged
and binder_sdk.version is bumped.

Refs #633, #32
Copilot AI review requested due to automatic review settings June 23, 2026 19:35
@Ulrond Ulrond added enhancement New feature or request scope:infrastructure Repo tooling, CI/CD, scripts, governance docs labels Jun 23, 2026
@github-project-automation github-project-automation Bot moved this to Architecture Review Required in halif_aidl Jun 23, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates the release freeze workflow so module snapshots embed a real, monotonic interface version (ordinal) and a deterministic contract hash into the generated Binder stubs at snapshot creation time. This addresses clients always seeing getInterfaceVersion() == 1 / getInterfaceHash() == "notfrozen" for frozen snapshots.

Changes:

  • Added helpers to compute a snapshot ordinal, compute a contract hash, and stamp current/interface.yaml with version: <ordinal>.
  • Updated create_snapshot() to temporarily stamp current/ with version + .hash, regenerate bindings, copy into <version>/, then restore current/.
  • Writes the computed contract hash into the frozen snapshot as <snapshot_dir>/.hash.

Comment thread scripts/release.sh Outdated
…n ordinal

Per review: getInterfaceVersion() should reflect the actual version
(0.2.0.0 -> 0200 -> 200), not a sequential 1/2/3 ordinal. Replace
_snapshot_ordinal with _snapshot_version_int = base-10 of the version with
dots removed. Verified: audiodecoder 0.2.0.0 emits VERSION = 200 (+ real hash),
builds clean.
@Ulrond
Ulrond requested a review from a team June 24, 2026 07:51
Ulrond added 2 commits June 24, 2026 19:14
…633)

The dot-strip encoding (0.2.0.0 -> 200) breaks once any field reaches 10
(0.10.0.0 -> 1000 == 1.0.0.0). Replace with a fixed-width positional scheme:
each X.Y.Z.W field is zero-padded to two digits and concatenated, so the
int32 is monotonic AND losslessly decodable (0.2.0.0 -> 20000, 1.0.0.0 ->
1000000, 0.10.0.0 -> 100000 distinct from 1.0.0.0). Fields must be 0-99; a
wider field returns empty and the caller now warns + leaves the snapshot
unfrozen rather than emit a wrong number.

Documents that getInterfaceVersion() ordering is an additive-compat test
only WITHIN a generation — a correct consumer check is
generation(server)==generation(client) && server>=client, not bare >=,
because the generation field marks breaking changes (see #633).

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 3 comments.

Comment thread scripts/release.sh Outdated
Comment thread scripts/release.sh
Comment thread scripts/release.sh
- _contract_hash: use sha256 (64 hex) to match the committed <version>/.hash
  format across the repo (was sha1, wrong length).
- _snapshot_version_int: require exactly four numeric dot-separated fields
  (X.Y.Z.W) before encoding; reject malformed inputs (0.2.0, 0.2.0.0., 0.2..0)
  that word-splitting would silently accept and that could collide.
- Correct 'ordinal' wording to 'fixed-width positional version int' in the
  freeze-stamp comments (the scheme encodes X.Y.Z.W, not a sequence ordinal).
@Ulrond Ulrond self-assigned this Jul 3, 2026
@Ulrond Ulrond added this to the Next Release milestone Jul 3, 2026
@Ulrond Ulrond added the Minor Change Additive, backwards-compatible interface change — bumps minor; the default for real work label Jul 4, 2026
@Ulrond

Ulrond commented Jul 4, 2026

Copy link
Copy Markdown
Collaborator Author

Copilot review: the three flagged items (SHA-1→SHA-256 contract hash, strict 4-field version validation, and the positional version-int naming) were already addressed in follow-up commits — resolving the now-outdated threads.

@Ulrond

Ulrond commented Jul 4, 2026

Copy link
Copy Markdown
Collaborator Author

Re-scope (decided 2026-07-04)

The ABI signals belong in the generator, not here — see linux_binder_idl#32 (now on the 2.5.0 milestone), which owns:

  • getInterfaceVersion() = ABI-only ordinal (generation.minor; the patch/doc field is dropped, so a doc-only bump doesn't move it).
  • getInterfaceHash() = normalized-API hash (contract-level; comment/doc/format-insensitive), computed by the tool that understands the AIDL.

That makes two changes here:

  1. Drop _contract_hash. release.sh should not define a hash. A byte-level sha256 of the .aidl files (a) is a second hash definition parallel to the generator's, and (b) changes on pure reformatting/comments — a false "contract changed" signal. The real hash comes from the generator (Feature: New Future / Interfaces #32).
  2. Drop _snapshot_version_int's 4-field positional encoding. It encodes all four fields (0.2.0.1 → 20001), so getInterfaceVersion() would tick on a doc-only bump — a false ABI-change signal. The generator derives the ABI-only ordinal (Feature: New Future / Interfaces #32).

What stays here: release.sh sets the 4-field release version in the snapshot's interface.yaml / metadata (its existing, legitimate job). The generator reads that and emits both ABI signals.

So this PR should shrink to version-stamping only, with the hash + version-emission deferred to linux_binder_idl#32. Once #32 lands (2.5.0) and the pin bump (#683 → binder 2.4.0, and later 2.5.0) is in, snapshots get correct, ABI-only getInterfaceVersion()/getInterfaceHash() for free.

cc: this came out of the review question on whether baking the hash in release.sh was the right place — it isn't; the generator is.

@Ulrond

Ulrond commented Jul 5, 2026

Copy link
Copy Markdown
Collaborator Author

Tracking note: the versioning design is consolidated in #633 (now trustworthy frozen interface versioning, with #658 folded in). Per that + linux_binder_idl#32, this PR re-scopes to version-setting only — the version-ordinal and contract-hash emission move to the generator (linux_binder_idl#32/#33).

Copilot AI review requested due to automatic review settings July 5, 2026 15:42
@Ulrond
Ulrond merged commit f03e6f2 into develop Jul 5, 2026
3 checks passed
@Ulrond
Ulrond deleted the feature/633-interface-version-freeze-wiring branch July 5, 2026 15:43
@github-project-automation github-project-automation Bot moved this from Architecture Review Required to Resolved in halif_aidl Jul 5, 2026
@github-actions github-actions Bot locked and limited conversation to collaborators Jul 5, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

enhancement New feature or request Minor Change Additive, backwards-compatible interface change — bumps minor; the default for real work scope:infrastructure Repo tooling, CI/CD, scripts, governance docs

Projects

Status: Resolved

Development

Successfully merging this pull request may close these issues.

2 participants