Skip to content
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
147 changes: 147 additions & 0 deletions crates/aube-lockfile/src/graph_hash.rs
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,63 @@ fn transitively_requires_build(
result
}

/// The set of dep_paths whose final graph hash folds in a content
/// fingerprint: every globally-shareable source dependency (git /
/// remote tarball — see [`LocalSource::is_globally_shareable`]) plus
/// every package that transitively depends on one.
///
/// This exists to keep the GVS-prewarm materializer honest. Prewarm
/// runs *concurrently with fetch*, so it can't fingerprint source trees
/// that haven't been imported yet — it hashes with [`ContentHashFn`]
/// returning `None` for everything. The link phase runs after fetch and
/// folds the real fingerprints in via [`compute_graph_hashes_full`]. For
/// any package in this set the two passes compute *different* hashes, so
/// a node the prewarm materializes lands at a content-less path the link
/// phase never references — stranding a duplicate cohort in the global
/// store. Worse, prewarm skips the shareable-source *leaves* themselves
/// (it can't materialize an un-fetched tree), so that stranded cohort's
/// sibling symlinks dangle and Node's module walk silently resolves a
/// second copy of the package higher up the tree (a duplicate-singleton
/// "Cannot find module" class of bug). Prewarm must skip exactly this set
/// and defer it to the link phase, which materializes every node at its
/// final content-ful path.
///
/// Computed by reverse reachability from the shareable-source seeds so
/// it's order- and cycle-independent: a source-backed subtree can sit
/// inside a self-referential peer-dependency cycle, which a forward DFS
/// memo would mis-handle depending on traversal entry point.
///
/// [`LocalSource::is_globally_shareable`]: crate::LocalSource::is_globally_shareable
pub fn content_affected_dep_paths(graph: &LockfileGraph) -> FxHashSet<String> {
let mut parents_of: FxHashMap<String, Vec<String>> = FxHashMap::default();
let mut stack: Vec<String> = Vec::new();
for (dep_path, pkg) in &graph.packages {
if pkg
.local_source
.as_ref()
.is_some_and(|source| source.is_globally_shareable())
{
stack.push(dep_path.clone());
}
for (alias, child_tail) in &pkg.dependencies {
let child = child_dep_path(alias, child_tail);
if graph.packages.contains_key(&child) {
parents_of.entry(child).or_default().push(dep_path.clone());
}
}
}
let mut affected: FxHashSet<String> = FxHashSet::default();
while let Some(dep_path) = stack.pop() {
if !affected.insert(dep_path.clone()) {
continue;
}
if let Some(parents) = parents_of.get(&dep_path) {
stack.extend(parents.iter().cloned());
}
}
affected
}

/// `full_pkg_id` — pnpm uses `${pkgIdWithPatchHash}:${resolution}`; we
/// use `${name}@${version}[:patch:<hex>]:${source?}[:content:<hex>]:${integrity}`.
/// Source-backed packages fold in their stable specifier so two local
Expand Down Expand Up @@ -721,6 +778,96 @@ mod tests {
assert!(h.node_hash.contains_key("b@1.0.0"));
}

fn shareable_source() -> LocalSource {
LocalSource::RemoteTarball(crate::RemoteTarballSource {
url: "https://example.com/dep.tgz".into(),
integrity: "sha512-Z".into(),
git_hosted: false,
})
}

#[test]
fn content_affected_covers_shareable_source_and_all_ancestors() {
// parent -> midware -> tarball(shareable); `pure` is an unrelated
// sibling whose subtree contains no source dep.
let mut g = empty_graph();
let mut parent = mk_pkg("parent", "1.0.0", Some("sha512-P"));
parent.dependencies.insert("midware".into(), "1.0.0".into());
g.packages.insert("parent@1.0.0".into(), parent);

let mut midware = mk_pkg("midware", "1.0.0", Some("sha512-M"));
midware
.dependencies
.insert("tardep".into(), "url+aaa".into());
g.packages.insert("midware@1.0.0".into(), midware);

let mut tardep = mk_pkg("tardep", "1.0.0", None);
tardep.dep_path = "tardep@url+aaa".into();
tardep.local_source = Some(shareable_source());
g.packages.insert("tardep@url+aaa".into(), tardep);

g.packages.insert(
"pure@1.0.0".into(),
mk_pkg("pure", "1.0.0", Some("sha512-X")),
);

let affected = content_affected_dep_paths(&g);
assert!(
affected.contains("tardep@url+aaa"),
"the source leaf itself"
);
assert!(affected.contains("midware@1.0.0"), "direct ancestor");
assert!(affected.contains("parent@1.0.0"), "transitive ancestor");
assert!(
!affected.contains("pure@1.0.0"),
"a source-free subtree must stay prewarm-eligible"
);
}

