feat(index): MetadataIndex interface, types, and MockIndex - #1
Merged
Conversation
Implements RFD 0001 Phase 3.1: the architectural seam between the resolver
and storage, per RFD Section 6 with the interface decisions settled in
Section 16.
MetadataIndex carries the three methods the RFD specifies -- Versions,
Metadata, Files -- plus the two sentinel errors. The settled decisions are
implemented as specified: DistFile.Location is a scheme-prefixed string the
interface treats as opaque, PackageMetadata.Origin is a string,
DistFile.UploadTime is a time.Time, and Files returns ALL wheels with no
platform filtering so the consumer decides.
Distinctions the interface draws deliberately, because collapsing either
produces a silently wrong resolution rather than an error:
* ErrPackageNotFound vs an empty version list. An unknown name is probably
a typo; a known name with no acceptable version is a constraint conflict.
A resolver must report them differently.
* ErrMetadataUnavailable vs ErrPackageNotFound. Treating an sdist-only
release whose metadata needs a build as "not found" would let the
resolver quietly select an older version and call it success.
Types carry only what resolution consumes. Descriptive metadata (summary,
author, classifiers) is excluded: it is not an input to any resolution
decision and would inflate a type on a per-candidate path.
Three judgment calls worth flagging, none of them settled by the RFD:
* PackageName is a distinct type constructed through NewPackageName, which
PEP 503-normalizes. This is correctness, not style: if "Flask" and
"flask" reach the solver as separate identities it builds two nodes for
one package and can report a conflict that does not exist. Normalization
delegates to go-python-packaging's extras.Normalize, which implements
exactly the PEP 503 algorithm -- PEP 685 extras and PEP 503 project
names are the same transformation, and that function documents itself as
mirroring canonicalize_name for both. Reusing it beats a second copy
that can drift. The name reads oddly at this call site; a PEP 503-named
entry point belongs upstream, filed as #19425.
* DistFile.Yanked / YankedReason go slightly beyond the four decisions
Section 16 settles. PEP 592 makes yanking per-FILE, and Section 6
assigns yanked policy to FilteredIndex -- that policy needs the fact
carried somewhere, and this is the only per-file type.
* DistFile.RequiresPython is per-file and distinct from
PackageMetadata.RequiresPython, since a release can ship one wheel for
older interpreters and another for newer ones.
DistKindUnknown is the zero value on purpose, so a DistFile built by an
incomplete implementation is obviously incomplete rather than silently
claiming to be an installable wheel.
MockIndex is a non-test file because candidate, provider, and resolver tests
all need an index to drive and Go cannot share _test.go helpers across
packages; it pulls in no test-only dependency. It is concurrency-safe per the
interface contract, returns copies so a resolver that sorts in place cannot
corrupt the fixture for later assertions, and registers a version on AddFiles
so a files-only setup does not surface as ErrPackageNotFound -- which would
look like a bug in the code under test rather than in its setup.
Versions returns REVERSE insertion order: deterministic, but deliberately
unsorted. The interface promises no ordering, and a mock returning sorted
versions would let an ordering assumption pass here and fail against a real
index. A shuffle would achieve the same thing at the cost of flaky tests.
Verified: 98.1% statement coverage on index/; -race clean; gofmt silent;
golangci-lint v2.11.2 reports 0 issues from the module root. The tests were
mutation tested rather than assumed effective -- removing the metadata slice
copy fails TestMockIndexReturnsCopies, and skipping name normalization fails
three tests including TestMockIndexNormalizesNames.
Pins go-python-packaging v0.2.0, the first release carrying the PEP 440/508
conformance work and the local-label ordering fix.
Refs rstudio/package-manager#18646
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
RFD 0001 Phase 3.1 — the architectural seam between the resolver and storage, per RFD §6 with the interface decisions settled in §16.
Tracked as rstudio/package-manager#18646.
The settled decisions, as specified
DistFile.LocationsemanticsPackageMetadata.OriginstringDistFile.UploadTimetime.TimeFiles()platform filteringMetadataIndexTwo distinctions that are load-bearing
Collapsing either produces a silently wrong resolution rather than an error, so both are enforced and tested:
ErrPackageNotFoundvs an empty version list. An unknown name is probably a typo; a known name with no acceptable version is a constraint conflict. A resolver must report them differently, so a known-but-empty package is([], nil).ErrMetadataUnavailablevsErrPackageNotFound. Treating an sdist-only release whose metadata needs a build as "not found" would let the resolver quietly select an older version and call it a success.Three judgment calls the RFD does not settle
1.
PackageNameis a distinct type, normalized at construction. This is correctness, not style: ifFlaskandflaskreach the solver as separate identities, it builds two nodes for one package and can report a conflict that does not exist.Normalization delegates to
go-python-packaging'sextras.Normalize, which implements exactly the PEP 503 algorithm — PEP 685 extra names and PEP 503 project names are the same transformation, and that function's own doc describes it as mirroringcanonicalize_namefor both. Reusing it beats a second copy that can drift from the first. The name reads oddly at this call site, so I filed rstudio/package-manager#19425 to add a PEP 503-named entry point upstream, and the code comment points there.2.
DistFile.Yanked/YankedReasongo slightly beyond §16's four decisions. PEP 592 makes yanking a per-file property, and §6 assigns yanked policy toFilteredIndex— that policy needs the fact carried somewhere, and this is the only per-file type. Flagging it explicitly since it is an addition rather than a settled decision.3.
DistFile.RequiresPythonis per-file and distinct fromPackageMetadata.RequiresPython. A release can ship one wheel for older interpreters and another for newer ones, so the file-level constraint is what decides whether a particular file is usable.Also:
DistKindUnknownis the zero value on purpose, so aDistFilebuilt by an incomplete implementation is obviously incomplete rather than silently claiming to be an installable wheel.MockIndex
A non-test file, because
candidate,provider, andresolvertests all need an index to drive and Go cannot share_test.gohelpers across packages. It pulls in no test-only dependency, so this costs nothing at build time.AddFilesregisters the version. Otherwise a files-only setup surfaces asErrPackageNotFound, which looks like a bug in the code under test rather than in its setup.Versionsreturns reverse insertion order — deterministic, but deliberately unsorted. The interface promises no ordering, and a mock returning sorted versions would let an ordering assumption pass here and then fail against a real index. A shuffle would achieve the same at the cost of flaky tests.Testing
index/;-raceclean;gofmt -l .silent;golangci-lint run ./...at CI's pinned v2.11.2 → 0 issues from the module root.TestMockIndexReturnsCopies; skipping name normalization fails three tests includingTestMockIndexNormalizesNames. Both mutations were reverted and the tree confirmed free of residue.Dependencies
Pins
go-python-packagingv0.2.0 — the first release carrying the PEP 440/508 conformance work and the local-label ordering fix.go-versioncomes in as an indirect.Not in this PR
RSFIndexandCachedJSONIndexare #18647.OfflineIndex,DBIndex,FilteredIndex, andMultiIndexare named inindex/doc.gowith their intended scope but are not in the initial release.🤖 Generated with Claude Code