Pin protoc so the build is reproducible - #73
Merged
Merged
Conversation
Building the generated code required a `protoc` on $PATH, unpinned. protoc 25
and newer embed a runtime version check into their output that refuses to load
under an older google.protobuf than the one they were built against, so a
contributor with a current Homebrew protoc (35.x) gets gencode that fails to
import in the very same virtualenv that built it:
google.protobuf.runtime_version.VersionError: Detected incompatible
Protobuf Gencode/Runtime versions ... gencode 7.35.1 runtime 6.32.1
which reads as "this repo is broken" rather than "your protoc is too new".
dumper/run.py now downloads and uses a pinned protoc 21.9 - the same version CI
already used - from .protoc/, and deliberately does not fall back to $PATH.
21.9 predates the version check, which keeps `protobuf>=3.13.0` an honest floor
for end users rather than silently requiring a much newer runtime. `--protoc`
overrides it.
Also:
- Import dumper.protodump lazily. It uses Protobuf internals removed in
protobuf 4, and importing it at module scope made even the compile-only path
(which needs none of that) fail on a modern protobuf.
- Pass check=True to protoc, so a compile failure stops the build instead of
leaving a half-populated gencode directory behind.
- Drop the now-redundant protoc download and the disabled ::set-env command
from both workflows, along with a `pip install protobuf>=4` that contradicted
the pin installed two lines later.
- Document how to rebuild gencode and how to add a new Keynote version. The
README told people to run `cd dumper && make`, and there has never been a
dumper/Makefile.
Fixes #67.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0179M4xvAKPGrgKpy4AeCsM7
CI installs ruff unpinned and the repo declares no rule selection, so the lint gate was whatever that month's ruff considered default. Ruff's defaults have since grown well beyond the original E4/E7/E9/F, and `ruff check .` now reports 125 errors on unmodified master - across LOG015, UP031, BLE001, SIM115 and others - so every pull request fails lint for reasons that have nothing to do with its contents. Select the rules explicitly and pin ruff in CI. The selection is exactly what master already passes, so this restores the gate to what it was actually enforcing rather than silently adopting a much stricter one; broadening it is now a deliberate edit instead of a side effect of a ruff release. target-version is py310 rather than the py39 inferred from requires-python, because dumper/extract_mapping.py already uses a match statement and the CI matrix starts at 3.10. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0179M4xvAKPGrgKpy4AeCsM7
This was referenced Aug 7, 2026
Merged
…-protoc # Conflicts: # .github/workflows/python-package.yml
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Building the generated code needed a
protocon$PATH, unpinned. protoc 25+ embeds a runtime-version check into its output that refuses to load under an oldergoogle.protobufthan the one it was built against — so a contributor with a current Homebrew protoc (35.x) gets generated code that won't import in the same virtualenv that just built it:That reads as "this repo is broken" rather than "your protoc is too new". Combined with the README pointing at a
dumper/Makefilethat has never existed, this is #67.Fix
dumper/run.pydownloads and uses a pinned protoc 21.9 — the version CI already used — into.protoc/, and deliberately does not fall back to$PATH.21.9 predates the runtime-version check, which keeps
protobuf>=3.13.0an honest floor for end users. Pinning a modern protoc instead would have forced every installed user up toprotobuf>=5.28.1; that's a separate decision, not a side effect of fixing the build.--protoc /path/to/protocoverrides.Also in here
dumper.protodumpis now imported lazily. It depends on Protobuf internals removed in protobuf 4 (SkipField), and importing it at module scope made even the compile-only path — which needs none of that — fail on a modern protobuf. Recompiling the checked-in protos now works on any supportedprotobuf; only the--app-pathextraction step still needsprotobuf<4, which is documented and declared in the script header.check=Trueon the protoc call. A protoc failure previously left a half-populated gencode directory and let the build carry on.::set-envcommand (disabled by GitHub since 2020, kept alive only byACTIONS_ALLOW_UNSECURE_COMMANDS), and apip install protobuf>=4that contradicted theprotobuf<4pin installed two lines later.cd dumper && make clean && makeinstructions.Verification
From a clean tree with no
.protoc/and no gencode, underprotobuf==6.32.1(which failed before this change):Second run reuses the cached protoc without re-downloading.
Fixes #67.