diff --git a/crates/aube-linker/src/hoisted.rs b/crates/aube-linker/src/hoisted.rs index 266298db9..9ca8c00ae 100644 --- a/crates/aube-linker/src/hoisted.rs +++ b/crates/aube-linker/src/hoisted.rs @@ -315,7 +315,12 @@ pub(crate) fn plan_importer( HoistingLimits::Dependencies => outcome.node_idx, }; for (dep_name, dep_tail) in &pkg.dependencies { - let child_dep_path = format!("{dep_name}@{dep_tail}"); + // Git / remote-tarball deps are recorded by their resolved URL + // spec but keyed under the short `name@git+` / + // `name@url+` form, so the verbatim `name@tail` key would + // miss `graph.packages` and silently drop the dep's subtree. + let child_dep_path = aube_lockfile::shared_local_dep_path(dep_name, dep_tail) + .unwrap_or_else(|| format!("{dep_name}@{dep_tail}")); if !graph.packages.contains_key(&child_dep_path) { continue; } diff --git a/crates/aube-linker/src/link.rs b/crates/aube-linker/src/link.rs index 1148ea9a4..1a300fde0 100644 --- a/crates/aube-linker/src/link.rs +++ b/crates/aube-linker/src/link.rs @@ -127,11 +127,20 @@ impl Linker { // Step 1: Populate .aube virtual store // - // Local packages (file:/link:) never go into the shared global - // virtual store — their source is project-specific, so we - // materialize them straight into per-project `.aube/` below. - // `link:` entries don't need any `.aube/` entry at all; their - // top-level symlink points directly at the target. + // `file:` / `link:` / `portal:` / `exec:` sources resolve + // against a path inside the project, so they never go into the + // shared global virtual store — we materialize them straight + // into per-project `.aube/` below. `link:` entries don't need + // any `.aube/` entry at all; their top-level symlink points + // directly at the target. + // + // Git and remote-tarball sources are the exception: they're + // pinned to immutable content and shared across projects like + // registry packages. Under the global virtual store they must + // be materialized into the shared store (and have their + // `.aube/` entry symlinked at it) so a registry + // dependent that also lives in the shared store doesn't get a + // dangling sibling symlink to them. for (dep_path, pkg) in &graph.packages { let Some(ref local) = pkg.local_source else { continue; @@ -139,6 +148,39 @@ impl Linker { if matches!(local, LocalSource::Link(_)) { continue; } + if self.use_global_virtual_store && local.is_globally_shareable() { + // A git / remote-tarball dep shared globally must be + // materialized into its content-addressed GVS path so a + // registry dependent's sibling symlink resolves. The + // package index *defines* that path — the content + // fingerprint is folded into the graph hash — so a + // missing index is not a skippable cache hit the way a + // per-project `file:` dep can be; silently continuing + // would leave the GVS entry absent and dangle every + // dependent (`require()` throwing `Cannot find module`). + // Fail loudly, exactly like the registry pass below. + // + // There is deliberately no `store.load_index` fallback + // here (unlike the registry pass): git/tarball indices + // are never persisted by `(name, version, integrity)`, + // because a prepared tree and its `--ignore-scripts` raw + // checkout share that coordinate and the content + // fingerprint is precisely what keeps them on separate + // GVS paths. The fetch driver guarantees the index is + // present by always re-importing source deps. + let index = package_indices + .get(dep_path) + .ok_or_else(|| Error::MissingPackageIndex(dep_path.to_string()))?; + self.ensure_shared_local_in_global_store( + &aube_dir, + dep_path, + pkg, + index, + &mut stats, + nested_link_targets.as_ref(), + )?; + continue; + } let Some(index) = package_indices.get(dep_path) else { continue; }; @@ -645,11 +687,19 @@ impl Linker { let nested_link_targets = build_nested_link_targets(root_dir, graph); - // Step 1a: Materialize local (`file:` dir/tarball) packages - // straight into the shared per-project `.aube/`. They never - // participate in the global virtual store since their source - // is project-specific. `link:` deps get no `.aube/` entry at - // all — step 2 symlinks directly to the target. + // Step 1a: Materialize local (`file:` dir/tarball, `portal:`, + // `exec:`) packages straight into the shared per-project + // `.aube/`. They never participate in the global virtual store + // since their source resolves against a path inside the + // project. `link:` deps get no `.aube/` entry at all — step 2 + // symlinks directly to the target. + // + // Git and remote-tarball sources are content-pinned and shared + // across projects like registry packages, so under the global + // virtual store they're materialized into the shared store with + // a `.aube/` symlink pointing at it — otherwise a + // registry dependent in the shared store would get a dangling + // sibling symlink to them. for (dep_path, pkg) in &graph.packages { let Some(ref local) = pkg.local_source else { continue; @@ -657,6 +707,26 @@ impl Linker { if matches!(local, LocalSource::Link(_)) { continue; } + if self.use_global_virtual_store && local.is_globally_shareable() { + // See the matching block in `link_isolated`: a globally + // shared git/tarball dep's index defines its + // content-addressed GVS path, so a missing index must + // fail loudly rather than dangle every dependent. No + // `load_index` fallback — those indices aren't persisted + // by coordinate (prepared vs raw checkout would collide). + let index = package_indices + .get(dep_path) + .ok_or_else(|| Error::MissingPackageIndex(dep_path.to_string()))?; + self.ensure_shared_local_in_global_store( + &aube_dir, + dep_path, + pkg, + index, + &mut stats, + nested_link_targets.as_ref(), + )?; + continue; + } let Some(index) = package_indices.get(dep_path) else { continue; }; diff --git a/crates/aube-linker/src/materialize.rs b/crates/aube-linker/src/materialize.rs index 08540c012..b7021c726 100644 --- a/crates/aube-linker/src/materialize.rs +++ b/crates/aube-linker/src/materialize.rs @@ -1,9 +1,9 @@ use tracing::{debug, trace, warn}; use crate::patches::apply_multi_file_patch; -use crate::sweep::mkdirp; +use crate::sweep::{EntryState, classify_entry_state, mkdirp, try_remove_entry}; use crate::{Error, LinkStats, LinkStrategy, Linker, sys}; -use aube_lockfile::LockedPackage; +use aube_lockfile::{LockedPackage, shared_local_dep_path}; use aube_store::{PackageIndex, StoredFile}; use std::collections::BTreeMap; use std::path::{Path, PathBuf}; @@ -213,6 +213,57 @@ impl Linker { Ok(()) } + /// Materialize a globally-reproducible local source (a `git` + /// dependency or a remote `.tgz`) into the shared virtual store and + /// point the per-project `.aube/` entry at it — the exact + /// arrangement Step 1 produces for a registry package. + /// + /// Used by the isolated linker in global-virtual-store mode. Plain + /// `file:` / `link:` / `portal:` / `exec:` sources resolve against + /// a path inside the project and are materialized per-project + /// instead (see `materialize_into` with `apply_hashes = false`), + /// but git and remote-tarball sources are content-pinned and shared + /// like registry packages. They MUST live in the shared store when + /// it is enabled: a registry dependent in the shared store links + /// its dependency siblings to the hashed global path + /// (`virtual_store_subdir(dep_path)`), so a git/tarball dep that + /// only existed in the per-project `.aube/` would leave that + /// sibling symlink dangling — and Node would resolve whatever + /// unrelated `` it found walking up the tree. + pub(crate) fn ensure_shared_local_in_global_store( + &self, + aube_dir: &Path, + dep_path: &str, + pkg: &LockedPackage, + index: &PackageIndex, + stats: &mut LinkStats, + nested_link_targets: Option<&BTreeMap>, + ) -> Result<(), Error> { + let local_aube_entry = aube_dir.join(self.aube_dir_entry_name(dep_path)); + let global_entry = self.virtual_store.join(self.virtual_store_subdir(dep_path)); + let state = classify_entry_state(&local_aube_entry, &global_entry); + if matches!(state, EntryState::Fresh) { + stats.packages_cached += 1; + return Ok(()); + } + self.ensure_in_virtual_store(dep_path, pkg, index, stats, nested_link_targets)?; + if matches!(state, EntryState::Stale) { + // A prior install — or an older aube that always + // materialized git/remote sources per-project — may have + // left a real directory or a stale symlink here. Clear + // either shape before pointing the entry at the shared + // store (`try_remove_entry` handles dir, symlink, and + // dangling-link cases). + try_remove_entry(&local_aube_entry); + } + if let Some(parent) = local_aube_entry.parent() { + mkdirp(parent)?; + } + sys::create_dir_link(&global_entry, &local_aube_entry) + .map_err(|e| Error::Io(local_aube_entry.clone(), e))?; + Ok(()) + } + /// Materialize a single package directly into the per-project /// virtual store at `aube_dir//node_modules//`. /// @@ -393,7 +444,13 @@ impl Linker { // a per-project `.aube/` that the caller just // ensured is empty), so nothing can be in the way. for (dep_name, dep_version) in &pkg.dependencies { - let dep_dep_path = format!("{dep_name}@{dep_version}"); + // Git / remote-tarball deps are recorded by their resolved + // URL spec but keyed in the graph under the short + // `name@git+` / `name@url+` form. Translate so the + // sibling symlink targets the same `dep_path` the package was + // materialized under; everything else keeps `name@version`. + let dep_dep_path = shared_local_dep_path(dep_name, dep_version) + .unwrap_or_else(|| format!("{dep_name}@{dep_version}")); // Skip any dep whose name matches the package being // materialized, regardless of version. The symlink would // land at `pkg_nm_parent.join(dep_name)` which is exactly diff --git a/crates/aube-linker/src/tests.rs b/crates/aube-linker/src/tests.rs index a44c819ff..de07180e0 100644 --- a/crates/aube-linker/src/tests.rs +++ b/crates/aube-linker/src/tests.rs @@ -530,6 +530,77 @@ fn test_second_install_reuses_global_store() { ); } +#[test] +fn gvs_shareable_source_dep_without_index_errors_loudly() { + // A git / remote-tarball dep is keyed in the GVS under a + // content-addressed path whose hash folds in the dep's content + // fingerprint. That fingerprint comes from its package index, so + // under the global virtual store the dep MUST have an index — a + // missing one would otherwise leave the dep unmaterialized and + // dangle every dependent's sibling symlink. The loop used to + // silently `continue`; assert it now fails loudly with a + // `MissingPackageIndex` diagnostic instead (the registry pass + // already does, but git/tarball deps have no `load_index` fallback + // because their indices aren't persisted by coordinate). + use aube_lockfile::{GitSource, LocalSource}; + + let dir = tempfile::tempdir().unwrap(); + let store = Store::at(dir.path().join("store/files")); + + let git = LocalSource::Git(GitSource { + url: "https://github.com/request/request.git".to_string(), + committish: None, + resolved: "0123456789abcdef0123456789abcdef01234567".to_string(), + integrity: None, + subpath: None, + }); + let dep_path = git.dep_path("request"); + + let mut packages = BTreeMap::new(); + packages.insert( + dep_path.clone(), + LockedPackage { + name: "request".to_string(), + version: "2.88.0".to_string(), + integrity: None, + dependencies: BTreeMap::new(), + dep_path: dep_path.clone(), + local_source: Some(git), + ..Default::default() + }, + ); + let mut importers = BTreeMap::new(); + importers.insert( + ".".to_string(), + vec![DirectDep { + name: "request".to_string(), + dep_path: dep_path.clone(), + dep_type: DepType::Production, + specifier: None, + }], + ); + let graph = LockfileGraph { + importers, + packages, + ..Default::default() + }; + + // Deliberately omit `dep_path` from the indices map — the contract + // violation the fetch driver normally prevents. + let indices: BTreeMap = BTreeMap::new(); + + let project_dir = dir.path().join("project"); + std::fs::create_dir_all(&project_dir).unwrap(); + let linker = Linker::new_with_gvs(&store, LinkStrategy::Copy, true); + let err = linker + .link_all(&project_dir, &graph, &indices) + .expect_err("a shareable source dep with no index must error, not dangle"); + assert!( + matches!(err, Error::MissingPackageIndex(ref dp) if dp == &dep_path), + "expected MissingPackageIndex({dep_path}), got: {err:?}" + ); +} + /// Regression: a version bump keeps the same top-level name /// (`foo`) but must repoint `node_modules/foo` at the new /// `.aube/foo@` entry. The old `.aube/foo@/` is left diff --git a/crates/aube-lockfile/src/graph_hash.rs b/crates/aube-lockfile/src/graph_hash.rs index ec71ee115..135941ad5 100644 --- a/crates/aube-lockfile/src/graph_hash.rs +++ b/crates/aube-lockfile/src/graph_hash.rs @@ -26,10 +26,31 @@ //! gives us alphabetized keys for free. BLAKE3 is the project default //! for non-crypto-verifying hashes (3-5x faster than SHA-256). -use crate::{LockedPackage, LockfileGraph}; +use crate::{LockedPackage, LockfileGraph, shared_local_dep_path}; use serde::Serialize; use std::collections::BTreeMap; +/// Resolve a child dependency's recorded `(alias, tail)` to the graph +/// key the target package is stored under. +/// +/// Registry deps record their version verbatim, so `alias@tail` is the +/// key. Git / remote-tarball deps record their *resolved URL* as the +/// tail while the package is keyed under the hashed +/// `alias@git+` / `alias@url+` form; [`shared_local_dep_path`] +/// performs that translation. Falling back to the raw `alias@tail` +/// keeps the common case allocation-light and behaves identically to +/// the pre-canonicalization lookup for everything that isn't a +/// content-pinned source. +/// +/// Keeping this in lockstep with the linker's sibling-symlink keying +/// (which calls the same helper) is load-bearing: if the hasher skipped +/// a URL-shaped git child, the parent's GVS hash would omit that child's +/// content fingerprint and build/engine taint, and two materially +/// different trees would collide on one virtual-store path. +fn child_dep_path(alias: &str, tail: &str) -> String { + shared_local_dep_path(alias, tail).unwrap_or_else(|| format!("{alias}@{tail}")) +} + use aube_util::collections::FxMap as FxHashMap; use aube_util::collections::FxSet as FxHashSet; @@ -122,6 +143,22 @@ fn append_hex_to_leaf(dep_path: &str, hex: &str) -> String { /// package also lands at a fresh virtual-store path. pub type PatchHashFn<'a> = &'a dyn Fn(&str, &str) -> Option; +/// Per-`dep_path` materialized-content fingerprint. Folded into +/// `full_pkg_id` so a source-backed dependency (git / remote tarball) +/// whose lockfile coordinate is identical to another's but whose +/// on-disk bytes differ hashes to a distinct value. +/// +/// The motivating case is a git dep installed once normally (its +/// `prepare` built `dist/`) and once under `--ignore-scripts` (raw +/// checkout): same `#` coordinate, no integrity in the +/// lockfile, but different trees. Keying the global virtual store by +/// coordinate alone would let the first project's built tree leak into +/// the second's scripts-free install; folding the content fingerprint +/// in keeps them at separate paths. Returns `None` for packages whose +/// content the caller doesn't fingerprint (registry packages already +/// carry an integrity, so they need no extra disambiguation). +pub type ContentHashFn<'a> = &'a dyn Fn(&str) -> Option; + /// Compute final hashes for every package in `graph`. When /// `engine` is `Some`, packages whose transitive subtree contains a /// build-allowed package fold the engine name into their hash; when @@ -143,6 +180,21 @@ pub fn compute_graph_hashes_with_patches( allow_build: AllowBuildFn<'_>, engine: Option<&EngineName>, patch_hash: PatchHashFn<'_>, +) -> GraphHashes { + compute_graph_hashes_full(graph, allow_build, engine, patch_hash, &|_| None) +} + +/// Variant of [`compute_graph_hashes_with_patches`] that additionally +/// folds a per-`dep_path` materialized-content fingerprint into each +/// node's identity. See [`ContentHashFn`] for why this is needed for +/// source-backed (git / remote-tarball) dependencies under the global +/// virtual store. +pub fn compute_graph_hashes_full( + graph: &LockfileGraph, + allow_build: AllowBuildFn<'_>, + engine: Option<&EngineName>, + patch_hash: PatchHashFn<'_>, + content_hash: ContentHashFn<'_>, ) -> GraphHashes { // Pass 1: identify every dep_path whose `(name, version)` is // allowed to run its scripts. This is the "builds" set. @@ -162,6 +214,7 @@ pub fn compute_graph_hashes_with_patches( &mut deps_hash_cache, &mut FxHashSet::default(), patch_hash, + content_hash, ); } @@ -213,6 +266,7 @@ fn calc_deps_hash( cache: &mut FxHashMap, parents: &mut FxHashSet, patch_hash: PatchHashFn<'_>, + content_hash: ContentHashFn<'_>, ) -> String { if let Some(cached) = cache.get(dep_path) { return cached.clone(); @@ -227,17 +281,24 @@ fn calc_deps_hash( let hash = match graph.packages.get(dep_path) { Some(pkg) => { - let id = full_pkg_id(pkg, patch_hash); + let id = full_pkg_id(pkg, patch_hash, content_hash(dep_path).as_deref()); let mut deps: BTreeMap = BTreeMap::new(); for (alias, child_tail) in &pkg.dependencies { - let child_dep_path = format!("{alias}@{child_tail}"); + let child_dep_path = child_dep_path(alias, child_tail); // The child might not be in the graph if the lockfile // has a dangling reference (e.g. after manual edits); // skip rather than panic. if !graph.packages.contains_key(&child_dep_path) { continue; } - let child_hash = calc_deps_hash(graph, &child_dep_path, cache, parents, patch_hash); + let child_hash = calc_deps_hash( + graph, + &child_dep_path, + cache, + parents, + patch_hash, + content_hash, + ); deps.insert(alias.clone(), child_hash); } hash_canonical(&DepsHashInput { @@ -274,7 +335,7 @@ fn transitively_requires_build( } let result = match graph.packages.get(dep_path) { Some(pkg) => pkg.dependencies.iter().any(|(alias, tail)| { - let child_dep_path = format!("{alias}@{tail}"); + let child_dep_path = child_dep_path(alias, tail); transitively_requires_build(graph, builds, &child_dep_path, cache, parents) }), None => false, @@ -285,23 +346,31 @@ fn transitively_requires_build( } /// `full_pkg_id` — pnpm uses `${pkgIdWithPatchHash}:${resolution}`; we -/// use `${name}@${version}[:patch:]:${source?}:${integrity}`. +/// use `${name}@${version}[:patch:]:${source?}[:content:]:${integrity}`. /// Source-backed packages fold in their stable specifier so two local /// or git dependencies with the same manifest version don't collapse /// onto the same graph hash when they point at different bytes. -fn full_pkg_id(pkg: &LockedPackage, patch_hash: PatchHashFn<'_>) -> String { +/// +/// `content` is the materialized-content fingerprint (see +/// [`ContentHashFn`]). It disambiguates source-backed deps that share a +/// coordinate but not bytes — e.g. a git dep whose `prepare` ran versus +/// the same commit installed under `--ignore-scripts`. +fn full_pkg_id(pkg: &LockedPackage, patch_hash: PatchHashFn<'_>, content: Option<&str>) -> String { let integrity = pkg.integrity.as_deref().unwrap_or(""); let source = pkg .local_source .as_ref() .map(|source| format!(":source:{}", source.specifier())) .unwrap_or_default(); + let content = content + .map(|hex| format!(":content:{hex}")) + .unwrap_or_default(); match patch_hash(&pkg.name, &pkg.version) { Some(hex) => format!( - "{}@{}:patch:{hex}{source}:{integrity}", + "{}@{}:patch:{hex}{source}{content}:{integrity}", pkg.name, pkg.version ), - None => format!("{}@{}{source}:{}", pkg.name, pkg.version, integrity), + None => format!("{}@{}{source}{content}:{integrity}", pkg.name, pkg.version), } } @@ -474,6 +543,169 @@ mod tests { assert_eq!(h_a.node_hash["pure@1.0.0"], h_b.node_hash["pure@1.0.0"]); } + #[test] + fn content_hash_disambiguates_same_coordinate() { + // A git dep with no integrity: two installs share the same + // `(name, version, source)` coordinate but materialize + // different trees (prepare ran vs `--ignore-scripts`). Folding + // the content fingerprint in must split them onto distinct + // hashes; an absent fingerprint must leave the hash unchanged. + let mut g = empty_graph(); + let mut pkg = mk_pkg("gitdep", "1.0.0", None); + pkg.dep_path = "gitdep@git+abc".into(); + pkg.local_source = Some(LocalSource::Directory(PathBuf::from("clone"))); + g.packages.insert("gitdep@git+abc".into(), pkg); + + let none = compute_graph_hashes_full(&g, &|_| false, None, &|_, _| None, &|_| None); + let prepared = compute_graph_hashes_full(&g, &|_| false, None, &|_, _| None, &|dp| { + (dp == "gitdep@git+abc").then(|| "prepared".to_string()) + }); + let raw = compute_graph_hashes_full(&g, &|_| false, None, &|_, _| None, &|dp| { + (dp == "gitdep@git+abc").then(|| "raw".to_string()) + }); + + assert_ne!( + prepared.node_hash["gitdep@git+abc"], raw.node_hash["gitdep@git+abc"], + "different content fingerprints must produce different hashes" + ); + assert_ne!( + none.node_hash["gitdep@git+abc"], prepared.node_hash["gitdep@git+abc"], + "folding in a fingerprint must change the hash vs none" + ); + // A no-op content fn reproduces the with-patches result exactly, + // so existing GVS paths for the common case stay stable. + let with_patches = compute_graph_hashes_with_patches(&g, &|_| false, None, &|_, _| None); + assert_eq!(none.node_hash, with_patches.node_hash); + } + + #[test] + fn content_hash_cascades_to_parent() { + // A parent that depends on the fingerprinted git dep must also + // get a fresh hash, so its sibling symlink lands on the dep's + // content-disambiguated path rather than dangling. + let mut g = empty_graph(); + let mut parent = mk_pkg("parent", "1.0.0", Some("sha512-P")); + parent + .dependencies + .insert("gitdep".into(), "git+abc".into()); + g.packages.insert("parent@1.0.0".into(), parent); + let mut child = mk_pkg("gitdep", "1.0.0", None); + child.dep_path = "gitdep@git+abc".into(); + child.local_source = Some(LocalSource::Directory(PathBuf::from("clone"))); + g.packages.insert("gitdep@git+abc".into(), child); + + let a = compute_graph_hashes_full(&g, &|_| false, None, &|_, _| None, &|dp| { + (dp == "gitdep@git+abc").then(|| "prepared".to_string()) + }); + let b = compute_graph_hashes_full(&g, &|_| false, None, &|_, _| None, &|dp| { + (dp == "gitdep@git+abc").then(|| "raw".to_string()) + }); + assert_ne!(a.node_hash["parent@1.0.0"], b.node_hash["parent@1.0.0"]); + } + + const URL_SHA: &str = "0123456789abcdef0123456789abcdef01234567"; + + #[test] + fn url_shaped_git_child_content_cascades_to_parent() { + // Real pnpm lockfiles record a git dependency by its *resolved + // URL* in the parent's `dependencies:` map, while the package is + // keyed under the hashed `name@git+` form. The hasher must + // canonicalize that URL-shaped value — a raw `name@` lookup + // misses the child, so its content fingerprint never reaches the + // parent and two materially different trees collide on one GVS + // path. (Distinct from `content_hash_cascades_to_parent`, which + // feeds the already-canonical synthetic `git+abc` value.) + let url = format!("https://github.com/request/request.git#{URL_SHA}"); + let child_key = shared_local_dep_path("request", &url).expect("git url is shareable"); + assert!( + child_key.starts_with("request@git+"), + "unexpected: {child_key}" + ); + + let mut g = empty_graph(); + let mut parent = mk_pkg("parent", "1.0.0", Some("sha512-P")); + parent.dependencies.insert("request".into(), url); + g.packages.insert("parent@1.0.0".into(), parent); + let mut child = mk_pkg("request", "2.88.0", None); + child.dep_path = child_key.clone(); + child.local_source = Some(LocalSource::Directory(PathBuf::from("clone"))); + g.packages.insert(child_key.clone(), child); + + let prepared = compute_graph_hashes_full(&g, &|_| false, None, &|_, _| None, &|dp| { + (dp == child_key.as_str()).then(|| "prepared".to_string()) + }); + let raw = compute_graph_hashes_full(&g, &|_| false, None, &|_, _| None, &|dp| { + (dp == child_key.as_str()).then(|| "raw".to_string()) + }); + assert_ne!( + prepared.node_hash["parent@1.0.0"], raw.node_hash["parent@1.0.0"], + "URL-shaped git child fingerprint must cascade into the parent hash" + ); + } + + #[test] + fn url_shaped_tarball_child_content_cascades_to_parent() { + // The codeload-archive form pnpm records for a `github:` dep that + // resolves to a tarball. Keyed under `name@url+`; the raw + // `name@` lookup would skip it just like the git case. + let url = format!("https://codeload.github.com/request/request/tar.gz/{URL_SHA}"); + let child_key = shared_local_dep_path("request", &url).expect("tarball url is shareable"); + assert!( + child_key.starts_with("request@url+"), + "unexpected: {child_key}" + ); + + let mut g = empty_graph(); + let mut parent = mk_pkg("parent", "1.0.0", Some("sha512-P")); + parent.dependencies.insert("request".into(), url); + g.packages.insert("parent@1.0.0".into(), parent); + let mut child = mk_pkg("request", "2.88.0", None); + child.dep_path = child_key.clone(); + child.local_source = Some(LocalSource::Directory(PathBuf::from("clone"))); + g.packages.insert(child_key.clone(), child); + + let prepared = compute_graph_hashes_full(&g, &|_| false, None, &|_, _| None, &|dp| { + (dp == child_key.as_str()).then(|| "prepared".to_string()) + }); + let raw = compute_graph_hashes_full(&g, &|_| false, None, &|_, _| None, &|dp| { + (dp == child_key.as_str()).then(|| "raw".to_string()) + }); + assert_ne!( + prepared.node_hash["parent@1.0.0"], raw.node_hash["parent@1.0.0"], + "URL-shaped tarball child fingerprint must cascade into the parent hash" + ); + } + + #[test] + fn url_shaped_git_child_engine_taint_cascades_to_parent() { + // An allowlisted (building) git child recorded by URL must make + // the parent engine-sensitive too; otherwise a parent installed + // under a different engine reuses a GVS path built for the wrong + // ABI. Requires the same canonical child lookup in + // `transitively_requires_build`. + let url = format!("https://github.com/request/request.git#{URL_SHA}"); + let child_key = shared_local_dep_path("request", &url).expect("git url is shareable"); + + let mut g = empty_graph(); + let mut parent = mk_pkg("parent", "1.0.0", Some("sha512-P")); + parent.dependencies.insert("request".into(), url); + g.packages.insert("parent@1.0.0".into(), parent); + let mut child = mk_pkg("request", "2.88.0", None); + child.dep_path = child_key.clone(); + child.local_source = Some(LocalSource::Directory(PathBuf::from("clone"))); + g.packages.insert(child_key, child); + + let allow_request = |pkg: &LockedPackage| pkg.registry_name() == "request"; + let engine_a = EngineName("linux-x64-node20".into()); + let engine_b = EngineName("linux-x64-node22".into()); + let h_a = compute_graph_hashes(&g, &allow_request, Some(&engine_a)); + let h_b = compute_graph_hashes(&g, &allow_request, Some(&engine_b)); + assert_ne!( + h_a.node_hash["parent@1.0.0"], h_b.node_hash["parent@1.0.0"], + "URL-shaped building git child must make the parent engine-sensitive" + ); + } + #[test] fn cycles_do_not_panic() { let mut g = empty_graph(); diff --git a/crates/aube-lockfile/src/lib.rs b/crates/aube-lockfile/src/lib.rs index b4535cefb..1b9bd4a8b 100644 --- a/crates/aube-lockfile/src/lib.rs +++ b/crates/aube-lockfile/src/lib.rs @@ -22,7 +22,7 @@ pub use merge::{MergeReport, merge_branch_lockfiles}; pub(crate) use source::normalize_git_fragment; pub use source::{ GitSource, HostedGit, HostedGitHost, LocalSource, RemoteTarballSource, git_commits_match, - parse_git_spec, parse_hosted_git, + parse_git_spec, parse_hosted_git, shared_local_dep_path, }; use smallvec::SmallVec; diff --git a/crates/aube-lockfile/src/source.rs b/crates/aube-lockfile/src/source.rs index d3a912a9f..0e1184a3b 100644 --- a/crates/aube-lockfile/src/source.rs +++ b/crates/aube-lockfile/src/source.rs @@ -114,6 +114,30 @@ impl LocalSource { } } + /// Whether this source is pinned to immutable, globally + /// reproducible content and can therefore be shared across + /// projects inside aube's global virtual store, exactly like a + /// registry package. + /// + /// `Git` is pinned to a 40-char commit SHA and `RemoteTarball` to + /// a fetched URL (and, once resolved, an integrity hash), so two + /// projects that depend on the same one resolve to the same files. + /// `file:` / `link:` / `portal:` / `exec:` all resolve against a + /// path inside the depending project, so they stay per-project and + /// are never promoted into the shared store. + /// + /// Load-bearing for global-virtual-store correctness: a registry + /// package materialized into the shared store points its + /// dependency siblings at the hashed global path + /// (`virtual_store_subdir(dep_path)`). If one of those deps were a + /// git/tarball source that only ever landed in the per-project + /// `.aube/`, the sibling symlink would dangle and Node's module + /// walk would silently fall back to some unrelated `` found + /// higher up the tree. + pub fn is_globally_shareable(&self) -> bool { + matches!(self, LocalSource::Git(_) | LocalSource::RemoteTarball(_)) + } + /// The path as a POSIX-style string with forward-slash separators. /// `Path::display()` and `to_string_lossy()` honor the host's /// separator (backslash on Windows), which would make `dep_path` @@ -265,6 +289,62 @@ impl LocalSource { } } +/// Resolve a transitive dependency's recorded spec *value* to the same +/// `dep_path` key the lockfile parser assigns the target package, for +/// the two content-pinned source kinds that get shared globally (git +/// and remote tarball). +/// +/// pnpm records a git / remote-tarball dependency inside a snapshot's +/// `dependencies:` map by its *resolved spec* — `#` for git, +/// the tarball URL for remote tarballs (e.g. request-promise-core lists +/// `request: https://github.com/request/request.git#`). The parser, +/// however, keys the package itself under [`LocalSource::dep_path`] — the +/// short `name@git+` / `name@url+` form. A naive +/// `format!("{name}@{value}")` lookup therefore points at a key that was +/// never inserted into the graph, so: +/// +/// * the linker's sibling symlink dangles (Node resolves the wrong +/// `` or none — the request-promise-core crash), and +/// * the graph hasher skips the child entirely, so neither its content +/// fingerprint nor its build/engine taint cascades into the parent's +/// global-virtual-store hash. +/// +/// Mirror `pnpm::read::push_direct`'s keying so the resolved value lands +/// on the exact `dep_path` the package was materialized under. Returns +/// `None` for every other value (plain semver, `file:`, `link:`, npm +/// aliases, …) so callers keep the verbatim `name@value` key those +/// already resolve correctly with. +pub fn shared_local_dep_path(dep_name: &str, dep_value: &str) -> Option { + // pnpm appends a `(peer@ver)` suffix to some spec values; the parser + // strips it before classifying the source, so strip it here too. + // + // This MUST stay byte-for-byte identical to `pnpm::read::push_direct`'s + // `classify_version` (`info.version.split('(').next()`), which is what + // produced the `dep_path` keys in `graph.packages` we're matching + // against. A "smarter" strip (e.g. only a trailing `(peer@…)` via + // rfind) would *desync* the two: any value with a non-peer `(` would + // hash differently here than the key the parser inserted, silently + // re-skipping that child in the linker and graph hasher. If the + // first-`(` truncation is ever wrong for a real spec, fix it in + // `push_direct` and here together — never in isolation. + let classify = dep_value.split('(').next().unwrap_or(dep_value); + match LocalSource::parse(classify, Path::new("")) { + Some(LocalSource::Git(mut git)) => { + // Snapshot specs carry the pinned commit after `#`, which + // `parse` records as `committish` rather than `resolved`. The + // package was keyed with that commit promoted to `resolved` + // (see `push_direct`), so promote it here too — otherwise the + // `url#resolved` hash diverges from the package's dep_path. + if git.resolved.is_empty() { + git.resolved = git.committish.take()?; + } + Some(LocalSource::Git(git).dep_path(dep_name)) + } + Some(tarball @ LocalSource::RemoteTarball(_)) => Some(tarball.dep_path(dep_name)), + _ => None, + } +} + /// Parse a git dependency specifier into `(clone_url, committish)`. /// /// Recognized forms: @@ -1014,4 +1094,113 @@ mod tests { }); assert_ne!(a.dep_path("dep"), b.dep_path("dep")); } + + const SHARED_SHA: &str = "0123456789abcdef0123456789abcdef01234567"; + + /// The dep_path the lockfile parser keys a git package under, given + /// its normalized clone URL and pinned commit. + fn git_key(url: &str, resolved: &str) -> String { + LocalSource::Git(GitSource { + url: url.to_string(), + committish: None, + resolved: resolved.to_string(), + integrity: None, + subpath: None, + }) + .dep_path("request") + } + + /// The dep_path the lockfile parser keys a remote-tarball package + /// under, given its fetch URL. + fn tarball_key(url: &str) -> String { + LocalSource::RemoteTarball(RemoteTarballSource { + url: url.to_string(), + integrity: String::new(), + git_hosted: false, + }) + .dep_path("request") + } + + #[test] + fn shared_github_shorthand_maps_to_git_dep_path() { + // A dependent records its git `request` via the `github:` spec, + // but the package is keyed under the hashed `git+` dep_path. The + // sibling symlink / hasher lookup must use that same key or it + // dangles / silently skips the child. + let got = shared_local_dep_path("request", &format!("github:request/request#{SHARED_SHA}")) + .expect("github: spec is a shareable local source"); + assert_eq!( + got, + git_key("https://github.com/request/request.git", SHARED_SHA) + ); + assert!(got.starts_with("request@git+"), "unexpected key: {got}"); + } + + #[test] + fn shared_git_url_and_shorthand_converge() { + // Whether the dependent recorded the shorthand or the resolved + // `.git#` form, both must canonicalize to one key. + let from_shorthand = + shared_local_dep_path("request", &format!("github:request/request#{SHARED_SHA}")) + .unwrap(); + let from_url = shared_local_dep_path( + "request", + &format!("https://github.com/request/request.git#{SHARED_SHA}"), + ) + .unwrap(); + assert_eq!(from_shorthand, from_url); + } + + #[test] + fn shared_missing_resolved_is_promoted_from_committish() { + // A lockfile round-trip that never re-resolved leaves `resolved` + // empty and only carries `#`; the helper must promote + // it so the hash matches the package's `#` key. + let got = shared_local_dep_path( + "request", + &format!("https://github.com/request/request.git#{SHARED_SHA}"), + ) + .unwrap(); + assert_eq!( + got, + git_key("https://github.com/request/request.git", SHARED_SHA) + ); + } + + #[test] + fn shared_codeload_tarball_maps_to_url_dep_path() { + // The exact form pnpm records for a `github:` dep that resolves to + // a codeload archive. This is the case that crashed + // request-promise-core under the global virtual store. + let url = format!("https://codeload.github.com/request/request/tar.gz/{SHARED_SHA}"); + let got = shared_local_dep_path("request", &url).unwrap(); + assert_eq!(got, tarball_key(&url)); + assert!(got.starts_with("request@url+"), "unexpected key: {got}"); + } + + #[test] + fn shared_strips_peer_suffix_before_classifying() { + let url = format!("https://codeload.github.com/request/request/tar.gz/{SHARED_SHA}"); + let with_peer = format!("{url}(typescript@5.8.3)"); + assert_eq!( + shared_local_dep_path("request", &with_peer), + shared_local_dep_path("request", &url), + ); + } + + #[test] + fn shared_returns_none_for_non_shareable_specs() { + for value in [ + "4.18.1", + "^1.2.3", + "link:../sibling", + "file:./vendor/x", + "npm:lodash@4.18.1", + ] { + assert!( + shared_local_dep_path("dep", value).is_none(), + "{value:?} must not be treated as a shareable local source", + ); + } + } } diff --git a/crates/aube-store/src/index.rs b/crates/aube-store/src/index.rs index e77e39909..e049629c2 100644 --- a/crates/aube-store/src/index.rs +++ b/crates/aube-store/src/index.rs @@ -32,6 +32,40 @@ pub struct StoredFile { /// file content). pub type PackageIndex = aube_util::collections::FxMap; +/// Deterministic content fingerprint of a materialized package. +/// +/// Hashes the package's full file set — every relative path plus its +/// CAS content hash and executable bit — in sorted order, so two +/// imports with byte-identical trees produce the same fingerprint and +/// two imports that differ in any file (presence, contents, or mode) +/// produce different fingerprints. +/// +/// Used by the global virtual store to disambiguate source-backed +/// dependencies (git / remote tarball) whose lockfile coordinate is +/// identical but whose materialized bytes are not — e.g. the same git +/// commit installed once normally (its `prepare` script built `dist/`) +/// and once under `--ignore-scripts` (raw checkout, no `dist/`). The +/// graph hash folds this in so the two land at distinct GVS paths +/// instead of the first writer's tree leaking into the second project. +/// +/// `PackageIndex` is an `FxMap` with non-deterministic iteration order, +/// so the entries are collected and sorted by path before hashing. +pub fn index_content_fingerprint(index: &PackageIndex) -> String { + let mut entries: Vec<(&str, &str, bool)> = index + .iter() + .map(|(path, file)| (path.as_str(), file.hex_hash.as_str(), file.executable)) + .collect(); + entries.sort_unstable(); + let mut hasher = blake3::Hasher::new(); + for (path, hex_hash, executable) in entries { + hasher.update(path.as_bytes()); + hasher.update(b"\0"); + hasher.update(hex_hash.as_bytes()); + hasher.update(if executable { b"\x01" } else { b"\x00" }); + } + hasher.finalize().to_hex().to_string() +} + fn index_files_match_metadata(index: &PackageIndex, verify_all: bool) -> bool { let mut files = index.values(); if verify_all { diff --git a/crates/aube-store/src/lib.rs b/crates/aube-store/src/lib.rs index 155f199c2..85289cc3d 100644 --- a/crates/aube-store/src/lib.rs +++ b/crates/aube-store/src/lib.rs @@ -22,7 +22,7 @@ use cas::copy_dir_recursive; use git::{ codeload_cache_paths, extract_codeload_tarball_at, git_commit_matches, validate_git_positional, }; -pub use index::{PackageIndex, StoredFile}; +pub use index::{PackageIndex, StoredFile, index_content_fingerprint}; pub use integrity::{ SHA512_INTEGRITY_PREFIX, integrity_to_hex, sha512_integrity, validate_and_encode_name, validate_pkg_content, validate_version, verify_integrity, verify_precomputed_sha512, diff --git a/crates/aube/src/commands/install/link.rs b/crates/aube/src/commands/install/link.rs index c6ac57e1e..e6e0554b4 100644 --- a/crates/aube/src/commands/install/link.rs +++ b/crates/aube/src/commands/install/link.rs @@ -190,6 +190,55 @@ pub(super) fn run_link_phase(input: LinkPhaseInput<'_>) -> miette::Result#` coordinate. Two + // installs of the same coordinate can still materialize + // different bytes — most commonly a git dep whose `prepare` + // built `dist/` versus the same commit installed under + // `--ignore-scripts` (raw checkout). Fingerprint the actual + // imported tree and fold it into the hash so those two land at + // distinct GVS paths instead of the first writer's tree leaking + // into the second project. + let mut content_hashes: aube_util::collections::FxMap = + aube_util::collections::FxMap::default(); + for (dep_path, pkg) in &graph_for_link.packages { + let is_shareable_source = pkg + .local_source + .as_ref() + .is_some_and(|s| s.is_globally_shareable()); + if !is_shareable_source { + continue; + } + // The fingerprint *defines* this dep's GVS path, and the + // linker keys its dependents' sibling symlinks off the same + // hash. Silently dropping a dep whose index is absent would + // compute the parent's path with a fingerprint-less hash + // while the dep itself was materialized at the + // fingerprinted path — a dangling sibling and a runtime + // `Cannot find module`. The fetch driver guarantees every + // in-graph source dep is imported (and thus indexed), so a + // miss is a contract violation, not a recoverable cache + // gap: there is no `store.load_index` fallback because + // git/tarball indices aren't persisted by coordinate (a + // prepared tree and its raw `--ignore-scripts` checkout + // would collide). Fail loudly to keep the invariant honest. + let index = package_indices.get(dep_path).ok_or_else(|| { + miette!( + code = aube_codes::errors::ERR_AUBE_MISSING_PACKAGE_INDEX, + "internal: globally-shared source dependency {dep_path} is in the link \ + graph but missing from package_indices; cannot fingerprint its content \ + for the global virtual store" + ) + })?; + content_hashes.insert( + dep_path.clone(), + aube_store::index_content_fingerprint(index), + ); + } + let content_hash_fn = + |dep_path: &str| -> Option { content_hashes.get(dep_path).cloned() }; + // Reuse the prewarm task's `compute_graph_hashes` output when // the link-phase graph matches what the prewarm hashed. The // prewarm hashed the unfiltered post-resolve graph; if no @@ -198,8 +247,15 @@ pub(super) fn run_link_phase(input: LinkPhaseInput<'_>) -> miette::Result) -> miette::Result