Repository navigation
Represent validated source archives before persistence - #21246
Merged
Merged
Conversation
This was referenced Aug 20, 2026
zanieb
commented
Aug 20, 2026
zanieb
commented
Aug 20, 2026
uv test inventory changesThis PR changes the tests when compared with the
|
zanieb
force-pushed
the
zb/validated-source-archive-refactor
branch
from
August 21, 2026 00:08
d18df58 to
8268bb0
Compare
zanieb
marked this pull request as ready for review
August 21, 2026 00:47
woodruffw
reviewed
Aug 21, 2026
Comment on lines
+214
to
+216
| // Two zero-filled records form an empty, valid tar archive. | ||
| const EMPTY_TAR: &[u8] = &[0; 1024]; | ||
| const TAR: SourceDistExtension = SourceDistExtension::Legacy(LegacySourceDistExtension::Tar); |
Member
There was a problem hiding this comment.
Nitpick: it might be nice to use a slightly more realistic fixture here, particularly since we're considering bare .tar legacy at this point. But minor IMO as we can change going forwards.
woodruffw
reviewed
Aug 21, 2026
| algorithms: &[HashAlgorithm], | ||
| origin: ArchiveOrigin, | ||
| expected_size: Option<u64>, | ||
| error_context: impl Fn(&Path) -> String, |
Member
There was a problem hiding this comment.
Confirming my understanding: this is a closure that produces an appropriate piece of context for an error by transforming a path into a string?
If so, maybe we could save this complexity and perform that transform at the callsite? It looks pretty minimal for our two existing callers; FWICT the only downside would be an eager allocation but that's probably not saving us any perf in either a network or FS pathway.
(Super minor though.)
woodruffw
approved these changes
Aug 21, 2026
woodruffw
left a comment
woodruffw
left a comment
Member
There was a problem hiding this comment.
LGTM, two extreme nitpicks.
zanieb
added a commit
that referenced
this pull request
Aug 24, 2026
`HashStrategy` currently represents hash generation and verification as mutually exclusive choices. A resolution may need both: verify artifacts whose hashes have been supplied, while generating hashes for new artifacts. Store generation separately from `HashVerification` in `HashStrategy`. Each distribution still receives a `HashPolicy` specifying whether to generate or check hashes. Update existing call sites to use the separate settings while preserving their hash inputs and which hashes uv treats as trusted. Generation remains available for artifacts without supplied hashes, unless the caller requires a hash for every artifact. This does not read hashes from `uv.lock` or add new archive comparisons. Based on #21246. #21248 checks caller-supplied hashes before source archives enter the cache. #21223 reads hashes from an existing `uv.lock` and supplies them for verification.
zanieb
added a commit
that referenced
this pull request
Sep 8, 2026
`uv.lock` records archive digests, but checking an existing lockfile or resolving with `--locked` can obtain source metadata without comparing those digests. A replaced source archive can therefore run its build backend before a later hash or lockfile check rejects it. Construct a `HashStrategy` from the existing lockfile's hashes for registry packages, direct URLs, and local archives. Enforce the recorded hashes when reusing cached source metadata, resolving with `--locked`, and installing build dependencies recorded in the lockfile into isolated build environments. Normalize equivalent archive subdirectories so they find the same recorded hashes. Keep recorded hashes while checking whether an existing lockfile is still usable, but allow unlocked fresh resolution to replace them. Build dependencies without recorded hashes remain allowed. Based on #21248, which checks source archives before saving them to the cache. #21246 contains the structural refactor, and #21247 separates hash generation from verification. --------- Co-authored-by: Zanie Blue <contact@zanie.dev> Co-authored-by: Tomasz Kramkowski <tom@astral.sh> Co-authored-by: Charlie Marsh <charlie.r.marsh@gmail.com>
This branch was previously deployed
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
2 participants
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.
Source archive helpers currently extract an archive and save it to the cache in the same call. Separate those steps so the type system records that extraction and its existing checks have completed before persistence.
Introduce a private
ValidatedSourceArchivethat owns the temporary directory. Its HTTP and local constructors retain their transport setup, diagnostics, and cleanup behavior, while a private reader helper performs the shared extraction and hash collection. The consumingpersistmethod saves the source tree and returns the collected hashes and byte count. This preserves existing behavior; it does not add trusted-hash comparisons or change cache repair.#21247 separates hash generation from verification, and #21248 checks caller-supplied hashes before persistence.