Skip to content

CI: gate .bo/.ba format changes on their header-tag bumps#1041

Open
nanavati wants to merge 1 commit into
B-Lang-org:mainfrom
nanavati:bin-format-tag-gate
Open

CI: gate .bo/.ba format changes on their header-tag bumps#1041
nanavati wants to merge 1 commit into
B-Lang-org:mainfrom
nanavati:bin-format-tag-gate

Conversation

@nanavati

Copy link
Copy Markdown
Collaborator

The .bo and .ba readers decide file compatibility solely by the header tag strings in src/comp/GenBin.hs and src/comp/GenABin.hs. When a PR changes the serialized format but forgets the tag bump, a same-tag reader misparses stale files and dies with an internalError ("please report") instead of the intended EBinFileVerMismatch "recompile" error. Recent history shows this slip is easy to make — several open PRs changed Bin instances with the tag untouched, and I made the same mistake myself last week.

This adds a cheap, build-free CI job (only triggered when the three files change):

  • a PR touching src/comp/GenBin.hs must change the .bo tag;
  • touching src/comp/GenABin.hs must change the .ba tag;
  • touching src/comp/BinData.hs (the Bin instances shared by both formats) must change both.

Not every change to these files alters serialized bytes (comments, hash plumbing, dump code), so there is an auditable escape hatch: a commit trailer Bin-format: bo-unchanged / Bin-format: ba-unchanged asserting the change is byte-neutral. The trailer lives in history, so a reviewer can hold the commit to its claim.

Validated against real ranges before opening this PR:

  • a PR range that changed GenABin.hs without a bump → fails with the actionable message;
  • a PR range that bumped both tags → passes;
  • a byte-neutral BinData.hs change (empirically verified bit-identical output) with a bumped .ba but unchanged .bo tag → fails on .bo until the trailer is added — the intended behavior for that case.

The gate deliberately does not touch the gated files itself. Note for existing open PRs: a few will trip this on their next push until they bump tags (correctly) or add trailers — that is the gate doing its job.

The .bo and .ba readers decide compatibility solely by the tag strings
in GenBin.hs and GenABin.hs; a format change that forgets the bump
makes same-tag readers misparse stale files and crash with an
internalError instead of the intended recompile error.  Several
recent PRs made this slip independently.

The gate: a PR touching GenBin.hs must change the .bo tag, GenABin.hs
the .ba tag, and BinData.hs (instances shared by both formats) both --
with an auditable escape hatch for provably byte-neutral changes, a
commit trailer 'Bin-format: bo-unchanged' / 'ba-unchanged'.

Validated against history: the ranges of PRs that forgot the bump
fail, ranges that bumped correctly pass, and a byte-neutral BinData
change fails until it carries the trailer.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019cqQdWKp7jr73GHQYnvhiq
nanavati added a commit to nanavati/bsc that referenced this pull request Jul 12, 2026
The BinData/GenBin changes in this PR thread a Position transform
through the encoder without altering any serialized bytes when the
flag is unused -- verified by byte-comparison against the previous
compiler (see the test's no-flag check).  The .ba tag was bumped for
the real format change (new Flags field); the .bo format is unchanged.

Bin-format: bo-unchanged

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019cqQdWKp7jr73GHQYnvhiq

@quark17 quark17 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This is good to check! Just a few comments.

(I also wouldn't mind a check for Error.getErrorText that there are no tag clashes and that the clauses are in tag order.)

#
# Not every change to these files alters serialized bytes (comments,
# the hash plumbing, dump/trace code). For those, add a commit
# trailer to any commit in the range:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

It seems dangerous for one commit to be able to exempt other commits from the check. I thought maybe you were doing this to allow someone to just push a commit to a PR, rather than force-push a change to an earlier commit -- but they couldn't push an empty commit just for its message, so I think that's OK.

# - a change to src/comp/BinData.hs (Bin instances shared by both
# formats) requires BOTH tags to change
#
# Not every change to these files alters serialized bytes (comments,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

These are also not the only files that would require a bump, right? I think some types have their format generated using generics, and so merely changing the ISyntax or ASyntax could require a bump? But this at least catches a lot of differences.

- 'src/comp/GenBin.hs'
- 'src/comp/GenABin.hs'

jobs:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

My preference would be to include this in ci.yml alongside the cleanliness checks, so that all of the checks are on one page and don't require looking elsewhere (assuming you know to even look for another page) (and it's particularly bad on the GitHub phone app).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants