From dbc8a3bc5c2bb1458acab7cbcf424fcb5d6af7e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Thalheim?= Date: Sun, 4 Jan 2026 13:57:33 +0000 Subject: [PATCH] prefetch-npm-deps: normalize packuments for deterministic hashes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixed-output derivations for npm dependencies fail with hash mismatches when rebuilt on different days. The root cause is that npm registry metadata (packuments) contain volatile fields that change whenever upstream publishes a new version. For example, the TypeScript packument changes daily due to nightly releases, even though the lockfile pins a specific version. The cached packument includes _rev, time, modified, and the full versions list - all of which drift over time. Strip these volatile fields and filter the versions map to only include versions actually referenced in the lockfile. Signed-off-by: Jörg Thalheim --- .../node/prefetch-npm-deps/src/main.rs | 173 ++++++++++++------ .../node/prefetch-npm-deps/src/parse/lock.rs | 3 + .../node/prefetch-npm-deps/src/parse/mod.rs | 2 + 3 files changed, 127 insertions(+), 51 deletions(-) diff --git a/pkgs/build-support/node/prefetch-npm-deps/src/main.rs b/pkgs/build-support/node/prefetch-npm-deps/src/main.rs index fa594ce5b2af..7666474f9fe1 100644 --- a/pkgs/build-support/node/prefetch-npm-deps/src/main.rs +++ b/pkgs/build-support/node/prefetch-npm-deps/src/main.rs @@ -31,6 +31,57 @@ fn get_packument_url(registry: &str, package_name: &str) -> anyhow::Result .map_err(|e| anyhow!("failed to construct packument URL: {e}")) } +/// Normalize packument data to ensure determinism. +/// +/// Strips volatile fields like `_rev`, `time`, and `modified`. +/// Filters the `versions` map to only include versions requested in the lockfile. +fn normalize_packument( + package_name: &str, + data: &[u8], + requested_versions: &HashSet, +) -> anyhow::Result> { + let mut json: Value = serde_json::from_slice(data) + .map_err(|e| anyhow!("failed to parse packument JSON for {package_name}: {e}"))?; + + let obj = json + .as_object_mut() + .ok_or_else(|| anyhow!("packument for {package_name} is not a JSON object"))?; + + // Strip volatile top-level fields + obj.remove("_rev"); + obj.remove("time"); + obj.remove("modified"); + + // Filter versions to only those in lockfile + if let Some(Value::Object(versions)) = obj.get_mut("versions") { + versions.retain(|version, _| requested_versions.contains(version)); + + // Normalize each version object + for version_val in versions.values_mut() { + if let Some(version_obj) = version_val.as_object_mut() { + // Strip fields starting with underscore (volatile/internal) + version_obj.retain(|key, _| !key.starts_with('_')); + // Strip other often-volatile fields + version_obj.remove("gitHead"); + } + } + } + + // Filter dist-tags to only point to versions we kept + if let Some(Value::Object(tags)) = obj.get_mut("dist-tags") { + tags.retain(|_, version_val| { + if let Some(version) = version_val.as_str() { + requested_versions.contains(version) + } else { + false + } + }); + } + + serde_json::to_vec(&json) + .map_err(|e| anyhow!("failed to re-serialize packument for {package_name}: {e}")) +} + /// Fetch and cache packuments (package metadata) for all packages. /// /// This is needed because npm may query package metadata for optional peer dependencies @@ -43,60 +94,68 @@ fn get_packument_url(registry: &str, package_name: &str) -> anyhow::Result /// /// We cache both versions to ensure cache hits regardless of which header npm uses. /// See: pacote/lib/registry.js and @npmcli/arborist/lib/arborist/build-ideal-tree.js -fn fetch_packuments(cache: &Cache, package_names: HashSet) -> anyhow::Result<()> { +fn fetch_packuments( + cache: &Cache, + package_versions: HashMap>, +) -> anyhow::Result<()> { const CORGI_DOC: &str = "application/vnd.npm.install-v1+json; q=1.0, application/json; q=0.8, */*"; const FULL_DOC: &str = "application/json"; - info!("Fetching {} packuments", package_names.len()); + info!("Fetching {} packuments", package_versions.len()); - package_names.into_par_iter().try_for_each(|package_name| { - let packument_url = get_packument_url("https://registry.npmjs.org", &package_name)?; + package_versions + .into_par_iter() + .try_for_each(|(package_name, requested_versions)| { + let packument_url = get_packument_url("https://registry.npmjs.org", &package_name)?; - match util::get_url_body_with_retry(&packument_url) { - Ok(packument_data) => { - // npm's make-fetch-happen uses the URL-encoded form for cache keys - // e.g., "https://registry.npmjs.org/@types%2freact-dom" not "@types/react-dom" - // We must use the encoded form in both the cache key string AND the metadata URL + match util::get_url_body_with_retry(&packument_url) { + Ok(packument_data) => { + let normalized_data = + normalize_packument(&package_name, &packument_data, &requested_versions)?; - // Cache with corgiDoc header (for initial requests) - cache - .put( - format!("make-fetch-happen:request-cache:{packument_url}"), - packument_url.clone(), - &packument_data, - None, // Packuments don't have integrity hashes - Some(ReqHeaders { - accept: String::from(CORGI_DOC), - }), - ) - .map_err(|e| { - anyhow!("couldn't insert packument cache entry (corgi) for {package_name}: {e:?}") - })?; + // npm's make-fetch-happen uses the URL-encoded form for cache keys + // e.g., "https://registry.npmjs.org/@types%2freact-dom" not "@types/react-dom" + // We must use the encoded form in both the cache key string AND the metadata URL - // Cache with fullDoc header (for workspace/full metadata requests) - cache - .put( - format!("make-fetch-happen:request-cache:{packument_url}"), - packument_url.clone(), - &packument_data, - None, - Some(ReqHeaders { - accept: String::from(FULL_DOC), - }), - ) - .map_err(|e| { - anyhow!("couldn't insert packument cache entry (full) for {package_name}: {e:?}") - })?; + // Cache with corgiDoc header (for initial requests) + cache + .put( + format!("make-fetch-happen:request-cache:{packument_url}"), + packument_url.clone(), + &normalized_data, + None, // Packuments don't have integrity hashes + Some(ReqHeaders { + accept: String::from(CORGI_DOC), + }), + ) + .map_err(|e| { + anyhow!("couldn't insert packument cache entry (corgi) for {package_name}: {e:?}") + })?; + + // Cache with fullDoc header (for workspace/full metadata requests) + cache + .put( + format!("make-fetch-happen:request-cache:{packument_url}"), + packument_url.clone(), + &normalized_data, + None, + Some(ReqHeaders { + accept: String::from(FULL_DOC), + }), + ) + .map_err(|e| { + anyhow!("couldn't insert packument cache entry (full) for {package_name}: {e:?}") + })?; + } + Err(e) => { + // Log but don't fail - some packages might not need packuments + info!("Warning: couldn't fetch packument for {package_name}: {e}"); + } } - Err(e) => { - // Log but don't fail - some packages might not need packuments - info!("Warning: couldn't fetch packument for {package_name}: {e}"); - } - } - Ok::<_, anyhow::Error>(()) - }) + Ok::<_, anyhow::Error>(()) + }) } /// `fixup_lockfile` rewrites `integrity` hashes to match cache and removes the `integrity` field from Git dependencies. @@ -330,12 +389,24 @@ fn main() -> anyhow::Result<()> { let cache = Cache::new(out.join("_cacache")); cache.init()?; - // Collect unique package names for packument fetching - // Extract from lockfile keys like "node_modules/@scope/name" -> "@scope/name" - let package_names: HashSet = packages - .iter() - .filter_map(|p| p.name.rsplit_once("node_modules/").map(|(_, n)| n.to_string())) - .collect(); + // Collect package names and their versions from the lockfile for packument filtering. + // For non-aliased packages: extract from lockfile keys like "node_modules/@scope/name" -> "@scope/name" + // For aliased packages: the name is already the real package name (e.g., "string-width" not "string-width-cjs") + // We only care about packages that have a version string (registry packages). + let mut package_versions: HashMap> = HashMap::new(); + for p in &packages { + if let Some(version) = &p.version { + let pkg_name = p + .name + .rsplit_once("node_modules/") + .map(|(_, name)| name.to_string()) + .unwrap_or_else(|| p.name.clone()); + package_versions + .entry(pkg_name) + .or_default() + .insert(version.to_string()); + } + } // Fetch and cache tarballs packages.into_par_iter().try_for_each(|package| { @@ -364,7 +435,7 @@ fn main() -> anyhow::Result<()> { .unwrap_or(1); if fetcher_version >= 2 { - fetch_packuments(&cache, package_names)?; + fetch_packuments(&cache, package_versions)?; fs::write(out.join(".fetcher-version"), format!("{fetcher_version}"))?; } diff --git a/pkgs/build-support/node/prefetch-npm-deps/src/parse/lock.rs b/pkgs/build-support/node/prefetch-npm-deps/src/parse/lock.rs index 266ba2585913..c420fb1e3ede 100644 --- a/pkgs/build-support/node/prefetch-npm-deps/src/parse/lock.rs +++ b/pkgs/build-support/node/prefetch-npm-deps/src/parse/lock.rs @@ -78,6 +78,7 @@ struct OldPackage { pub(super) struct Package { #[serde(default)] pub(super) name: Option, + pub(super) version: Option, pub(super) resolved: Option, pub(super) integrity: Option, } @@ -254,6 +255,7 @@ fn to_new_packages( new.push(Package { name: Some(name), + version: None, // v1 lockfiles don't include version in the same structure resolved: if matches!(package.version, UrlOrString::Url(_)) { Some(package.version) } else { @@ -315,6 +317,7 @@ mod tests { assert_eq!(new.len(), 1, "new packages map should contain 1 value"); assert_eq!(new[0], Package { name: Some(String::from("sqlite3")), + version: None, resolved: Some(UrlOrString::Url(Url::parse("git+ssh://git@github.com/mapbox/node-sqlite3.git#593c9d498be2510d286349134537e3bf89401c4a").unwrap())), integrity: None }); diff --git a/pkgs/build-support/node/prefetch-npm-deps/src/parse/mod.rs b/pkgs/build-support/node/prefetch-npm-deps/src/parse/mod.rs index 6c54d404f3cb..3e6f5019ea33 100644 --- a/pkgs/build-support/node/prefetch-npm-deps/src/parse/mod.rs +++ b/pkgs/build-support/node/prefetch-npm-deps/src/parse/mod.rs @@ -112,6 +112,7 @@ pub fn lockfile( #[derive(Debug)] pub struct Package { pub name: String, + pub version: Option, pub url: Url, specifics: Specifics, } @@ -175,6 +176,7 @@ impl Package { Ok(Package { name: pkg.name.unwrap(), + version: pkg.version, url: resolved, specifics, })