feat(pypirsf): PyPI RSF record layout and dependency-blob decoder - #3
Merged
Conversation
Ports the PyPI record layout and the dependency-blob decode side out of
Package Manager so there is ONE implementation, shared by the standalone
resolver and by PPM.
Placed here rather than in rstudio/repository-snapshot-format because that
library is deliberately generic -- it knows records, fields and indexes and
nothing about any ecosystem -- and a PyPI record schema is Python-packaging
knowledge rather than format knowledge. The dependency direction already
works: PPM imports this module, and this module imports the generic format
library, so PPM -> go-pyresolver -> repository-snapshot-format stays acyclic.
The one coupling to PPM was three fields, so the neutral type is three
fields: VersionDeps carries RequiresDist, RequiresPython and ProvidesExtra as
RAW unparsed strings. That keeps this package's only dependency a zstd
implementation and leaves PEP 508/440 parsing to the caller, which also lets
a consumer that just echoes the strings back skip the parsing entirely.
PPM's storetypes.PyPIVersionMetadata carries Blocked, BlockingRule, Vulns and
MetadataMD5Sum, none of which the codec ever touched.
Two properties carried over deliberately, both now tested:
* Hardening. Every length-prefixed read is bounded against the bytes
actually remaining, so a crafted length errors instead of allocating, and
MaxDecompressedBytes (64 MiB) caps zstd expansion. This matters MORE here
than in a server: a standalone tool decodes a file the user downloaded and
nothing upstream vouched for it.
* The load-bearing field order. ReleaseDate must sit between Version and
Summary with Summary last, because released readers navigate subfields by
name using a forward-only skip and therefore depend on Summary being
last. Violating it desynchronizes every shipped reader, which already
happened in production. Documented as wire format, not struct layout.
The golden fixtures come from the PRODUCER's own test vectors, copied
verbatim. That is what makes the conformance test meaningful: PPM's
codec_test.go asserts the same expected decode against the same
golden_blob.bin, so both decoders are pinned to one producer-authored fixture
rather than each merely agreeing with itself. A test also asserts the fixture
actually exercises BOTH requirement encodings (dictionary-referenced and
inline name), so a decoder that broke one could not pass by luck.
Two deliberate API differences from PPM's version:
* Dict.Decoder() is not exposed. PPM used it only so a test could assert
the decoder was released; TestDecodeAfterCloseErrors covers the same
ground by decoding through a closed Dict, without leaking zstd types into
a public API. PPM's migration will need to adapt that one test.
* Names() is exported, since the dep-name table doubles as a bounded set
usable for integer-id comparison instead of string comparison.
* decompress guards an empty field rather than indexing field[0] on faith.
Unreachable today because DecodePackage checks first, but a future caller
should get an error rather than a panic.
Verified: 86.5% statement coverage on pypirsf/; -race clean; gofmt silent;
golangci-lint v2.11.2 reports 0 issues from the module root.
Refs rstudio/package-manager#18647
Co-Authored-By: Claude Opus 5 <noreply@anthropic.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
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.
What
Ports the PyPI record layout and the dependency-blob decode side out of Package Manager, so there is one implementation shared by the standalone resolver and by PPM.
Step 1 of the direction settled with Jonathan: a standalone runnable resolver that resolves from an RSF file it is given, with PPM keeping its own tight in-process integration.
Why here and not in
repository-snapshot-formatThat library is deliberately generic — records, fields, indexes, and nothing about any ecosystem. A PyPI record schema is Python-packaging knowledge, not format knowledge, so it belongs with the code that understands Python packaging.
The dependency direction already works out: PPM imports this module, and this module imports the generic format library, so
PPM → go-pyresolver → repository-snapshot-formatstays acyclic.The neutral type is three fields, because that was the whole coupling
PPM's
depsblobreferencedstoretypes.PyPIVersionMetadatafor exactly three fields. SoVersionDepscarriesRequiresDist,RequiresPython,ProvidesExtra— as raw unparsed strings.That keeps this package's only dependency a zstd implementation and leaves PEP 508/440 parsing to the caller, which also lets a consumer that just echoes the strings back into an API response skip parsing entirely. PPM's type additionally carries
Blocked,BlockingRule,VulnsandMetadataMD5Sum— none of which the codec ever touched.Two properties carried over deliberately, both now tested
Hardening. Every length-prefixed read is bounded against bytes actually remaining, so a crafted length errors instead of allocating, and
MaxDecompressedBytes(64 MiB) caps zstd expansion. This matters more here than in a server: a standalone tool decodes a file the user downloaded, and nothing upstream vouched for it.The load-bearing field order.
ReleaseDatemust sit betweenVersionandSummary, withSummarylast, because released readers navigate subfields by name using a forward-only skip and therefore silently depend onSummarybeing last. Violating it leaks bytes into the next element and desynchronizes every shipped reader — which already happened in production. Documented as wire format rather than struct layout.The golden fixtures are the point
testdata/golden_blob.binandgolden_depsdict.binare copied verbatim from the producer's own test vectors. That is what makes the conformance test meaningful: PPM'scodec_test.goasserts the same expected decode against the same fixture, so both decoders are pinned to one producer-authored vector rather than each merely agreeing with itself.TestGoldenCoversDictionaryCompressionBothWaysadditionally asserts the fixture exercises both requirement encodings — dictionary-referenced (flask>=2.0) and inline-name (unknownpkg>=1.0) — so a decoder that broke one could not pass the golden test by luck.The fixture also covers the empty-case distinction that matters to a resolver: version
3.0.0decodes to a zeroVersionDeps, meaning captured and declares nothing, which is different from a version being absent from the map, meaning nothing was captured. Collapsing those would let a resolver silently drop a subtree.Three deliberate API differences from PPM's version
Dict.Decoder()is not exposed. PPM used it only so a test could assert the decoder was released.TestDecodeAfterCloseErrorscovers the same ground by decoding through a closedDict, without leaking zstd types into a public API. PPM's migration will need to adapt that one test.Names()is exported, since the dep-name table doubles as a bounded set usable for integer-id comparison instead of string comparison.decompressguards an empty field rather than indexingfield[0]on faith. Unreachable today becauseDecodePackagechecks first, but a future caller gets an error rather than a panic.Testing
pypirsf/;-race -count=1clean;gofmt -l .silent;golangci-lint run ./...at CI's pinned v2.11.2 → 0 issues from the module root.github.com/klauspost/compress(zstd), attributed inNOTICEalong with the port itself.Not in this PR
MetadataIndexbuilt on this — next.src/rsf/python_listing_index.gocallsParseDepsdictFieldduring index build, and Rev 15's design depends on the listing hot path never materializing the trailing deps fields — so the migration needs to confirm it doesn't change when that dict is parsed.🤖 Generated with Claude Code