diff --git a/src/install/isolated_install.rs b/src/install/isolated_install.rs index d89105ac2ead..8eae768ab17c 100644 --- a/src/install/isolated_install.rs +++ b/src/install/isolated_install.rs @@ -213,6 +213,98 @@ impl<'a, 'b> Wait<'a, 'b> { } } +/// `dep` is a `file:` dependency whose path is relative to the package declaring it +/// (resolved as `declarer`): `Package::from_npm` stores these paths as declared, +/// `Package::parse` and `overrides` store top-level relative ones. Such a folder is +/// linked from inside the package and is not a peer provider for the packages below. +fn dependency_is_contained_folder( + lockfile: &Lockfile, + declarer: &Resolution, + dep: &install::Dependency, +) -> bool { + if matches!( + declarer.tag, + ResolutionTag::Root | ResolutionTag::Workspace | ResolutionTag::Folder + ) || dep.version.tag != VersionTag::Folder + { + return false; + } + let string_buf = lockfile.buffers.string_bytes.as_slice(); + !lockfile + .overrides + .contains_name(dep.name_hash, dep.name.slice(string_buf), string_buf) +} + +/// `dependency_is_contained_folder` evaluated once for the whole lockfile. +struct ContainedFolders { + /// Indexed by `DependencyID`. + dependencies: DynamicBitSet, + /// Indexed by `PackageID`: folders only such dependencies resolve to. They get + /// no store entry (`store::entry::Entry::nested_folder`). + packages: DynamicBitSet, +} + +fn contained_folders(lockfile: &Lockfile) -> Result { + let pkgs = lockfile.packages.slice(); + let pkg_resolutions = pkgs.items_resolution(); + let resolutions = &lockfile.buffers.resolutions[..]; + let dependencies = &lockfile.buffers.dependencies[..]; + + let mut contained = ContainedFolders { + dependencies: DynamicBitSet::init_empty(dependencies.len())?, + packages: DynamicBitSet::init_empty(pkg_resolutions.len())?, + }; + for (pkg_id, pkg_res) in pkg_resolutions.iter().enumerate() { + if pkg_res.tag == ResolutionTag::Folder { + contained.packages.set(pkg_id); + } + } + if contained.packages.count() == 0 { + return Ok(contained); + } + + for (pkg_id, pkg_deps) in pkgs.items_dependencies().iter().enumerate() { + for dep_id in pkg_deps.begin()..pkg_deps.end() { + let target = resolutions[dep_id as usize]; + if target == invalid_package_id + || target as usize >= pkg_resolutions.len() + || pkg_resolutions[target as usize].tag != ResolutionTag::Folder + { + continue; + } + if dependency_is_contained_folder( + lockfile, + &pkg_resolutions[pkg_id], + &dependencies[dep_id as usize], + ) { + contained.dependencies.set(dep_id as usize); + } else { + contained.packages.unset(target as usize); + } + } + } + + Ok(contained) +} + +/// `pkg_id` has a `dependency_is_contained_folder` dependency resolving to `folder_pkg_id`. +pub(crate) fn folder_is_inside_package( + lockfile: &Lockfile, + pkg_id: PackageID, + folder_pkg_id: PackageID, +) -> bool { + let pkgs = lockfile.packages.slice(); + let declarer = &pkgs.items_resolution()[pkg_id as usize]; + let pkg_deps = pkgs.items_dependencies()[pkg_id as usize]; + let resolutions = &lockfile.buffers.resolutions[..]; + let dependencies = &lockfile.buffers.dependencies[..]; + + (pkg_deps.begin()..pkg_deps.end()).any(|dep_id| { + resolutions[dep_id as usize] == folder_pkg_id + && dependency_is_contained_folder(lockfile, declarer, &dependencies[dep_id as usize]) + }) +} + pub(crate) fn build_store( manager: &PackageManager, lockfile: &Lockfile, @@ -231,6 +323,9 @@ pub(crate) fn build_store( let dependencies = &lockfile.buffers.dependencies[..]; let string_buf = &lockfile.buffers.string_bytes[..]; + let contained = contained_folders(lockfile)?; + let is_contained_folder = |dep_id: DependencyID| contained.dependencies.is_set(dep_id as usize); + let mut nodes: store::node::List = store::node::List::default(); // DFS so a deduplicated node's full subtree (and therefore its `peers`) @@ -289,7 +384,8 @@ pub(crate) fn build_store( // Per-package bits computed once: own peer-dep names, and non-peer // dependency names that will appear in `node_dependencies` (i.e., not - // filtered out by bundled/disabled/unresolved). + // filtered out by bundled/disabled/unresolved) and are peer providers for + // the packages below (contained folders are not, see the walk below). let own_peers: DynamicBitSetList = DynamicBitSetList::init_empty(lockfile.packages.len(), peer_name_count as usize)?; let provides: DynamicBitSetList = @@ -305,15 +401,17 @@ pub(crate) fn build_store( }; if dep.behavior.is_peer() { own_peers.set(pkg_id as usize, bit); - } else if !is_filtered_dependency_or_workspace( - dep_id, - pkg_id, - workspace_filters, - install_root_dependencies, - manager, - lockfile, - resolutions, - ) { + } else if !is_contained_folder(dep_id) + && !is_filtered_dependency_or_workspace( + dep_id, + pkg_id, + workspace_filters, + install_root_dependencies, + manager, + lockfile, + resolutions, + ) + { provides.set(pkg_id as usize, bit); } } @@ -504,6 +602,7 @@ pub(crate) fn build_store( for ids in &node_dependencies[curr_id.get() as usize] { if dependencies[ids.dep_id as usize].name_hash == peer_name_hash + && !is_contained_folder(ids.dep_id) { break 'resolved ids.pkg_id; } @@ -581,7 +680,9 @@ pub(crate) fn build_store( let mut curr_id = entry.parent_id; 'walk: while curr_id != store::node::Id::INVALID { for ids in &node_dependencies[curr_id.get() as usize] { - if dependencies[ids.dep_id as usize].name_hash == peer_name_hash { + if dependencies[ids.dep_id as usize].name_hash == peer_name_hash + && !is_contained_folder(ids.dep_id) + { break 'walk; } } @@ -746,6 +847,11 @@ pub(crate) fn build_store( continue; } + // private to the ancestor; a package's own one still satisfies its own peer + if curr_id != node_id && is_contained_folder(ids.dep_id) { + continue; + } + let res = &pkg_resolutions[ids.pkg_id as usize]; if peer_dep.version.tag != VersionTag::Npm || res.tag != ResolutionTag::Npm @@ -1007,8 +1113,11 @@ pub(crate) fn build_store( let new_entry_parents: Vec = vec![entry.entry_parent_id]; + // never hoisted, so it must not claim the hoist slots for its name below + let new_entry_nested_folder = contained.packages.is_set(pkg_id as usize); + let hoisted = 'hoisted: { - if !manager.options.hoist { + if !manager.options.hoist || new_entry_nested_folder { break 'hoisted false; } @@ -1039,6 +1148,7 @@ pub(crate) fn build_store( parents: new_entry_parents, peer_hash: new_entry_peer_hash, hoisted, + nested_folder: new_entry_nested_folder, step: core::sync::atomic::AtomicU32::new(0), entry_hash: 0, scripts: core::cell::Cell::new(None), @@ -1080,7 +1190,7 @@ pub(crate) fn build_store( .name .slice(string_buf); public_hoisted.put(dep_name, ())?; - } else { + } else if !new_entry_nested_folder { // transitive dependencies (also direct dependencies of workspaces!) let dep_name = dependencies[new_entry_dep_id as usize] .name @@ -1110,6 +1220,10 @@ pub(crate) fn build_store( peers: node_peers[entry.node_id.get() as usize].clone(), }); + debug_assert!( + !new_entry_nested_folder || node_nodes[entry.node_id.get() as usize].is_empty(), + "a folder declared by a remote package is resolved without dependencies" + ); for &child_node_id in &node_nodes[entry.node_id.get() as usize] { entry_queue.write_item(QueuedEntry { node_id: child_node_id, @@ -1968,6 +2082,7 @@ pub(crate) fn install_isolated_packages( let entry_steps = entries.items_step(); let entry_dependencies = entries.items_dependencies(); let entry_hoisted = entries.items_hoisted(); + let entry_nested_folder = entries.items_nested_folder(); // Reborrow through a // `BackRef` so `string_buf` / `pkgs` don't tie up `&mut lockfile` for @@ -2182,6 +2297,31 @@ pub(crate) fn install_isolated_packages( continue; } ResolutionTag::Folder => { + if entry_nested_folder[entry_id.get() as usize] { + // linked by the packages containing it (`Installer::symlink_dependencies`) + debug_assert!(entry_dependencies[entry_id.get() as usize].list.is_empty()); + let folder = pkg_res.folder().slice(string_buf); + let state = if crate::bin::bin_target_escapes_package_dir(folder) { + // the resolver rejects these; only an edited or old lockfile gets here + Output::err_generic( + "refusing to install dependency {} with unsafe folder path \"{}\"", + (BStr::new(pkg_name.slice(string_buf)), BStr::new(folder)), + ); + Output::flush(); + if installer.manager().options.enable.fail_early() { + Global::exit(1); + } + installer::CompleteState::Fail + } else { + installer::CompleteState::Skipped + }; + // .monotonic is okay because the task isn't running on another thread. + entry_steps[entry_id.get() as usize] + .store(installer::Step::Done as u32, Ordering::Relaxed); + installer.on_task_complete(entry_id, state); + continue; + } + // folders are always hardlinked to keep them up-to-date installer.start_task(entry_id); continue; diff --git a/src/install/isolated_install/Installer.rs b/src/install/isolated_install/Installer.rs index ce346a116b37..db04e9fdac4c 100644 --- a/src/install/isolated_install/Installer.rs +++ b/src/install/isolated_install/Installer.rs @@ -2277,8 +2277,32 @@ impl<'a> Installer<'a> { let _ = dest.append(dep_name); // OOM/capacity: fire-and-forget } + let dep_node_id = self.store.entries.items_node_id()[dep.entry_id.get() as usize]; + let dep_pkg_id = self.store.nodes.items_pkg_id()[dep_node_id.get() as usize]; + let dep_res = &pkg_resolutions[dep_pkg_id as usize]; + let folder_is_inside_this_package = dep_res.tag == ResolutionTag::Folder + && super::folder_is_inside_package(lockfile, pkg_id, dep_pkg_id); + let dep_is_nested_folder = + self.store.entries.items_nested_folder()[dep.entry_id.get() as usize]; + // `build_store` only binds a nested folder to the packages containing it. + debug_assert!(folder_is_inside_this_package || !dep_is_nested_folder); + let mut dep_store_path = AutoAbsPath::init_top_level_dir(); - if uses_global_store { + if folder_is_inside_this_package { + let Some(package_dir_name) = entry_node_modules_name else { + continue; + }; + if !self.append_nested_folder_path( + &mut dep_store_path, + entry_id, + package_dir_name, + dep_res.folder().slice(string_buf), + )? { + continue; + } + } else if dep_is_nested_folder { + continue; + } else if uses_global_store { debug_assert!(self.entry_uses_global_store(dep.entry_id)); self.append_real_store_path(&mut dep_store_path, dep.entry_id, Which::Final); } else { @@ -2791,6 +2815,37 @@ impl<'a> Installer<'a> { } } + /// Appends `//` to `buf`. + /// `Ok(false)`: nothing to link (the path leaves the package, or no such directory). + fn append_nested_folder_path( + &self, + buf: &mut AutoAbsPath, + entry_id: StoreEntryId, + package_dir_name: &[u8], + folder: &[u8], + ) -> sys::Result { + if bin_real::bin_target_escapes_package_dir(folder) { + return Ok(false); + } + + self.append_real_store_node_modules_path(buf, entry_id, Which::Staging); + buf.append(package_dir_name).assume_ok(); + // `folder` comes from a third-party manifest, so its length is unchecked. + if buf.len() + 1 + folder.len() >= paths::MAX_PATH_BYTES { + return Err(sys::Error::from_code( + sys::Errno::ENAMETOOLONG, + sys::Tag::fstatat, + )); + } + buf.append_join(folder).assume_ok(); + + match sys::directory_exists_at(Fd::cwd(), buf.slice_z()) { + Ok(is_dir) => Ok(is_dir), + Err(err) if err.get_errno() == sys::Errno::ENOTDIR => Ok(false), + Err(err) => Err(err), + } + } + /// The directory name for the entry store node_modules install /// folder. /// ./node_modules/.bun/jquery@3.7.1/node_modules/jquery diff --git a/src/install/isolated_install/Store.rs b/src/install/isolated_install/Store.rs index 25be802357ff..1c09a983d0ee 100644 --- a/src/install/isolated_install/Store.rs +++ b/src/install/isolated_install/Store.rs @@ -283,6 +283,10 @@ pub mod entry { // if true this entry gets symlinked to `node_modules/.bun/node_modules` pub hoisted: bool, + /// Exists only inside the packages declaring it (`isolated_install::ContainedFolders`): + /// no store directory, task or hoist slot; linked in `Installer::symlink_dependencies`. + pub nested_folder: bool, + pub peer_hash: PeerHash, /// Content hash of (package + sorted resolved dependency global-store keys), @@ -313,6 +317,7 @@ pub mod entry { parents: Vec, step: core::sync::atomic::AtomicU32, hoisted: bool, + nested_folder: bool, peer_hash: PeerHash, entry_hash: u64, scripts: core::cell::Cell>, @@ -328,6 +333,7 @@ pub mod entry { // `Step::LinkPackage as u32 == 0`. step: core::sync::atomic::AtomicU32::new(0), hoisted: false, + nested_folder: false, peer_hash: PeerHash::NONE, entry_hash: 0, scripts: core::cell::Cell::new(None), diff --git a/src/install/prune.rs b/src/install/prune.rs index 01aa08b4607c..45b0c2228d0a 100644 --- a/src/install/prune.rs +++ b/src/install/prune.rs @@ -1491,11 +1491,15 @@ fn push_store_entry_names( ) { let pkg_res = lockfile.packages.items_resolution(); let node_pkg_ids = store.nodes.items_pkg_id(); + let entry_nested_folder = store.entries.items_nested_folder(); names.reserve(store.entries.len()); let mut name: Vec = Vec::new(); for (entry_idx, node_id) in store.entries.items_node_id().iter().enumerate() { let pkg_id = node_pkg_ids[node_id.get() as usize] as usize; - if pkg_res[pkg_id].tag == ResolutionTag::Workspace || !wanted.is_set(pkg_id) { + if pkg_res[pkg_id].tag == ResolutionTag::Workspace + || entry_nested_folder[entry_idx] + || !wanted.is_set(pkg_id) + { continue; } name.clear(); diff --git a/test/cli/install/bun-prune.test.ts b/test/cli/install/bun-prune.test.ts index 3c5574753fee..77efe37c3f0c 100644 --- a/test/cli/install/bun-prune.test.ts +++ b/test/cli/install/bun-prune.test.ts @@ -2646,6 +2646,30 @@ test.concurrent( }, ); +test.concurrent( + "isolated: a file: dependency declared by a registry package is not a store entry, a leftover one is removed", + async () => { + // file-dep@1.0.0 declares `"files": "file:./the-files"`; the installer links the + // folder inside the package's own store entry instead of creating one for it. + const dir = await setupWithLinker("isolated", { name: "foo", dependencies: { "file-dep": "1.0.0" } }); + expect(storeEntries(dir)).toStrictEqual(["file-dep@1.0.0"]); + const filesLink = join(dir, "node_modules", ".bun", "file-dep@1.0.0", "node_modules", "files"); + expect(isSymlink(filesLink)).toBeTrue(); + plant(dir, "node_modules/.bun/files@file+.+the-files/node_modules/files"); + + const { stdout, stderr, exitCode } = await prune(dir, "--linker", "isolated"); + expect(stderr).not.toContain("warn:"); + expect(lines(stdout)).toContain("- files@file+.+the-files"); + expect(exitCode).toBe(0); + expect(storeEntries(dir)).toStrictEqual(["file-dep@1.0.0"]); + expect(existsSync(join(filesLink, "package.json"))).toBeTrue(); + + const again = await prune(dir, "--linker", "isolated"); + expect(out(again.stdout)).toEndWith(NOTHING(2, 2)); + expect(again.exitCode).toBe(0); + }, +); + test.concurrent( "workspaces: without --linker, a bun.lock with configVersion 1 is pruned with the isolated linker", async () => { diff --git a/test/cli/install/isolated-install.test.ts b/test/cli/install/isolated-install.test.ts index b7bb923765ea..865efd12c2c5 100644 --- a/test/cli/install/isolated-install.test.ts +++ b/test/cli/install/isolated-install.test.ts @@ -367,6 +367,306 @@ test("can install folder dependencies on root package", async () => { ]); }); +// A `file:` dependency declared by a registry package points at a folder inside +// that package (the same fixtures as "transitive file dependencies" in +// bun-install-registry.test.ts, which covers the hoisted linker). +describe("transitive file dependencies of registry packages", () => { + const dependencies = { + // depends on file-dep@1.0.0 + "dep-file-dep": "1.0.0", + // "files": "file:./the-files" + "file-dep": "1.0.0", + // "files": "file:./missing-folder", which is not in the tarball + "missing-file-dep": "1.0.0", + // file-dep@1.0.1 has "files": "file:." + "aliased-file-dep": "npm:file-dep@1.0.1", + // both have "@scoped/files": "file:./the-files", so bun.lock holds one + // folder package with two dependents + "@scoped/file-dep": "1.0.0", + "@another-scope/file-dep": "1.0.0", + // "self-file-dep": "file:." + "self-file-dep": "1.0.0", + }; + + const storeEntries = [ + "@another-scope+file-dep@1.0.0", + "@scoped+file-dep@1.0.0", + "dep-file-dep@1.0.0", + "file-dep@1.0.0", + "file-dep@1.0.1", + "missing-file-dep@1.0.0", + "node_modules", + "self-file-dep@1.0.0", + ]; + + async function checkStore(packageDir: string) { + const bunDir = join(packageDir, "node_modules", ".bun"); + const storePackage = (entry: string, ...name: string[]) => join(bunDir, entry, "node_modules", ...name); + + // the folders never become store entries of their own + expect(await readdirSorted(bunDir)).toEqual(storeEntries); + + expect([ + readlinkSync(storePackage("file-dep@1.0.0", "files")), + readlinkSync(storePackage("file-dep@1.0.1", "files")), + readlinkSync(storePackage("@scoped+file-dep@1.0.0", "@scoped", "files")), + readlinkSync(storePackage("@another-scope+file-dep@1.0.0", "@scoped", "files")), + readlinkSync(storePackage("self-file-dep@1.0.0", "self-file-dep", "node_modules", "self-file-dep")), + await readdirSorted(storePackage("missing-file-dep@1.0.0")), + await readdirSorted(storePackage("dep-file-dep@1.0.0")), + ]).toEqual([ + join("file-dep", "the-files"), + "file-dep", + join("file-dep", "the-files"), + join("..", "@another-scope", "file-dep", "the-files"), + "..", + ["missing-file-dep"], + ["dep-file-dep", "file-dep"], + ]); + + expect( + await Promise.all([ + file(storePackage("file-dep@1.0.0", "files", "package.json")).json(), + file(storePackage("file-dep@1.0.1", "files", "package.json")).json(), + file(storePackage("@another-scope+file-dep@1.0.0", "@scoped", "files", "index.js")).text(), + file( + storePackage("self-file-dep@1.0.0", "self-file-dep", "node_modules", "self-file-dep", "package.json"), + ).json(), + ]), + ).toEqual([ + { name: "files", version: "1.1.1", dependencies: { "no-deps": "2.0.0" } }, + { name: "file-dep", version: "1.0.1", dependencies: { files: "file:." } }, + 'console.log("hello files");', + { name: "self-file-dep", version: "1.0.0", dependencies: { "self-file-dep": "file:." } }, + ]); + + // the packages resolve their folders from their real location in the store + const fromFileDep = createRequire(storePackage("file-dep@1.0.0", "file-dep", "package.json")); + expect(fromFileDep.resolve("files")).toEndWith( + join(".bun", "file-dep@1.0.0", "node_modules", "file-dep", "the-files", "index.js"), + ); + const fromMissingFileDep = createRequire( + storePackage("missing-file-dep@1.0.0", "missing-file-dep", "package.json"), + ); + expect(() => fromMissingFileDep.resolve("files")).toThrow(expect.objectContaining({ code: "MODULE_NOT_FOUND" })); + } + + test("are linked from inside the package that declares them", async () => { + const { packageJson, packageDir } = await registry.createTestDir({ bunfigOpts: { linker: "isolated" } }); + await write(packageJson, JSON.stringify({ name: "transitive-file-deps", dependencies })); + + let { out } = await runBunInstall(bunEnv, packageDir); + expect(out).toContain("7 packages installed"); + await checkStore(packageDir); + + // from bun.lock, where the two `@scoped/files` rows load as one package + await rm(join(packageDir, "node_modules"), { recursive: true, force: true }); + ({ out } = await runBunInstall(bunEnv, packageDir, { savesLockfile: false })); + expect(out).toContain("7 packages installed"); + await checkStore(packageDir); + + // relinking an existing node_modules + ({ out } = await runBunInstall(bunEnv, packageDir, { savesLockfile: false })); + expect(out).toContain("no changes"); + await checkStore(packageDir); + }); + + test("a folder declared by the root keeps its store entry when a registry package peer-depends on it", async () => { + const { packageJson, packageDir } = await registry.createTestDir({ bunfigOpts: { linker: "isolated" } }); + await Promise.all([ + write( + packageJson, + JSON.stringify({ + name: "root-folder-satisfies-peer", + dependencies: { + // peerDependencies: { "no-deps": "*" } + "peer-deps": "1.0.0", + "no-deps": "file:./no-deps", + }, + }), + ), + write(join(packageDir, "no-deps", "package.json"), JSON.stringify({ name: "no-deps", version: "9.9.9" })), + ]); + + await runBunInstall(bunEnv, packageDir); + + const bunDir = join(packageDir, "node_modules", ".bun"); + const entries = await readdirSorted(bunDir); + expect(entries).toContain("no-deps@file+no-deps"); + const peerDeps = entries.find(entry => entry.startsWith("peer-deps@1.0.0"))!; + expect(readlinkSync(join(bunDir, peerDeps, "node_modules", "no-deps"))).toBe( + join("..", "..", "no-deps@file+no-deps", "node_modules", "no-deps"), + ); + expect(await file(join(bunDir, peerDeps, "node_modules", "no-deps", "package.json")).json()).toEqual({ + name: "no-deps", + version: "9.9.9", + }); + }); + + // file-dep-with-peer-on-files@1.0.0 declares `"files": "file:the-files"` and depends on + // peer-on-files@1.0.0, which has an optional peer on `files` and ships an unrelated + // the-files folder of its own (registry/packages/create-file-dep-with-peer-packages.ts). + test("a folder is not bound to a peer dependency below the package containing it", async () => { + const { packageJson, packageDir } = await registry.createTestDir({ bunfigOpts: { linker: "isolated" } }); + await write( + packageJson, + JSON.stringify({ name: "contained-folder-peer", dependencies: { "file-dep-with-peer-on-files": "1.0.0" } }), + ); + + await runBunInstall(bunEnv, packageDir); + + const bunDir = join(packageDir, "node_modules", ".bun"); + const declaring = join(bunDir, "file-dep-with-peer-on-files@1.0.0", "node_modules"); + const peer = join(bunDir, "peer-on-files@1.0.0", "node_modules"); + expect( + await Promise.all([ + readdirSorted(bunDir), + readdirSorted(declaring), + readlink(join(declaring, "files")), + file(join(declaring, "files", "index.js")).text(), + // nothing provides the peer, so it is not linked even though peer-on-files + // ships a folder with the declared path itself + readdirSorted(peer), + ]), + ).toEqual([ + ["file-dep-with-peer-on-files@1.0.0", "node_modules", "peer-on-files@1.0.0"], + ["file-dep-with-peer-on-files", "files", "peer-on-files"], + join("file-dep-with-peer-on-files", "the-files"), + 'module.exports = "the-files shipped by file-dep-with-peer-on-files";\n', + ["peer-on-files"], + ]); + const fromPeer = createRequire(join(peer, "peer-on-files", "package.json")); + expect(() => fromPeer.resolve("files")).toThrow(expect.objectContaining({ code: "MODULE_NOT_FOUND" })); + }); + + test("a folder the root also declares: the root links its store entry, the package links its own copy, fresh and from bun.lock", async () => { + const { packageJson, packageDir } = await registry.createTestDir({ bunfigOpts: { linker: "isolated" } }); + await Promise.all([ + write( + packageJson, + JSON.stringify({ + name: "shared-folder-row", + dependencies: { + "file-dep-with-peer-on-files": "1.0.0", + // normalizes to the-files, the path the registry package declares, so the + // two rows load from bun.lock as one folder package + "files": "file:./the-files", + }, + }), + ), + write(join(packageDir, "the-files", "package.json"), JSON.stringify({ name: "files", version: "2.0.0" })), + write(join(packageDir, "the-files", "index.js"), 'module.exports = "the-files in the project";\n'), + ]); + + const bunDir = join(packageDir, "node_modules", ".bun"); + async function checkLayout() { + const entries = await readdirSorted(bunDir); + const declaring = join( + bunDir, + entries.find(entry => entry.startsWith("file-dep-with-peer-on-files@"))!, + "node_modules", + ); + const peer = join(bunDir, entries.find(entry => entry.startsWith("peer-on-files@"))!, "node_modules"); + expect( + await Promise.all([ + entries.map(entry => entry.replace(/\+[0-9a-f]{16}$/, "")), + readlink(join(packageDir, "node_modules", "files")), + file(join(packageDir, "node_modules", "files", "index.js")).text(), + readlink(join(declaring, "files")), + file(join(declaring, "files", "index.js")).text(), + // the peer is satisfied by the root's folder, not by the copy inside its parent + readlink(join(peer, "files")), + file(join(peer, "files", "index.js")).text(), + ]), + ).toEqual([ + ["file-dep-with-peer-on-files@1.0.0", "files@file+the-files", "node_modules", "peer-on-files@1.0.0"], + join(".bun", "files@file+the-files", "node_modules", "files"), + 'module.exports = "the-files in the project";\n', + join("file-dep-with-peer-on-files", "the-files"), + 'module.exports = "the-files shipped by file-dep-with-peer-on-files";\n', + join("..", "..", "files@file+the-files", "node_modules", "files"), + 'module.exports = "the-files in the project";\n', + ]); + return entries; + } + + await runBunInstall(bunEnv, packageDir); + const fresh = await checkLayout(); + + await rm(join(packageDir, "node_modules"), { recursive: true, force: true }); + await runBunInstall(bunEnv, packageDir, { savesLockfile: false }); + expect(await checkLayout()).toEqual(fresh); + }); + + test("does not take the hoist slots of a real package with the same name", async () => { + const { packageJson, packageDir } = await registry.createTestDir({ + bunfigOpts: { linker: "isolated", publicHoistPattern: ["files"] }, + }); + await Promise.all([ + write( + packageJson, + JSON.stringify({ + name: "folder-hoist-slots", + // file-dep's `files` folder entry is created before mid's `files` below + dependencies: { "file-dep": "1.0.0", "mid": "file:./mid" }, + }), + ), + write( + join(packageDir, "mid", "package.json"), + JSON.stringify({ name: "mid", version: "1.0.0", dependencies: { files: "npm:no-deps@1.0.0" } }), + ), + ]); + + await runBunInstall(bunEnv, packageDir); + + const bunDir = join(packageDir, "node_modules", ".bun"); + expect( + await Promise.all([ + readdirSorted(bunDir), + // publicHoistPattern and the hidden fallback directory both hold no-deps + readlink(join(packageDir, "node_modules", "files")), + readdirSorted(join(bunDir, "node_modules")), + readlink(join(bunDir, "file-dep@1.0.0", "node_modules", "files")), + readlink(join(bunDir, "mid@file+mid", "node_modules", "files")), + ]), + ).toEqual([ + ["file-dep@1.0.0", "mid@file+mid", "no-deps@1.0.0", "node_modules"], + join(".bun", "no-deps@1.0.0", "node_modules", "no-deps"), + ["file-dep", "no-deps"], + join("file-dep", "the-files"), + join("..", "..", "no-deps@1.0.0", "node_modules", "no-deps"), + ]); + }); + + test("a folder path escaping its package is refused when it comes from bun.lock", async () => { + const { packageJson, packageDir } = await registry.createTestDir({ bunfigOpts: { linker: "isolated" } }); + await write(packageJson, JSON.stringify({ name: "escaping-folder", dependencies: { "file-dep": "1.0.0" } })); + + // the resolver refuses to record such a path, so install first and edit bun.lock + await runBunInstall(bunEnv, packageDir); + const lockfile = join(packageDir, "bun.lock"); + const contents = await file(lockfile).text(); + expect(contents).toContain('"files@file:./the-files"'); + const bunDir = join(packageDir, "node_modules", ".bun"); + await Promise.all([ + write(lockfile, contents.replace('"files@file:./the-files"', '"files@file:../../../the-files"')), + // exists, so only the refusal keeps it from being linked: ../../../the-files + // from .bun/file-dep@1.0.0/node_modules/file-dep is .bun/the-files + write(join(bunDir, "the-files", "package.json"), JSON.stringify({ name: "files" })), + rm(join(bunDir, "file-dep@1.0.0"), { recursive: true, force: true }), + ]); + + const { out, err } = await runBunInstall(bunEnv, packageDir, { + allowErrors: true, + expectedExitCode: 1, + savesLockfile: false, + }); + expect(err).toContain('error: refusing to install dependency files with unsafe folder path "../../../the-files"'); + expect(out).toContain("Failed to install 1 package"); + expect(await readdirSorted(join(bunDir, "file-dep@1.0.0", "node_modules"))).toEqual(["file-dep"]); + }); +}); + describe("isolated workspaces", () => { test("basic", async () => { const { packageJson, packageDir } = await registry.createTestDir({ bunfigOpts: { linker: "isolated" } }); diff --git a/test/cli/install/registry/packages/create-file-dep-with-peer-packages.ts b/test/cli/install/registry/packages/create-file-dep-with-peer-packages.ts new file mode 100644 index 000000000000..e242cd749583 --- /dev/null +++ b/test/cli/install/registry/packages/create-file-dep-with-peer-packages.ts @@ -0,0 +1,67 @@ +#!/usr/bin/env bun +/** + * Generates the fixtures for a registry package that declares a `file:` + * dependency while one of its dependencies peer-depends on the same name + * (isolated-install.test.ts, "transitive file dependencies of registry packages"). + * + * - file-dep-with-peer-on-files@1.0.0 "files": "file:the-files", -> peer-on-files@1.0.0; + * ships the-files/ + * - peer-on-files@1.0.0 optional peer on "files"; also ships a the-files/ + * folder of its own, which nothing declares + * + * The `file:` path is written without `./` so that a root dependency on a folder + * named the-files (normalized to the same path) loads as the same bun.lock row. + */ + +import { mkdir, writeFile } from "fs/promises"; +import { join } from "path"; + +const packagesDir = import.meta.dir; + +const packages: Record = { + "file-dep-with-peer-on-files": { + dependencies: { "files": "file:the-files", "peer-on-files": "1.0.0" }, + }, + "peer-on-files": { + peerDependencies: { files: "*" }, + peerDependenciesMeta: { files: { optional: true } }, + }, +}; + +for (const [name, manifest] of Object.entries(packages)) { + const version = "1.0.0"; + const dir = join(packagesDir, name); + await mkdir(dir, { recursive: true }); + + const pkgJson = { name, version, ...manifest }; + const tarball = join(dir, `${name}-${version}.tgz`); + await Bun.Archive.write( + tarball, + { + "package/package.json": JSON.stringify(pkgJson, null, 2), + "package/the-files/package.json": JSON.stringify({ name: "files", version }, null, 2), + "package/the-files/index.js": `module.exports = "the-files shipped by ${name}";\n`, + }, + { compress: "gzip" }, + ); + + const bytes = await Bun.file(tarball).bytes(); + const versions = { + [version]: { + ...pkgJson, + _id: `${name}@${version}`, + dist: { + integrity: `sha512-${Buffer.from(new Bun.CryptoHasher("sha512").update(bytes).digest()).toString("base64")}`, + shasum: new Bun.CryptoHasher("sha1").update(bytes).digest("hex"), + tarball: `http://localhost:4873/${name}/-/${name}-${version}.tgz`, + }, + }, + }; + + await writeFile( + join(dir, "package.json"), + JSON.stringify({ _id: name, name, "dist-tags": { latest: version }, versions }, null, 2), + ); +} + +console.log("Created file-dep-with-peer-on-files and peer-on-files test packages"); diff --git a/test/cli/install/registry/packages/file-dep-with-peer-on-files/file-dep-with-peer-on-files-1.0.0.tgz b/test/cli/install/registry/packages/file-dep-with-peer-on-files/file-dep-with-peer-on-files-1.0.0.tgz new file mode 100644 index 000000000000..c17184d1ba85 Binary files /dev/null and b/test/cli/install/registry/packages/file-dep-with-peer-on-files/file-dep-with-peer-on-files-1.0.0.tgz differ diff --git a/test/cli/install/registry/packages/file-dep-with-peer-on-files/package.json b/test/cli/install/registry/packages/file-dep-with-peer-on-files/package.json new file mode 100644 index 000000000000..b0560b594807 --- /dev/null +++ b/test/cli/install/registry/packages/file-dep-with-peer-on-files/package.json @@ -0,0 +1,23 @@ +{ + "_id": "file-dep-with-peer-on-files", + "name": "file-dep-with-peer-on-files", + "dist-tags": { + "latest": "1.0.0" + }, + "versions": { + "1.0.0": { + "name": "file-dep-with-peer-on-files", + "version": "1.0.0", + "dependencies": { + "files": "file:the-files", + "peer-on-files": "1.0.0" + }, + "_id": "file-dep-with-peer-on-files@1.0.0", + "dist": { + "integrity": "sha512-ZmSsoAZOGAqssZPTcwdidOg1673007uwXYBG+uFVMARB3y6sjupPm+jGh3/5+bT/Jdp2usHUQWLc1nvsXGBvfQ==", + "shasum": "904c29b2d64c8a863793d0615f735094687b64a1", + "tarball": "http://localhost:4873/file-dep-with-peer-on-files/-/file-dep-with-peer-on-files-1.0.0.tgz" + } + } + } +} \ No newline at end of file diff --git a/test/cli/install/registry/packages/peer-on-files/package.json b/test/cli/install/registry/packages/peer-on-files/package.json new file mode 100644 index 000000000000..1714a5bdbd09 --- /dev/null +++ b/test/cli/install/registry/packages/peer-on-files/package.json @@ -0,0 +1,27 @@ +{ + "_id": "peer-on-files", + "name": "peer-on-files", + "dist-tags": { + "latest": "1.0.0" + }, + "versions": { + "1.0.0": { + "name": "peer-on-files", + "version": "1.0.0", + "peerDependencies": { + "files": "*" + }, + "peerDependenciesMeta": { + "files": { + "optional": true + } + }, + "_id": "peer-on-files@1.0.0", + "dist": { + "integrity": "sha512-hzameqZFR3zTGv4G767dFQyyq7nfOp6BegLlAdadjl0APSDLELkgjWnG+KFApM4H0qPdymVCekdPESCB9fusjg==", + "shasum": "a95df32378fdf3bcd591969a2427b1cfca78a0d9", + "tarball": "http://localhost:4873/peer-on-files/-/peer-on-files-1.0.0.tgz" + } + } + } +} \ No newline at end of file diff --git a/test/cli/install/registry/packages/peer-on-files/peer-on-files-1.0.0.tgz b/test/cli/install/registry/packages/peer-on-files/peer-on-files-1.0.0.tgz new file mode 100644 index 000000000000..c9064b05401a Binary files /dev/null and b/test/cli/install/registry/packages/peer-on-files/peer-on-files-1.0.0.tgz differ