#[test]
fn content_affected_handles_self_referential_cycle() {
// host <-> srcdep(shareable) cycle, mirroring a real-world
// self-referential peer cycle. Both nodes must be flagged
// regardless of the back-edge; reverse reachability from the
// source seed makes this order-independent.
let mut g = empty_graph();
let mut host = mk_pkg("host", "2.0.0", Some("sha512-L"));
host.dependencies.insert("srcdep".into(), "url+bbb".into());
g.packages.insert("host@2.0.0".into(), host);

let mut srcdep = mk_pkg("srcdep", "2.0.0", None);
srcdep.dep_path = "srcdep@url+bbb".into();
srcdep.local_source = Some(shareable_source());
srcdep.dependencies.insert("host".into(), "2.0.0".into());
g.packages.insert("srcdep@url+bbb".into(), srcdep);

let affected = content_affected_dep_paths(&g);
assert!(affected.contains("srcdep@url+bbb"));
assert!(
affected.contains("host@2.0.0"),
"ancestor inside a cycle with the source must still be flagged"
);
}

#[test]
fn content_affected_ignores_non_shareable_local_sources() {
// A `file:` directory dep is not globally shareable: it gets no
// content fingerprint, so its hash is identical across prewarm
// and link and prewarm may safely materialize its ancestors.
let mut g = empty_graph();
let mut parent = mk_pkg("parent", "1.0.0", Some("sha512-P"));
parent.dependencies.insert("dir".into(), "file+ccc".into());
g.packages.insert("parent@1.0.0".into(), parent);

let mut dir = mk_pkg("dir", "1.0.0", None);
dir.dep_path = "dir@file+ccc".into();
dir.local_source = Some(LocalSource::Directory(PathBuf::from("vendor/dir")));
g.packages.insert("dir@file+ccc".into(), dir);

let affected = content_affected_dep_paths(&g);
assert!(affected.is_empty(), "got: {affected:?}");
}

#[test]
fn hashed_dep_path_appends_to_leaf() {
let mut h = GraphHashes::default();
Expand Down
2 changes: 1 addition & 1 deletion crates/aube-lockfile/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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, shared_local_dep_path,
parse_git_spec, parse_hosted_git, resolve_dep_edge, shared_local_dep_path,
};

use smallvec::SmallVec;
Expand Down
166 changes: 166 additions & 0 deletions crates/aube-lockfile/src/pnpm/dep_path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,102 @@ pub(super) fn peerless_dep_path(name: &str, value: &str) -> String {
version_to_dep_path(name, value.split('(').next().unwrap_or(value))
}

/// Split a dep_path tail's peer suffix into outer-level paren segments,
/// each returned *with* its enclosing parens. `react-dom@18.2.0(react@18.2.0)`
/// yields `["(react@18.2.0)"]`; a nested form
/// `consumer@1.0.0(react-dom@18.2.0(react@18.2.0))` yields the single outer
/// segment `["(react-dom@18.2.0(react@18.2.0))"]` with the inner segment
/// preserved verbatim. The canonical `name@version` head (everything before
/// the first `(`) is skipped. Mirrors the resolver's identically-named
/// helper; duplicated here because the two crates can't share it without a
/// public surface neither wants to commit to.
Comment thread
greptile-apps[bot] marked this conversation as resolved.
Resolved
fn outer_paren_segments(s: &str) -> Vec<&str> {
let bytes = s.as_bytes();
let mut segments = Vec::new();
let mut i = 0;
while i < bytes.len() && bytes[i] != b'(' {
i += 1;
}
while i < bytes.len() {
if bytes[i] != b'(' {
i += 1;
continue;
}
let start = i;
let mut depth: i32 = 0;
while i < bytes.len() {
match bytes[i] {
b'(' => depth += 1,
b')' => {
depth -= 1;
if depth == 0 {
i += 1;
segments.push(&s[start..i]);
break;
}
}
_ => {}
}
i += 1;
}
if depth != 0 {
break;
}
}
segments
}

