From 1972e8470153e75550fa7405bf62a2a9aefeab51 Mon Sep 17 00:00:00 2001 From: Zoltan Kochan Date: Thu, 17 Sep 2026 20:29:30 +0200 Subject: [PATCH] refactor(tarball): expose one cache identity, not two (#15038) Nothing outside the crate builds a cache key for a projection other than the ordinary package one, so `ArchiveStoreProjection::mem_cache_key` goes back to being internal and `package_mem_cache_key` carries the contract. Co-authored-by: Claude Opus 5 (1M context) --- pnpm/crates/tarball/src/download.rs | 34 +++++++++++++++-------------- pnpm/crates/tarball/src/lib.rs | 3 ++- 2 files changed, 20 insertions(+), 17 deletions(-) diff --git a/pnpm/crates/tarball/src/download.rs b/pnpm/crates/tarball/src/download.rs index 5b038dbbad..1d717dcaf7 100644 --- a/pnpm/crates/tarball/src/download.rs +++ b/pnpm/crates/tarball/src/download.rs @@ -61,21 +61,10 @@ impl<'a> ArchiveStoreProjection<'a> { } } - /// Identity of the archive a shared [`crate::MemCache`] slot holds. - /// - /// The expected hash is part of it: a slot's bytes were verified against - /// one integrity, and a request pinning a different one must not be - /// served them. An archive whose hash the fetch discovers is published - /// under the computed hash, which is the one its resolution then records. - /// Projections that can produce a different file set carry their own - /// discriminator; synthesized manifests are content-addressed so equal - /// projections still share work. - /// - /// The parts are tab-separated, and neither a URL, an integrity, nor a - /// projection tag can contain a tab, so two distinct identities cannot - /// spell the same key. - #[must_use] - pub fn mem_cache_key( + /// Identity of the archive a shared [`crate::MemCache`] slot holds. See + /// [`package_mem_cache_key`], the shape every caller outside this crate + /// uses. + pub(crate) fn mem_cache_key( self, package_url: &str, integrity: Option<&Integrity>, @@ -117,7 +106,20 @@ impl<'a> ArchiveStoreProjection<'a> { /// Cache identity of an ordinary package archive, the projection every /// resolve-time publisher, speculative prefetch and install-time lookup -/// has to agree on. See [`ArchiveStoreProjection::mem_cache_key`]. +/// has to agree on. +/// +/// The expected hash is part of it: a slot's bytes were verified against +/// one integrity, and a request pinning a different one must not be served +/// them. An archive whose hash the fetch discovers is published under the +/// computed hash, which is the one its resolution then records. The network +/// policy is part of it too, since a revision-addressed fetch and a direct +/// one are different requests for the same URL. Projections that can produce +/// a different file set carry their own discriminator; synthesized manifests +/// are content-addressed so equal projections still share work. +/// +/// The parts are tab-separated, and neither a URL, an integrity, nor a +/// projection tag can contain a tab, so two distinct identities cannot spell +/// the same key. #[must_use] pub fn package_mem_cache_key( package_url: &str, diff --git a/pnpm/crates/tarball/src/lib.rs b/pnpm/crates/tarball/src/lib.rs index 7e126ee81b..b067b1a7a5 100644 --- a/pnpm/crates/tarball/src/lib.rs +++ b/pnpm/crates/tarball/src/lib.rs @@ -206,7 +206,8 @@ impl<'a> IngestTarballToStore<'a> { /// /// # Caller invariant: stable filter per archive /// - /// The cache is keyed on [`ArchiveStoreProjection::mem_cache_key`]. Within + /// The cache is keyed on the archive's identity, which + /// [`package_mem_cache_key`] spells for an ordinary package. Within /// one key, a second caller fetching the same archive with a different /// [`ignore_file_pattern`] silently receives the map the first caller's /// filter produced. Every fetch of an archive must use the same filter.