Decode pkl-binary Bytes, and tolerate future extra slots - #16
Open
JahJajaka wants to merge 1 commit into
Open
Conversation
Reviewed the apple/pkl 0.30.0 release diff (0.29.1...0.30.0) against
docs/modules/bindings-specification/pages/binary-encoding.adoc:
- `Bytes` (type code 0x0F, introduced in Pkl 0.29 and used more heavily
since 0.30, e.g. YAML binary scalars and the new pkl-binary renderer)
had no decoder registered in pkl.parser.Parser, so any Bytes-valued
property silently decoded to `{"type": "Unknown", "value": obj}`
instead of the actual bytes.
- 0.30.0 formalizes a forward-compatibility contract: "Additional slots
may be added to types in future Pkl releases. Decoders must be
designed to defensively discard values beyond the number of known
slots for a type". Several decoders used fixed-arity tuple unpacking
(`_, value, unit = obj`), which raises "too many values to unpack" the
moment a future release appends a slot to Typed/Dynamic, Duration,
Property, Entry, or Element. Switched these to positional indexing so
extra trailing slots are ignored per spec.
Also bumps scripts/download_binary.py's pinned VERSION to 0.30.0
(verified all 5 referenced asset filenames still exist on that release)
and documents Bytes -> bytes in the README type-mapping table.
Out of scope, left for follow-up: Class/TypeAlias decoding was already
a pre-existing no-op unrelated to 0.30.0; the 0.26 Http/proxy evaluator
option and 0.30's new pkl-binary in-language renderer are unrelated
protocol/stdlib additions that don't affect existing wire decoding.
Signed-off-by: JahJajaka <r2e4d6@gmail.com>
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
1 participant
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.
apple/pkl 0.30.0 formalizes a forward-compatibility contract in its wire format docs, and Pkl 0.29 introduced a
Bytesvalue type that pkl-python never learned to decode. Concretely, comparingbinary-encoding.adocbetween the0.29.1and0.30.0tags:Bytes(type code0x0F) had no registered decoder inpkl.parser.Parser, so anyBytes-valued property silently became{"type": "Unknown", "value": obj}instead of actual bytes._, value, unit = obj), which would raiseValueError: too many values to unpackthe moment a future Pkl release appends a slot toTyped/Dynamic,Duration,Property,Entry, orElement.This PR:
Bytesdecoding, mapping it to native Pythonbytes(documented in the README type-mapping table alongsideRegex,Pair, etc.).tests/test_parser_binary_encoding.py) covering both, exercisingParser.handle_typedirectly against hand-built wire-format arrays so they don't need a runningpklbinary.VERSIONin the dev-onlyscripts/download_binary.pyfrom0.25.2to0.30.0(verified the referenced asset filenames still exist on that release).No behavior change for existing well-formed payloads — this only adds a previously-missing decode path and makes existing decoders more permissive of trailing data.