diff --git a/src/install/PackageInstall.rs b/src/install/PackageInstall.rs index 3bd8f1baa916..34055ab52d37 100644 --- a/src/install/PackageInstall.rs +++ b/src/install/PackageInstall.rs @@ -1,6 +1,6 @@ use core::sync::atomic::{AtomicU8, Ordering}; -use bun_collections::{ArrayHashMap, DynamicBitSet}; +use bun_collections::{ArrayHashMap, DynamicBitSet, StringHashMap}; use bun_core::Progress::Progress; use bun_core::{Global, Output}; use bun_core::{MutableString, ZStr}; @@ -14,6 +14,7 @@ use bun_threading::thread_pool::{Batch, Node as ThreadPoolNode}; use bun_threading::work_pool::Task as WorkPoolTask; use bun_threading::{ThreadPool, WaitGroup}; +use crate::lockfile::package::PackageColumns as _; use crate::package_installer::NodeModulesFolder; use crate::{ BuntagHashBuf, Lockfile, Npm, PackageID, PackageManager, Repository, Resolution, @@ -2562,4 +2563,256 @@ impl<'a> PackageInstall<'a> { } } +/// Remove top-level entries from an already-open root `node_modules` +/// directory whose names are not in `expected`. Scoped packages are handled +/// by descending one level into `@scope/` and removing the empty scope +/// directory if nothing remains. Dot-prefixed entries (`.bin`, `.bun`, +/// `.cache`, …) are never touched. Finishes with a sweep of `bin_path` that +/// unlinks any `.bin` entry left dangling by a deleted package. +/// +/// Called by both the hoisted and isolated install paths after +/// `Lockfile::clean_with_logger` has rebuilt the lockfile, so a dependency +/// removed from `package.json` is also removed from disk on the next +/// `bun install` instead of remaining importable until the user deletes +/// `node_modules` by hand. +/// +/// Each linker builds `expected` as "every folder name the lockfile can +/// legitimately place at the root", independent of install flags: the root +/// package's declared dependency aliases (all behaviors, so `--production` +/// or `--omit` never turn a reinstall into a delete) unioned with the +/// linker's own root placements (the hoisted root tree, or every lockfile +/// alias matching `publicHoistPattern` for the isolated store). +/// +/// `keep_symlinks` is set by the hoisted linker: its extraneous entries are +/// always real extracted directories, while a root symlink there is a +/// workspace link, a `file:` folder dependency, or a `bun link ` +/// registration. The last of those is recorded in no manifest (`bun link` is +/// `--no-save` by default), so it can never be in `expected` and must not be +/// deleted. The isolated linker's own root entries are symlinks into +/// `node_modules/.bun`, so it cannot skip them. +/// +/// The prune is best effort: a stale directory the filesystem refuses to +/// delete (locked, read-only) is left behind rather than failing the +/// install, matching the uninstall paths elsewhere in this file. +pub(crate) fn prune_extraneous_node_modules( + expected: &StringHashMap<()>, + node_modules: Fd, + bin_path: &[u8], + keep_symlinks: bool, +) { + let dir = Dir::borrow(&node_modules); + + // Snapshot the listing before deleting anything. `delete_tree` mutates + // the directory, and on some filesystems a readdir refill after a delete + // can re-surface entries; iterating an owned list sidesteps that. + let mut names: Vec> = Vec::new(); + let mut iter = sys::iterate_dir(node_modules); + loop { + let entry = match iter.next() { + Ok(Some(e)) => e, + Ok(None) => break, + Err(_) => return, + }; + let name = entry.name.slice_u8(); + if name.is_empty() || name[0] == b'.' { + continue; + } + if entry_is_symlink(node_modules, &entry) { + // A `@scope` symlink is never descended into: `delete_tree` on a + // symlink unlinks the link itself without following it, but the + // scope walk below opens through the link and could delete the + // contents of a directory outside `node_modules`. + if keep_symlinks || name[0] == b'@' { + continue; + } + } + names.push(name.to_vec()); + } + + let mut removed_any = false; + for name in &names { + if name[0] == b'@' { + removed_any |= prune_extraneous_scope(node_modules, name, expected, keep_symlinks); + continue; + } + if expected.contains_key(name.as_slice()) { + continue; + } + if dir.delete_tree(name).is_ok() { + removed_any = true; + } + } + + // A deleted package's `.bin` symlink now dangles; sweep it so scripts get + // "command not found" instead of an ENOENT on the link target. + if removed_any { + if let Ok(bin_dir) = sys::open_dir_for_iteration(Fd::cwd(), bin_path) { + prune_dangling_bin_links(bin_dir); + } + } +} + +/// Insert the alias of every package with a `patchedDependencies` entry into +/// `expected`. `bun patch` materializes the package being patched at the root +/// `node_modules/` (under the isolated linker, as a symlink into the +/// store) even when no tree places it there, so the prune must treat those +/// names as expected or `bun patch --commit` removes the folder it just told +/// the user about. Matches packages to patches the same way the installer +/// does: by the hash of `name@version`. +pub(crate) fn extend_expected_with_patched_packages( + expected: &mut StringHashMap<()>, + lockfile: &Lockfile, +) -> Result<(), bun_alloc::AllocError> { + if lockfile.patched_dependencies.count() == 0 { + return Ok(()); + } + let string_buf = lockfile.buffers.string_bytes.as_slice(); + let pkgs = lockfile.packages.slice(); + let names = pkgs.items_name(); + let resolutions = pkgs.items_resolution(); + let mut name_and_version: Vec = Vec::new(); + for i in 0..pkgs.len() { + name_and_version.clear(); + use std::io::Write; + write!( + &mut name_and_version, + "{}@{}", + bstr::BStr::new(names[i].slice(string_buf)), + resolutions[i].fmt(string_buf, bun_core::fmt::PathSep::Posix), + ) + .expect("writing to a Vec cannot fail"); + let name_and_version_hash = bun_semver::string::Builder::string_hash(&name_and_version); + if lockfile + .patched_dependencies + .get(&name_and_version_hash) + .is_some() + { + expected.put(names[i].slice(string_buf), ())?; + } + } + Ok(()) +} + +/// Whether a directory entry is a symlink. `readdir` leaves `d_type` unset on +/// some filesystems (NFS, FUSE, bind mounts), so `Unknown` is resolved with an +/// `lstat` rather than treated as "not a symlink": misclassifying a symlink +/// here would delete a `bun link` registration. +#[cfg(not(windows))] +fn entry_is_symlink(dir: Fd, entry: &sys::dir_iterator::IteratorResult) -> bool { + match entry.kind { + EntryKind::SymLink => true, + EntryKind::Unknown => matches!( + sys::lstatat(dir, entry.name.as_zstr()), + Ok(st) if sys::kind_from_mode(st.st_mode as sys::Mode) == EntryKind::SymLink + ), + _ => false, + } +} +/// Windows directory iteration always reports accurate kinds. +#[cfg(windows)] +fn entry_is_symlink(_dir: Fd, entry: &sys::dir_iterator::IteratorResult) -> bool { + matches!(entry.kind, EntryKind::SymLink) +} + +/// Unlink every dangling symlink in the already-open `.bin` directory and +/// close `bin_dir`. Shared by the install-time prune above and the +/// `bun remove` cleanup in `updatePackageJSONAndInstall.rs`. +pub(crate) fn prune_dangling_bin_links(bin_dir: Fd) { + let mut name_buf = PathBuffer::uninit(); + let mut iter = sys::iterate_dir(bin_dir); + loop { + let Ok(Some(entry)) = iter.next() else { break }; + if !entry_is_symlink(bin_dir, &entry) { + continue; + } + // A symlink whose target no longer exists cannot be opened. + // `access` would not work here because it does not resolve + // symlinks. Only a missing target makes a link dangling; + // other open failures (EACCES, EMFILE, transient I/O) can + // happen to a perfectly good shim and must not delete it. + let name = entry.name.slice_u8(); + name_buf[..name.len()].copy_from_slice(name); + name_buf[name.len()] = 0; + let buf: &ZStr = ZStr::from_buf(&name_buf, name.len()); + + match sys::File::openat(bin_dir, buf, sys::O::RDONLY, 0) { + Ok(file) => { + let _ = file.close(); + } + Err(err) if err.get_errno() == sys::E::ENOENT || err.get_errno() == sys::E::ENOTDIR => { + let _ = sys::unlinkat(bin_dir, buf); + } + Err(_) => {} + } + } + let _ = sys::close(bin_dir); +} + +/// Returns `true` if anything was deleted. +fn prune_extraneous_scope( + parent: Fd, + scope: &[u8], + expected: &StringHashMap<()>, + keep_symlinks: bool, +) -> bool { + let scope_fd = match sys::open_dir_for_iteration(parent, scope) { + Ok(fd) => fd, + Err(_) => return false, + }; + let scope_dir = Dir::from_fd(scope_fd); + + let mut names: Vec> = Vec::new(); + let mut has_remaining = false; + let mut iter = sys::iterate_dir(scope_dir.fd()); + loop { + let entry = match iter.next() { + Ok(Some(e)) => e, + Ok(None) => break, + Err(_) => return false, + }; + let name = entry.name.slice_u8(); + if name.is_empty() { + continue; + } + if name[0] == b'.' || (keep_symlinks && entry_is_symlink(scope_dir.fd(), &entry)) { + has_remaining = true; + continue; + } + names.push(name.to_vec()); + } + + let mut removed_any = false; + for name in &names { + let mut full = Vec::with_capacity(scope.len() + 1 + name.len()); + full.extend_from_slice(scope); + full.push(b'/'); + full.extend_from_slice(name); + + if expected.contains_key(full.as_slice()) { + has_remaining = true; + continue; + } + + if scope_dir.delete_tree(name).is_ok() { + removed_any = true; + } else { + has_remaining = true; + } + } + + // `scope_dir` owns the fd and must be dropped before removing the + // directory it points at (Windows cannot remove a dir with open handles). + drop(scope_dir); + + if !has_remaining { + let mut buf = Vec::with_capacity(scope.len() + 1); + buf.extend_from_slice(scope); + buf.push(0); + let z = ZStr::from_buf(&buf, scope.len()); + let _ = sys::rmdirat(parent, z); + } + + removed_any +} + type Walker = walker_skippable::Walker; diff --git a/src/install/PackageManager/updatePackageJSONAndInstall.rs b/src/install/PackageManager/updatePackageJSONAndInstall.rs index bf38804aeb70..a796fa5c6c08 100644 --- a/src/install/PackageManager/updatePackageJSONAndInstall.rs +++ b/src/install/PackageManager/updatePackageJSONAndInstall.rs @@ -692,38 +692,7 @@ fn update_package_json_and_install_with_manager_with_updates( // This could be slow if there are a lot of symlinks match bun_sys::open_dir_for_iteration(cwd.fd(), manager.options.bin_path.as_bytes()) { Ok(node_modules_bin) => { - // `defer node_modules_bin.close()` — explicit close below (Fd is Copy, no Drop). - let mut iter = bun_sys::iterate_dir(node_modules_bin); - 'iterator: loop { - let Ok(Some(entry)) = iter.next() else { break }; - match entry.kind { - bun_sys::EntryKind::SymLink => { - // any symlinks which we are unable to open are assumed to be dangling - // note that using access won't work here, because access doesn't resolve symlinks - let name = entry.name.slice_u8(); - node_modules_buf[..name.len()].copy_from_slice(name); - node_modules_buf[name.len()] = 0; - let buf: &ZStr = ZStr::from_buf(&node_modules_buf, name.len()); - - match bun_sys::File::openat( - node_modules_bin, - buf, - bun_sys::O::RDONLY, - 0, - ) { - Ok(file) => { - let _ = file.close(); - } - Err(_) => { - let _ = bun_sys::unlinkat(node_modules_bin, buf); - continue 'iterator; - } - } - } - _ => {} - } - } - let _ = bun_sys::close(node_modules_bin); + crate::package_install::prune_dangling_bin_links(node_modules_bin); } Err(err) => { if err.get_errno() != bun_sys::E::ENOENT { diff --git a/src/install/hoisted_install.rs b/src/install/hoisted_install.rs index 86206cbe8949..2a0a232b7d15 100644 --- a/src/install/hoisted_install.rs +++ b/src/install/hoisted_install.rs @@ -127,6 +127,65 @@ pub(crate) fn install_hoisted_packages( }, ); + // Every folder name the lockfile can legitimately place at the root + // `node_modules`, so stale entries on disk can be pruned once the + // directory is opened below: the root package's declared dependency + // aliases (all behaviors, so `--production`/`--omit` never turn a + // reinstall into a delete) unioned with the unfiltered root tree (the + // hoisted transitives that belong there; the pre-`filter()` snapshot is + // read back through `_restore_buffers`, so `--filter` cannot shrink it). + // Built after the restore guard is armed so a fallible step cannot + // return with the original buffers dropped. + // + // `None` (no prune) for: + // - global installs: the global `node_modules` is also the `bun link` + // registry, whose entries are bare symlinks recorded in no manifest + // and must survive `bun add -g`. + // - the security scanner's narrowed pre-install pass; the full install + // that follows performs the prune. + let expected_root_entries: Option> = 'expected: { + if this.options.global || packages_to_install.is_some() || this.lockfile.packages.is_empty() + { + break 'expected None; + } + let deps = this.lockfile.buffers.dependencies.as_slice(); + let string_buf = this.lockfile.buffers.string_bytes.as_slice(); + let (original_trees, original_tree_dep_ids) = &*_restore_buffers; + // A lockfile with no dependencies at all has zero trees; that is a + // legitimate state (the last dependency was just removed) in which + // nothing belongs at the root, not a reason to skip the prune. + let root_tree_dep_ids: &[DependencyID] = if original_trees.is_empty() { + &[] + } else { + original_trees[0].dependencies.get(original_tree_dep_ids) + }; + let root_pkg_deps = this.lockfile.packages.slice().items_dependencies()[0]; + let mut set = StringHashMap::<()>::with_capacity( + root_tree_dep_ids.len() + root_pkg_deps.len as usize, + ); + // An incomplete kept set would delete entries the lockfile still + // places at the root, so insertion failures must not be ignored. + let mut keep = |dep_id: DependencyID| -> Result<(), bun_alloc::AllocError> { + if let Some(dep) = deps.get(dep_id as usize) { + let alias = dep.name.slice(string_buf); + if !alias.is_empty() { + set.put(alias, ())?; + } + } + Ok(()) + }; + for &dep_id in root_tree_dep_ids { + keep(dep_id)?; + } + for dep_id in root_pkg_deps.begin()..root_pkg_deps.end() { + keep(dep_id)?; + } + // `bun patch` materializes the patched package at the root even when + // no tree places it there; its folder must survive reinstalls. + package_install::extend_expected_with_patched_packages(&mut set, &this.lockfile)?; + break 'expected Some(set); + }; + let mut download_node: ProgressNode; let mut install_node: ProgressNode = ProgressNode::default(); let mut scripts_node: ProgressNode; @@ -215,6 +274,24 @@ pub(crate) fn install_hoisted_packages( } }; + // Remove entries the cleaned lockfile no longer places at the root, so a + // dependency dropped from `package.json` is also removed from disk by + // `bun install`. + if !new_node_modules { + if let Some(expected) = expected_root_entries.as_ref() { + // `keep_symlinks`: a root symlink here is a workspace link, a + // `file:` folder dependency, or an unsaved `bun link ` + // registration; the last is recorded in no manifest and must + // survive `bun install`. + package_install::prune_extraneous_node_modules( + expected, + node_modules_folder.fd(), + this.options.bin_path.as_bytes(), + true, + ); + } + } + let mut skip_delete = new_node_modules; let mut skip_verify_installed_version_number = new_node_modules; diff --git a/src/install/isolated_install.rs b/src/install/isolated_install.rs index 40d22bd7e0b6..766a4f8138ad 100644 --- a/src/install/isolated_install.rs +++ b/src/install/isolated_install.rs @@ -1971,6 +1971,70 @@ pub(crate) fn install_isolated_packages( let pkg_name_hashes = pkgs.items_name_hash(); let pkg_resolutions = pkgs.items_resolution(); + // Remove root `node_modules` entries the cleaned lockfile no longer + // places there, so a dependency dropped from `package.json` is also + // removed from disk by `bun install`. The kept set is everything the + // isolated linker can legitimately place at the root, independent of + // install flags: the root package's declared dependency aliases (all + // behaviors, so `--production`/`--omit` never turn a reinstall into a + // delete) unioned with every lockfile alias that matches + // `publicHoistPattern` (a superset of the hoists any one install + // performs; the store's own root entry cannot be used here because it + // is narrowed by `--filter`). A dependency that became transitive + // only is in neither set and is pruned, so the root symlink for it + // stops being importable. + // + // Skipped for global installs (the global `node_modules` is also the + // `bun link` registry, whose entries are recorded in no manifest) and + // for the security scanner's narrowed pre-install pass; the full + // install that follows performs it. + if !is_new_bun_modules + && !manager.options.global + && packages_to_install.is_none() + && !lockfile_ro.packages.is_empty() + { + let root_pkg_deps = pkgs.items_dependencies()[0]; + let deps = lockfile_ro.buffers.dependencies.as_slice(); + // An incomplete kept set would delete entries the lockfile still + // places at the root, so insertion failures must not be ignored. + let mut expected = + bun_collections::StringHashMap::<()>::with_capacity(root_pkg_deps.len as usize); + for dep_id in root_pkg_deps.begin()..root_pkg_deps.end() { + let alias = deps[dep_id as usize].name.slice(string_buf); + if !alias.is_empty() { + expected.put(alias, ())?; + } + } + if let Some(public_hoist_pattern) = &manager.options.public_hoist_pattern { + for dep in deps { + let alias = dep.name.slice(string_buf); + if !alias.is_empty() && public_hoist_pattern.is_match(alias) { + expected.put(alias, ())?; + } + } + } + // `bun patch` materializes the patched package as a root symlink + // into the store even when it is not a root dependency; it must + // survive reinstalls. + crate::package_install::extend_expected_with_patched_packages( + &mut expected, + lockfile_ro, + )?; + if let Ok(fd) = sys::open_dir_for_iteration(Fd::cwd(), b"node_modules") { + // `keep_symlinks: false`: the isolated linker's own root + // entries are symlinks into `node_modules/.bun`, so they must + // be prunable. + crate::package_install::prune_extraneous_node_modules( + &expected, + fd, + manager.options.bin_path.as_bytes(), + false, + ); + use bun_sys::FdExt as _; + fd.close(); + } + } + let mut seen_entry_ids: HashMap = HashMap::default(); seen_entry_ids.reserve(store.entries.len()); diff --git a/test/cli/install/bun-install-lifecycle-scripts.test.ts b/test/cli/install/bun-install-lifecycle-scripts.test.ts index d92d8bdc2c96..86415f9a5097 100644 --- a/test/cli/install/bun-install-lifecycle-scripts.test.ts +++ b/test/cli/install/bun-install-lifecycle-scripts.test.ts @@ -3068,8 +3068,9 @@ for (const forceWaiterThread of isLinux ? [false, true] : [false]) { }); describe("add trusted, delete, then add again", async () => { - // when we change bun install to delete dependencies from node_modules - // for both cases, we need to update this test + // `bun install` prunes node_modules entries removed from the lockfile, + // so a dependency deleted from package.json by hand (withRm: false) is + // removed from disk the same way `bun rm` (withRm: true) removes it. for (const withRm of [true, false]) { test(withRm ? "withRm" : "withoutRm", async () => { using ctx = await setupTest(); @@ -3193,7 +3194,9 @@ for (const forceWaiterThread of isLinux ? [false, true] : [false]) { expected = [expect.stringContaining("bun install v1."), ...expected]; expect(out.replace(/\s*\[[0-9\.]+m?s\]\s*$/, "").split(/\r?\n/)).toEqual(expected); expect(await exited).toBe(0); - expect(await exists(join(packageDir, "node_modules", "uses-what-bin"))).toBe(!withRm); + // pruned from node_modules on both paths now that the lockfile + // no longer reaches it + expect(await exists(join(packageDir, "node_modules", "uses-what-bin"))).toBe(false); // add again, bun pm untrusted should report it as untrusted @@ -3222,18 +3225,19 @@ for (const forceWaiterThread of isLinux ? [false, true] : [false]) { expect(err).not.toContain("error:"); expect(err).not.toContain("warn:"); out = await stdout.text(); - expected = withRm - ? [ - "", - expect.stringContaining("+ uses-what-bin@1.0.0"), - "", - "1 package installed", - "", - "Blocked 1 postinstall. Run `bun pm untrusted` for details.", - "", - ] - : ["", expect.stringContaining("Checked 3 installs across 4 packages (no changes)"), ""]; - expected = [expect.stringContaining("bun install v1."), ...expected]; + // The prune removed node_modules/uses-what-bin and its hoisted + // transitive dependency what-bin on both paths, so re-adding it + // always performs a fresh, untrusted install of both. + expected = [ + expect.stringContaining("bun install v1."), + "", + expect.stringContaining("+ uses-what-bin@1.0.0"), + "", + "2 packages installed", + "", + "Blocked 1 postinstall. Run `bun pm untrusted` for details.", + "", + ]; expect(out.replace(/\s*\[[0-9\.]+m?s\]$/m, "").split(/\r?\n/)).toEqual(expected); ({ stdout, stderr, exited } = spawn({ diff --git a/test/cli/install/bun-install-registry.test.ts b/test/cli/install/bun-install-registry.test.ts index 1ab503b6fabb..f61df1ed166c 100644 --- a/test/cli/install/bun-install-registry.test.ts +++ b/test/cli/install/bun-install-registry.test.ts @@ -9097,3 +9097,416 @@ registry = { url = "https://127.0.0.1:${otherRegistry.port}/", token = "${token} expect(await exited).not.toBe(0); } }); + +// A dependency removed from package.json must also be removed from +// node_modules on the next `bun install`, not just from the lockfile. +// Otherwise `require("")` keeps working on machines that had it +// installed and breaks on fresh clones / CI. +for (const linker of ["hoisted", "isolated"] as const) { + describe(`prunes stale node_modules entries (${linker})`, () => { + test("removing a direct dependency removes it from node_modules", async () => { + const { packageDir, packageJson } = await registry.createTestDir({ + bunfigOpts: { linker, saveTextLockfile: true }, + }); + + await write( + packageJson, + JSON.stringify({ + name: "foo", + dependencies: { + "no-deps": "1.0.0", + "a-dep": "1.0.1", + }, + }), + ); + await runBunInstall(env, packageDir); + expect(await exists(join(packageDir, "node_modules", "no-deps", "package.json"))).toBe(true); + expect(await exists(join(packageDir, "node_modules", "a-dep", "package.json"))).toBe(true); + expect(await file(join(packageDir, "bun.lock")).text()).toContain('"a-dep"'); + + // drop a-dep from package.json and reinstall + await write( + packageJson, + JSON.stringify({ + name: "foo", + dependencies: { + "no-deps": "1.0.0", + }, + }), + ); + const { out } = await runBunInstall(env, packageDir, { savesLockfile: false }); + + const lock = await file(join(packageDir, "bun.lock")).text(); + expect(lock).not.toContain('"a-dep"'); + expect(lock).toContain('"no-deps"'); + + // node_modules must reflect the lockfile: the surviving dep stays, the + // removed one is gone. Use a dot-prefix filter so .cache/.bun don't fail + // the assertion across linkers. + const entries = (await readdirSorted(join(packageDir, "node_modules"))).filter(e => !e.startsWith(".")); + expect(entries).toEqual(["no-deps"]); + expect(await exists(join(packageDir, "node_modules", "a-dep"))).toBe(false); + expect(await file(join(packageDir, "node_modules", "no-deps", "package.json")).json()).toMatchObject({ + name: "no-deps", + version: "1.0.0", + }); + expect(out).toContain("1 package removed"); + }); + + test("removing a scoped dependency removes it and cleans up the empty @scope dir", async () => { + const { packageDir, packageJson } = await registry.createTestDir({ + bunfigOpts: { linker, saveTextLockfile: true }, + }); + + await write( + packageJson, + JSON.stringify({ + name: "foo", + dependencies: { + "no-deps": "1.0.0", + "@types/no-deps": "1.0.0", + }, + }), + ); + await runBunInstall(env, packageDir); + expect(await exists(join(packageDir, "node_modules", "@types", "no-deps", "package.json"))).toBe(true); + + await write( + packageJson, + JSON.stringify({ + name: "foo", + dependencies: { + "no-deps": "1.0.0", + }, + }), + ); + await runBunInstall(env, packageDir, { savesLockfile: false }); + + const entries = (await readdirSorted(join(packageDir, "node_modules"))).filter(e => !e.startsWith(".")); + expect(entries).toEqual(["no-deps"]); + expect(await exists(join(packageDir, "node_modules", "@types", "no-deps"))).toBe(false); + // empty scope directory should be cleaned up too + expect(await exists(join(packageDir, "node_modules", "@types"))).toBe(false); + }); + + test("removing a dependency with a transitive dep removes both", async () => { + const { packageDir, packageJson } = await registry.createTestDir({ + bunfigOpts: { linker, saveTextLockfile: true }, + }); + + await write( + packageJson, + JSON.stringify({ + name: "foo", + dependencies: { + "a-dep": "1.0.1", + "one-dep": "1.0.0", + }, + }), + ); + await runBunInstall(env, packageDir); + expect(await exists(join(packageDir, "node_modules", "one-dep", "package.json"))).toBe(true); + // one-dep's transitive dep no-deps@1.0.1 is hoisted to the root + expect(await exists(join(packageDir, "node_modules", "no-deps", "package.json"))).toBe(linker === "hoisted"); + + await write( + packageJson, + JSON.stringify({ + name: "foo", + dependencies: { + "a-dep": "1.0.1", + }, + }), + ); + await runBunInstall(env, packageDir, { savesLockfile: false }); + + const entries = (await readdirSorted(join(packageDir, "node_modules"))).filter(e => !e.startsWith(".")); + expect(entries).toEqual(["a-dep"]); + expect(await exists(join(packageDir, "node_modules", "one-dep"))).toBe(false); + expect(await exists(join(packageDir, "node_modules", "no-deps"))).toBe(false); + }); + + test("does not remove unrelated user directories or dotfiles", async () => { + const { packageDir, packageJson } = await registry.createTestDir({ + bunfigOpts: { linker, saveTextLockfile: true }, + }); + + await write( + packageJson, + JSON.stringify({ + name: "foo", + dependencies: { + "no-deps": "1.0.0", + "a-dep": "1.0.1", + }, + }), + ); + await runBunInstall(env, packageDir); + + // user-created entries that must survive a prune + await write(join(packageDir, "node_modules", ".cache", "something"), "keep"); + await write(join(packageDir, "node_modules", ".keep-me"), "keep"); + + await write( + packageJson, + JSON.stringify({ + name: "foo", + dependencies: { + "no-deps": "1.0.0", + }, + }), + ); + await runBunInstall(env, packageDir, { savesLockfile: false }); + + expect(await exists(join(packageDir, "node_modules", "a-dep"))).toBe(false); + expect(await exists(join(packageDir, "node_modules", "no-deps", "package.json"))).toBe(true); + expect(await exists(join(packageDir, "node_modules", ".cache", "something"))).toBe(true); + expect(await exists(join(packageDir, "node_modules", ".keep-me"))).toBe(true); + }); + + test("removing the last dependency removes it from node_modules", async () => { + const { packageDir, packageJson } = await registry.createTestDir({ + bunfigOpts: { linker, saveTextLockfile: true }, + }); + + await write(packageJson, JSON.stringify({ name: "foo", dependencies: { "no-deps": "1.0.0" } })); + await runBunInstall(env, packageDir); + expect(await exists(join(packageDir, "node_modules", "no-deps", "package.json"))).toBe(true); + + await write(packageJson, JSON.stringify({ name: "foo", dependencies: {} })); + await runBunInstall(env, packageDir, { savesLockfile: false }); + + expect((await readdirSorted(join(packageDir, "node_modules"))).filter(e => !e.startsWith("."))).toEqual([]); + expect(await exists(join(packageDir, "node_modules", "no-deps"))).toBe(false); + }); + + test("a file: folder dependency survives the prune", async () => { + const { packageDir, packageJson } = await registry.createTestDir({ + bunfigOpts: { linker, saveTextLockfile: true }, + }); + + await mkdir(join(packageDir, "local-pkg")); + await write( + join(packageDir, "local-pkg", "package.json"), + JSON.stringify({ name: "local-pkg", version: "1.0.0" }), + ); + await write( + packageJson, + JSON.stringify({ + name: "foo", + dependencies: { + "local-pkg": "file:./local-pkg", + "a-dep": "1.0.1", + }, + }), + ); + await runBunInstall(env, packageDir); + expect(await exists(join(packageDir, "node_modules", "local-pkg", "package.json"))).toBe(true); + + // drop a-dep so a prune actually runs; the folder dep must survive it + await write(packageJson, JSON.stringify({ name: "foo", dependencies: { "local-pkg": "file:./local-pkg" } })); + await runBunInstall(env, packageDir, { savesLockfile: false }); + + expect(await exists(join(packageDir, "node_modules", "a-dep"))).toBe(false); + expect(await exists(join(packageDir, "node_modules", "local-pkg", "package.json"))).toBe(true); + }); + + test("a dependency demoted to transitive-only leaves the root under the isolated linker", async () => { + const { packageDir, packageJson } = await registry.createTestDir({ + bunfigOpts: { linker, saveTextLockfile: true }, + }); + + // `one-dep` depends on `no-deps@1.0.1`, so after `no-deps` stops being a + // direct dependency it is still reachable through `one-dep`. + await write( + packageJson, + JSON.stringify({ + name: "foo", + dependencies: { + "one-dep": "1.0.0", + "no-deps": "1.0.1", + "a-dep": "1.0.1", + }, + }), + ); + await runBunInstall(env, packageDir); + expect(await exists(join(packageDir, "node_modules", "no-deps", "package.json"))).toBe(true); + + await write(packageJson, JSON.stringify({ name: "foo", dependencies: { "one-dep": "1.0.0" } })); + await runBunInstall(env, packageDir, { savesLockfile: false }); + + // `a-dep` left the graph entirely: gone on both linkers. + expect(await exists(join(packageDir, "node_modules", "a-dep"))).toBe(false); + // `no-deps` is still in the graph as `one-dep`'s transitive. The hoisted + // linker legitimately places it at the root; the isolated linker only + // places direct dependencies there, so its root symlink must be removed + // or `require("no-deps")` keeps working with no declared dependency. + expect(await exists(join(packageDir, "node_modules", "no-deps", "package.json"))).toBe(linker === "hoisted"); + }); + + test("removing a dependency removes its dangling .bin link", async () => { + const { packageDir, packageJson } = await registry.createTestDir({ + bunfigOpts: { linker, saveTextLockfile: true }, + }); + + await write( + packageJson, + JSON.stringify({ + name: "foo", + dependencies: { + "what-bin": "1.0.0", + "a-dep": "1.0.1", + }, + }), + ); + await runBunInstall(env, packageDir); + const binEntries = async () => { + try { + return (await readdirSorted(join(packageDir, "node_modules", ".bin"))).filter(e => e.startsWith("what-bin")); + } catch (err: any) { + // only a missing .bin directory counts as "no bin entries" + if (err?.code === "ENOENT") return []; + throw err; + } + }; + expect(await exists(join(packageDir, "node_modules", "what-bin", "package.json"))).toBe(true); + expect((await binEntries()).length).toBeGreaterThan(0); + + await write(packageJson, JSON.stringify({ name: "foo", dependencies: { "a-dep": "1.0.1" } })); + await runBunInstall(env, packageDir, { savesLockfile: false }); + + expect(await exists(join(packageDir, "node_modules", "what-bin"))).toBe(false); + if (!isWindows) { + // On Windows `.bin` holds `.exe`/`.bunx` shim files rather than + // symlinks, which the dangling-symlink sweep (shared with `bun rm`) + // does not cover. + expect(await binEntries()).toEqual([]); + } + }); + + test("a publicHoistPattern hoist from a filtered-out workspace survives --filter", async () => { + const { packageDir } = await registry.createTestDir({ + bunfigOpts: { linker, saveTextLockfile: true, publicHoistPattern: ["no-deps"] }, + files: { + "package.json": JSON.stringify({ name: "foo", workspaces: ["packages/*"] }), + // `one-dep` depends on `no-deps@1.0.1`; the hoist comes from a + // workspace's transitive, not from the root's own dependencies. + "packages/wsa/package.json": JSON.stringify({ + name: "wsa", + version: "1.0.0", + dependencies: { "one-dep": "1.0.0" }, + }), + "packages/wsb/package.json": JSON.stringify({ + name: "wsb", + version: "1.0.0", + dependencies: { "a-dep": "1.0.1" }, + }), + }, + }); + + await runBunInstall(env, packageDir); + expect(await exists(join(packageDir, "node_modules", "no-deps", "package.json"))).toBe(true); + + // A filtered install excludes `wsa` (the hoist's origin). The lockfile + // still places `no-deps` at the root, so the prune must not delete it. + await using proc = Bun.spawn({ + cmd: [bunExe(), "install", "--filter", "wsb"], + cwd: packageDir, + stdout: "pipe", + stderr: "pipe", + env, + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect({ stdout, stderr: stderrForInstall(stderr), exitCode }).toMatchObject({ exitCode: 0 }); + + expect(await exists(join(packageDir, "node_modules", "no-deps", "package.json"))).toBe(true); + }); + }); +} + +// The global install directory doubles as the `bun link` registry: `bun link` +// records a package as a bare `node_modules/` symlink there without +// touching the global package.json or lockfile. A global install must never +// prune it, or every link registration is destroyed by the next `bun add -g`. +test("a bun link registration survives a global install", async () => { + const { packageDir } = await registry.createTestDir({ + bunfigOpts: { saveTextLockfile: true }, + files: { + "linked-pkg/package.json": JSON.stringify({ name: "my-linked-pkg", version: "1.0.0" }), + }, + }); + + const bunInstallDir = join(packageDir, "global-home"); + const globalEnv = { + ...env, + BUN_INSTALL: bunInstallDir, + BUN_CONFIG_REGISTRY: registry.registryUrl(), + }; + const globalNodeModules = join(bunInstallDir, "install", "global", "node_modules"); + + { + await using proc = Bun.spawn({ + cmd: [bunExe(), "link"], + cwd: join(packageDir, "linked-pkg"), + stdout: "pipe", + stderr: "pipe", + env: globalEnv, + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect({ stdout, stderr: stderrForInstall(stderr), exitCode }).toMatchObject({ exitCode: 0 }); + } + expect(await exists(join(globalNodeModules, "my-linked-pkg", "package.json"))).toBe(true); + + { + await using proc = Bun.spawn({ + cmd: [bunExe(), "add", "--global", "no-deps@1.0.0"], + cwd: packageDir, + stdout: "pipe", + stderr: "pipe", + env: globalEnv, + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect({ stdout, stderr: stderrForInstall(stderr), exitCode }).toMatchObject({ exitCode: 0 }); + } + + expect(await exists(join(globalNodeModules, "no-deps", "package.json"))).toBe(true); + // The link registration must not have been pruned by the global install. + expect(await exists(join(globalNodeModules, "my-linked-pkg", "package.json"))).toBe(true); +}); + +// `bun link ` in a consumer is `--no-save` by default: it creates +// `node_modules/` as a symlink without writing package.json or bun.lock. +// The prune must not remove it on the next `bun install`, even while pruning a +// genuinely removed dependency in the same pass. +test("an unsaved bun link in a consumer survives bun install", async () => { + const { packageDir, packageJson } = await registry.createTestDir({ + bunfigOpts: { saveTextLockfile: true }, + files: { + "linked-src/package.json": JSON.stringify({ name: "my-linked-pkg", version: "1.0.0" }), + }, + }); + const globalEnv = { ...env, BUN_INSTALL: join(packageDir, "global-home") }; + const run = async (cmd: string[], cwd: string) => { + await using proc = Bun.spawn({ cmd, cwd, stdout: "pipe", stderr: "pipe", env: globalEnv }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect({ cmd, stdout, stderr: stderrForInstall(stderr), exitCode }).toMatchObject({ cmd, exitCode: 0 }); + }; + + await run([bunExe(), "link"], join(packageDir, "linked-src")); + + await write(packageJson, JSON.stringify({ name: "foo", dependencies: { "no-deps": "1.0.0", "a-dep": "1.0.1" } })); + await run([bunExe(), "install"], packageDir); + await run([bunExe(), "link", "my-linked-pkg"], packageDir); + expect(await exists(join(packageDir, "node_modules", "my-linked-pkg", "package.json"))).toBe(true); + // unsaved by default: neither manifest records the link + expect(await file(packageJson).text()).not.toContain("my-linked-pkg"); + expect(await file(join(packageDir, "bun.lock")).text()).not.toContain("my-linked-pkg"); + + // drop a-dep so the prune actually runs; the link must survive it + await write(packageJson, JSON.stringify({ name: "foo", dependencies: { "no-deps": "1.0.0" } })); + await run([bunExe(), "install"], packageDir); + + expect(await exists(join(packageDir, "node_modules", "a-dep"))).toBe(false); + expect(await exists(join(packageDir, "node_modules", "no-deps", "package.json"))).toBe(true); + expect(await exists(join(packageDir, "node_modules", "my-linked-pkg", "package.json"))).toBe(true); +}); diff --git a/test/harness.ts b/test/harness.ts index 365bfda948b2..57d08db23537 100644 --- a/test/harness.ts +++ b/test/harness.ts @@ -1828,15 +1828,23 @@ export class VerdaccioRegistry { async start(silent: boolean = true) { await rm(join(dirname(this.configPath), "htpasswd"), { force: true }); - this.process = fork(require.resolve("verdaccio/bin/verdaccio"), ["-c", this.configPath, "-l", `${this.port}`], { - silent, - // Prefer using a release build of Bun since it's faster - execPath: isCI ? bunExe() : Bun.which("bun") || bunExe(), - env: { - ...(bunEnv as any), - NODE_NO_WARNINGS: "1", + // Bind to 127.0.0.1 explicitly. With only a port, verdaccio listens on + // whatever getaddrinfo("localhost") returns first; on hosts where that is + // ::1, bun's HTTP client (which resolves localhost to 127.0.0.1) gets + // ConnectionRefused and every registry-backed test fails. + this.process = fork( + require.resolve("verdaccio/bin/verdaccio"), + ["-c", this.configPath, "-l", `127.0.0.1:${this.port}`], + { + silent, + // Prefer using a release build of Bun since it's faster + execPath: isCI ? bunExe() : Bun.which("bun") || bunExe(), + env: { + ...(bunEnv as any), + NODE_NO_WARNINGS: "1", + }, }, - }); + ); this.process.stderr?.on("data", data => { console.error(`[verdaccio] stderr: ${data}`);