Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
7 changes: 6 additions & 1 deletion crates/aube-linker/src/hoisted.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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+<hash>` /
// `name@url+<hash>` 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;
}
Expand Down
90 changes: 80 additions & 10 deletions crates/aube-linker/src/link.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,18 +127,60 @@ 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/<dep_path>` 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;
};
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;
};
Expand Down Expand Up @@ -645,18 +687,46 @@ 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/<dep_path>` 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;
};
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;
};
Expand Down
63 changes: 60 additions & 3 deletions crates/aube-linker/src/materialize.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand Down Expand Up @@ -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/<dep_path>` 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 `<name>` 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<String, PathBuf>>,
) -> 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/<dep_path>/node_modules/<name>/`.
///
Expand Down Expand Up @@ -393,7 +444,13 @@ impl Linker {
// a per-project `.aube/<dep_path>` 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+<hash>` / `name@url+<hash>` 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
Expand Down
71 changes: 71 additions & 0 deletions crates/aube-linker/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<String, aube_store::PackageIndex> = 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@<new>` entry. The old `.aube/foo@<old>/` is left
Expand Down
Loading
Loading