diff --git a/src/install/PackageManager/runTasks.rs b/src/install/PackageManager/runTasks.rs index 3feca04e87af..80a1c5a14b55 100644 --- a/src/install/PackageManager/runTasks.rs +++ b/src/install/PackageManager/runTasks.rs @@ -80,6 +80,7 @@ pub trait RunTasksCallbacks { // `package_id: PackageID` otherwise. Static dispatch via two trait // methods lets impls receive the correctly-typed id without a // `Task::Id` round-trip pun. + // `is_required == false`: only optional dependencies need the download. fn on_package_download_error_store( _ctx: &mut Self::Ctx, _task_id: Task::Id, @@ -87,6 +88,7 @@ pub trait RunTasksCallbacks { _resolution: &bun_install::Resolution, _err: crate::Error, _url: &[u8], + _is_required: bool, ) { unreachable!() } @@ -97,6 +99,7 @@ pub trait RunTasksCallbacks { _resolution: &bun_install::Resolution, _err: crate::Error, _url: &[u8], + _is_required: bool, ) { unreachable!() } @@ -133,9 +136,9 @@ struct ErasedCallbacks { is_store_installer: bool, on_package_manifest_error: fn(*mut (), &[u8], crate::Error, &[u8]), on_package_download_error_store: - fn(*mut (), Task::Id, &[u8], &bun_install::Resolution, crate::Error, &[u8]), + fn(*mut (), Task::Id, &[u8], &bun_install::Resolution, crate::Error, &[u8], bool), on_package_download_error_pkg: - fn(*mut (), PackageID, &[u8], &bun_install::Resolution, crate::Error, &[u8]), + fn(*mut (), PackageID, &[u8], &bun_install::Resolution, crate::Error, &[u8], bool), on_extract_package_installer: fn(*mut (), Task::Id, DependencyID, &mut bun_install::ExtractData, Options::LogLevel), on_extract_store_installer: fn(*mut (), Task::Id), @@ -163,19 +166,30 @@ impl ErasedCallbacks { on_package_manifest_error: |c, name, err, url| { C::on_package_manifest_error(ctx::(c), name, err, url) }, - on_package_download_error_store: |c, task_id, name, resolution, err, url| { - C::on_package_download_error_store(ctx::(c), task_id, name, resolution, err, url) - }, - on_package_download_error_pkg: |c, package_id, name, resolution, err, url| { - C::on_package_download_error_pkg( - ctx::(c), - package_id, - name, - resolution, - err, - url, - ) - }, + on_package_download_error_store: + |c, task_id, name, resolution, err, url, is_required| { + C::on_package_download_error_store( + ctx::(c), + task_id, + name, + resolution, + err, + url, + is_required, + ) + }, + on_package_download_error_pkg: + |c, package_id, name, resolution, err, url, is_required| { + C::on_package_download_error_pkg( + ctx::(c), + package_id, + name, + resolution, + err, + url, + is_required, + ) + }, on_extract_package_installer: |c, task_id, dependency_id, data, log_level| { C::on_extract_package_installer( ctx::(c), @@ -785,6 +799,27 @@ fn run_tasks_erased( let is_required = manager.is_network_task_required(task.task_id); manager.mark_network_task_failed(task.task_id); + if !is_required { + bun_ast::add_warning_pretty!( + manager.log_mut(), + None, + bun_ast::Loc::EMPTY, + "{} downloading tarball {}@{}", + DownloadFailure(err.name(), &task.response), + bstr::BStr::new(extract.name.slice()), + extract + .resolution + .fmt(&manager.lockfile.buffers.string_bytes, PathSep::Auto,), + ); + } + + fail_update_requests( + manager, + task.task_id, + extract.name.slice(), + Some(extract.dependency_id), + ); + if cb.has_on_package_download_error { if cb.is_store_installer { (cb.on_package_download_error_store)( @@ -794,6 +829,7 @@ fn run_tasks_erased( &extract.resolution, err, &task.url_buf, + is_required, ); } else { let package_id = manager.lockfile.buffers.resolutions @@ -805,6 +841,7 @@ fn run_tasks_erased( &extract.resolution, err, &task.url_buf, + is_required, ); } continue; @@ -822,25 +859,7 @@ fn run_tasks_erased( .resolution .fmt(&manager.lockfile.buffers.string_bytes, PathSep::Auto,), ); - } else { - bun_ast::add_warning_pretty!( - manager.log_mut(), - None, - bun_ast::Loc::EMPTY, - "{} downloading tarball {}@{}", - DownloadFailure(err.name(), &task.response), - bstr::BStr::new(extract.name.slice()), - extract - .resolution - .fmt(&manager.lockfile.buffers.string_bytes, PathSep::Auto,), - ); } - fail_update_requests( - manager, - task.task_id, - extract.name.slice(), - Some(extract.dependency_id), - ); if let Some(removed) = manager.task_queue.remove(&task.task_id) { drop(removed); @@ -860,6 +879,24 @@ fn run_tasks_erased( let is_required = manager.is_network_task_required(task.task_id); manager.mark_network_task_failed(task.task_id); + if !is_required { + bun_ast::add_warning_pretty!( + manager.log_mut(), + None, + bun_ast::Loc::EMPTY, + "GET {} - {}", + bstr::BStr::new(metadata.url.slice()), + response.status_code, + ); + } + + fail_update_requests( + manager, + task.task_id, + extract.name.slice(), + Some(extract.dependency_id), + ); + if cb.has_on_package_download_error { let err = match response.status_code { 400 => crate::Error::TarballHTTP400, @@ -879,6 +916,7 @@ fn run_tasks_erased( &extract.resolution, err, &task.url_buf, + is_required, ); } else { let package_id = manager.lockfile.buffers.resolutions @@ -890,6 +928,7 @@ fn run_tasks_erased( &extract.resolution, err, &task.url_buf, + is_required, ); } continue; @@ -904,22 +943,7 @@ fn run_tasks_erased( bstr::BStr::new(metadata.url.slice()), response.status_code, ); - } else { - bun_ast::add_warning_pretty!( - manager.log_mut(), - None, - bun_ast::Loc::EMPTY, - "GET {} - {}", - bstr::BStr::new(metadata.url.slice()), - response.status_code, - ); } - fail_update_requests( - manager, - task.task_id, - extract.name.slice(), - Some(extract.dependency_id), - ); if let Some(removed) = manager.task_queue.remove(&task.task_id) { drop(removed); @@ -1166,6 +1190,7 @@ fn run_tasks_erased( resolution, err, fail_url, + true, ); } else { (cb.on_package_download_error_pkg)( @@ -1175,6 +1200,7 @@ fn run_tasks_erased( resolution, err, fail_url, + true, ); } continue; @@ -1365,6 +1391,7 @@ fn run_tasks_erased( res, err, url, + true, ); } } @@ -1386,6 +1413,7 @@ fn run_tasks_erased( &clone.res, err, url, + true, ); } } else if log_level != Options::LogLevel::Silent { @@ -1554,6 +1582,7 @@ fn run_tasks_erased( resolution, err, manager.lockfile.str(repo), + true, ); } else { bun_ast::add_error_pretty!( diff --git a/src/install/isolated_install.rs b/src/install/isolated_install.rs index 65128085d4be..3ccf997c6c87 100644 --- a/src/install/isolated_install.rs +++ b/src/install/isolated_install.rs @@ -155,8 +155,9 @@ impl<'a> run_tasks::RunTasksCallbacks for StoreRunTasksCallbacks<'a> { resolution: &Resolution, err: crate::Error, url: &[u8], + is_required: bool, ) { - ctx.on_package_download_error(id, name, resolution, err, url); + ctx.on_package_download_error(id, name, resolution, err, url, is_required); } } @@ -2041,6 +2042,10 @@ pub(crate) fn install_isolated_packages( manager: manager_ptr, command_ctx, installed, + missing: (0..store.entries.len()) + .map(|_| std::sync::atomic::AtomicBool::new(false)) + .collect(), + required_packages: None, install_node: if show_progress { Some(&mut install_node) } else { @@ -2429,12 +2434,7 @@ pub(crate) fn install_isolated_packages( crate::network_task::ForTarballError::AlreadyFailed | crate::network_task::ForTarballError::Offline, ) => { - // .monotonic is okay because an error means 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, installer::CompleteState::Fail); + installer.on_download_not_queued(entry_id); continue; } Err(err) => { @@ -2471,10 +2471,7 @@ pub(crate) fn install_isolated_packages( ) == crate::package_manager::GitEnqueueResult::OfflineMiss { // --offline and not cached: nothing was queued - entry_steps[entry_id.get() as usize] - .store(installer::Step::Done as u32, Ordering::Relaxed); - installer - .on_task_complete(entry_id, installer::CompleteState::Fail); + installer.on_download_not_queued(entry_id); continue; } } @@ -2499,12 +2496,7 @@ pub(crate) fn install_isolated_packages( crate::network_task::ForTarballError::AlreadyFailed | crate::network_task::ForTarballError::Offline, ) => { - // .monotonic is okay because an error means 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, installer::CompleteState::Fail); + installer.on_download_not_queued(entry_id); continue; } Err(err) => { @@ -2555,12 +2547,7 @@ pub(crate) fn install_isolated_packages( crate::network_task::ForTarballError::AlreadyFailed | crate::network_task::ForTarballError::Offline, ) => { - // .monotonic is okay because an error means 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, installer::CompleteState::Fail); + installer.on_download_not_queued(entry_id); continue; } Err(err) => { @@ -2678,6 +2665,8 @@ pub(crate) fn install_isolated_packages( debug_assert!(done); } + installer.unlink_missing_dependencies(); + let mut summary = core::mem::take(&mut installer.summary); summary.successfully_installed = Some(core::mem::take(&mut installer.installed)); diff --git a/src/install/isolated_install/Installer.rs b/src/install/isolated_install/Installer.rs index 1ee1715dbf53..ade5bdc382cf 100644 --- a/src/install/isolated_install/Installer.rs +++ b/src/install/isolated_install/Installer.rs @@ -1,4 +1,4 @@ -use core::sync::atomic::{AtomicU8, Ordering}; +use core::sync::atomic::{AtomicBool, AtomicU8, Ordering}; use std::io::Write as _; use bun_ast::Log; @@ -23,6 +23,7 @@ use crate::resolution; use crate::{ self as install, DependencyID, Lockfile, PackageID, PackageManager, PackageNameHash, Resolution, TaskCallbackContext, TruncatedPackageNameHash, bin, invalid_dependency_id, + invalid_package_id, }; // Bring `items_()` column accessors into scope for // `MultiArrayList` / `Slice`. @@ -75,6 +76,10 @@ pub struct Installer<'a> { pub(crate) summary: InstallSummary, pub(crate) installed: Bitset, + /// Entries left out because their download failed and only optional dependencies need them. + pub(crate) missing: Box<[AtomicBool]>, + /// Main thread only. Packages that a dependency without `Behavior::OPTIONAL` resolves to. + pub(crate) required_packages: Option, pub(crate) install_node: Option<&'a mut ProgressNode>, pub(crate) is_new_bun_modules: bool, @@ -247,17 +252,33 @@ impl<'a> Installer<'a> { resolution: &Resolution, err: crate::Error, url: &[u8], + is_required: bool, ) { - if let Some(removed) = self.manager_mut().task_queue.remove(&task_id) { - let callbacks = removed; + let Some(callbacks) = self.manager_mut().task_queue.remove(&task_id) else { + if is_required { + // No waiting entry — still surface the error so it isn't lost. + let string_buf = self.lockfile().buffers.string_bytes.as_slice(); + Output::err_generic( + "failed to download {}@{}: {}\n {}", + ( + bstr::BStr::new(name), + resolution.fmt(string_buf, bun_core::fmt::PathSep::Auto), + bstr::BStr::new(download_error_reason(err)), + bstr::BStr::new(url), + ), + ); + Output::flush(); + } + return; + }; - let entry_steps = self.store.entries.items_step(); - for install_ctx in callbacks.as_slice() { - // `TaskCallbackContext` is an enum, so destructure. - let &TaskCallbackContext::IsolatedPackageInstallContext(entry_id) = install_ctx - else { - continue; - }; + let entry_steps = self.store.entries.items_step(); + for install_ctx in callbacks.as_slice() { + // `TaskCallbackContext` is an enum, so destructure. + let &TaskCallbackContext::IsolatedPackageInstallContext(entry_id) = install_ctx else { + continue; + }; + if is_required || self.entry_is_required(entry_id) { entry_steps[entry_id.get() as usize].store(Step::Done as u32, Ordering::Relaxed); self.on_task_fail( entry_id, @@ -266,22 +287,11 @@ impl<'a> Installer<'a> { url: url.into(), }), ); + } else { + self.on_optional_download_fail(entry_id); } - // callbacks dropped here - } else { - // No waiting entry — still surface the error so it isn't lost. - let string_buf = self.lockfile().buffers.string_bytes.as_slice(); - Output::err_generic( - "failed to download {}@{}: {}\n {}", - ( - bstr::BStr::new(name), - resolution.fmt(string_buf, bun_core::fmt::PathSep::Auto), - bstr::BStr::new(download_error_reason(err)), - bstr::BStr::new(url), - ), - ); - Output::flush(); } + // callbacks dropped here } pub(crate) fn apply_package_patch( @@ -444,6 +454,65 @@ impl<'a> Installer<'a> { self.resume_unblocked_tasks(entry_id); } + /// Called from main thread when the download for `entry_id` already failed or `--offline` has no cached copy. + pub(crate) fn on_download_not_queued(&mut self, entry_id: StoreEntryId) { + if self.entry_is_required(entry_id) { + // .monotonic is okay because the task isn't running on another thread. + self.store.entries.items_step()[entry_id.get() as usize] + .store(Step::Done as u32, Ordering::Relaxed); + self.on_task_complete(entry_id, CompleteState::Fail); + } else { + self.on_optional_download_fail(entry_id); + } + } + + /// Called from main thread. The entry is left out, neither installed nor failed. + fn on_optional_download_fail(&mut self, entry_id: StoreEntryId) { + self.missing[entry_id.get() as usize].store(true, Ordering::Relaxed); + // The dependents that wait on this entry made their links before they blocked. + self.unlink_links_to(entry_id); + // Release pairs with the Acquire load in `is_task_blocked`. + self.store.entries.items_step()[entry_id.get() as usize] + .store(Step::Done as u32, Ordering::Release); + self.decrement_pending_tasks(); + self.resume_unblocked_tasks(entry_id); + + if let Some(node) = self.install_node.as_mut() { + node.complete_one(); + } + } + + /// Main thread only. Reads the lockfile edges: a deduped store node keeps its first dependency only. + fn entry_is_required(&mut self, entry_id: StoreEntryId) -> bool { + let lockfile = self.lockfile(); + let node_pkg_ids = self.store.nodes.items_pkg_id(); + let entry_node_ids = self.store.entries.items_node_id(); + if self.required_packages.is_none() { + let dependencies = lockfile.buffers.dependencies.as_slice(); + let resolutions = lockfile.buffers.resolutions.as_slice(); + let pkg_deps = lockfile.packages.items_dependencies(); + let mut required = bun_core::handle_oom(Bitset::init_empty(lockfile.packages.len())); + for &node_id in entry_node_ids { + let list = pkg_deps[node_pkg_ids[node_id.get() as usize] as usize]; + for dep_id in list.off..list.off + list.len { + let behavior = dependencies[dep_id as usize].behavior; + let res = resolutions[dep_id as usize]; + if res != invalid_package_id + && !behavior.is_optional() + && !behavior.is_optional_peer() + { + required.set(res as usize); + } + } + } + self.required_packages = Some(required); + } + let pkg_id = node_pkg_ids[entry_node_ids[entry_id.get() as usize].get() as usize]; + self.required_packages + .as_ref() + .is_some_and(|required| required.is_set(pkg_id as usize)) + } + pub(crate) fn decrement_pending_tasks(&mut self) { self.manager_mut().decrement_pending_tasks(); } @@ -629,6 +698,20 @@ pub enum Relink { Changed, } +/// A dependency with the same name as the entry itself nests one `node_modules` deeper. +fn append_dependency_link_name( + dest: &mut AutoPath, + dep_name: &[u8], + entry_node_modules_name: Option<&[u8]>, +) { + // OOM/capacity: fire-and-forget + let _ = dest.append(dep_name); + if entry_node_modules_name.is_some_and(|name| strings::eql_long(dep_name, name, true)) { + let _ = dest.append(b"node_modules"); + let _ = dest.append(dep_name); + } +} + fn download_error_reason(e: crate::Error) -> &'static [u8] { match e { crate::Error::TarballHTTP400 => b"400 Bad Request", @@ -2259,17 +2342,16 @@ impl<'a> Installer<'a> { self.append_real_store_node_modules_path(&mut dest, entry_id, Which::Staging); let base_len = dest.len(); + let skips_missing = self.links_each_install(entry_id); let mut changed = false; for dep in self.store.entries.items_dependencies()[entry_id.get() as usize].slice() { + if skips_missing && self.missing[dep.entry_id.get() as usize].load(Ordering::Relaxed) { + continue; + } let dep_name = dependencies[dep.dep_id as usize].name.slice(string_buf); dest.set_length(base_len); - let _ = dest.append(dep_name); // OOM/capacity: fire-and-forget - if entry_node_modules_name.is_some_and(|name| strings::eql_long(dep_name, name, true)) { - // same name as the entry itself: nest one node_modules deeper to avoid the collision - let _ = dest.append(b"node_modules"); // OOM/capacity: fire-and-forget - let _ = dest.append(dep_name); // OOM/capacity: fire-and-forget - } + append_dependency_link_name(&mut dest, dep_name, entry_node_modules_name); let mut dep_store_path = AutoAbsPath::init_top_level_dir(); if uses_global_store { @@ -2298,6 +2380,72 @@ impl<'a> Installer<'a> { Ok(changed) } + /// Main thread, after every task finished: `symlink_dependencies` runs before a download can fail. + pub(crate) fn unlink_missing_dependencies(&self) { + for (entry, missing) in self.missing.iter().enumerate() { + if missing.load(Ordering::Relaxed) { + self.unlink_links_to(StoreEntryId::from(entry as u32)); + } + } + } + + /// A store entry relinks instead: a link it misses is a change, and that runs its scripts again. + fn links_each_install(&self, entry_id: StoreEntryId) -> bool { + let node_id = self.store.entries.items_node_id()[entry_id.get() as usize]; + let pkg_id = self.store.nodes.items_pkg_id()[node_id.get() as usize]; + let tag = self.lockfile().packages.items_resolution()[pkg_id as usize].tag; + matches!( + tag, + ResolutionTag::Root | ResolutionTag::Workspace | ResolutionTag::Folder + ) + } + + /// Main thread. Removes the links to `entry_id` from the entries that link each install. + fn unlink_links_to(&self, entry_id: StoreEntryId) { + let lockfile = self.lockfile(); + let string_buf = lockfile.buffers.string_bytes.as_slice(); + let dependencies = lockfile.buffers.dependencies.as_slice(); + let pkg_names = lockfile.packages.items_name(); + let pkg_resolutions = lockfile.packages.items_resolution(); + let entries = &self.store.entries; + let entry_node_ids = entries.items_node_id(); + let node_pkg_ids = self.store.nodes.items_pkg_id(); + let node_dep_ids = self.store.nodes.items_dep_id(); + + // Not `items_parents()`: a public hoist adds the entry to the root's list only. + for (parent, deps) in entries.items_dependencies().iter().enumerate() { + let parent_id = StoreEntryId::from(parent as u32); + if !self.links_each_install(parent_id) { + continue; + } + let mut dest: Option<(AutoPath, usize, Option<&[u8]>)> = None; + for dep in deps.slice() { + if dep.entry_id != entry_id { + continue; + } + let (dest, base_len, entry_node_modules_name) = dest.get_or_insert_with(|| { + let node_id = entry_node_ids[parent]; + let pkg_id = node_pkg_ids[node_id.get() as usize]; + let dep_id = node_dep_ids[node_id.get() as usize]; + let mut dest = AutoPath::init_top_level_dir(); + self.append_real_store_node_modules_path(&mut dest, parent_id, Which::Final); + let base_len = dest.len(); + let name = self.entry_store_node_modules_package_name( + dep_id, + pkg_id, + &pkg_resolutions[pkg_id as usize], + pkg_names, + ); + (dest, base_len, name) + }); + let dep_name = dependencies[dep.dep_id as usize].name.slice(string_buf); + dest.set_length(*base_len); + append_dependency_link_name(dest, dep_name, *entry_node_modules_name); + symlinker::remove_link(dest.slice_z()); + } + } + } + pub(crate) fn link_dependency_bins(&self, parent_entry_id: StoreEntryId) -> crate::Result<()> { let lockfile = self.lockfile(); let store = self.store; diff --git a/src/install/isolated_install/Symlinker.rs b/src/install/isolated_install/Symlinker.rs index 52c90735abba..6bbe48ff0773 100644 --- a/src/install/isolated_install/Symlinker.rs +++ b/src/install/isolated_install/Symlinker.rs @@ -123,32 +123,34 @@ impl Symlinker { { return Ok(false); } - - // this existing link is pointing to the wrong package. - // on windows rmdir must be used for symlinks created to point - // at directories, even if the target no longer exists - match bun_sys::rmdir(self.dest.slice_z()) { - Ok(()) => {} - Err(err) => match err.get_errno() { - Errno::EPERM => { - let _ = bun_sys::unlink(self.dest.slice_z()); - } - _ => {} - }, - } - } - #[cfg(not(windows))] - { - // this existing link is pointing to the wrong package - let _ = bun_sys::unlink(self.dest.slice_z()); } + // this existing link is pointing to the wrong package + remove_link(self.dest.slice_z()); + return self.symlink().map(|()| true); } } } } +/// Removes the link at `dest`. Errors are ignored. +pub(crate) fn remove_link(dest: &bun_core::ZStr) { + #[cfg(windows)] + { + // a directory symlink needs rmdir, even if the target no longer exists + if let Err(err) = bun_sys::rmdir(dest) { + if matches!(err.get_errno(), Errno::EPERM) { + let _ = bun_sys::unlink(dest); + } + } + } + #[cfg(not(windows))] + { + let _ = bun_sys::unlink(dest); + } +} + #[derive(Clone, Copy)] pub enum Strategy { ExpectExisting, diff --git a/src/jsc/AsyncModule.rs b/src/jsc/AsyncModule.rs index 3db53bf12564..2d06271c36c5 100644 --- a/src/jsc/AsyncModule.rs +++ b/src/jsc/AsyncModule.rs @@ -262,6 +262,7 @@ impl run_tasks::RunTasksCallbacks for QueueRunTasksCallbac resolution: &Resolution, err: bun_install::Error, url: &[u8], + _is_required: bool, ) { ctx.on_package_download_error(package_id, name, resolution, err.name(), url) } diff --git a/test/cli/install/bun-install-retry.test.ts b/test/cli/install/bun-install-retry.test.ts index d9706157a9be..eb9e7338fd85 100644 --- a/test/cli/install/bun-install-retry.test.ts +++ b/test/cli/install/bun-install-retry.test.ts @@ -1,5 +1,6 @@ import { file, spawn } from "bun"; import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, setDefaultTimeout } from "bun:test"; +import { lstatSync } from "fs"; import { access, writeFile } from "fs/promises"; import { bunExe, bunEnv as env, readdirSorted, tmpdirSync, toBeValidBin, toBeWorkspaceLink, toHaveBins } from "harness"; import { join } from "path"; @@ -15,6 +16,16 @@ import { setHandler, } from "./dummy.registry"; +/** Whether `path` exists, as a directory, a file, or a link (even a dangling one). */ +function entryExists(path: string): boolean { + try { + lstatSync(path); + return true; + } catch { + return false; + } +} + beforeAll(dummyBeforeAll); afterAll(dummyAfterAll); @@ -433,9 +444,224 @@ describe.each(["hoisted", "isolated"])("linker=%s", linker => { name: "baz", version: "0.0.3", }); - if (linker === "hoisted") { - expect(out).not.toContain("Failed to install"); - expect(exitCode).toBe(0); + expect(out).not.toContain("Failed to install"); + expect(exitCode).toBe(0); + }); + + // From a lockfile the install phase asks for the tarball first, so the + // failure reaches the linker instead of the resolver. + it("warns and exits 0 when an optional dependency's tarball fails to download from a lockfile", async () => { + const urls: string[] = []; + setHandler(async request => { + const { pathname } = new URL(request.url); + urls.push(pathname); + if (pathname === "/BaR") { + return Response.json({ + name: "BaR", + "dist-tags": { latest: "0.0.2" }, + versions: { + "0.0.2": { name: "BaR", version: "0.0.2", dist: { tarball: `${root_url}/BaR-0.0.2.tgz` } }, + }, + }); + } + if (pathname === "/BaR-0.0.2.tgz") return new Response("no", { status: 404 }); + return new Response("unexpected", { status: 404 }); + }); + await writeFile( + join(package_dir, "bunfig.toml"), + Bun.TOML.stringify({ + install: { + cache: false, + registry: `${root_url}/`, + linker, + }, + }), + ); + await writeFile( + join(package_dir, "package.json"), + JSON.stringify({ + name: "foo", + version: "0.0.1", + optionalDependencies: { BaR: "0.0.2" }, + }), + ); + async function install(...args: string[]) { + await using proc = spawn({ + cmd: [bunExe(), ...args, "--no-progress", "--ignore-scripts"], + cwd: package_dir, + stdout: "pipe", + stdin: "pipe", + stderr: "pipe", + env, + }); + const [err, out, exitCode] = await Promise.all([proc.stderr.text(), proc.stdout.text(), proc.exited]); + return { + warnLines: err.split("\n").filter(l => l.startsWith("warn:")), + errorLines: err.split("\n").filter(l => l.startsWith("error:")), + failed: out.includes("Failed to install"), + // The package is left out. No dangling link to it either. + linked: entryExists(join(package_dir, "node_modules", "BaR")), + exitCode, + }; + } + const warned = { + warnLines: [`warn: GET ${root_url}/BaR-0.0.2.tgz - 404`], + errorLines: [], + failed: false, + linked: false, + }; + + // The first install resolves BaR and saves the lockfile. Its tarball fails while resolving. + expect(await install("install")).toEqual({ ...warned, exitCode: 0 }); + await access(join(package_dir, "bun.lock")); + + urls.length = 0; + expect(await install("install")).toEqual({ ...warned, exitCode: 0 }); + expect(urls).toEqual(["/BaR-0.0.2.tgz"]); + + // A package that the user asked for fails the command, optional or not. + expect(await install("add", "--optional", "BaR@0.0.2")).toEqual({ ...warned, exitCode: 1 }); + }); + + // The dependent is a store entry in the isolated linker. It keeps its links across installs, so + // a repeat install reports no changes. + it("a dependency whose optional dependency's tarball fails installs once", async () => { + const tarball = await new Bun.Archive( + { + "package/package.json": JSON.stringify({ + name: "pkg", + version: "1.0.0", + optionalDependencies: { BaR: "0.0.2" }, + }), + }, + { compress: "gzip" }, + ).bytes(); + setHandler(async request => { + const { pathname } = new URL(request.url); + const manifest = (name: string, version: string, extra: object) => + Response.json({ + name, + "dist-tags": { latest: version }, + versions: { [version]: { name, version, ...extra, dist: { tarball: `${root_url}/${name}-${version}.tgz` } } }, + }); + if (pathname === "/BaR") return manifest("BaR", "0.0.2", {}); + if (pathname === "/pkg") return manifest("pkg", "1.0.0", { optionalDependencies: { BaR: "0.0.2" } }); + if (pathname === "/pkg-1.0.0.tgz") return new Response(tarball); + return new Response("no", { status: 404 }); + }); + await writeFile( + join(package_dir, "bunfig.toml"), + Bun.TOML.stringify({ install: { cache: false, registry: `${root_url}/`, linker } }), + ); + await writeFile( + join(package_dir, "package.json"), + JSON.stringify({ name: "foo", version: "0.0.1", dependencies: { pkg: "1.0.0" } }), + ); + async function install() { + await using proc = spawn({ + cmd: [bunExe(), "install", "--no-progress", "--ignore-scripts"], + cwd: package_dir, + stdout: "pipe", + stdin: "pipe", + stderr: "pipe", + env, + }); + const [err, out, exitCode] = await Promise.all([proc.stderr.text(), proc.stdout.text(), proc.exited]); + return { + warnLines: err.split("\n").filter(l => l.startsWith("warn:")), + errorLines: err.split("\n").filter(l => l.startsWith("error:")), + summary: out.includes("no changes") ? "no changes" : out.includes("installed") ? "installed" : out, + linked: entryExists(join(package_dir, "node_modules", "BaR")), + exitCode, + }; + } + const warned = { + warnLines: [`warn: GET ${root_url}/BaR-0.0.2.tgz - 404`], + errorLines: [], + linked: false, + exitCode: 0, + }; + expect(await install()).toEqual({ ...warned, summary: "installed" }); + expect(await install()).toEqual({ ...warned, summary: "no changes" }); + expect(await file(join(package_dir, "node_modules", "pkg", "package.json")).json()).toMatchObject({ + name: "pkg", + version: "1.0.0", + }); + }); + + // One package is optional for one dependent and required by another. Its failed download + // must fail the install whichever dependent reaches it first. The hoisted linker has the + // same hole on main; #43197 closes it for both linkers. + describe.skipIf(linker === "hoisted")("a failed download that an optional and a required dependency share", () => { + async function project(root: object, packages: Record) { + const tarballs = new Map(); + for (const [name, pkg] of Object.entries(packages)) { + tarballs.set( + name, + await new Bun.Archive( + { "package/package.json": JSON.stringify({ name, version: "1.0.0", ...pkg }) }, + { compress: "gzip" }, + ).bytes(), + ); + } + setHandler(async request => { + const { pathname } = new URL(request.url); + const manifest = (name: string, version: string, extra: object) => + Response.json({ + name, + "dist-tags": { latest: version }, + versions: { + [version]: { name, version, ...extra, dist: { tarball: `${root_url}/${name}-${version}.tgz` } }, + }, + }); + if (pathname === "/BaR") return manifest("BaR", "0.0.2", {}); + const name = pathname.slice(1); + if (name in packages) return manifest(name, "1.0.0", packages[name]); + const tarball = tarballs.get(name.replace(/-1\.0\.0\.tgz$/, "")); + if (tarball && name.endsWith("-1.0.0.tgz")) return new Response(tarball); + return new Response("no", { status: 404 }); + }); + await writeFile( + join(package_dir, "bunfig.toml"), + Bun.TOML.stringify({ install: { cache: false, registry: `${root_url}/`, linker } }), + ); + await writeFile(join(package_dir, "package.json"), JSON.stringify({ name: "foo", version: "0.0.1", ...root })); + } + async function install() { + await using proc = spawn({ + cmd: [bunExe(), "install", "--no-progress", "--ignore-scripts"], + cwd: package_dir, + stdout: "pipe", + stdin: "pipe", + stderr: "pipe", + env, + }); + const [err, out, exitCode] = await Promise.all([proc.stderr.text(), proc.stdout.text(), proc.exited]); + return { + failed: out.includes("Failed to install") || err.split("\n").some(l => l.startsWith("error:")), + exitCode, + }; } + + it("optional for the root, required by a dependency", async () => { + await project( + { dependencies: { needs: "1.0.0" }, optionalDependencies: { BaR: "0.0.2" } }, + { needs: { dependencies: { BaR: "0.0.2" } } }, + ); + expect(await install()).toEqual({ failed: true, exitCode: 1 }); + await access(join(package_dir, "bun.lock")); + expect(await install()).toEqual({ failed: true, exitCode: 1 }); + }); + + // `pkg` reaches BaR first in one order, the required dependent in the other. + it.each(["aneeds", "zneeds"])("optional for a dependency, required by %s", async required => { + await project( + { dependencies: { pkg: "1.0.0", [required]: "1.0.0" } }, + { pkg: { optionalDependencies: { BaR: "0.0.2" } }, [required]: { dependencies: { BaR: "0.0.2" } } }, + ); + expect(await install()).toEqual({ failed: true, exitCode: 1 }); + await access(join(package_dir, "bun.lock")); + expect(await install()).toEqual({ failed: true, exitCode: 1 }); + }); }); });