/// Rewrite the peer-suffix references of a dep_path (or a dep *value*,
/// which is a headless `version{suffix}`) by passing each peer
/// reference's flat `name@version` head through `translate`. The head
/// before the first `(` is left untouched — only the parenthesized peer
/// references are rewritten, and nested suffixes recurse.
///
/// `translate(head)` returns `Some(new_head)` to replace a *flat* peer
/// reference, or `None` to keep it verbatim. The two boundary passes use
/// it in opposite directions:
///
/// * writer: hashed `request@url+<hash>` → spec `request@https://…/tar.gz/<sha>`
/// (a `graph.packages` lookup yields the target's `local_source.specifier()`).
/// * reader: spec → hashed (`shared_local_dep_path` re-derives the dep_path),
/// keeping the in-memory graph FS-safe so a round-trip matches a fresh
/// resolve byte-for-byte.
///
/// Registry peers (`react@18.2.0`) translate to `None` and pass through
/// unchanged, so this is a no-op on graphs with no git / tarball peers.
pub(super) fn rewrite_peer_suffix(s: &str, translate: &impl Fn(&str) -> Option<String>) -> String {
let Some(head_end) = s.find('(') else {
return s.to_string();
};
let mut out = String::with_capacity(s.len());
out.push_str(&s[..head_end]);
for seg in outer_paren_segments(&s[head_end..]) {
out.push('(');
out.push_str(&rewrite_peer_reference(&seg[1..seg.len() - 1], translate));
out.push(')');
}
out
}

/// Rewrite a single peer reference (the contents between one pair of
/// suffix parens). A flat reference (`request@url+<hash>`) is handed to
/// `translate`; a reference that carries its own nested suffix
/// (`request-promise@4.2.6(request@url+<hash>)`) keeps its head and
/// recurses, so the nested git / tarball peer is still translated.
fn rewrite_peer_reference(inner: &str, translate: &impl Fn(&str) -> Option<String>) -> String {
let Some(nested_start) = inner.find('(') else {
return translate(inner).unwrap_or_else(|| inner.to_string());
};
let mut out = String::with_capacity(inner.len());
out.push_str(&inner[..nested_start]);
for seg in outer_paren_segments(&inner[nested_start..]) {
out.push('(');
out.push_str(&rewrite_peer_reference(&seg[1..seg.len() - 1], translate));
out.push(')');
}
out
Comment thread
greptile-apps[bot] marked this conversation as resolved.
Resolved
}

pub(super) fn peerless_alias_target<'a>(
packages: &'a BTreeMap<String, LockedPackage>,
real_dep_path: &str,
Expand Down Expand Up @@ -84,3 +180,73 @@ pub(super) fn rewrite_snapshot_alias_deps(
*dep_value = format!("{resolved}{peer_suffix}");
}
}

#[cfg(test)]
mod rewrite_peer_suffix_tests {
use super::rewrite_peer_suffix;

// Stand-in for the writer's `graph.packages` lookup / reader's
// `shared_local_dep_path`: only the one git/tarball peer translates.
fn translate(head: &str) -> Option<String> {
(head == "request@url+1ff5271859b51655")
.then(|| "request@https://codeload.github.com/owner/request/tar.gz/abc".to_string())
}

#[test]
fn no_suffix_is_unchanged() {
assert_eq!(
rewrite_peer_suffix("lodash@4.18.1", &translate),
"lodash@4.18.1"
);
// A headless dep value with no suffix passes through too.
assert_eq!(rewrite_peer_suffix("4.18.1", &translate), "4.18.1");
}

#[test]
fn flat_local_peer_renders_as_spec() {
assert_eq!(
rewrite_peer_suffix(
"request-promise-core@1.1.4(request@url+1ff5271859b51655)",
&translate
),
"request-promise-core@1.1.4(request@https://codeload.github.com/owner/request/tar.gz/abc)"
);
// Headless dep-value form is rewritten identically.
assert_eq!(
rewrite_peer_suffix("1.1.4(request@url+1ff5271859b51655)", &translate),
"1.1.4(request@https://codeload.github.com/owner/request/tar.gz/abc)"
);
}

#[test]
fn registry_peer_is_left_untouched() {
// No translation entry → verbatim, so this is a no-op for graphs
// with only registry peers.
let s = "react-dom@18.2.0(react@18.2.0)";
assert_eq!(rewrite_peer_suffix(s, &translate), s);
}

#[test]
fn nested_suffix_translates_only_the_inner_local_peer() {
// A registry peer (`request-promise`) keeps its head; the nested
// git/tarball peer inside it still renders as the spec.
assert_eq!(
rewrite_peer_suffix(
"consumer@1.0.0(request-promise@4.2.6(request@url+1ff5271859b51655))",
&translate
),
"consumer@1.0.0(request-promise@4.2.6(request@https://codeload.github.com/owner/request/tar.gz/abc))"
);
}

#[test]
fn multiple_segments_each_handled_independently() {
assert_eq!(
rewrite_peer_suffix(
"pkg@1.0.0(react@18.2.0)(request@url+1ff5271859b51655)",
&translate
),
"pkg@1.0.0(react@18.2.0)(request@https://codeload.github.com/owner/request/tar.gz/abc)"
);
}
}
Loading
Loading