From 840f198d916397f3f1ec8a42fe1b6f518fd4d192 Mon Sep 17 00:00:00 2001 From: Alistair Smith Date: Mon, 8 Jun 2026 15:01:48 -0700 Subject: [PATCH 1/7] Consolidate package manager and CLI command helpers --- src/install/PackageInstall.rs | 505 ++++++++---------- src/install/PackageInstaller.rs | 262 +++++---- src/install/PackageManager.rs | 332 +++++------- .../PackageManager/PackageJSONEditor.rs | 197 ++----- .../PackageManager/PackageManagerEnqueue.rs | 262 ++++----- .../WorkspacePackageJSONCache.rs | 25 + .../PackageManager/install_with_manager.rs | 182 +++---- src/install/PackageManager/patchPackage.rs | 304 ++++------- src/install/PackageManager/runTasks.rs | 196 +++---- .../updatePackageJSONAndInstall.rs | 32 +- src/install/hosted_git_info.rs | 352 ++++-------- src/install/isolated_install.rs | 80 +-- src/install/npm.rs | 78 ++- src/install/yarn.rs | 171 +----- src/runtime/cli/bunx_command.rs | 206 +++---- src/runtime/cli/create_command.rs | 78 +-- src/runtime/cli/filter_run.rs | 122 +---- src/runtime/cli/install_command.rs | 94 +--- src/runtime/cli/link_command.rs | 195 ++++--- src/runtime/cli/mod.rs | 2 + src/runtime/cli/multi_run.rs | 119 +---- src/runtime/cli/outdated_command.rs | 188 +------ src/runtime/cli/pack_command.rs | 394 +++++++------- src/runtime/cli/pm_update_package_json.rs | 183 ++++--- src/runtime/cli/repl.rs | 165 ++---- src/runtime/cli/repl_command.rs | 16 +- src/runtime/cli/run_command.rs | 47 +- src/runtime/cli/run_processes_shared.rs | 121 +++++ src/runtime/cli/unlink_command.rs | 96 +--- src/runtime/cli/update_interactive_command.rs | 166 +----- src/runtime/cli/workspace_helpers.rs | 160 ++++++ 31 files changed, 2081 insertions(+), 3249 deletions(-) create mode 100644 src/runtime/cli/run_processes_shared.rs create mode 100644 src/runtime/cli/workspace_helpers.rs diff --git a/src/install/PackageInstall.rs b/src/install/PackageInstall.rs index 00ab60a992c3..e10810796587 100644 --- a/src/install/PackageInstall.rs +++ b/src/install/PackageInstall.rs @@ -1319,232 +1319,153 @@ impl<'a> PackageInstall<'a> { return res; } - #[cfg(windows)] - type WinSlice<'b> = &'b mut [u16]; #[cfg(not(windows))] - type WinSlice<'b> = (); - #[cfg(windows)] - type WinOffset = usize; - #[cfg(not(windows))] - type WinOffset = (); - - // Two overlapping slices into the same buffer (`head` is the whole - // buffer, `to_copy_into` is its tail) would be two live aliasing - // `&mut [u16]`, which is UB — pass head buffer + tail offset and - // reslice inside. fn copy( destination_dir_: &Dir, walker: &mut Walker, mut progress_: Option<&mut Progress>, - to_copy_into1_offset: WinOffset, - head1: WinSlice<'_>, - to_copy_into2_offset: WinOffset, - head2: WinSlice<'_>, ) -> crate::Result { - #[cfg(not(windows))] let mut real_file_count: u32 = 0; - #[cfg(windows)] - let real_file_count: u32 = 0; - #[cfg(not(windows))] let mut copy_file_state = bun_sys::copy_file::CopyFileState::default(); - #[cfg(not(windows))] - let _ = (to_copy_into1_offset, head1, to_copy_into2_offset, head2); while let Some(entry) = walker.next()? { - #[cfg(windows)] - { - use bun_sys::windows::{self, Win32ErrorExt as _}; - match entry.kind { - EntryKind::Directory | EntryKind::File => {} - _ => continue, - } - - if entry.path.len() > head1.len() - to_copy_into1_offset - || entry.path.len() > head2.len() - to_copy_into2_offset - { - return Err(crate::Error::Sys(bun_errno::SystemErrno::ENAMETOOLONG)); - } - - let dest_len = to_copy_into1_offset + entry.path.len(); - head1[to_copy_into1_offset..dest_len].copy_from_slice(entry.path.as_slice()); - head1[dest_len] = 0; - let dest = bun_core::WStr::from_buf(head1, dest_len); + if entry.kind != EntryKind::File { + continue; + } + real_file_count += 1; - let src_len = to_copy_into2_offset + entry.path.len(); - head2[to_copy_into2_offset..src_len].copy_from_slice(entry.path.as_slice()); - head2[src_len] = 0; - let src = bun_core::WStr::from_buf(head2, src_len); + let in_file = sys::openat(entry.dir, entry.basename, sys::O::RDONLY, 0)?; + let _close_in = sys::CloseOnDrop::new(in_file); - match entry.kind { - EntryKind::Directory => { - // SAFETY: FFI — src/dest are valid NUL-terminated WStr buffers built - // into head1/head2 above. - if unsafe { - windows::CreateDirectoryExW( - src.as_ptr(), - dest.as_ptr(), - core::ptr::null_mut(), - ) - } == 0 - { - let _ = bun_sys::MakePath::make_path_u16( - destination_dir_, - entry.path.as_slice(), - ); - } + bun_output::scoped_log!( + install, + "createFile {} {}\n", + destination_dir_.fd(), + bstr::BStr::new(entry.path.as_bytes()) + ); + // Open O_WRONLY|O_CREAT|O_TRUNC, mode 0o666. + let create = |path: &ZStr| { + sys::openat( + destination_dir_.fd(), + path, + sys::O::WRONLY | sys::O::CREAT | sys::O::TRUNC, + 0o666, + ) + }; + let outfile = match create(entry.path) { + Ok(f) => f, + Err(_) => 'brk: { + let entry_dirname = bun_paths::resolve_path::dirname::< + bun_paths::platform::Auto, + >(entry.path.as_bytes()); + if !entry_dirname.is_empty() { + let _ = bun_sys::MakePath::make_path::( + destination_dir_, + entry_dirname, + ); } - EntryKind::File => { - // SAFETY: FFI — src/dest are valid NUL-terminated WStr buffers. - if unsafe { windows::CopyFileW(src.as_ptr(), dest.as_ptr(), 0) } == 0 { - if let Some(entry_dirname) = - bun_paths::Dirname::dirname_u16(entry.path.as_slice()) - { - let _ = bun_sys::MakePath::make_path_u16( - destination_dir_, - entry_dirname, - ); - // SAFETY: FFI — src/dest are valid NUL-terminated WStr buffers. - if unsafe { windows::CopyFileW(src.as_ptr(), dest.as_ptr(), 0) } - != 0 - { - continue; - } - } - - if let Some(progress) = progress_.as_deref_mut() { + match create(entry.path) { + Ok(f) => break 'brk f, + Err(err) => { + if let Some(progress) = progress_ { progress.root.end(); progress.refresh(); } - if let Some(err) = windows::Win32Error::get().to_system_errno() { - bun_core::pretty_errorln!( - "{}: copying file {}", - <&'static str>::from(err), - bun_core::fmt::fmt_os_path( - entry.path.as_slice(), - Default::default() - ) - ); - } else { - bun_core::pretty_errorln!( - "error copying file {}", - bun_core::fmt::fmt_os_path( - entry.path.as_slice(), - Default::default() - ) - ); - } - + bun_core::pretty_errorln!( + "{}: copying file {}", + bstr::BStr::new(err.name()), + bun_core::fmt::fmt_os_path( + entry.path.as_bytes(), + Default::default() + ) + ); Global::crash(); } } - _ => unreachable!(), // handled above } - } - #[cfg(not(windows))] + }; + let _close_out = sys::CloseOnDrop::new(outfile); + + #[cfg(unix)] { - if entry.kind != EntryKind::File { + let Ok(stat) = sys::fstat(in_file) else { continue; - } - real_file_count += 1; + }; + // `sys::fchmod` is the safe by-value-fd wrapper (kernel + // validates the fd; no memory-safety preconditions). + // Result intentionally ignored. + let _ = sys::fchmod(outfile, stat.st_mode as bun_sys::Mode); + } - let in_file = sys::openat(entry.dir, entry.basename, sys::O::RDONLY, 0)?; - let _close_in = sys::CloseOnDrop::new(in_file); + if let Err(err) = + bun_sys::copy_file::copy_file_with_state(in_file, outfile, &mut copy_file_state) + { + if let Some(progress) = progress_.as_deref_mut() { + progress.root.end(); + progress.refresh(); + } - bun_output::scoped_log!( - install, - "createFile {} {}\n", - destination_dir_.fd(), - bstr::BStr::new(entry.path.as_bytes()) + bun_core::pretty_errorln!( + "{}: copying file {}", + bstr::BStr::new(err.name()), + bun_core::fmt::fmt_os_path(entry.path.as_bytes(), Default::default()) ); - // Open O_WRONLY|O_CREAT|O_TRUNC, mode 0o666. - let create = |path: &ZStr| { - sys::openat( - destination_dir_.fd(), - path, - sys::O::WRONLY | sys::O::CREAT | sys::O::TRUNC, - 0o666, - ) - }; - let outfile = match create(entry.path) { - Ok(f) => f, - Err(_) => 'brk: { - let entry_dirname = bun_paths::resolve_path::dirname::< - bun_paths::platform::Auto, - >(entry.path.as_bytes()); - if !entry_dirname.is_empty() { - let _ = bun_sys::MakePath::make_path::( - destination_dir_, - entry_dirname, - ); - } - match create(entry.path) { - Ok(f) => break 'brk f, - Err(err) => { - if let Some(progress) = progress_ { - progress.root.end(); - progress.refresh(); - } + Global::crash(); + } + } - bun_core::pretty_errorln!( - "{}: copying file {}", - bstr::BStr::new(err.name()), - bun_core::fmt::fmt_os_path( - entry.path.as_bytes(), - Default::default() - ) - ); - Global::crash(); - } + Ok(real_file_count) + } + + #[cfg(windows)] + let result = { + use bun_sys::windows::{self, Win32ErrorExt as _}; + let destination_dir_ = &state.subdir; + let mut progress_ = self.progress.as_deref_mut(); + walk_install_dir_windows( + destination_dir_, + state.walker.as_mut().unwrap(), + state.to_copy_buf_off, + &mut state.buf[..], + state.to_copy_buf2_off, + &mut state.buf2[..], + |dest, src, entry_path| { + // SAFETY: FFI — src/dest are valid NUL-terminated WStr buffers. + if unsafe { windows::CopyFileW(src.as_ptr(), dest.as_ptr(), 0) } == 0 { + if let Some(entry_dirname) = bun_paths::Dirname::dirname_u16(entry_path) { + let _ = + bun_sys::MakePath::make_path_u16(destination_dir_, entry_dirname); + // SAFETY: FFI — src/dest are valid NUL-terminated WStr buffers. + if unsafe { windows::CopyFileW(src.as_ptr(), dest.as_ptr(), 0) } != 0 { + return Ok(()); } } - }; - let _close_out = sys::CloseOnDrop::new(outfile); - - #[cfg(unix)] - { - let Ok(stat) = sys::fstat(in_file) else { - continue; - }; - // `sys::fchmod` is the safe by-value-fd wrapper (kernel - // validates the fd; no memory-safety preconditions). - // Result intentionally ignored. - let _ = sys::fchmod(outfile, stat.st_mode as bun_sys::Mode); - } - if let Err(err) = bun_sys::copy_file::copy_file_with_state( - in_file, - outfile, - &mut copy_file_state, - ) { if let Some(progress) = progress_.as_deref_mut() { progress.root.end(); progress.refresh(); } - bun_core::pretty_errorln!( - "{}: copying file {}", - bstr::BStr::new(err.name()), - bun_core::fmt::fmt_os_path(entry.path.as_bytes(), Default::default()) - ); + if let Some(err) = windows::Win32Error::get().to_system_errno() { + bun_core::pretty_errorln!( + "{}: copying file {}", + <&'static str>::from(err), + bun_core::fmt::fmt_os_path(entry_path, Default::default()) + ); + } else { + bun_core::pretty_errorln!( + "error copying file {}", + bun_core::fmt::fmt_os_path(entry_path, Default::default()) + ); + } + Global::crash(); } - } - } - - Ok(real_file_count) - } - - #[cfg(windows)] - let result = copy( - &state.subdir, - state.walker.as_mut().unwrap(), - self.progress.as_deref_mut(), - state.to_copy_buf_off, - &mut state.buf[..], - state.to_copy_buf2_off, - &mut state.buf2[..], - ); + Ok(()) + }, + ) + }; #[cfg(not(windows))] let result = copy( &state.subdir, @@ -1552,10 +1473,6 @@ impl<'a> PackageInstall<'a> { // (`state.walker()` would reborrow `&mut state` and conflict). state.walker.as_mut().unwrap(), self.progress.as_deref_mut(), - (), - (), - (), - (), ); self.file_count = match result { @@ -1792,37 +1709,18 @@ impl<'a> PackageInstall<'a> { } } - #[cfg(windows)] - type WinSlice<'b> = &'b mut [u16]; - #[cfg(not(windows))] - type WinSlice<'b> = (); - #[cfg(windows)] - type WinOffset = usize; - #[cfg(not(windows))] - type WinOffset = (); - #[cfg(windows)] - type Head2Char = u16; - #[cfg(not(windows))] - type Head2Char = u8; - // Two overlapping slices into the same buffer (`head` is the whole // buffer, `to_copy_into` is its tail) would be two live aliasing // `&mut`, which is UB — pass head buffer + tail offset and reslice // inside. + #[cfg(not(windows))] fn copy( destination_dir: &Dir, walker: &mut Walker, - to_copy_into1_offset: WinOffset, - head1: WinSlice<'_>, to_copy_into2_offset: usize, - head2: &mut [Head2Char], + head2: &mut [u8], ) -> crate::Result { - #[cfg(not(windows))] let mut real_file_count: u32 = 0; - #[cfg(windows)] - let real_file_count: u32 = 0; - #[cfg(not(windows))] - let _ = (to_copy_into1_offset, head1); while let Some(entry) = walker.next()? { #[cfg(unix)] { @@ -1857,99 +1755,51 @@ impl<'a> PackageInstall<'a> { _ => {} } } - #[cfg(not(unix))] - { - use bun_sys::windows; - match entry.kind { - EntryKind::Directory | EntryKind::File => {} - _ => continue, - } - - if entry.path.len() > head1.len() - to_copy_into1_offset - || entry.path.len() > head2.len() - to_copy_into2_offset - { - return Err(crate::Error::Sys(bun_errno::SystemErrno::ENAMETOOLONG)); - } - - let dest_len = to_copy_into1_offset + entry.path.len(); - head1[to_copy_into1_offset..dest_len].copy_from_slice(entry.path.as_slice()); - head1[dest_len] = 0; - let dest = bun_core::WStr::from_buf(head1, dest_len); + } - let src_len = to_copy_into2_offset + entry.path.len(); - head2[to_copy_into2_offset..src_len].copy_from_slice(entry.path.as_slice()); - head2[src_len] = 0; - let src = bun_core::WStr::from_buf(head2, src_len); + Ok(real_file_count) + } - match entry.kind { - EntryKind::Directory => { - // SAFETY: FFI — src/dest are valid NUL-terminated WStr buffers built - // into head1/head2 above. - if unsafe { - windows::CreateDirectoryExW( - src.as_ptr(), - dest.as_ptr(), - core::ptr::null_mut(), - ) - } == 0 - { - let _ = bun_sys::MakePath::make_path_u16( - destination_dir, - entry.path.as_slice(), - ); + #[cfg(windows)] + let result = { + let destination_dir = &state.subdir; + walk_install_dir_windows( + destination_dir, + state.walker.as_mut().unwrap(), + state.to_copy_buf_off, + &mut state.buf[..], + state.to_copy_buf2_off, + &mut state.buf2[..], + |dest, src, entry_path| match sys::symlink_w(dest, src, Default::default()) { + Err(err) => { + if let Some(entry_dirname) = bun_paths::Dirname::dirname_u16(entry_path) { + let _ = + bun_sys::MakePath::make_path_u16(destination_dir, entry_dirname); + if sys::symlink_w(dest, src, Default::default()).is_ok() { + return Ok(()); } } - EntryKind::File => match sys::symlink_w(dest, src, Default::default()) { - Err(err) => { - if let Some(entry_dirname) = - bun_paths::Dirname::dirname_u16(entry.path.as_slice()) - { - let _ = bun_sys::MakePath::make_path_u16( - destination_dir, - entry_dirname, - ); - if sys::symlink_w(dest, src, Default::default()).is_ok() { - continue; - } - } - if PackageManager::verbose_install() { - bun_core::run_once! {{ - bun_core::warn!( - "CreateHardLinkW failed, falling back to CopyFileW: {} -> {}\n", - bun_core::fmt::fmt_os_path(src.as_slice(), Default::default()), - bun_core::fmt::fmt_os_path(dest.as_slice(), Default::default()), - ); - }} - } + if PackageManager::verbose_install() { + bun_core::run_once! {{ + bun_core::warn!( + "CreateHardLinkW failed, falling back to CopyFileW: {} -> {}\n", + bun_core::fmt::fmt_os_path(src.as_slice(), Default::default()), + bun_core::fmt::fmt_os_path(dest.as_slice(), Default::default()), + ); + }} + } - return Err(err.into()); - } - Ok(_) => {} - }, - _ => unreachable!(), // handled above + Err(err.into()) } - } - } - - Ok(real_file_count) - } - - #[cfg(windows)] - let result = copy( - &state.subdir, - state.walker.as_mut().unwrap(), - state.to_copy_buf_off, - &mut state.buf[..], - state.to_copy_buf2_off, - &mut state.buf2[..], - ); + Ok(_) => Ok(()), + }, + ) + }; #[cfg(not(windows))] let result = copy( &state.subdir, state.walker.as_mut().unwrap(), - (), - (), to_copy_buf2_offset, &mut buf2[..], ); @@ -2517,3 +2367,66 @@ impl<'a> PackageInstall<'a> { } type Walker = walker_skippable::Walker; + +/// Shared Windows directory walk for the copyfile/symlink install backends: +/// builds NUL-terminated wide dest/src paths for each entry, creates +/// directories (`CreateDirectoryExW` with a `make_path` fallback), and calls +/// `per_file(dest, src, entry_path)` for each file. +/// +/// Two overlapping slices into the same buffer (`head` is the whole buffer, +/// `to_copy_into` is its tail) would be two live aliasing `&mut [u16]`, which +/// is UB — pass head buffer + tail offset and reslice inside. +#[cfg(windows)] +fn walk_install_dir_windows( + destination_dir: &Dir, + walker: &mut Walker, + to_copy_into1_offset: usize, + head1: &mut [u16], + to_copy_into2_offset: usize, + head2: &mut [u16], + mut per_file: impl FnMut(&bun_core::WStr, &bun_core::WStr, &[u16]) -> crate::Result<()>, +) -> crate::Result { + use bun_sys::windows; + + while let Some(entry) = walker.next()? { + match entry.kind { + EntryKind::Directory | EntryKind::File => {} + _ => continue, + } + + if entry.path.len() > head1.len() - to_copy_into1_offset + || entry.path.len() > head2.len() - to_copy_into2_offset + { + return Err(crate::Error::Sys(bun_errno::SystemErrno::ENAMETOOLONG)); + } + + let dest_len = to_copy_into1_offset + entry.path.len(); + head1[to_copy_into1_offset..dest_len].copy_from_slice(entry.path.as_slice()); + head1[dest_len] = 0; + let dest = bun_core::WStr::from_buf(head1, dest_len); + + let src_len = to_copy_into2_offset + entry.path.len(); + head2[to_copy_into2_offset..src_len].copy_from_slice(entry.path.as_slice()); + head2[src_len] = 0; + let src = bun_core::WStr::from_buf(head2, src_len); + + match entry.kind { + EntryKind::Directory => { + // SAFETY: FFI — src/dest are valid NUL-terminated WStr buffers built + // into head1/head2 above. + if unsafe { + windows::CreateDirectoryExW(src.as_ptr(), dest.as_ptr(), core::ptr::null_mut()) + } == 0 + { + let _ = + bun_sys::MakePath::make_path_u16(destination_dir, entry.path.as_slice()); + } + } + EntryKind::File => per_file(dest, src, entry.path.as_slice())?, + _ => unreachable!(), // handled above + } + } + + // Windows installs don't track a real file count. + Ok(0) +} diff --git a/src/install/PackageInstaller.rs b/src/install/PackageInstaller.rs index a74f458d2cbc..ec2d8187acb2 100644 --- a/src/install/PackageInstaller.rs +++ b/src/install/PackageInstaller.rs @@ -427,6 +427,16 @@ pub(crate) fn alias_is_safe_install_target(alias: &[u8]) -> bool { component_count == 1 || (component_count == 2 && alias[0] == b'@') } +/// Where to record a newly trusted dependency once its lifecycle scripts are +/// enqueued. +#[derive(Clone, Copy)] +struct TrustedDepRecord { + /// Add the alias to `trusted_deps_to_add_to_package_json`. + package_json: bool, + /// Add the alias to the lockfile's `trusted_dependencies`. + lockfile: bool, +} + impl<'a> PackageInstaller<'a> { // ────────────────────────────────────────────────────────────────────── // BACKREF accessors @@ -1947,73 +1957,20 @@ impl<'a> PackageInstaller<'a> { if resolution.tag != resolution::Tag::Root && (resolution.tag == resolution::Tag::Workspace || is_trusted) { - let mut folder_path = - AutoAbsPath::from(self.node_modules.path.as_slice()).unwrap_or_oom(); - // `defer folder_path.deinit()` — AbsPath impls Drop. - folder_path - .append(alias.slice(string_buf!())) - .unwrap_or_oom(); - - 'enqueue_lifecycle_scripts: { - if self - .manager() - .postinstall_optimizer - .should_ignore_lifecycle_scripts( - &postinstall_optimizer::PkgInfo { - name_hash: pkg_name_hash, - version: if resolution.tag == resolution::Tag::Npm { - Some(resolution.npm().version) - } else { - None - }, - version_buf: string_buf!(), - }, - self.lockfile().packages.items_resolutions() - [package_id as usize] - .get(self.lockfile().buffers.resolutions.as_slice()), - self.lockfile().packages.items_meta(), - self.manager().options.cpu, - self.manager().options.os, - ) - { - if PackageManager::verbose_install() { - bun_core::pretty_errorln!( - "[Lifecycle Scripts] ignoring {} lifecycle scripts", - bstr::BStr::new(pkg_name.slice(string_buf!())), - ); - } - break 'enqueue_lifecycle_scripts; - } - - if self.enqueue_lifecycle_scripts( - alias.slice(string_buf!()), - log_level, - &mut folder_path, - package_id, - dep_behavior.contains(crate::dependency::Behavior::OPTIONAL), - resolution, - ) { - if is_trusted_through_update_request { - self.manager_mut() - .trusted_deps_to_add_to_package_json - .push(Box::<[u8]>::from(alias.slice(string_buf!()))); - - if self.lockfile().trusted_dependencies.is_none() { - self.lockfile_mut().trusted_dependencies = - Some(Default::default()); - } - self.lockfile_mut() - .trusted_dependencies - .as_mut() - .unwrap() - .put( - truncated_dep_name_hash, - Box::<[u8]>::from(alias.slice(string_buf!())), - ) - .unwrap_or_oom(); - } - } - } + self.enqueue_lifecycle_scripts_for_trusted( + log_level, + package_id, + pkg_name, + pkg_name_hash, + alias, + truncated_dep_name_hash, + dep_behavior.contains(crate::dependency::Behavior::OPTIONAL), + resolution, + TrustedDepRecord { + package_json: is_trusted_through_update_request, + lockfile: is_trusted_through_update_request, + }, + ); } match resolution.tag { @@ -2262,72 +2219,20 @@ impl<'a> PackageInstaller<'a> { }; if resolution.tag != resolution::Tag::Root && is_trusted { - let mut folder_path = - AutoAbsPath::from(self.node_modules.path.as_slice()).unwrap_or_oom(); - folder_path - .append(alias.slice(string_buf!())) - .unwrap_or_oom(); - - 'enqueue_lifecycle_scripts: { - if self - .manager() - .postinstall_optimizer - .should_ignore_lifecycle_scripts( - &postinstall_optimizer::PkgInfo { - name_hash: pkg_name_hash, - version: if resolution.tag == resolution::Tag::Npm { - Some(resolution.npm().version) - } else { - None - }, - version_buf: string_buf!(), - }, - self.lockfile().packages.items_resolutions()[package_id as usize] - .get(self.lockfile().buffers.resolutions.as_slice()), - self.lockfile().packages.items_meta(), - self.manager().options.cpu, - self.manager().options.os, - ) - { - if PackageManager::verbose_install() { - bun_core::pretty_errorln!( - "[Lifecycle Scripts] ignoring {} lifecycle scripts", - bstr::BStr::new(pkg_name.slice(string_buf!())), - ); - } - break 'enqueue_lifecycle_scripts; - } - - if self.enqueue_lifecycle_scripts( - alias.slice(string_buf!()), - log_level, - &mut folder_path, - package_id, - dep_behavior.contains(crate::dependency::Behavior::OPTIONAL), - resolution, - ) { - if is_trusted_through_update_request { - self.manager_mut() - .trusted_deps_to_add_to_package_json - .push(Box::<[u8]>::from(alias.slice(string_buf!()))); - } - - if add_to_lockfile { - if self.lockfile().trusted_dependencies.is_none() { - self.lockfile_mut().trusted_dependencies = Some(Default::default()); - } - self.lockfile_mut() - .trusted_dependencies - .as_mut() - .unwrap() - .put( - truncated_dep_name_hash, - Box::<[u8]>::from(alias.slice(string_buf!())), - ) - .unwrap_or_oom(); - } - } - } + self.enqueue_lifecycle_scripts_for_trusted( + log_level, + package_id, + pkg_name, + pkg_name_hash, + alias, + truncated_dep_name_hash, + dep_behavior.contains(crate::dependency::Behavior::OPTIONAL), + resolution, + TrustedDepRecord { + package_json: is_trusted_through_update_request, + lockfile: add_to_lockfile, + }, + ); } // `destination_dir` is `LazyPackageDestinationDir::NodeModulesPath` @@ -2357,6 +2262,97 @@ impl<'a> PackageInstaller<'a> { ); } + /// Enqueue lifecycle scripts for a trusted (or workspace) dependency, then + /// record it in `trusted_deps_to_add_to_package_json` and/or the lockfile's + /// `trusted_dependencies` as requested. + fn enqueue_lifecycle_scripts_for_trusted( + &mut self, + log_level: Options::LogLevel, + package_id: PackageID, + pkg_name: String, + pkg_name_hash: PackageNameHash, + alias: String, + truncated_dep_name_hash: TruncatedPackageNameHash, + optional: bool, + resolution: &Resolution, + record: TrustedDepRecord, + ) { + // SAFETY: `buffers.string_bytes` is append-only and never freed + // for the lifetime of this `PackageInstaller`. + let string_buf_ptr = + bun_ptr::RawSlice::new(self.lockfile().buffers.string_bytes.as_slice()); + macro_rules! string_buf { + () => { + string_buf_ptr.slice() + }; + } + + let mut folder_path = AutoAbsPath::from(self.node_modules.path.as_slice()).unwrap_or_oom(); + // `defer folder_path.deinit()` — AbsPath impls Drop. + folder_path + .append(alias.slice(string_buf!())) + .unwrap_or_oom(); + + if self + .manager() + .postinstall_optimizer + .should_ignore_lifecycle_scripts( + &postinstall_optimizer::PkgInfo { + name_hash: pkg_name_hash, + version: if resolution.tag == resolution::Tag::Npm { + Some(resolution.npm().version) + } else { + None + }, + version_buf: string_buf!(), + }, + self.lockfile().packages.items_resolutions()[package_id as usize] + .get(self.lockfile().buffers.resolutions.as_slice()), + self.lockfile().packages.items_meta(), + self.manager().options.cpu, + self.manager().options.os, + ) + { + if PackageManager::verbose_install() { + bun_core::pretty_errorln!( + "[Lifecycle Scripts] ignoring {} lifecycle scripts", + bstr::BStr::new(pkg_name.slice(string_buf!())), + ); + } + return; + } + + if self.enqueue_lifecycle_scripts( + alias.slice(string_buf!()), + log_level, + &mut folder_path, + package_id, + optional, + resolution, + ) { + if record.package_json { + self.manager_mut() + .trusted_deps_to_add_to_package_json + .push(Box::<[u8]>::from(alias.slice(string_buf!()))); + } + + if record.lockfile { + if self.lockfile().trusted_dependencies.is_none() { + self.lockfile_mut().trusted_dependencies = Some(Default::default()); + } + self.lockfile_mut() + .trusted_dependencies + .as_mut() + .unwrap() + .put( + truncated_dep_name_hash, + Box::<[u8]>::from(alias.slice(string_buf!())), + ) + .unwrap_or_oom(); + } + } + } + /// returns true if scripts are enqueued fn enqueue_lifecycle_scripts( &mut self, diff --git a/src/install/PackageManager.rs b/src/install/PackageManager.rs index ce93caa60233..af9dfa9f04e0 100644 --- a/src/install/PackageManager.rs +++ b/src/install/PackageManager.rs @@ -1323,6 +1323,123 @@ pub(crate) fn get() -> *mut PackageManager { // init // ────────────────────────────────────────────────────────────────────────── +/// Placement-writes one field of the `PackageManager` singleton through the +/// raw pointer `$p` — see the PERF NOTE in [`init`] for why the struct must +/// not be built by value. +macro_rules! wr { + ($p:ident, $field:ident, $val:expr) => { + core::ptr::addr_of_mut!((*$p).$field).write($val) + }; +} + +/// Writes the `PackageManager` fields that take identical default values in +/// both init paths ([`init`] and [`init_with_runtime_once`]). Each caller +/// writes the remaining (divergent) fields itself; together they must fully +/// initialize the singleton. +/// +/// Uses per-field placement writes — see the PERF NOTE in [`init`] for why +/// the struct must not be built by value. +/// +/// # Safety +/// `p` must point to the allocated (possibly uninitialized) singleton from +/// `allocate_package_manager()`, with no other references to it live. +unsafe fn write_shared_default_fields(p: *mut PackageManager) { + // SAFETY: caller guarantees `p` is valid for per-field placement writes. + unsafe { + // The two large pools: in-place init that only zeros the 256 B + // occupancy bitset and leaves `[MaybeUninit; N]` untouched — no + // stack temporary, no memcpy. + PreallocatedNetworkTasks::init_in_place(core::ptr::addr_of_mut!( + (*p).preallocated_network_tasks + )); + PreallocatedTaskStore::init_in_place(core::ptr::addr_of_mut!( + (*p).preallocated_resolve_tasks + )); + + wr!(p, cache_directory, None); + wr!(p, cache_directory_path, ZBox::from_bytes(b"")); + wr!( + p, + active_lifecycle_scripts, + crate::lifecycle_script_runner::List { + root: core::ptr::null_mut(), + // `lifecycle_script_runner::List`'s heap comparator never + // dereferences its context arg, so it is modeled as a ZST + // (`StartedAtCtx`) instead of threading a back-pointer. + context: crate::lifecycle_script_runner::StartedAtCtx, + } + ); + wr!(p, network_task_fifo, NetworkQueue::init()); + wr!(p, patch_task_fifo, PatchTaskFifo::init()); + wr!(p, ast_arena, bun_alloc::Arena::new()); + wr!(p, resolve_tasks, ResolveTaskQueue::default()); + // `Lockfile` contains `HashMap`/`Vec`/`NonNull` fields, so a + // zero-bit pattern is UB; allocate the real (empty) lockfile here directly. + // `Lockfile::default()` ≡ `Lockfile::init_empty()`. + wr!(p, lockfile, Box::new(Lockfile::default())); + wr!(p, timestamp_for_manifest_cache_control, 0); + wr!(p, extracted_count, 0); + wr!(p, summary, Default::default()); + wr!(p, progress, Progress::default()); + wr!(p, downloads_node, None); + wr!(p, scripts_node, None); + wr!(p, progress_name_buf, [0; 768]); + wr!(p, track_installed_bin, TrackInstalledBin::None); + wr!(p, root_progress_node, core::ptr::null_mut()); + wr!(p, to_update, false); + wr!(p, update_requests, Box::default()); + wr!(p, root_package_id, RootPackageId::default()); + wr!(p, task_batch, thread_pool::Batch::default()); + wr!(p, task_queue, TaskDependencyQueue::default()); + wr!(p, manifests, PackageManifestMap::default()); + wr!(p, folders, Default::default()); + wr!(p, git_repositories, RepositoryMap::default()); + wr!(p, network_dedupe_map, Default::default()); + wr!( + p, + async_network_task_queue, + AsyncNetworkTaskQueue::default() + ); + wr!(p, network_tarball_batch, thread_pool::Batch::default()); + wr!(p, network_resolve_batch, thread_pool::Batch::default()); + wr!(p, patch_apply_batch, thread_pool::Batch::default()); + wr!(p, patch_calc_hash_batch, thread_pool::Batch::default()); + wr!(p, patch_task_queue, PatchTaskQueue::default()); + wr!(p, pending_pre_calc_hashes, AtomicU32::new(0)); + wr!(p, pending_tasks, AtomicU32::new(0)); + wr!(p, total_tasks, 0); + wr!( + p, + lifecycle_script_time_log, + LifecycleScriptTimeLog::default() + ); + wr!(p, pending_lifecycle_script_tasks, AtomicU32::new(0)); + wr!(p, finished_installing, AtomicBool::new(false)); + wr!(p, total_scripts, 0); + wr!(p, root_lifecycle_scripts, None); + wr!(p, node_gyp_tempdir_name, Box::default()); + wr!(p, preinstall_state, Vec::new()); + wr!(p, postinstall_optimizer, Default::default()); + wr!(p, global_link_dir, None); + wr!(p, global_dir, None); + wr!(p, global_link_dir_path, Box::default()); + wr!(p, on_wake, WakeHandler::default()); + wr!( + p, + peer_dependencies, + LinearFifo::>::init() + ); + wr!(p, known_npm_aliases, NpmAliasMap::default()); + wr!(p, trusted_deps_to_add_to_package_json, Vec::new()); + wr!(p, any_failed_to_install, false); + wr!(p, updating_packages, StringArrayHashMap::default()); + wr!(p, updating_catalogs, Vec::new()); + wr!(p, patched_dependencies_to_remove, ArrayHashMap::default()); + wr!(p, last_reported_slow_lifecycle_script_at, 0); + wr!(p, cached_tick_for_slow_lifecycle_script_logging, 0); + } +} + /// Returns `&'static mut PackageManager` — the process-singleton (held in /// `holder::RAW_PTR`) is leaked for the process lifetime and `init()` is called /// exactly once on the single CLI dispatch thread. Every @@ -1840,123 +1957,44 @@ pub fn init( // directly to the heap and keeps the frame under 16 KB. unsafe { let p = manager_ptr; - macro_rules! wr { - ($field:ident, $val:expr) => { - core::ptr::addr_of_mut!((*p).$field).write($val) - }; - } - // The two large pools: in-place init that only zeros the 256 B - // occupancy bitset and leaves `[MaybeUninit; N]` untouched — no - // stack temporary, no memcpy. - PreallocatedNetworkTasks::init_in_place(core::ptr::addr_of_mut!( - (*p).preallocated_network_tasks - )); - PreallocatedTaskStore::init_in_place(core::ptr::addr_of_mut!( - (*p).preallocated_resolve_tasks - )); + write_shared_default_fields(p); - wr!(cache_directory, None); - wr!(cache_directory_path, ZBox::from_bytes(b"")); - wr!(options, options); - wr!( - active_lifecycle_scripts, - crate::lifecycle_script_runner::List { - root: core::ptr::null_mut(), - // `lifecycle_script_runner::List`'s heap comparator never - // dereferences its context arg, so it is modeled as a ZST - // (`StartedAtCtx`) instead of threading a back-pointer. - context: crate::lifecycle_script_runner::StartedAtCtx, - } - ); - wr!(network_task_fifo, NetworkQueue::init()); - wr!(patch_task_fifo, PatchTaskFifo::init()); - wr!(log, ctx.log); - wr!(root_dir, entries_option); - wr!(ast_arena, bun_alloc::Arena::new()); + wr!(p, options, options); + wr!(p, log, ctx.log); + wr!(p, root_dir, entries_option); // reborrow `&mut *env` so the local stays usable for // the post-construction `BUN_MANIFEST_CACHE` / `options.load` // reads. `BackRef` stores a raw pointer — // ending the reborrow here does not alias the later uses. - wr!(env, Some(bun_ptr::BackRef::new_mut(&mut *env))); + wr!(p, env, Some(bun_ptr::BackRef::new_mut(&mut *env))); wr!( + p, thread_pool, ThreadPool::init(thread_pool::Config { max_threads: cpu_count, ..Default::default() }) ); - wr!(resolve_tasks, ResolveTaskQueue::default()); - // `Lockfile` contains `HashMap`/`Vec`/`NonNull` fields, so a - // zero-bit pattern is UB; allocate the real (empty) lockfile here directly. - // `Lockfile::default()` ≡ `Lockfile::init_empty()`. - wr!(lockfile, Box::new(Lockfile::default())); - wr!(root_package_json_file, root_package_json_file); + wr!(p, root_package_json_file, root_package_json_file); // .progress - wr!(event_loop, AnyEventLoop::init()); + wr!(p, event_loop, AnyEventLoop::init()); wr!( + p, original_package_json_path, ZBox::from_vec_with_nul(original_package_json_path_buf) ); - wr!(workspace_package_json_cache, workspace_package_json_cache); - wr!(workspace_name_hash, workspace_name_hash); - wr!(subcommand, subcommand); wr!( - root_package_json_name_at_time_of_init, - root_package_json_name_at_time_of_init + p, + workspace_package_json_cache, + workspace_package_json_cache ); - - // remaining defaults: - wr!(timestamp_for_manifest_cache_control, 0); - wr!(extracted_count, 0); - wr!(summary, Default::default()); - wr!(progress, Progress::default()); - wr!(downloads_node, None); - wr!(scripts_node, None); - wr!(progress_name_buf, [0; 768]); - wr!(track_installed_bin, TrackInstalledBin::None); - wr!(root_progress_node, core::ptr::null_mut()); - wr!(to_update, false); - wr!(update_requests, Box::default()); - wr!(root_package_id, RootPackageId::default()); - wr!(task_batch, thread_pool::Batch::default()); - wr!(task_queue, TaskDependencyQueue::default()); - wr!(manifests, PackageManifestMap::default()); - wr!(folders, Default::default()); - wr!(git_repositories, RepositoryMap::default()); - wr!(network_dedupe_map, Default::default()); - wr!(async_network_task_queue, AsyncNetworkTaskQueue::default()); - wr!(network_tarball_batch, thread_pool::Batch::default()); - wr!(network_resolve_batch, thread_pool::Batch::default()); - wr!(patch_apply_batch, thread_pool::Batch::default()); - wr!(patch_calc_hash_batch, thread_pool::Batch::default()); - wr!(patch_task_queue, PatchTaskQueue::default()); - wr!(pending_pre_calc_hashes, AtomicU32::new(0)); - wr!(pending_tasks, AtomicU32::new(0)); - wr!(total_tasks, 0); - wr!(lifecycle_script_time_log, LifecycleScriptTimeLog::default()); - wr!(pending_lifecycle_script_tasks, AtomicU32::new(0)); - wr!(finished_installing, AtomicBool::new(false)); - wr!(total_scripts, 0); - wr!(root_lifecycle_scripts, None); - wr!(node_gyp_tempdir_name, Box::default()); - wr!(preinstall_state, Vec::new()); - wr!(postinstall_optimizer, Default::default()); - wr!(global_link_dir, None); - wr!(global_dir, None); - wr!(global_link_dir_path, Box::default()); - wr!(on_wake, WakeHandler::default()); + wr!(p, workspace_name_hash, workspace_name_hash); + wr!(p, subcommand, subcommand); wr!( - peer_dependencies, - LinearFifo::>::init() + p, + root_package_json_name_at_time_of_init, + root_package_json_name_at_time_of_init ); - wr!(known_npm_aliases, NpmAliasMap::default()); - wr!(trusted_deps_to_add_to_package_json, Vec::new()); - wr!(any_failed_to_install, false); - wr!(updating_packages, StringArrayHashMap::default()); - wr!(updating_catalogs, Vec::new()); - wr!(patched_dependencies_to_remove, ArrayHashMap::default()); - wr!(last_reported_slow_lifecycle_script_at, 0); - wr!(cached_tick_for_slow_lifecycle_script_logging, 0); } holder::INITIALIZED.store(true, core::sync::atomic::Ordering::Release); // The per-field placement above fully initialized the singleton; the @@ -2268,23 +2306,10 @@ fn init_with_runtime_once( // directly to the heap singleton. unsafe { let p = manager_ptr; - macro_rules! wr { - ($field:ident, $val:expr) => { - core::ptr::addr_of_mut!((*p).$field).write($val) - }; - } - // The two large pools: in-place init that only zeros the 256 B - // occupancy bitset and leaves `[MaybeUninit; N]` untouched. - PreallocatedNetworkTasks::init_in_place(core::ptr::addr_of_mut!( - (*p).preallocated_network_tasks - )); - PreallocatedTaskStore::init_in_place(core::ptr::addr_of_mut!( - (*p).preallocated_resolve_tasks - )); + write_shared_default_fields(p); - wr!(cache_directory, None); - wr!(cache_directory_path, ZBox::from_bytes(b"")); wr!( + p, options, Options { max_concurrent_lifecycle_scripts: cli @@ -2293,108 +2318,45 @@ fn init_with_runtime_once( ..Default::default() } ); - wr!( - active_lifecycle_scripts, - crate::lifecycle_script_runner::List { - root: core::ptr::null_mut(), - context: crate::lifecycle_script_runner::StartedAtCtx, - } - ); - wr!(network_task_fifo, NetworkQueue::init()); - wr!(log, std::ptr::from_mut(log)); - wr!(root_dir, root_dir); - wr!(ast_arena, bun_alloc::Arena::new()); + wr!(p, log, std::ptr::from_mut(log)); + wr!(p, root_dir, root_dir); // reborrow `&mut *env` so the local stays usable for // the post-construction `BUN_MANIFEST_CACHE` / `options.load` // reads. `BackRef` stores a raw pointer — // ending the reborrow here does not alias the later uses. - wr!(env, Some(bun_ptr::BackRef::new_mut(&mut *env))); + wr!(p, env, Some(bun_ptr::BackRef::new_mut(&mut *env))); wr!( + p, thread_pool, ThreadPool::init(thread_pool::Config { max_threads: cpu_count, ..Default::default() }) ); - // `Lockfile` holds `HashMap`/`Vec`/`NonNull` (zero-bit pattern is - // UB), so allocate the real empty lockfile here directly instead of a zeroed placeholder. - wr!(lockfile, Box::new(Lockfile::default())); // `.root_package_json_file` is never read in the runtime // path. Use the explicit invalid-fd sentinel rather than `mem::zeroed()` — // on posix `Fd(0)` is stdin, not the invalid marker. wr!( + p, root_package_json_file, bun_sys::File::from_fd(Fd::invalid()) ); // erased *mut () set by tier-6; `js_current()` resolves the per-thread JS // event loop via `bun_io::__bun_get_vm_ctx` (link-time, definer in bun_runtime). - wr!(event_loop, AnyEventLoop::js_current()); + wr!(p, event_loop, AnyEventLoop::js_current()); wr!( + p, original_package_json_path, ZBox::from_vec_with_nul(original_package_json_path) ); - wr!(subcommand, Subcommand::Install); - - // remaining defaults: - wr!(resolve_tasks, ResolveTaskQueue::default()); - wr!(timestamp_for_manifest_cache_control, 0); - wr!(extracted_count, 0); - wr!(summary, Default::default()); - wr!(progress, Progress::default()); - wr!(downloads_node, None); - wr!(scripts_node, None); - wr!(progress_name_buf, [0; 768]); - wr!(track_installed_bin, TrackInstalledBin::None); - wr!(root_progress_node, core::ptr::null_mut()); - wr!(to_update, false); - wr!(update_requests, Box::default()); - wr!(root_package_json_name_at_time_of_init, Box::default()); - wr!(root_package_id, RootPackageId::default()); - wr!(task_batch, thread_pool::Batch::default()); - wr!(task_queue, TaskDependencyQueue::default()); - wr!(manifests, PackageManifestMap::default()); - wr!(folders, Default::default()); - wr!(git_repositories, RepositoryMap::default()); - wr!(network_dedupe_map, Default::default()); - wr!(async_network_task_queue, AsyncNetworkTaskQueue::default()); - wr!(network_tarball_batch, thread_pool::Batch::default()); - wr!(network_resolve_batch, thread_pool::Batch::default()); - wr!(patch_apply_batch, thread_pool::Batch::default()); - wr!(patch_calc_hash_batch, thread_pool::Batch::default()); - wr!(patch_task_fifo, PatchTaskFifo::init()); - wr!(patch_task_queue, PatchTaskQueue::default()); - wr!(pending_pre_calc_hashes, AtomicU32::new(0)); - wr!(pending_tasks, AtomicU32::new(0)); - wr!(total_tasks, 0); - wr!(lifecycle_script_time_log, LifecycleScriptTimeLog::default()); - wr!(pending_lifecycle_script_tasks, AtomicU32::new(0)); - wr!(finished_installing, AtomicBool::new(false)); - wr!(total_scripts, 0); - wr!(root_lifecycle_scripts, None); - wr!(node_gyp_tempdir_name, Box::default()); - wr!(preinstall_state, Vec::new()); - wr!(postinstall_optimizer, Default::default()); - wr!(global_link_dir, None); - wr!(global_dir, None); - wr!(global_link_dir_path, Box::default()); - wr!(on_wake, WakeHandler::default()); - wr!( - peer_dependencies, - LinearFifo::>::init() - ); - wr!(known_npm_aliases, NpmAliasMap::default()); - wr!(trusted_deps_to_add_to_package_json, Vec::new()); - wr!(any_failed_to_install, false); - wr!(workspace_name_hash, None); + wr!(p, subcommand, Subcommand::Install); + wr!(p, root_package_json_name_at_time_of_init, Box::default()); + wr!(p, workspace_name_hash, None); wr!( + p, workspace_package_json_cache, WorkspacePackageJSONCache::default() ); - wr!(updating_packages, StringArrayHashMap::default()); - wr!(updating_catalogs, Vec::new()); - wr!(patched_dependencies_to_remove, ArrayHashMap::default()); - wr!(last_reported_slow_lifecycle_script_at, 0); - wr!(cached_tick_for_slow_lifecycle_script_logging, 0); } holder::INITIALIZED.store(true, core::sync::atomic::Ordering::Release); // SAFETY: per-field placement above fully initialized the PackageManager; diff --git a/src/install/PackageManager/PackageJSONEditor.rs b/src/install/PackageManager/PackageJSONEditor.rs index a1beb791c272..aca597a00b13 100644 --- a/src/install/PackageManager/PackageJSONEditor.rs +++ b/src/install/PackageManager/PackageJSONEditor.rs @@ -39,6 +39,46 @@ fn arena_dup<'a>(arena: &'a bun_alloc::Arena, bytes: &[u8]) -> &'a [u8] { arena.alloc_slice_copy(bytes) } +/// Builds the replacement version string for an updated npm dependency, +/// preserving the original pin style (`1.2.3` / `~1.2.3` / `^1.2.3`) and, for +/// aliases, the `npm:@scope/pkg@` prefix from `dep_literal`. +fn replacement_version_literal( + version_fmt: impl std::fmt::Display, + original_version_literal: &[u8], + is_alias: bool, + dep_literal: &[u8], + exact_versions: bool, +) -> Vec { + let mut v = Vec::new(); + if is_alias { + // negative because the real package might have a scope + // e.g. "dep": "npm:@foo/bar@1.2.3" + if let Some(at_index) = strings::last_index_of_char(dep_literal, b'@') { + write!(&mut v, "{}@", bstr::BStr::new(&dep_literal[0..at_index])) + .expect("infallible: in-memory write"); + } + } + let pin_prefix = if exact_versions { + "" + } else { + let version_literal = if is_alias { + match strings::last_index_of_char(original_version_literal, b'@') { + Some(at_index) => &original_version_literal[at_index + 1..], + None => original_version_literal, + } + } else { + original_version_literal + }; + match semver::Version::which_version_is_pinned(version_literal) { + semver::PinnedVersion::Patch => "", + semver::PinnedVersion::Minor => "~", + semver::PinnedVersion::Major => "^", + } + }; + write!(&mut v, "{}{}", pin_prefix, version_fmt).expect("infallible: in-memory write"); + v +} + /// Shallow-copy a `G::Property` for the JSON-editing path. Only `key`/`value` /// (both `Option`, `Copy`) are populated by the JSON parser; the rest /// (`ts_decorators`, `class_static_block`, …) are always default for parsed @@ -441,82 +481,14 @@ pub(crate) fn edit_update_no_args( } } - let new_version: Vec = 'new_version: { - // `resolution.tag == Npm` checked above. - let version_fmt = resolution.npm().version.fmt(string_buf); - if options.exact_versions { - let mut v = Vec::new(); - write!(&mut v, "{}", version_fmt) - .expect("infallible: in-memory write"); - break 'new_version v; - } - - let version_literal: &[u8] = 'version_literal: { - if !is_alias { - break 'version_literal &entry - .value - .original_version_literal; - } - if let Some(at_index) = strings::last_index_of_char( - &entry.value.original_version_literal, - b'@', - ) { - break 'version_literal &entry - .value - .original_version_literal[at_index + 1..]; - } - &entry.value.original_version_literal - }; - - let pinned_version = - semver::Version::which_version_is_pinned( - version_literal, - ); - let mut v = Vec::new(); - match pinned_version { - semver::PinnedVersion::Patch => { - write!(&mut v, "{}", version_fmt) - .expect("infallible: in-memory write") - } - semver::PinnedVersion::Minor => { - write!(&mut v, "~{}", version_fmt) - .expect("infallible: in-memory write") - } - semver::PinnedVersion::Major => { - write!(&mut v, "^{}", version_fmt) - .expect("infallible: in-memory write") - } - } - v - }; - - if is_alias { - let dep_literal = - workspace_dep.version.literal.slice(string_buf); - - // negative because the real package might have a scope - // e.g. "dep": "npm:@foo/bar@1.2.3" - if let Some(at_index) = - strings::last_index_of_char(dep_literal, b'@') - { - let mut v = Vec::new(); - write!( - &mut v, - "{}@{}", - bstr::BStr::new(&dep_literal[0..at_index]), - bstr::BStr::new(&new_version) - ) - .unwrap(); - dep.value = Some(Expr::allocate( - arena, - E::EString::init(arena_str(arena, &v)), - bun_ast::Loc::EMPTY, - )); - break 'updated; - } - - // fallthrough and replace entire version. - } + // `resolution.tag == Npm` checked above. + let new_version = replacement_version_literal( + resolution.npm().version.fmt(string_buf), + &entry.value.original_version_literal, + is_alias, + workspace_dep.version.literal.slice(string_buf), + options.exact_versions, + ); dep.value = Some(Expr::allocate( arena, @@ -1459,73 +1431,16 @@ pub(crate) fn edit( if let Some(entry) = manager.updating_packages.fetch_swap_remove(request.name) { - let new_version: Vec = 'new_version: { - let version_fmt = resolutions[request.package_id as usize] + let new_version = replacement_version_literal( + resolutions[request.package_id as usize] .npm() .version - .fmt(manager.lockfile.buffers.string_bytes.as_slice()); - if options.exact_versions { - let mut v = Vec::new(); - write!(&mut v, "{}", version_fmt) - .expect("infallible: in-memory write"); - break 'new_version v; - } - - let version_literal: &[u8] = 'version_literal: { - if !entry.value.is_alias { - break 'version_literal &entry - .value - .original_version_literal; - } - if let Some(at_index) = strings::last_index_of_char( - &entry.value.original_version_literal, - b'@', - ) { - break 'version_literal &entry - .value - .original_version_literal[at_index + 1..]; - } - - &entry.value.original_version_literal - }; - - let pinned_version = - semver::Version::which_version_is_pinned(version_literal); - let mut v = Vec::new(); - match pinned_version { - semver::PinnedVersion::Patch => { - write!(&mut v, "{}", version_fmt) - .expect("infallible: in-memory write") - } - semver::PinnedVersion::Minor => { - write!(&mut v, "~{}", version_fmt) - .expect("infallible: in-memory write") - } - semver::PinnedVersion::Major => { - write!(&mut v, "^{}", version_fmt) - .expect("infallible: in-memory write") - } - } - v - }; - - if entry.value.is_alias { - let dep_literal = &entry.value.original_version_literal; - - if let Some(at_index) = - strings::last_index_of_char(dep_literal, b'@') - { - let mut v = Vec::new(); - write!( - &mut v, - "{}@{}", - bstr::BStr::new(&dep_literal[0..at_index]), - bstr::BStr::new(&new_version) - ) - .unwrap(); - break 'npm arena_str(arena, &v); - } - } + .fmt(manager.lockfile.buffers.string_bytes.as_slice()), + &entry.value.original_version_literal, + entry.value.is_alias, + &entry.value.original_version_literal, + options.exact_versions, + ); break 'npm arena_str(arena, &new_version); } diff --git a/src/install/PackageManager/PackageManagerEnqueue.rs b/src/install/PackageManager/PackageManagerEnqueue.rs index f0c91da56b53..6eab8f2a1d90 100644 --- a/src/install/PackageManager/PackageManagerEnqueue.rs +++ b/src/install/PackageManager/PackageManagerEnqueue.rs @@ -619,6 +619,36 @@ pub unsafe fn enqueue_patch_task_pre(this: &mut PackageManager, task: *mut Patch let _ = this.pending_pre_calc_hashes.fetch_add(1, Ordering::Relaxed); } +/// Returns the task-callback list for `task_id`, creating and initializing it +/// if this is the first callback registered for the task. +fn task_callback_list<'a>( + this: &'a mut PackageManager, + task_id: Task::Id, +) -> crate::Result<&'a mut TaskCallbackList> { + let entry = this.task_queue.get_or_put_context(task_id, ())?; + if !entry.found_existing { + *entry.value_ptr = TaskCallbackList::default(); + } + Ok(entry.value_ptr) +} + +/// Registers dependency `id` as a callback for `task_id`, tagging it as a +/// root or transitive dependency. +fn push_dependency_task_callback( + this: &mut PackageManager, + task_id: Task::Id, + id: DependencyID, + is_root: bool, +) -> crate::Result<()> { + let ctx = if is_root { + TaskCallbackContext::RootDependency(id) + } else { + TaskCallbackContext::Dependency(id) + }; + task_callback_list(this, task_id)?.push(ctx); + Ok(()) +} + /// Q: "What do we do with a dependency in a package.json?" /// A: "We enqueue it!" pub fn enqueue_dependency_with_main_and_success_fn( @@ -1155,18 +1185,7 @@ pub fn enqueue_dependency_with_main_and_success_fn( return Ok(()); } - let manifest_entry_parse = - this.task_queue.get_or_put_context(task_id, ())?; - if !manifest_entry_parse.found_existing { - *manifest_entry_parse.value_ptr = TaskCallbackList::default(); - } - - let ctx = if is_root { - TaskCallbackContext::RootDependency(id) - } else { - TaskCallbackContext::Dependency(id) - }; - manifest_entry_parse.value_ptr.push(ctx); + push_dependency_task_callback(this, task_id, id, is_root)?; } return Ok(()); } @@ -1191,11 +1210,6 @@ pub fn enqueue_dependency_with_main_and_success_fn( let alias = this.lockfile.str_detached(&dependency.name); let url = this.lockfile.str_detached(&dep.repo); let clone_id = Task::Id::for_git_clone(url); - let ctx = if is_root { - TaskCallbackContext::RootDependency(id) - } else { - TaskCallbackContext::Dependency(id) - }; if cfg!(debug_assertions) { bun_output::scoped_log!( @@ -1222,15 +1236,11 @@ pub fn enqueue_dependency_with_main_and_success_fn( let needs_ctx = this.lockfile.buffers.resolutions[id as usize] == invalid_package_id; - let entry = this - .task_queue - .get_or_put_context(checkout_id, ()) - .expect("unreachable"); - if !entry.found_existing { - *entry.value_ptr = TaskCallbackList::default(); - } if needs_ctx { - entry.value_ptr.push(ctx); + push_dependency_task_callback(this, checkout_id, id, is_root) + .expect("unreachable"); + } else { + task_callback_list(this, checkout_id).expect("unreachable"); } if dependency.behavior.is_peer() { @@ -1256,14 +1266,7 @@ pub fn enqueue_dependency_with_main_and_success_fn( ); this.task_batch.push(ThreadPool::Batch::from(task)); } else { - let entry = this - .task_queue - .get_or_put_context(clone_id, ()) - .expect("unreachable"); - if !entry.found_existing { - *entry.value_ptr = TaskCallbackList::default(); - } - entry.value_ptr.push(ctx); + push_dependency_task_callback(this, clone_id, id, is_root).expect("unreachable"); if dependency.behavior.is_peer() { if !install_peer { @@ -1308,24 +1311,7 @@ pub fn enqueue_dependency_with_main_and_success_fn( ); } - let ctx = if is_root { - TaskCallbackContext::RootDependency(id) - } else { - TaskCallbackContext::Dependency(id) - }; - // reshaped for borrowck — `entry` mutably borrows - // `this.task_queue`; scope it tightly so the calls below can - // reborrow `*this`. - { - let entry = this - .task_queue - .get_or_put_context(task_id, ()) - .expect("unreachable"); - if !entry.found_existing { - *entry.value_ptr = TaskCallbackList::default(); - } - entry.value_ptr.push(ctx); - } + push_dependency_task_callback(this, task_id, id, is_root).expect("unreachable"); if dependency.behavior.is_peer() { if !install_peer { @@ -1506,22 +1492,7 @@ pub fn enqueue_dependency_with_main_and_success_fn( ); } - let ctx = if is_root { - TaskCallbackContext::RootDependency(id) - } else { - TaskCallbackContext::Dependency(id) - }; - // reshaped for borrowck — scope `entry` tightly. - { - let entry = this - .task_queue - .get_or_put_context(task_id, ()) - .expect("unreachable"); - if !entry.found_existing { - *entry.value_ptr = TaskCallbackList::default(); - } - entry.value_ptr.push(ctx); - } + push_dependency_task_callback(this, task_id, id, is_root).expect("unreachable"); if dependency.behavior.is_peer() { if !install_peer { @@ -2153,6 +2124,41 @@ fn get_or_put_resolved_package_with_find_result( // `guard` drops here → success_fn(this, dependency_id, package.meta.id) } +/// Scans the root package's dependencies for a workspace entry matching +/// `name_hash`; on a match, records the resolution via `success_fn` and +/// returns the already-resolved workspace package. +fn resolve_root_workspace_package( + this: &mut PackageManager, + name_hash: PackageNameHash, + dependency_id: DependencyID, + success_fn: SuccessFn, +) -> Option { + let root_package = this.lockfile.root_package()?; + let root_dependencies = root_package + .dependencies + .get(this.lockfile.buffers.dependencies.as_slice()); + let root_resolutions = root_package + .resolutions + .get(this.lockfile.buffers.resolutions.as_slice()); + + debug_assert_eq!(root_dependencies.len(), root_resolutions.len()); + for (root_dep, &workspace_package_id) in root_dependencies.iter().zip(root_resolutions) { + if workspace_package_id != invalid_package_id + && root_dep.version.tag == dependency::version::Tag::Workspace + && root_dep.name_hash == name_hash + { + // make sure verifyResolutions sees this resolution as a valid package id + success_fn(this, dependency_id, workspace_package_id); + return Some(ResolvedPackageResult { + package: *this.lockfile.packages.get(workspace_package_id as usize), + is_first_time: false, + task: None, + }); + } + } + None +} + fn get_or_put_resolved_package( this: &mut PackageManager, name_hash: PackageNameHash, @@ -2279,54 +2285,27 @@ fn get_or_put_resolved_package( match version.tag { dependency::version::Tag::Npm | dependency::version::Tag::DistTag => { - 'resolve_from_workspace: { - if version.tag == dependency::version::Tag::Npm { - let workspace_path = if this.lockfile.workspace_paths.count() > 0 { - this.lockfile.workspace_paths.get(&name_hash) - } else { - None - }; - let workspace_version = this.lockfile.workspace_versions.get(&name_hash); - let buf = this.lockfile.buffers.string_bytes.as_slice(); - let npm_group = &version.npm().version; - if this.options.link_workspace_packages - && ((workspace_version.is_some() - && npm_group.satisfies(*workspace_version.unwrap(), buf, buf)) - // https://github.com/oven-sh/bun/pull/10899#issuecomment-2099609419 - // if the workspace doesn't have a version, it can still be used if - // dependency version is wildcard - || (workspace_path.is_some() && npm_group.is_star())) + if version.tag == dependency::version::Tag::Npm { + let workspace_path = if this.lockfile.workspace_paths.count() > 0 { + this.lockfile.workspace_paths.get(&name_hash) + } else { + None + }; + let workspace_version = this.lockfile.workspace_versions.get(&name_hash); + let buf = this.lockfile.buffers.string_bytes.as_slice(); + let npm_group = &version.npm().version; + if this.options.link_workspace_packages + && ((workspace_version.is_some() + && npm_group.satisfies(*workspace_version.unwrap(), buf, buf)) + // https://github.com/oven-sh/bun/pull/10899#issuecomment-2099609419 + // if the workspace doesn't have a version, it can still be used if + // dependency version is wildcard + || (workspace_path.is_some() && npm_group.is_star())) + { + if let Some(resolved) = + resolve_root_workspace_package(this, name_hash, dependency_id, success_fn) { - let Some(root_package) = this.lockfile.root_package() else { - break 'resolve_from_workspace; - }; - let root_dependencies = root_package - .dependencies - .get(this.lockfile.buffers.dependencies.as_slice()); - let root_resolutions = root_package - .resolutions - .get(this.lockfile.buffers.resolutions.as_slice()); - - debug_assert_eq!(root_dependencies.len(), root_resolutions.len()); - for (root_dep, &workspace_package_id) in - root_dependencies.iter().zip(root_resolutions) - { - if workspace_package_id != invalid_package_id - && root_dep.version.tag == dependency::version::Tag::Workspace - && root_dep.name_hash == name_hash - { - // make sure verifyResolutions sees this resolution as a valid package id - success_fn(this, dependency_id, workspace_package_id); - return Ok(Some(ResolvedPackageResult { - package: *this - .lockfile - .packages - .get(workspace_package_id as usize), - is_first_time: false, - task: None, - })); - } - } + return Ok(Some(resolved)); } } } @@ -2440,46 +2419,21 @@ fn get_or_put_resolved_package( let find_result = match find_result_opt { Some(r) => r, None => { - 'resolve_workspace_from_dist_tag: { - // choose a workspace for a dist_tag only if a version was not found - if version.tag == dependency::version::Tag::DistTag { - let workspace_path = if this.lockfile.workspace_paths.count() > 0 { - this.lockfile.workspace_paths.get(&name_hash) - } else { - None - }; - if workspace_path.is_some() { - let Some(root_package) = this.lockfile.root_package() else { - break 'resolve_workspace_from_dist_tag; - }; - let root_dependencies = root_package - .dependencies - .get(this.lockfile.buffers.dependencies.as_slice()); - let root_resolutions = root_package - .resolutions - .get(this.lockfile.buffers.resolutions.as_slice()); - - debug_assert_eq!(root_dependencies.len(), root_resolutions.len()); - for (root_dep, &workspace_package_id) in - root_dependencies.iter().zip(root_resolutions) - { - if workspace_package_id != invalid_package_id - && root_dep.version.tag - == dependency::version::Tag::Workspace - && root_dep.name_hash == name_hash - { - // make sure verifyResolutions sees this resolution as a valid package id - success_fn(this, dependency_id, workspace_package_id); - return Ok(Some(ResolvedPackageResult { - package: *this - .lockfile - .packages - .get(workspace_package_id as usize), - is_first_time: false, - task: None, - })); - } - } + // choose a workspace for a dist_tag only if a version was not found + if version.tag == dependency::version::Tag::DistTag { + let workspace_path = if this.lockfile.workspace_paths.count() > 0 { + this.lockfile.workspace_paths.get(&name_hash) + } else { + None + }; + if workspace_path.is_some() { + if let Some(resolved) = resolve_root_workspace_package( + this, + name_hash, + dependency_id, + success_fn, + ) { + return Ok(Some(resolved)); } } } diff --git a/src/install/PackageManager/WorkspacePackageJSONCache.rs b/src/install/PackageManager/WorkspacePackageJSONCache.rs index 41287a66c09c..8b6d4a75dce6 100644 --- a/src/install/PackageManager/WorkspacePackageJSONCache.rs +++ b/src/install/PackageManager/WorkspacePackageJSONCache.rs @@ -2,6 +2,7 @@ use crate::Error; use bun_collections::StringHashMap; +use bun_core::{Global, Output}; // `Expr` here is the JSON parser's AST node (`bun_ast::Expr`, re- // exported via `crate::bun_json`). It is intentionally NOT `bun_ast::Expr` // — that lives in a higher-tier crate and is a distinct type. Consumers of @@ -205,4 +206,28 @@ impl WorkspacePackageJSONCache { GetResult::Entry(entry.value_ptr) } + + /// `get_with_path`, except read/parse failures are fatal: pending log + /// messages and the error are printed to stderr, then the process exits. + pub fn get_with_path_or_exit( + &mut self, + log: &mut Log, + abs_package_json_path: &[u8], + opts: GetJSONOptions, + ) -> &mut MapEntry { + let (err, fmt) = match self.get_with_path(log, abs_package_json_path, opts) { + GetResult::Entry(entry) => return entry, + GetResult::ReadErr(err) => (err, "failed to read '{}'"), + GetResult::ParseErr(err) => (err, "failed to parse '{}'"), + }; + if log.errors > 0 { + let _ = log.print(std::ptr::from_mut(Output::error_writer())); + } + Output::err( + err, + fmt, + format_args!("{}", bstr::BStr::new(abs_package_json_path)), + ); + Global::crash(); + } } diff --git a/src/install/PackageManager/install_with_manager.rs b/src/install/PackageManager/install_with_manager.rs index a06d651a152c..be765f92e39b 100644 --- a/src/install/PackageManager/install_with_manager.rs +++ b/src/install/PackageManager/install_with_manager.rs @@ -8,7 +8,6 @@ use bun_core::{ZStr, strings}; use bun_glob as glob; use bun_semver::String as SemverString; -use crate::GetJsonResult as WorkspacePackageJsonCacheResult; use crate::Subcommand; use crate::dependency::{DependencyExt as _, Tag as DependencyVersionTag}; use crate::lockfile::{self, Lockfile}; @@ -140,72 +139,12 @@ pub fn install_with_manager( let mut lockfile = Lockfile::default(); let mut maybe_root = lockfile::Package::default(); - // SAFETY: `manager.log` is a non-null backref to the CLI log set at init(). - let root_package_json_entry = match manager - .workspace_package_json_cache - .get_with_path( - manager.log_mut(), - root_package_json_path.as_bytes(), - Default::default(), - ) { - WorkspacePackageJsonCacheResult::Entry(entry) => entry, - WorkspacePackageJsonCacheResult::ReadErr(err) => { - if manager.log_mut().errors > 0 { - manager - .log_mut() - .print(std::ptr::from_mut(Output::error_writer()))?; - } - Output::err( - err, - "failed to read '{}'", - format_args!("{}", bstr::BStr::new(root_package_json_path.as_bytes())), - ); - Global::exit(1); - } - WorkspacePackageJsonCacheResult::ParseErr(err) => { - if manager.log_mut().errors > 0 { - manager - .log_mut() - .print(std::ptr::from_mut(Output::error_writer()))?; - } - Output::err( - err, - "failed to parse '{}'", - format_args!("{}", bstr::BStr::new(root_package_json_path.as_bytes())), - ); - Global::exit(1); - } - }; - - // `Source` is not `Copy`, so - // clone it (cheap — `Source` is a few `Box<[u8]>` handles) so the - // `&mut *mgr` reborrow below doesn't conflict with the cache borrow. - let source_copy = root_package_json_entry.source.clone(); - - let mut resolver: () = (); - // `parse` needs `manager`, `manager.log` and a fresh - // stack `lockfile` simultaneously. Route through raw ptrs so - // borrowck doesn't see overlapping `&mut PackageManager` / - // `&mut Lockfile`. - { - // `log_mut()` reads the BACKREF `self.log: *mut Log` and - // returns the disjoint CLI `Log` allocation (lifetime - // decoupled from `&self`), so call it safely through - // `manager` *before* establishing the raw-ptr split — no - // borrow on `*manager` survives into the `&mut *mgr` below. - let log = manager.log_mut(); - let mgr: *mut PackageManager = manager; - maybe_root.parse( - &mut lockfile, - // SAFETY: `mgr` is the sole provenance root for `*manager`; `log` is a - // disjoint backref and `lockfile` is a stack local, so this `&mut` is unique. - unsafe { &mut *mgr }, - log, - &source_copy, - &mut resolver, - Features::main(), - )?; - } + parse_root_package( + manager, + root_package_json_path, + &mut lockfile, + &mut maybe_root, + )?; let mut mapping = vec![invalid_package_id; maybe_root.dependencies.len as usize] .into_boxed_slice(); // @memset already done via vec! init @@ -1475,6 +1414,49 @@ fn record_updating_package_versions(manager: &mut PackageManager) { } } +/// Load the root package.json from the workspace cache (exiting on read/parse +/// errors) and parse it as the root `Package` into `lockfile`. `lockfile` must +/// be storage disjoint from `*manager` (a stack local, or the heap allocation +/// behind `manager.lockfile`'s `Box`). +fn parse_root_package( + manager: &mut PackageManager, + root_package_json_path: &ZStr, + lockfile: &mut Lockfile, + root: &mut lockfile::Package, +) -> crate::Result<()> { + // SAFETY: `manager.log` is a non-null backref to the CLI log set at init(). + let root_package_json_entry = manager.workspace_package_json_cache.get_with_path_or_exit( + manager.log_mut(), + root_package_json_path.as_bytes(), + Default::default(), + ); + + // `Source` is not `Copy`, so clone it (cheap — `Source` is a few + // `Box<[u8]>` handles) so the `&mut *mgr` reborrow below doesn't conflict + // with the cache borrow. + let source_copy = root_package_json_entry.source.clone(); + + let mut resolver: () = (); + // `log_mut()` reads the BACKREF `self.log: *mut Log` and returns the + // disjoint CLI `Log` allocation (lifetime decoupled from `&self`), so call + // it safely through `manager` *before* establishing the raw-ptr split — no + // borrow on `*manager` survives into the `&mut *mgr` below. + let log = manager.log_mut(); + let mgr: *mut PackageManager = manager; + root.parse( + lockfile, + // SAFETY: `mgr` is the sole provenance root for `*manager`; `log` is a + // disjoint backref and `lockfile` is caller-guaranteed disjoint + // storage, so this `&mut` is unique. + unsafe { &mut *mgr }, + log, + &source_copy, + &mut resolver, + Features::main(), + )?; + Ok(()) +} + #[cold] #[inline(never)] fn create_new_lockfile_and_enqueue( @@ -1512,63 +1494,21 @@ fn create_new_lockfile_and_enqueue( Global::crash(); } - // SAFETY: `manager.log` is a non-null backref to the CLI log set at init(). - let root_package_json_entry = match manager.workspace_package_json_cache.get_with_path( - manager.log_mut(), - root_package_json_path.as_bytes(), - Default::default(), - ) { - WorkspacePackageJsonCacheResult::Entry(entry) => entry, - WorkspacePackageJsonCacheResult::ReadErr(err) => { - if manager.log_mut().errors > 0 { - manager - .log_mut() - .print(std::ptr::from_mut(Output::error_writer()))?; - } - Output::err( - err, - "failed to read '{}'", - format_args!("{}", bstr::BStr::new(root_package_json_path.as_bytes())), - ); - Global::exit(1); - } - WorkspacePackageJsonCacheResult::ParseErr(err) => { - if manager.log_mut().errors > 0 { - manager - .log_mut() - .print(std::ptr::from_mut(Output::error_writer()))?; - } - Output::err( - err, - "failed to parse '{}'", - format_args!("{}", bstr::BStr::new(root_package_json_path.as_bytes())), - ); - Global::exit(1); - } - }; - - let source_copy = root_package_json_entry.source.clone(); - - let mut resolver: () = (); { - // `log_mut()` reads the BACKREF `self.log` and returns the disjoint - // CLI `Log` allocation (lifetime decoupled from `&self`); call it - // safely *before* the raw-ptr split. - let log = manager.log_mut(); let mgr: *mut PackageManager = manager; - // SAFETY: `mgr` is the sole provenance root; `parse` reborrows the - // disjoint `lockfile` field through it. No other live `&mut` to - // `*mgr` exists across the call. - root.parse( - // SAFETY: disjoint field projection through the sole provenance root `mgr`. - unsafe { &mut (*mgr).lockfile }, - // SAFETY: `parse` touches only `PackageManager` fields disjoint from - // `lockfile` through this borrow; `mgr` is the sole provenance root. + // SAFETY: `mgr` is the sole provenance root; `manager.lockfile` is an + // owned `Box`, so this projects its heap allocation, which is disjoint + // storage from the `PackageManager` struct itself. + let lockfile: *mut Lockfile = unsafe { &raw mut *(*mgr).lockfile }; + parse_root_package( + // SAFETY: `parse_root_package` touches only `PackageManager` fields + // disjoint from `lockfile` through this borrow; `mgr` is the sole + // provenance root. unsafe { &mut *mgr }, - log, - &source_copy, - &mut resolver, - Features::main(), + root_package_json_path, + // SAFETY: points to the Box-owned heap allocation, disjoint from `*mgr`. + unsafe { &mut *lockfile }, + &mut root, )?; } diff --git a/src/install/PackageManager/patchPackage.rs b/src/install/PackageManager/patchPackage.rs index a416be8d9293..2f271749db86 100644 --- a/src/install/PackageManager/patchPackage.rs +++ b/src/install/PackageManager/patchPackage.rs @@ -152,101 +152,7 @@ pub fn do_patch_commit( let (cache_dir, cache_dir_subpath, changes_dir, pkg): (Fd, &ZStr, Vec, Package) = match arg_kind { PatchArgKind::Path => 'result: { - let package_json_path = - resolve_path::join_z::(&[argument, b"package.json"]); - let package_json_source: bun_ast::Source = - match bun_ast::to_source(package_json_path, Default::default()) { - Ok(s) => s, - Err(e) => { - Output::err( - e, - "failed to read {f}", - (bun_fmt::quote(package_json_path.as_bytes()),), - ); - Global::crash(); - } - }; - - initialize_store(); - let log = manager.log_mut(); - let parsed = match JSON::ParsedJson::parse_package_json(&package_json_source, log) { - Ok(p) => p, - Err(err) => { - let _ = log.print(std::ptr::from_mut(Output::error_writer())); - bun_core::pretty_errorln!( - "{} parsing package.json in \"{}\"", - err.name(), - bstr::BStr::new(package_json_source.path.pretty_dir()), - ); - Global::crash(); - } - }; - let json = parsed.root; - - let version: &[u8] = 'version: { - if let Some(v) = json.get(b"version") { - if let bun_ast::ExprData::EString(s) = &v.data { - let s = s.data.slice(); - break 'version s; - } - } - bun_core::pretty_error!( - "error: invalid package.json, missing or invalid property \"version\": {}\n", - bstr::BStr::new(package_json_source.path.text()), - ); - Global::crash(); - }; - - let mut resolver: () = (); - let mut package = Package::default(); - let log = manager.log_mut(); - package.parse_with_json::<()>( - &mut lockfile, - manager, - log, - &package_json_source, - json, - &mut resolver, - Features::FOLDER, - )?; - - let actual_package = match lockfile.package_index.get(&package.name_hash) { - None => { - bun_core::pretty_error!( - "error: failed to find package in lockfile package index, this is a bug in Bun. Please file a GitHub issue.\n", - ); - Global::crash(); - } - Some(PackageIndexEntry::Id(id)) => *lockfile.packages.get(*id as usize), - Some(PackageIndexEntry::Ids(ids)) => 'brk: { - for &id in ids.as_slice() { - let pkg = *lockfile.packages.get(id as usize); - let total = resolution_buf.len(); - let mut cursor: &mut [u8] = &mut resolution_buf[..]; - write!( - &mut cursor, - "{}", - pkg.resolution - .fmt(lockfile.buffers.string_bytes.as_slice(), PathSep::Posix) - ) - .expect("unreachable"); - let written = total - cursor.len(); - let resolution_label = &resolution_buf[..written]; - if resolution_label == version { - break 'brk pkg; - } - } - bun_core::pretty_error!( - "error: could not find package with name: {}\n", - bstr::BStr::new( - package.name.slice(lockfile.buffers.string_bytes.as_slice()) - ), - ); - Global::crash(); - } - }; - - let name = lockfile.str(&package.name).to_vec(); + let (name, actual_package) = load_path_package(manager, &mut lockfile, argument)?; let resolution_clone = actual_package.resolution; let cache_result = compute_cache_dir_and_subpath( manager, @@ -729,7 +635,6 @@ pub fn prepare_patch(manager: &mut PackageManager) -> Result<(), crate::Error> { let arg_kind: PatchArgKind = PatchArgKind::from_arg(argument); let mut folder_path_buf = PathBuffer::uninit(); - let mut resolution_buf = [0u8; 1024]; #[cfg(windows)] let mut win_normalizer = PathBuffer::uninit(); @@ -762,109 +667,18 @@ pub fn prepare_patch(manager: &mut PackageManager) -> Result<(), crate::Error> { let (cache_dir, cache_dir_subpath, module_folder, pkg_name): (Fd, &[u8], Vec, Vec) = match arg_kind { PatchArgKind::Path => 'brk: { - let package_json_path = - resolve_path::join_z::(&[argument, b"package.json"]); - let package_json_source: bun_ast::Source = - match bun_ast::to_source(package_json_path, Default::default()) { - Ok(s) => s, - Err(e) => { - Output::err( - e, - "failed to read {f}", - (bun_fmt::quote(package_json_path.as_bytes()),), - ); - Global::crash(); - } - }; - - initialize_store(); - let log = manager.log_mut(); - let parsed = match JSON::ParsedJson::parse_package_json(&package_json_source, log) { - Ok(p) => p, - Err(err) => { - let _ = log.print(std::ptr::from_mut(Output::error_writer())); - bun_core::pretty_errorln!( - "{} parsing package.json in \"{}\"", - err.name(), - bstr::BStr::new(package_json_source.path.pretty_dir()), - ); - Global::crash(); - } - }; - let json = parsed.root; - - let version: &[u8] = 'version: { - if let Some(v) = json.get(b"version") { - if let bun_ast::ExprData::EString(s) = &v.data { - let s = s.data.slice(); - break 'version s; - } - } - bun_core::pretty_error!( - "error: invalid package.json, missing or invalid property \"version\": {}\n", - bstr::BStr::new(package_json_source.path.text()), - ); - Global::crash(); - }; - - let mut resolver: () = (); - let mut package = Package::default(); - let log = manager.log_mut(); - // borrowck — `parse_with_json` needs `&mut Lockfile` and + // borrowck — `load_path_package` needs `&mut Lockfile` and // `&mut PackageManager` simultaneously, but the lockfile here is // `manager.lockfile`. Temporarily move the Box out so the two - // borrows are disjoint; `parse_with_json` never reads `pm.lockfile` - // (it takes the lockfile as its own parameter). Restore before - // propagating any error so `manager` is never left half-torn. + // borrows are disjoint. Restore before propagating any error so + // `manager` is never left half-torn. let mut lockfile: Box = core::mem::take(&mut manager.lockfile); - let parse_result = package.parse_with_json::<()>( - &mut lockfile, - manager, - log, - &package_json_source, - json, - &mut resolver, - Features::FOLDER, - ); + let result = load_path_package(manager, &mut lockfile, argument); manager.lockfile = lockfile; - parse_result?; + let (name, actual_package) = result?; let lockfile: &Lockfile = &manager.lockfile; let strbuf = lockfile.buffers.string_bytes.as_slice(); - let actual_package = match lockfile.package_index.get(&package.name_hash) { - None => { - bun_core::pretty_error!( - "error: failed to find package in lockfile package index, this is a bug in Bun. Please file a GitHub issue.\n", - ); - Global::crash(); - } - Some(PackageIndexEntry::Id(id)) => *lockfile.packages.get(*id as usize), - Some(PackageIndexEntry::Ids(ids)) => 'id: { - for &id in ids.as_slice() { - let pkg = *lockfile.packages.get(id as usize); - let total = resolution_buf.len(); - let mut cursor: &mut [u8] = &mut resolution_buf[..]; - write!( - &mut cursor, - "{}", - pkg.resolution.fmt(strbuf, PathSep::Posix) - ) - .expect("unreachable"); - let written = total - cursor.len(); - let resolution_label = &resolution_buf[..written]; - if resolution_label == version { - break 'id pkg; - } - } - bun_core::pretty_error!( - "error: could not find package with name: {}\n", - bstr::BStr::new(package.name.slice(strbuf)), - ); - Global::crash(); - } - }; - - let name = lockfile.str(&package.name).to_vec(); let existing_patchfile_hash: Option = 'existing_patchfile_hash: { let mut name_and_version = Vec::new(); write!( @@ -1280,6 +1094,112 @@ fn node_modules_folder_for_dependency_id( } } +/// Shared `PatchArgKind::Path` handling for `bun patch` and `bun patch --commit`: +/// parse `/package.json`, register it against the lockfile, and find +/// the matching package in the lockfile's package index. Returns the package +/// name and the lockfile's entry for the package. +/// +/// `parse_with_json` never reads `manager.lockfile` (it takes the lockfile as +/// its own parameter), so callers whose lockfile lives in `manager.lockfile` +/// may temporarily move the Box out to make the two `&mut` borrows disjoint. +fn load_path_package( + manager: &mut PackageManager, + lockfile: &mut Lockfile, + argument: &[u8], +) -> crate::Result<(Vec, Package)> { + let package_json_path = resolve_path::join_z::(&[argument, b"package.json"]); + let package_json_source: bun_ast::Source = + match bun_ast::to_source(package_json_path, Default::default()) { + Ok(s) => s, + Err(e) => { + Output::err( + e, + "failed to read {f}", + (bun_fmt::quote(package_json_path.as_bytes()),), + ); + Global::crash(); + } + }; + + initialize_store(); + let log = manager.log_mut(); + let parsed = match JSON::ParsedJson::parse_package_json(&package_json_source, log) { + Ok(p) => p, + Err(err) => { + let _ = log.print(std::ptr::from_mut(Output::error_writer())); + bun_core::pretty_errorln!( + "{} parsing package.json in \"{}\"", + err.name(), + bstr::BStr::new(package_json_source.path.pretty_dir()), + ); + Global::crash(); + } + }; + let json = parsed.root; + + let version: &[u8] = 'version: { + if let Some(v) = json.get(b"version") { + if let bun_ast::ExprData::EString(s) = &v.data { + break 'version s.data.slice(); + } + } + bun_core::pretty_error!( + "error: invalid package.json, missing or invalid property \"version\": {}\n", + bstr::BStr::new(package_json_source.path.text()), + ); + Global::crash(); + }; + + let mut resolver: () = (); + let mut package = Package::default(); + package.parse_with_json::<()>( + lockfile, + manager, + log, + &package_json_source, + json, + &mut resolver, + Features::FOLDER, + )?; + + let strbuf = lockfile.buffers.string_bytes.as_slice(); + let actual_package = match lockfile.package_index.get(&package.name_hash) { + None => { + bun_core::pretty_error!( + "error: failed to find package in lockfile package index, this is a bug in Bun. Please file a GitHub issue.\n", + ); + Global::crash(); + } + Some(PackageIndexEntry::Id(id)) => *lockfile.packages.get(*id as usize), + Some(PackageIndexEntry::Ids(ids)) => 'brk: { + let mut resolution_buf = [0u8; 1024]; + for &id in ids.as_slice() { + let pkg = *lockfile.packages.get(id as usize); + let total = resolution_buf.len(); + let mut cursor: &mut [u8] = &mut resolution_buf[..]; + write!( + &mut cursor, + "{}", + pkg.resolution.fmt(strbuf, PathSep::Posix) + ) + .expect("unreachable"); + let written = total - cursor.len(); + if &resolution_buf[..written] == version { + break 'brk pkg; + } + } + bun_core::pretty_error!( + "error: could not find package with name: {}\n", + bstr::BStr::new(package.name.slice(strbuf)), + ); + Global::crash(); + } + }; + + let name = lockfile.str(&package.name).to_vec(); + Ok((name, actual_package)) +} + type IdPair = (DependencyID, PackageID); fn pkg_info_for_name_and_version( diff --git a/src/install/PackageManager/runTasks.rs b/src/install/PackageManager/runTasks.rs index b4fee5fbb9d9..dcbd09383712 100644 --- a/src/install/PackageManager/runTasks.rs +++ b/src/install/PackageManager/runTasks.rs @@ -444,16 +444,7 @@ pub fn run_tasks( ); } - if manager.subcommand != Subcommand::Remove { - for request in manager.update_requests.iter_mut() { - if strings::eql(request.name, name) { - request.failed = true; - manager.options.do_.remove(Do::SAVE_LOCKFILE); - manager.options.do_.remove(Do::SAVE_YARN_LOCK); - manager.options.do_.remove(Do::INSTALL_PACKAGES); - } - } - } + mark_update_request_failed(manager, name); } continue; @@ -496,16 +487,7 @@ pub fn run_tasks( response.status_code, ); } - if manager.subcommand != Subcommand::Remove { - for request in manager.update_requests.iter_mut() { - if strings::eql(request.name, name) { - request.failed = true; - manager.options.do_.remove(Do::SAVE_LOCKFILE); - manager.options.do_.remove(Do::SAVE_YARN_LOCK); - manager.options.do_.remove(Do::INSTALL_PACKAGES); - } - } - } + mark_update_request_failed(manager, name); continue; } @@ -702,41 +684,11 @@ pub fn run_tasks( .map(crate::Error::from) .unwrap_or(crate::Error::TarballFailedToDownload); - // The download will not be retried for this task_id. Mark - // the dedupe entry as failed so a later - // `enqueuePackageForDownload` for the same package observes - // the failure and fails fast instead of either waiting - // forever on a callback that never arrives (entry kept) or - // re-running the entire download+retry cycle (entry removed). - // Runs before the callback branch so `Store.Installer` - // (which `continue`s from the callback) is covered too. - let is_required = manager.is_network_task_required(task.task_id); - manager.mark_network_task_failed(task.task_id); - - if C::HAS_ON_PACKAGE_DOWNLOAD_ERROR { - if C::IS_STORE_INSTALLER { - C::on_package_download_error_store( - extract_ctx, - task.task_id, - extract.name.slice(), - &extract.resolution, - err, - &task.url_buf, - ); - } else { - let package_id = manager.lockfile.buffers.resolutions - [extract.dependency_id as usize]; - C::on_package_download_error_pkg( - extract_ctx, - package_id, - extract.name.slice(), - &extract.resolution, - err, - &task.url_buf, - ); - } + let Some(is_required) = + dispatch_tarball_error::(manager, extract_ctx, task, extract, err) + else { continue; - } + }; if is_required { bun_ast::add_error_pretty!( @@ -763,16 +715,7 @@ pub fn run_tasks( .fmt(&manager.lockfile.buffers.string_bytes, PathSep::Auto,), ); } - if manager.subcommand != Subcommand::Remove { - for request in manager.update_requests.iter_mut() { - if strings::eql(request.name, extract.name.slice()) { - request.failed = true; - manager.options.do_.remove(Do::SAVE_LOCKFILE); - manager.options.do_.remove(Do::SAVE_YARN_LOCK); - manager.options.do_.remove(Do::INSTALL_PACKAGES); - } - } - } + mark_update_request_failed(manager, extract.name.slice()); if let Some(removed) = manager.task_queue.remove(&task.task_id) { drop(removed); @@ -784,48 +727,21 @@ pub fn run_tasks( let response = &metadata.response; if response.status_code > 399 { - // Non-retryable HTTP error: mark the dedupe entry as failed - // so a later enqueue for this task_id fails fast instead of - // waiting on this failed one or re-downloading it. Runs - // before the callback branch so `Store.Installer` (which - // `continue`s from the callback) is covered too. - let is_required = manager.is_network_task_required(task.task_id); - manager.mark_network_task_failed(task.task_id); - - if C::HAS_ON_PACKAGE_DOWNLOAD_ERROR { - let err = match response.status_code { - 400 => crate::Error::TarballHTTP400, - 401 => crate::Error::TarballHTTP401, - 402 => crate::Error::TarballHTTP402, - 403 => crate::Error::TarballHTTP403, - 404 => crate::Error::TarballHTTP404, - 405..=499 => crate::Error::TarballHTTP4xx, - _ => crate::Error::TarballHTTP5xx, - }; + let err = match response.status_code { + 400 => crate::Error::TarballHTTP400, + 401 => crate::Error::TarballHTTP401, + 402 => crate::Error::TarballHTTP402, + 403 => crate::Error::TarballHTTP403, + 404 => crate::Error::TarballHTTP404, + 405..=499 => crate::Error::TarballHTTP4xx, + _ => crate::Error::TarballHTTP5xx, + }; - if C::IS_STORE_INSTALLER { - C::on_package_download_error_store( - extract_ctx, - task.task_id, - extract.name.slice(), - &extract.resolution, - err, - &task.url_buf, - ); - } else { - let package_id = manager.lockfile.buffers.resolutions - [extract.dependency_id as usize]; - C::on_package_download_error_pkg( - extract_ctx, - package_id, - extract.name.slice(), - &extract.resolution, - err, - &task.url_buf, - ); - } + let Some(is_required) = + dispatch_tarball_error::(manager, extract_ctx, task, extract, err) + else { continue; - } + }; if is_required { bun_ast::add_error_pretty!( @@ -846,16 +762,7 @@ pub fn run_tasks( response.status_code, ); } - if manager.subcommand != Subcommand::Remove { - for request in manager.update_requests.iter_mut() { - if strings::eql(request.name, extract.name.slice()) { - request.failed = true; - manager.options.do_.remove(Do::SAVE_LOCKFILE); - manager.options.do_.remove(Do::SAVE_YARN_LOCK); - manager.options.do_.remove(Do::INSTALL_PACKAGES); - } - } - } + mark_update_request_failed(manager, extract.name.slice()); if let Some(removed) = manager.task_queue.remove(&task.task_id) { drop(removed); @@ -1552,6 +1459,67 @@ pub fn run_tasks( Ok(()) } +/// Non-retryable tarball download failure. The download will not be retried +/// for this task_id, so the dedupe entry is marked failed before the error is +/// dispatched: a later `enqueue_package_for_download` for the same package +/// then fails fast instead of waiting forever on a callback that never +/// arrives (entry kept) or re-running the whole download (entry removed). +/// The store installer's `on_package_download_error` drains `task_queue` +/// itself but does not touch `network_dedupe_map`, so the mark runs on the +/// callback path too. Returns `None` when the `on_package_download_error_*` +/// callback consumed the error, otherwise `Some(is_required)` for the +/// caller's log-and-mark fallback. +fn dispatch_tarball_error( + manager: &mut PackageManager, + extract_ctx: &mut C::Ctx, + task: &NetworkTask, + extract: &ExtractTarball, + err: crate::Error, +) -> Option { + let is_required = manager.is_network_task_required(task.task_id); + manager.mark_network_task_failed(task.task_id); + + if C::HAS_ON_PACKAGE_DOWNLOAD_ERROR { + if C::IS_STORE_INSTALLER { + C::on_package_download_error_store( + extract_ctx, + task.task_id, + extract.name.slice(), + &extract.resolution, + err, + &task.url_buf, + ); + } else { + let package_id = manager.lockfile.buffers.resolutions[extract.dependency_id as usize]; + C::on_package_download_error_pkg( + extract_ctx, + package_id, + extract.name.slice(), + &extract.resolution, + err, + &task.url_buf, + ); + } + return None; + } + + Some(is_required) +} + +fn mark_update_request_failed(manager: &mut PackageManager, name: &[u8]) { + if manager.subcommand == Subcommand::Remove { + return; + } + for request in manager.update_requests.iter_mut() { + if strings::eql(request.name, name) { + request.failed = true; + manager.options.do_.remove(Do::SAVE_LOCKFILE); + manager.options.do_.remove(Do::SAVE_YARN_LOCK); + manager.options.do_.remove(Do::INSTALL_PACKAGES); + } + } +} + #[inline] pub fn pending_task_count(manager: &PackageManager) -> u32 { manager.pending_tasks.load(Ordering::Acquire) diff --git a/src/install/PackageManager/updatePackageJSONAndInstall.rs b/src/install/PackageManager/updatePackageJSONAndInstall.rs index e9979f5cdf4c..8cf10cddc858 100644 --- a/src/install/PackageManager/updatePackageJSONAndInstall.rs +++ b/src/install/PackageManager/updatePackageJSONAndInstall.rs @@ -1,10 +1,9 @@ use crate::lockfile::package::PackageColumns as _; +use bstr::BStr; use bun_collections::VecExt; use core::fmt; use std::borrow::Cow; -use bstr::BStr; - use crate::Error; use crate::ShellCompletions; use crate::bun_fs::FileSystem; @@ -738,25 +737,22 @@ fn update_package_json_and_install_with_manager_with_updates( let (source, path): (&[u8], &ZStr) = if matches!(manager.options.patch_features, PatchFeatures::Commit { .. }) { 'source_and_path: { - let root_package_json_entry = match manager - .workspace_package_json_cache - .get_with_path( + let root_package_json_entry = + match manager.workspace_package_json_cache.get_with_path( manager.log_mut(), root_package_json_path.as_bytes(), GetJSONOptions::default(), - ) - .unwrap() - { - Ok(e) => e, - Err(err) => { - Output::err( - err, - "failed to read/parse package.json at '{s}'", - (BStr::new(root_package_json_path.as_bytes()),), - ); - Global::exit(1); - } - }; + ) { + GetResult::Entry(entry) => entry, + GetResult::ReadErr(err) | GetResult::ParseErr(err) => { + Output::err( + err, + "failed to read/parse package.json at '{s}'", + (BStr::new(root_package_json_path.as_bytes()),), + ); + Global::exit(1); + } + }; break 'source_and_path ( &root_package_json_entry.source.contents, diff --git a/src/install/hosted_git_info.rs b/src/install/hosted_git_info.rs index 62937c9302c6..d582666e433e 100644 --- a/src/install/hosted_git_info.rs +++ b/src/install/hosted_git_info.rs @@ -1040,52 +1040,17 @@ pub(crate) mod formatters { pub(crate) type Type = fn(url: &JscUrl) -> Result, HostedGitInfoError>; - pub(crate) fn github(url: &JscUrl) -> Result, HostedGitInfoError> { - let pathname_owned = url.pathname().to_owned_slice(); - let pathname = strings::trim_prefix(&pathname_owned, b"/"); - - let mut iter = strings::split(pathname, b"/"); - let Some(user_part) = iter.next() else { - return Ok(None); - }; - let Some(project_part) = iter.next() else { - return Ok(None); - }; - let type_part = iter.next(); - let committish_part = iter.next(); - - let project = strings::trim_suffix(project_part, b".git"); - - if user_part.is_empty() || project.is_empty() { - return Ok(None); - } - - // If the type part says something other than "tree", we're not looking at a - // github URL that we understand. - if let Some(tp) = type_part { - if tp != b"tree" { - return Ok(None); - } - } - - // Hold the owned fragment here so the `committish` borrow stays - // valid until it's copied into the StringBuilder. - let fragment_utf8; - let committish: Option<&[u8]> = if type_part.is_none() { - let fragment_str = OwnedString::new(url.fragment_identifier()); - fragment_utf8 = fragment_str.to_utf8(); - let fragment = fragment_utf8.slice(); - if !fragment.is_empty() { - Some(fragment) - } else { - None - } - } else { - committish_part - }; - + /// Percent-decode the parts into a single owned buffer and build the + /// `ExtractResult` over it. + fn build_result( + user: Option<&[u8]>, + project: &[u8], + committish: Option<&[u8]>, + ) -> Result { let mut sb = StringBuilder::default(); - sb.count(user_part); + if let Some(u) = user { + sb.count(u); + } sb.count(project); if let Some(c) = committish { sb.count(c); @@ -1093,45 +1058,69 @@ pub(crate) mod formatters { sb.allocate()?; - let user_slice = HostedGitInfo::decode_and_append(&mut sb, user_part)?; + let user_slice = match user { + Some(u) => Some(HostedGitInfo::decode_and_append(&mut sb, u)?), + None => None, + }; let project_slice = HostedGitInfo::decode_and_append(&mut sb, project)?; let committish_slice = match committish { Some(c) => Some(HostedGitInfo::decode_and_append(&mut sb, c)?), None => None, }; - Ok(Some(ExtractResult { - user: Some(user_slice), + Ok(ExtractResult { + user: user_slice, project: project_slice, committish: committish_slice, _owned_buffer: Some(sb.move_to_slice()), - })) + }) } - pub(crate) fn bitbucket(url: &JscUrl) -> Result, HostedGitInfoError> { + /// Shared tail for hosts whose URL shape is `/user/project[.git][/aux]` + /// with the committish in the fragment: reject `aux == reject_aux`, + /// trim `.git`, require a non-empty project (and user, unless + /// `user_optional`, in which case a lone segment is the project), then + /// build the result. With `error_as_none`, allocation or decode + /// failure maps to "not a hosted git URL" rather than an error. + fn user_project_committish( + url: &JscUrl, + reject_aux: &[u8], + user_optional: bool, + error_as_none: bool, + ) -> Result, HostedGitInfoError> { let pathname_owned = url.pathname().to_owned_slice(); let pathname = strings::trim_prefix(&pathname_owned, b"/"); let mut iter = strings::split(pathname, b"/"); - let Some(user_part) = iter.next() else { + let Some(mut user_part) = iter.next() else { return Ok(None); }; - let Some(project_part) = iter.next() else { + let mut project_part = iter.next(); + + if iter.next() == Some(reject_aux) { return Ok(None); - }; - let aux = iter.next(); + } - if let Some(a) = aux { - if a == b"get" { - return Ok(None); - } + if user_optional && project_part.is_none_or(<[u8]>::is_empty) { + project_part = Some(user_part); + user_part = b""; } + let Some(project_part) = project_part else { + return Ok(None); + }; let project = strings::trim_suffix(project_part, b".git"); - - if user_part.is_empty() || project.is_empty() { + if project.is_empty() { return Ok(None); } + let user: Option<&[u8]> = if user_part.is_empty() { + if !user_optional { + return Ok(None); + } + None + } else { + Some(user_part) + }; let fragment_str = OwnedString::new(url.fragment_identifier()); let fragment_utf8 = fragment_str.to_utf8(); @@ -1142,28 +1131,64 @@ pub(crate) mod formatters { None }; - let mut sb = StringBuilder::default(); - sb.count(user_part); - sb.count(project); - if let Some(c) = committish { - sb.count(c); + match build_result(user, project, committish) { + Ok(result) => Ok(Some(result)), + Err(_) if error_as_none => Ok(None), + Err(err) => Err(err), } + } - sb.allocate()?; + pub(crate) fn github(url: &JscUrl) -> Result, HostedGitInfoError> { + let pathname_owned = url.pathname().to_owned_slice(); + let pathname = strings::trim_prefix(&pathname_owned, b"/"); - let user_slice = HostedGitInfo::decode_and_append(&mut sb, user_part)?; - let project_slice = HostedGitInfo::decode_and_append(&mut sb, project)?; - let committish_slice = match committish { - Some(c) => Some(HostedGitInfo::decode_and_append(&mut sb, c)?), - None => None, + let mut iter = strings::split(pathname, b"/"); + let Some(user_part) = iter.next() else { + return Ok(None); + }; + let Some(project_part) = iter.next() else { + return Ok(None); }; + let type_part = iter.next(); + let committish_part = iter.next(); - Ok(Some(ExtractResult { - user: Some(user_slice), - project: project_slice, - committish: committish_slice, - _owned_buffer: Some(sb.move_to_slice()), - })) + let project = strings::trim_suffix(project_part, b".git"); + + if user_part.is_empty() || project.is_empty() { + return Ok(None); + } + + // If the type part says something other than "tree", we're not looking at a + // github URL that we understand. + if let Some(tp) = type_part { + if tp != b"tree" { + return Ok(None); + } + } + + // Hold the owned fragment here so the `committish` borrow stays + // valid until it's copied into the StringBuilder. + let fragment_utf8; + let committish: Option<&[u8]> = if type_part.is_none() { + let fragment_str = OwnedString::new(url.fragment_identifier()); + fragment_utf8 = fragment_str.to_utf8(); + let fragment = fragment_utf8.slice(); + if !fragment.is_empty() { + Some(fragment) + } else { + None + } + } else { + committish_part + }; + + Ok(Some(build_result(Some(user_part), project, committish)?)) + } + + pub(crate) fn bitbucket(url: &JscUrl) -> Result, HostedGitInfoError> { + user_project_committish( + url, b"get", /* user_optional */ false, /* error_as_none */ false, + ) } pub(crate) fn gitlab(url: &JscUrl) -> Result, HostedGitInfoError> { @@ -1221,176 +1246,15 @@ pub(crate) mod formatters { } pub(crate) fn gist(url: &JscUrl) -> Result, HostedGitInfoError> { - let pathname_owned = url.pathname().to_owned_slice(); - let pathname = strings::trim_prefix(&pathname_owned, b"/"); - - let mut iter = strings::split(pathname, b"/"); - let Some(mut user_part) = iter.next() else { - return Ok(None); - }; - let mut project_part = iter.next(); - let aux = iter.next(); - - if let Some(a) = aux { - if a == b"raw" { - return Ok(None); - } - } - - if project_part.is_none() || project_part.unwrap().is_empty() { - project_part = Some(user_part); - user_part = b""; - } - - let project = strings::trim_suffix(project_part.unwrap(), b".git"); - let user: Option<&[u8]> = if !user_part.is_empty() { - Some(user_part) - } else { - None - }; - - if project.is_empty() { - return Ok(None); - } - - let fragment_str = OwnedString::new(url.fragment_identifier()); - let fragment_utf8 = fragment_str.to_utf8(); - let fragment = fragment_utf8.slice(); - let committish: Option<&[u8]> = if !fragment.is_empty() { - Some(fragment) - } else { - None - }; - - let mut sb = StringBuilder::default(); - if let Some(u) = user { - sb.count(u); - } - sb.count(project); - if let Some(c) = committish { - sb.count(c); - } - - let Ok(()) = sb.allocate() else { - return Ok(None); - }; - - let user_slice = match user { - Some(u) => { - let Ok(r) = HostedGitInfo::decode_and_append(&mut sb, u) else { - return Ok(None); - }; - Some(r) - } - None => None, - }; - let Ok(project_slice) = HostedGitInfo::decode_and_append(&mut sb, project) else { - return Ok(None); - }; - let committish_slice = match committish { - Some(c) => { - let Ok(r) = HostedGitInfo::decode_and_append(&mut sb, c) else { - return Ok(None); - }; - Some(r) - } - None => None, - }; - - Ok(Some(ExtractResult { - user: user_slice, - project: project_slice, - committish: committish_slice, - _owned_buffer: Some(sb.move_to_slice()), - })) + user_project_committish( + url, b"raw", /* user_optional */ true, /* error_as_none */ true, + ) } pub(crate) fn sourcehut(url: &JscUrl) -> Result, HostedGitInfoError> { - let pathname_owned = url.pathname().to_owned_slice(); - let pathname = strings::trim_prefix(&pathname_owned, b"/"); - - let mut iter = strings::split(pathname, b"/"); - let Some(user_part) = iter.next() else { - return Ok(None); - }; - let Some(project_part) = iter.next() else { - return Ok(None); - }; - let aux = iter.next(); - - if let Some(a) = aux { - if a == b"archive" { - return Ok(None); - } - } - - let project = strings::trim_suffix(project_part, b".git"); - - if user_part.is_empty() || project.is_empty() { - return Ok(None); - } - - let fragment_str = OwnedString::new(url.fragment_identifier()); - let fragment_utf8 = fragment_str.to_utf8(); - let fragment = fragment_utf8.slice(); - let committish: Option<&[u8]> = if !fragment.is_empty() { - Some(fragment) - } else { - None - }; - - let mut sb = StringBuilder::default(); - sb.count(user_part); - sb.count(project); - if let Some(c) = committish { - sb.count(c); - } - - let Ok(()) = sb.allocate() else { - return Ok(None); - }; - - // Inline percent-decode rather than `decode_and_append`: this path - // returns None instead of erroring on decode failure. - let user_slice = 'blk: { - let start = sb.len; - let writable = sb.writable(); - let Ok(decoded_len) = PercentEncoding::decode_into(writable, user_part) else { - return Ok(None); - }; - let decoded_len = decoded_len as usize; - sb.len += decoded_len; - break 'blk start..start + decoded_len; - }; - let project_slice = 'blk: { - let start = sb.len; - let writable = sb.writable(); - let Ok(decoded_len) = PercentEncoding::decode_into(writable, project) else { - return Ok(None); - }; - let decoded_len = decoded_len as usize; - sb.len += decoded_len; - break 'blk start..start + decoded_len; - }; - let committish_slice = if let Some(c) = committish { - let start = sb.len; - let writable = sb.writable(); - let Ok(decoded_len) = PercentEncoding::decode_into(writable, c) else { - return Ok(None); - }; - let decoded_len = decoded_len as usize; - sb.len += decoded_len; - Some(start..start + decoded_len) - } else { - None - }; - - Ok(Some(ExtractResult { - user: Some(user_slice), - project: project_slice, - committish: committish_slice, - _owned_buffer: Some(sb.move_to_slice()), - })) + user_project_committish( + url, b"archive", /* user_optional */ false, /* error_as_none */ true, + ) } } } diff --git a/src/install/isolated_install.rs b/src/install/isolated_install.rs index 35e5ea78bcfc..ddef6671076a 100644 --- a/src/install/isolated_install.rs +++ b/src/install/isolated_install.rs @@ -2414,6 +2414,29 @@ pub(crate) fn install_isolated_packages( let dep = &lockfile_ro.buffers.dependencies[dep_id as usize]; + // Shared failure path for the enqueue-for-download arms + // below; the caller `continue`s after invoking it. + let fail_enqueue = |installer: &mut store::Installer, err, what| { + Output::err( + err, + "failed to enqueue {} for download: {}@{}", + ( + what, + BStr::new(pkg_name.slice(string_buf)), + pkg_res.fmt(string_buf, bun_fmt::PathSep::Auto), + ), + ); + Output::flush(); + if installer.manager().options.enable.fail_early() { + Global::exit(1); + } + // .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); + }; + match pkg_res_tag { ResolutionTag::Npm => { match installer.manager_mut().enqueue_package_for_download( @@ -2440,24 +2463,7 @@ pub(crate) fn install_isolated_packages( } Err(err) => { // error.InvalidURL - Output::err( - err, - "failed to enqueue package for download: {}@{}", - ( - BStr::new(pkg_name.slice(string_buf)), - pkg_res.fmt(string_buf, bun_fmt::PathSep::Auto), - ), - ); - Output::flush(); - if installer.manager().options.enable.fail_early() { - Global::exit(1); - } - // .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); + fail_enqueue(&mut installer, err, "package"); continue; } } @@ -2498,24 +2504,7 @@ pub(crate) fn install_isolated_packages( continue; } Err(err) => { - Output::err( - err, - "failed to enqueue github package for download: {}@{}", - ( - BStr::new(pkg_name.slice(string_buf)), - pkg_res.fmt(string_buf, bun_fmt::PathSep::Auto), - ), - ); - Output::flush(); - if installer.manager().options.enable.fail_early() { - Global::exit(1); - } - // .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); + fail_enqueue(&mut installer, err, "github package"); continue; } } @@ -2551,24 +2540,7 @@ pub(crate) fn install_isolated_packages( continue; } Err(err) => { - Output::err( - err, - "failed to enqueue tarball for download: {}@{}", - ( - BStr::new(pkg_name.slice(string_buf)), - pkg_res.fmt(string_buf, bun_fmt::PathSep::Auto), - ), - ); - Output::flush(); - if installer.manager().options.enable.fail_early() { - Global::exit(1); - } - // .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); + fail_enqueue(&mut installer, err, "tarball"); continue; } } diff --git a/src/install/npm.rs b/src/install/npm.rs index 7c1b77b4a08d..4a142465fe55 100644 --- a/src/install/npm.rs +++ b/src/install/npm.rs @@ -667,6 +667,38 @@ pub(crate) fn negatable_from_json_value(value: &JSON::E::JsonV this.combine() } +/// Resets and refills `bundled_deps_set` / `bundle_all_deps` from a version's +/// `bundleDependencies` (or legacy `bundledDependencies`) field. +fn extract_bundled_deps( + version_obj: Option<&JSON::E::ObjectJSON>, + bundled_deps_set: &mut StringSet, + bundle_all_deps: &mut bool, +) -> Result<(), AllocError> { + bundled_deps_set.map.clear_retaining_capacity(); + *bundle_all_deps = false; + let Some(bundled_deps_value) = version_obj + .and_then(|o| o.get(b"bundleDependencies")) + .or_else(|| version_obj.and_then(|o| o.get(b"bundledDependencies"))) + else { + return Ok(()); + }; + match bundled_deps_value { + JSON::E::JsonValue::Boolean(boolean) => { + *bundle_all_deps = *boolean; + } + JSON::E::JsonValue::Array(arr) => { + for bundled_dep in arr.get().items() { + let Some(s) = bundled_dep.as_str() else { + continue; + }; + bundled_deps_set.insert(s)?; + } + } + _ => {} + } + Ok(()) +} + // ────────────────────────────────────────────────────────────────────────── #[repr(C)] @@ -2016,7 +2048,7 @@ impl PackageManifest { let mut optional_peer_dep_names: Vec = Vec::new(); let mut bundled_deps_set = StringSet::init(); - let mut bundle_all_deps: bool; + let mut bundle_all_deps = false; let mut bundled_deps_count: usize = 0; @@ -2150,27 +2182,7 @@ impl PackageManifest { } } - bundled_deps_set.map.clear_retaining_capacity(); - bundle_all_deps = false; - if let Some(bundled_deps_value) = version_obj - .and_then(|o| o.get(b"bundleDependencies")) - .or_else(|| version_obj.and_then(|o| o.get(b"bundledDependencies"))) - { - match bundled_deps_value { - JSON::E::JsonValue::Boolean(boolean) => { - bundle_all_deps = *boolean; - } - JSON::E::JsonValue::Array(arr) => { - for bundled_dep in arr.get().items() { - let Some(s) = bundled_dep.as_str() else { - continue; - }; - bundled_deps_set.insert(s)?; - } - } - _ => {} - } - } + extract_bundled_deps(version_obj, &mut bundled_deps_set, &mut bundle_all_deps)?; for pair in &DEPENDENCY_GROUPS { if let Some(obj) = version_obj @@ -2374,27 +2386,7 @@ impl PackageManifest { let version_obj = prop.value.as_object(); - bundled_deps_set.map.clear_retaining_capacity(); - bundle_all_deps = false; - if let Some(bundled_deps_value) = version_obj - .and_then(|o| o.get(b"bundleDependencies")) - .or_else(|| version_obj.and_then(|o| o.get(b"bundledDependencies"))) - { - match bundled_deps_value { - JSON::E::JsonValue::Boolean(boolean) => { - bundle_all_deps = *boolean; - } - JSON::E::JsonValue::Array(arr) => { - for bundled_dep in arr.get().items() { - let Some(s) = bundled_dep.as_str() else { - continue; - }; - bundled_deps_set.insert(s)?; - } - } - _ => {} - } - } + extract_bundled_deps(version_obj, &mut bundled_deps_set, &mut bundle_all_deps)?; let mut package_version: PackageVersion = empty_version; diff --git a/src/install/yarn.rs b/src/install/yarn.rs index 1fcfd3ce50ea..6c0f8e6d97c9 100644 --- a/src/install/yarn.rs +++ b/src/install/yarn.rs @@ -1723,169 +1723,24 @@ pub(crate) fn migrate_yarn_lockfile<'a>( let deps_off = u32::try_from(this.buffers.dependencies.len()).expect("int cast"); let resolutions_off = u32::try_from(this.buffers.resolutions.len()).expect("int cast"); - if let Some(deps) = &entry.dependencies { - for (dep_name_key, dep_version_ref) in deps.iter() { - let dep_name: &[u8] = dep_name_key.as_ref(); - let dep_version_literal: &[u8] = *dep_version_ref; - - let name_hash = string_hash(dep_name); - let dep_name_string = sbuf!().append_with_hash(dep_name, name_hash)?; - let dep_version_string = sbuf!().append(dep_version_literal)?; - let sliced_string = SlicedString::init( - dep_version_string.slice(this.buffers.string_bytes.as_slice()), - dep_version_string.slice(this.buffers.string_bytes.as_slice()), - ); - - let mut parsed_version = Dependency::parse( - dep_name_string, - Some(name_hash), - dep_version_string.slice(this.buffers.string_bytes.as_slice()), - &sliced_string, - Some(&mut *log), - Some(&mut *manager), - ) - .unwrap_or_default(); - - parsed_version.literal = dep_version_string; - - this.buffers.dependencies.push(Dependency { - name: dep_name_string, - name_hash, - version: parsed_version, - behavior: dependency::Behavior::PROD, - }); - - let mut dep_spec = Vec::new(); - write!( - &mut dep_spec, - "{}@{}", - bstr::BStr::new(dep_name), - bstr::BStr::new(dep_version_literal) - ) - .expect("unreachable"); - - if let Some(res_pkg_id) = spec_to_package_id.get(dep_spec.as_slice()).copied() { - this.buffers.resolutions.push(res_pkg_id); - } else { - this.buffers.resolutions.push(install::INVALID_PACKAGE_ID); - } - - dep_count += 1; - } - } - - if let Some(optional_deps) = &entry.optional_dependencies { - for (dep_name_key, dep_version_ref) in optional_deps.iter() { - let dep_name: &[u8] = dep_name_key.as_ref(); - let dep_version_literal: &[u8] = *dep_version_ref; - - let name_hash = string_hash(dep_name); - let dep_name_string = sbuf!().append_with_hash(dep_name, name_hash)?; - - let dep_version_string = sbuf!().append(dep_version_literal)?; - let sliced_string = SlicedString::init( - dep_version_string.slice(this.buffers.string_bytes.as_slice()), - dep_version_string.slice(this.buffers.string_bytes.as_slice()), - ); - - let mut parsed_version = Dependency::parse( - dep_name_string, - Some(name_hash), - dep_version_string.slice(this.buffers.string_bytes.as_slice()), - &sliced_string, - Some(&mut *log), - Some(&mut *manager), - ) - .unwrap_or_default(); - - parsed_version.literal = dep_version_string; - - this.buffers.dependencies.push(Dependency { - name: dep_name_string, - name_hash, - version: parsed_version, - behavior: dependency::Behavior::OPTIONAL, - }); - - let mut dep_spec = Vec::new(); - write!( - &mut dep_spec, - "{}@{}", - bstr::BStr::new(dep_name), - bstr::BStr::new(dep_version_literal) - ) - .expect("unreachable"); - - if let Some(res_pkg_id) = spec_to_package_id.get(dep_spec.as_slice()).copied() { - this.buffers.resolutions.push(res_pkg_id); - } else { - this.buffers.resolutions.push(install::INVALID_PACKAGE_ID); - } - - dep_count += 1; - } - } - - if let Some(peer_deps) = &entry.peer_dependencies { - for (dep_name_key, dep_version_ref) in peer_deps.iter() { - let dep_name: &[u8] = dep_name_key.as_ref(); - let dep_version_literal: &[u8] = *dep_version_ref; - - let name_hash = string_hash(dep_name); - let dep_name_string = sbuf!().append_with_hash(dep_name, name_hash)?; - - let dep_version_string = sbuf!().append(dep_version_literal)?; - let sliced_string = SlicedString::init( - dep_version_string.slice(this.buffers.string_bytes.as_slice()), - dep_version_string.slice(this.buffers.string_bytes.as_slice()), - ); - - let mut parsed_version = Dependency::parse( - dep_name_string, - Some(name_hash), - dep_version_string.slice(this.buffers.string_bytes.as_slice()), - &sliced_string, - Some(&mut *log), - Some(&mut *manager), - ) - .unwrap_or_default(); - - parsed_version.literal = dep_version_string; - - this.buffers.dependencies.push(Dependency { - name: dep_name_string, - name_hash, - version: parsed_version, - behavior: dependency::Behavior::PEER, - }); - - let mut dep_spec = Vec::new(); - write!( - &mut dep_spec, - "{}@{}", - bstr::BStr::new(dep_name), - bstr::BStr::new(dep_version_literal) - ) - .expect("unreachable"); - - if let Some(res_pkg_id) = spec_to_package_id.get(dep_spec.as_slice()).copied() { - this.buffers.resolutions.push(res_pkg_id); - } else { - this.buffers.resolutions.push(install::INVALID_PACKAGE_ID); - } - - dep_count += 1; - } - } + let dep_groups = [ + (entry.dependencies.as_ref(), dependency::Behavior::PROD), + ( + entry.optional_dependencies.as_ref(), + dependency::Behavior::OPTIONAL, + ), + (entry.peer_dependencies.as_ref(), dependency::Behavior::PEER), + (entry.dev_dependencies.as_ref(), dependency::Behavior::DEV), + ]; - if let Some(dev_deps) = &entry.dev_dependencies { - for (dep_name_key, dep_version_ref) in dev_deps.iter() { + for (deps, behavior) in dep_groups { + let Some(deps) = deps else { continue }; + for (dep_name_key, dep_version_ref) in deps.iter() { let dep_name: &[u8] = dep_name_key.as_ref(); let dep_version_literal: &[u8] = *dep_version_ref; let name_hash = string_hash(dep_name); let dep_name_string = sbuf!().append_with_hash(dep_name, name_hash)?; - let dep_version_string = sbuf!().append(dep_version_literal)?; let sliced_string = SlicedString::init( dep_version_string.slice(this.buffers.string_bytes.as_slice()), @@ -1908,7 +1763,7 @@ pub(crate) fn migrate_yarn_lockfile<'a>( name: dep_name_string, name_hash, version: parsed_version, - behavior: dependency::Behavior::DEV, + behavior, }); let mut dep_spec = Vec::new(); diff --git a/src/runtime/cli/bunx_command.rs b/src/runtime/cli/bunx_command.rs index 15e2b2287af6..8d99d3363493 100644 --- a/src/runtime/cli/bunx_command.rs +++ b/src/runtime/cli/bunx_command.rs @@ -213,6 +213,19 @@ pub(crate) enum GetBinNameError { NeedToInstall, } +/// Inputs shared by both post-install cached-bin probes; invariant across the +/// initial-bin-name and package-json-bin-name attempts. +struct CachedBinProbe<'a> { + bunx_cache_dir: &'a [u8], + ignore_cwd: &'a [u8], + top_level_dir: &'a [u8], + #[cfg(unix)] + uid: libc::uid_t, + #[cfg(not(unix))] + uid: u32, + dirname_store: &'static bun_resolver::fs::DirnameStore, +} + impl BunxCommand { /// Adds `create-` to the string, but also handles scoped packages correctly. /// Always clones the string in the process. @@ -663,6 +676,77 @@ impl BunxCommand { true } + /// Post-install cache probe: builds + /// `/node_modules/.bin/`, resolves it + /// via `bun_which::which`, and execs it via `Run::run_binary` (noreturn) + /// once it passes the `is_trusted_cached_binary` TOCTOU check. Returns + /// `Ok(())` only on a miss: the binary is absent or untrusted. + fn try_run_cached_bin( + ctx: &mut ContextData, + path_buf: &mut PathBuffer, + absolute_in_cache_dir_buf: &mut PathBuffer, + probe: &CachedBinProbe, + bin_name: &[u8], + env_loader: &mut bun_dotenv::Loader, + passthrough: &[Box<[u8]>], + ) -> crate::Result<()> { + let buf_total = absolute_in_cache_dir_buf.len(); + let absolute_in_cache_dir: &[u8] = { + let mut cursor: &mut [u8] = &mut absolute_in_cache_dir_buf[..]; + write!( + cursor, + "{cache}{sep}node_modules{sep}.bin{sep}{bin}{exe}", + cache = BStr::new(probe.bunx_cache_dir), + sep = bun_paths::SEP as char, + bin = BStr::new(bin_name), + exe = EXE_SUFFIX, + ) + .expect("unreachable"); + let written = buf_total - cursor.len(); + // SAFETY: `written` bytes initialized above + unsafe { core::slice::from_raw_parts(absolute_in_cache_dir_buf.as_ptr(), written) } + }; + + // Similar to "npx": try the bin in the global cache. Do not try $PATH + // because we already checked it above if we should. + if let Some(destination) = bun_which::which( + path_buf, + probe.bunx_cache_dir, + if !probe.ignore_cwd.is_empty() { + b"".as_slice() + } else { + probe.top_level_dir + }, + absolute_in_cache_dir, + ) { + let out: &[u8] = destination.as_bytes(); + // The install we just ran should have created this symlink as the + // current user, but the cache lives in a world-writable temp dir; an + // attacker can race the install and plant a uid-mismatched entry. + // Bail out to the generic error rather than execute it. + if Self::is_trusted_cached_binary(destination, probe.uid) { + let stored = probe.dirname_store.append_slice(out)?; + Run::run_binary( + ctx, + stored, + destination, + probe.top_level_dir, + env_loader, + passthrough, + None, + )?; + // run_binary is noreturn + } else { + bun_output::scoped_log!( + bunx, + "refusing untrusted cached binary: {}", + BStr::new(out) + ); + } + } + Ok(()) + } + fn exit_with_usage() -> ! { crate::cli::command::tag_print_help(Command::Tag::BunxCommand, false); Global::exit(1); @@ -1450,61 +1534,23 @@ impl BunxCommand { _ => {} } - absolute_in_cache_dir = { - let mut cursor: &mut [u8] = &mut absolute_in_cache_dir_buf[..]; - write!( - cursor, - "{cache}{sep}node_modules{sep}.bin{sep}{bin}{exe}", - cache = BStr::new(bunx_cache_dir), - sep = bun_paths::SEP as char, - bin = BStr::new(initial_bin_name), - exe = EXE_SUFFIX, - ) - .expect("unreachable"); - let written = buf_total - cursor.len(); - // SAFETY: `written` bytes initialized above - unsafe { core::slice::from_raw_parts(absolute_in_cache_dir_buf.as_ptr(), written) } + let cached_bin_probe = CachedBinProbe { + bunx_cache_dir, + ignore_cwd: &ignore_cwd, + top_level_dir, + uid, + dirname_store: fs.dirname_store, }; - // Similar to "npx": - // - // 1. Try the bin in the global cache - // Do not try $PATH because we already checked it above if we should - if let Some(destination) = bun_which::which( + Self::try_run_cached_bin( + ctx, &mut path_buf, - bunx_cache_dir, - if !ignore_cwd.is_empty() { - b"".as_slice() - } else { - top_level_dir - }, - absolute_in_cache_dir, - ) { - let out: &[u8] = destination.as_bytes(); - // The install we just ran should have created this symlink as the - // current user, but the cache lives in a world-writable temp dir; an - // attacker can race the install and plant a uid-mismatched entry. - // Bail out to the generic error rather than execute it. - if Self::is_trusted_cached_binary(destination, uid) { - let stored = fs.dirname_store.append_slice(out)?; - Run::run_binary( - ctx, - stored, - destination, - top_level_dir, - env_loader, - passthrough, - None, - )?; - // run_binary is noreturn - } else { - bun_output::scoped_log!( - bunx, - "refusing untrusted cached binary: {}", - BStr::new(out) - ); - } - } + &mut absolute_in_cache_dir_buf, + &cached_bin_probe, + initial_bin_name, + env_loader, + passthrough, + )?; // 2. The "bin" is possibly not the same as the package name, so we load the package.json to figure out what "bin" to use // BUT: Skip this if --package was used, as the user explicitly specified the binary name @@ -1516,55 +1562,15 @@ impl BunxCommand { false, ) { if !strings::eql_long(&package_name_for_bin, initial_bin_name, true) { - absolute_in_cache_dir = { - let mut cursor: &mut [u8] = &mut absolute_in_cache_dir_buf[..]; - write!( - cursor, - "{}/node_modules/.bin/{}{}", - BStr::new(bunx_cache_dir), - BStr::new(&package_name_for_bin), - EXE_SUFFIX, - ) - .expect("unreachable"); - let written = buf_total - cursor.len(); - // SAFETY: `written` bytes initialized above - unsafe { - core::slice::from_raw_parts(absolute_in_cache_dir_buf.as_ptr(), written) - } - }; - - if let Some(destination) = bun_which::which( + Self::try_run_cached_bin( + ctx, &mut path_buf, - bunx_cache_dir, - if !ignore_cwd.is_empty() { - b"".as_slice() - } else { - top_level_dir - }, - absolute_in_cache_dir, - ) { - let out: &[u8] = destination.as_bytes(); - // Same TOCTOU hardening as the post-install probe above. - if Self::is_trusted_cached_binary(destination, uid) { - let stored = fs.dirname_store.append_slice(out)?; - Run::run_binary( - ctx, - stored, - destination, - top_level_dir, - env_loader, - passthrough, - None, - )?; - // run_binary is noreturn - } else { - bun_output::scoped_log!( - bunx, - "refusing untrusted cached binary: {}", - BStr::new(out) - ); - } - } + &mut absolute_in_cache_dir_buf, + &cached_bin_probe, + &package_name_for_bin, + env_loader, + passthrough, + )?; } } } diff --git a/src/runtime/cli/create_command.rs b/src/runtime/cli/create_command.rs index b0ec6e3ab5b4..8401dd7ab8e4 100644 --- a/src/runtime/cli/create_command.rs +++ b/src/runtime/cli/create_command.rs @@ -1342,64 +1342,40 @@ impl CreateCommand { } if !bun_paths::is_absolute(positional) { - 'outer: { - if let Some(home_dir) = env_loader.map.get(b"BUN_CREATE_DIR") { - let parts = [home_dir, positional]; - let outdir_path = filesystem.abs_buf(&parts, home_dir_buf); - let len = outdir_path.len(); - home_dir_buf[len] = 0; - // SAFETY: home_dir_buf[len] == 0 written above - let outdir_path_ = bun_core::ZStr::from_buf(&home_dir_buf[..], len); - if bun_paths::resolve_path::has_any_illegal_chars(outdir_path_.as_bytes()) { - break 'outer; - } - if bun_sys::directory_exists_at(bun_sys::Fd::cwd(), outdir_path_) - .unwrap_or(false) - { - example_tag = ExampleTag::LocalFolder; - break 'brk &home_dir_buf[..len]; - } - } - } - - 'outer: { - let parts = [filesystem.top_level_dir, BUN_CREATE_DIR, positional]; - let outdir_path = filesystem.abs_buf(&parts, home_dir_buf); - let len = outdir_path.len(); - home_dir_buf[len] = 0; - // SAFETY: home_dir_buf[len] == 0 written above - let outdir_path_ = bun_core::ZStr::from_buf(&home_dir_buf[..], len); - if bun_paths::resolve_path::has_any_illegal_chars(outdir_path_.as_bytes()) { - break 'outer; + // Returns the path length if `parts` joins to an existing template directory. + let probe_template_dir = |buf: &mut PathBuffer, parts: &[&[u8]]| -> Option { + let len = filesystem.abs_buf(parts, buf).len(); + buf[len] = 0; + // SAFETY: buf[len] == 0 written above + let outdir_path = bun_core::ZStr::from_buf(&buf[..], len); + if bun_paths::resolve_path::has_any_illegal_chars(outdir_path.as_bytes()) { + return None; } - if bun_sys::directory_exists_at(bun_sys::Fd::cwd(), outdir_path_) + bun_sys::directory_exists_at(bun_sys::Fd::cwd(), outdir_path) .unwrap_or(false) - { + .then_some(len) + }; + + // Empty parts are skipped by the path join, so the two-part + // BUN_CREATE_DIR candidate pads with `b""`. + let candidates: [Option<[&[u8]; 3]>; 3] = [ + env_loader + .map + .get(b"BUN_CREATE_DIR") + .map(|home_dir| [home_dir, positional, b"".as_slice()]), + Some([filesystem.top_level_dir, BUN_CREATE_DIR, positional]), + env_loader + .map + .get(b"HOME") + .map(|home_dir| [home_dir, BUN_CREATE_DIR, positional]), + ]; + for parts in candidates.into_iter().flatten() { + if let Some(len) = probe_template_dir(&mut *home_dir_buf, &parts) { example_tag = ExampleTag::LocalFolder; break 'brk &home_dir_buf[..len]; } } - 'outer: { - if let Some(home_dir) = env_loader.map.get(b"HOME") { - let parts = [home_dir, BUN_CREATE_DIR, positional]; - let outdir_path = filesystem.abs_buf(&parts, home_dir_buf); - let len = outdir_path.len(); - home_dir_buf[len] = 0; - // SAFETY: home_dir_buf[len] == 0 written above - let outdir_path_ = bun_core::ZStr::from_buf(&home_dir_buf[..], len); - if bun_paths::resolve_path::has_any_illegal_chars(outdir_path_.as_bytes()) { - break 'outer; - } - if bun_sys::directory_exists_at(bun_sys::Fd::cwd(), outdir_path_) - .unwrap_or(false) - { - example_tag = ExampleTag::LocalFolder; - break 'brk &home_dir_buf[..len]; - } - } - } - if bun_paths::is_absolute(positional) { example_tag = ExampleTag::LocalFolder; break 'brk positional; diff --git a/src/runtime/cli/filter_run.rs b/src/runtime/cli/filter_run.rs index e506cbd3f2f5..f9e242a79d62 100644 --- a/src/runtime/cli/filter_run.rs +++ b/src/runtime/cli/filter_run.rs @@ -1,14 +1,17 @@ use core::ffi::{c_char, c_void}; use std::io::Write as _; -use std::sync::atomic::{AtomicBool, Ordering}; +use std::sync::atomic::Ordering; use std::time::Instant; #[cfg(unix)] use crate::api::bun::process::SpawnResultExt as _; -use crate::api::bun::process::{self as spawn, Process, Rusage, SpawnOptions, Status}; +use crate::api::bun::process::{self as spawn, Process, SpawnOptions, Status}; use crate::cli::Command; use crate::cli::filter_arg as FilterArg; use crate::cli::run_command::RunCommand; +use crate::cli::run_processes_shared::{ + AbortHandler, SHOULD_ABORT, aggregate_exit_code, buffered_stdio, watch_or_reap, +}; use bun_collections::StringHashMap; use bun_core::{Global, Output}; use bun_core::{ZStr, strings}; @@ -199,16 +202,7 @@ impl<'a> ProcessHandle<'a> { ) }); - match process.watch_or_reap() { - Ok(_) => {} - Err(err) => { - if !process.has_exited() { - // SAFETY: all-zero is a valid Rusage (POD C struct) - let rusage = bun_core::ffi::zeroed::(); - process.on_exit(Status::Err(err), &rusage); - } - } - } + watch_or_reap(process); Ok(()) } @@ -666,91 +660,11 @@ impl<'a> State<'a> { if self.aborted { let _ = self.redraw(true); } - for handle in self.handles.iter() { - if let Some(proc) = &handle.process { - match &proc.status { - Status::Exited(exited) => { - if exited.code != 0 { - return exited.code; - } - } - Status::Signaled(signal) => { - return bun_sys::SignalCode(*signal).to_exit_code().unwrap_or(1); - } - _ => return 1, - } - } - } - 0 - } -} - -struct AbortHandler; - -static SHOULD_ABORT: AtomicBool = AtomicBool::new(false); -// Atomic because it is set from a signal handler. - -impl AbortHandler { - #[cfg(unix)] - extern "C" fn posix_signal_handler( - sig: i32, - info: *const bun_sys::posix::siginfo_t, - _: *const c_void, - ) { - let _ = sig; - let _ = info; - SHOULD_ABORT.store(true, Ordering::SeqCst); - } - - #[cfg(windows)] - extern "system" fn windows_ctrl_handler( - dw_ctrl_type: bun_sys::windows::DWORD, - ) -> bun_sys::windows::BOOL { - if dw_ctrl_type == bun_sys::windows::CTRL_C_EVENT { - SHOULD_ABORT.store(true, Ordering::SeqCst); - return bun_sys::windows::TRUE; - } - bun_sys::windows::FALSE - } - - fn install() { - #[cfg(unix)] - { - // SAFETY: libc::sigaction is #[repr(C)] POD; all-zero is a valid value (fields overwritten below). - let mut act: libc::sigaction = bun_core::ffi::zeroed(); - act.sa_sigaction = Self::posix_signal_handler as *const () as usize; - act.sa_flags = libc::SA_SIGINFO | libc::SA_RESTART | libc::SA_RESETHAND; - // SAFETY: sa_mask is a valid out-pointer; act is on the stack. - unsafe { - libc::sigemptyset(&raw mut act.sa_mask); - libc::sigaction(libc::SIGINT, &raw const act, core::ptr::null_mut()); - } - } - #[cfg(not(unix))] - { - let res = bun_sys::c::SetConsoleCtrlHandler( - Some(Self::windows_ctrl_handler), - bun_sys::windows::TRUE, - ); - if res == 0 { - if bun_core::env::IS_DEBUG { - bun_core::warn!("Failed to set abort handler\n"); - } - } - } - } - - fn uninstall() { - // only necessary on Windows, as on posix we pass the SA_RESETHAND flag - #[cfg(windows)] - { - // (None, FALSE) clears the ignore attribute; it does NOT unregister - // a handler routine — pass the address. - let _ = bun_sys::c::SetConsoleCtrlHandler( - Some(Self::windows_ctrl_handler), - bun_sys::windows::FALSE, - ); - } + aggregate_exit_code( + self.handles + .iter() + .map(|h| h.process.as_ref().map(|p| &p.status)), + ) } } @@ -1031,18 +945,8 @@ pub(crate) fn run_scripts_with_filter( process: None, options: SpawnOptions { stdin: spawn::Stdio::Ignore, - #[cfg(unix)] - stdout: spawn::Stdio::Buffer, - #[cfg(not(unix))] - stdout: spawn::Stdio::Buffer(bun_core::heap::into_raw(Box::new( - bun_core::ffi::zeroed::(), - ))), - #[cfg(unix)] - stderr: spawn::Stdio::Buffer, - #[cfg(not(unix))] - stderr: spawn::Stdio::Buffer(bun_core::heap::into_raw(Box::new( - bun_core::ffi::zeroed::(), - ))), + stdout: buffered_stdio(), + stderr: buffered_stdio(), cwd: bun_paths::resolve_path::dirname::( &script.package_json_path, ) diff --git a/src/runtime/cli/install_command.rs b/src/runtime/cli/install_command.rs index 523eed71e266..0967cb445d38 100644 --- a/src/runtime/cli/install_command.rs +++ b/src/runtime/cli/install_command.rs @@ -1,5 +1,4 @@ use crate::Error; -use bun_bundler::bundle_v2::{DependenciesScanner, DependenciesScannerResult}; use bun_core::{Global, Output}; use bun_install::package_manager_real::{ CommandLineArguments, PackageManager, ROOT_PACKAGE_JSON_PATH, Subcommand, install_with_manager, @@ -7,7 +6,7 @@ use bun_install::package_manager_real::{ }; use crate::Cli; -use crate::build_command::BuildCommand; +use crate::cli::pm_update_package_json::analyze_dependencies_and_install; use crate::command::ContextData; pub(crate) struct InstallCommand; @@ -54,97 +53,8 @@ impl InstallCommand { fn install(ctx: &mut ContextData) -> Result<(), Error> { let mut cli = CommandLineArguments::parse(Subcommand::Install)?; - // The way this works: - // 1. Run the bundler on source files - // 2. Rewrite positional arguments to act identically to the developer - // typing in the dependency names - // 3. Run the install command if cli.analyze { - // `ctx` is stored as a raw `*mut ContextData`; the `on_fetch` callback - // re-enters the install path while `BuildCommand::exec` still holds the - // global `Context`, so a `&mut` here would be aliased UB. - struct Analyzer { - ctx: *mut ContextData, - cli: *mut CommandLineArguments, - } - impl bun_bundler::bundle_v2::OnDependenciesAnalyze for Analyzer { - fn on_analyze( - &mut self, - result: &mut DependenciesScannerResult<'_, '_>, - ) -> Result<(), bun_bundler::Error> { - let this = self; - // TODO: add separate argument that makes it so positionals[1..] is not done and instead the positionals are passed - // - // Process-lifetime storage for the rewritten positionals — - // `Global::exit(0)` follows immediately. - // `OnceLock` (not leaking) per PORTING.md §Forbidden. - static OWNED_KEYS: std::sync::OnceLock>> = std::sync::OnceLock::new(); - static POSITIONALS: std::sync::OnceLock> = - std::sync::OnceLock::new(); - - let owned = OWNED_KEYS.get_or_init(|| { - result - .dependencies - .keys() - .iter() - .map(|k| Box::<[u8]>::from(&**k)) - .collect() - }); - let positionals = POSITIONALS.get_or_init(|| { - let mut v: Vec<&'static [u8]> = Vec::with_capacity(owned.len() + 1); - v.push(b"install"); - for k in owned { - v.push(&**k); - } - v - }); - - // SAFETY: `this.cli` / `this.ctx` were set from live stack - // locals in `install()` whose scope encloses the entire - // `BuildCommand::exec` call (and hence this callback). The - // bundler does not touch the global `ContextData` between - // dependency-scan completion and `on_fetch` invocation, so - // forming a fresh `&mut` here is exclusive for the duration of - // `install_with_cli`. - let cli = unsafe { &mut *this.cli }; - cli.positionals = positionals.as_slice(); - // SAFETY: see above — same invariant covers `this.ctx`. - let ctx = unsafe { &mut *this.ctx }; - - install_with_cli(ctx, cli.clone()).map_err(bun_bundler::Error::from)?; - - Global::exit(0); - } - } - - // `DependenciesScanner.entry_points` is `Box<[Box<[u8]>]>`. Clone the - // argv slices into an owned buffer (small one-shot list — no perf - // concern). Captured *before* - // raw-ptr aliasing of `cli` below so the access goes through the live - // `&mut cli` borrow. - let entry_points: Box<[Box<[u8]>]> = cli.positionals[1..] - .iter() - .map(|s| Box::<[u8]>::from(*s)) - .collect(); - - // Derive raw pointers from the existing `&mut` borrows; all subsequent - // access to `ctx` / `cli` in this branch goes through these. - let ctx_ptr: *mut ContextData = ctx; - let mut analyzer = Analyzer { - ctx: ctx_ptr, - cli: &raw mut cli, - }; - - let fetcher = DependenciesScanner::new(&mut analyzer, entry_points); - - // `Command.get()` resolves to the same `*ContextData` already held in - // `ctx`; reborrow through `ctx_ptr` rather than minting a fresh - // `&'static mut` from the global static (which would alias the - // still-live `ctx` parameter under stacked borrows). - // SAFETY: `ctx_ptr` was just derived from the live `ctx: &mut - // ContextData` parameter; `ctx` is not accessed again in this branch. - BuildCommand::exec(unsafe { &mut *ctx_ptr }, Some(&fetcher))?; - return Ok(()); + return analyze_dependencies_and_install(ctx, &mut cli, b"install", &mut install_with_cli); } install_with_cli(ctx, cli) diff --git a/src/runtime/cli/link_command.rs b/src/runtime/cli/link_command.rs index a52f8c3a3429..1df8ba37d3f2 100644 --- a/src/runtime/cli/link_command.rs +++ b/src/runtime/cli/link_command.rs @@ -10,7 +10,7 @@ use bun_install::Features; use bun_install::bin_real as bin; use bun_install::lockfile_real::{Lockfile, package::Package}; use bun_install::package_manager_real::{ - self as pm, CommandLineArguments, Subcommand, attempt_to_create_package_json, + self as pm, CommandLineArguments, PackageManager, Subcommand, attempt_to_create_package_json, options::LogLevel, package_manager_options, setup_global_dir, update_package_json_and_install_with_manager, }; @@ -25,6 +25,108 @@ impl LinkCommand { } } +/// Shared by `bun link` / `bun unlink`: parse the nearest package.json into an +/// empty lockfile and validate that it declares a valid npm package name. +/// Crashes with a user-facing error otherwise. The package name is re-derived +/// by callers via `lockfile.str(&package.name)`. +pub(crate) fn load_package_for_link( + manager: &mut PackageManager, + verb: &str, +) -> crate::Result<(Lockfile, Package)> { + let mut lockfile = Lockfile::default(); + let mut package = Package::default(); + + let package_json_source = match bun_ast::to_source( + manager.original_package_json_path.as_zstr(), + Default::default(), + ) { + Ok(s) => s, + Err(e) => { + Output::err_generic( + "failed to read \"{}\" for {}: {}", + ( + BStr::new(manager.original_package_json_path.as_bytes()), + verb, + BStr::new(e.name()), + ), + ); + Global::crash(); + } + }; + lockfile.init_empty(); + + let mut resolver: () = (); + // `log_mut()` returns a borrow decoupled from `&self`; disjoint + // storage from `&mut PackageManager` (owned by the CLI `Context`). + let log = manager.log_mut(); + package.parse::<()>( + &mut lockfile, + manager, + log, + &package_json_source, + &mut resolver, + Features::FOLDER, + )?; + let name = lockfile.str(&package.name); + if name.is_empty() { + if manager.options.log_level != LogLevel::Silent { + bun_core::pretty_errorln!( + "error: package.json missing \"name\" in \"{}\"", + BStr::new(package_json_source.path.text), + ); + } + Global::crash(); + } else if !strings::is_npm_package_name(name) { + if manager.options.log_level != LogLevel::Silent { + bun_core::pretty_errorln!( + "error: invalid package.json name \"{}\" in \"{}\"", + BStr::new(name), + BStr::new(package_json_source.path.text), + ); + } + Global::crash(); + } + + Ok((lockfile, package)) +} + +/// Shared by `bun link` / `bun unlink`: open the global directory (storing it +/// in `manager.global_dir`) and create+open its `node_modules` folder. +/// Crashes with a user-facing error if `node_modules` cannot be created. +pub(crate) fn open_global_node_modules( + manager: &mut PackageManager, + ctx: &mut command::ContextData, +) -> crate::Result { + bin::Linker::ensure_umask(); + let explicit_global_dir: &[u8] = match &ctx.install { + Some(install_) => install_.global_dir.as_deref().unwrap_or(b""), + None => b"", + }; + manager.global_dir = Some(Dir::from_fd(package_manager_options::open_global_dir( + explicit_global_dir, + )?)); + + setup_global_dir(manager, &ctx)?; + + match manager + .global_dir + .as_ref() + .unwrap() + .make_open_path(b"node_modules", Default::default()) + { + Ok(d) => Ok(d), + Err(e) => { + if manager.options.log_level != LogLevel::Silent { + bun_core::pretty_errorln!( + "error: failed to create node_modules in global dir due to error {}", + BStr::new(e.name()), + ); + } + Global::crash(); + } + } +} + fn link(ctx: command::Context) -> crate::Result<()> { let cli = CommandLineArguments::parse(Subcommand::Link)?; let (manager, original_cwd) = match pm::init(&mut *ctx, cli, Subcommand::Link) { @@ -51,98 +153,15 @@ fn link(ctx: command::Context) -> crate::Result<()> { if manager.options.positionals.len() == 1 { // bun link - let mut lockfile = Lockfile::default(); - let mut package = Package::default(); - // Step 1. parse the nearest package.json file - { - let package_json_source = match bun_ast::to_source( - manager.original_package_json_path.as_zstr(), - Default::default(), - ) { - Ok(s) => s, - Err(e) => { - Output::err_generic( - "failed to read \"{s}\" for linking: {s}", - ( - BStr::new(manager.original_package_json_path.as_bytes()), - BStr::new(e.name()), - ), - ); - Global::crash(); - } - }; - lockfile.init_empty(); + let (lockfile, package) = load_package_for_link(manager, "linking")?; - let mut resolver: () = (); - // `log_mut()` returns a borrow decoupled from `&self`; disjoint - // storage from `&mut PackageManager` (owned by the CLI `Context`). - let log = manager.log_mut(); - package.parse::<()>( - &mut lockfile, - manager, - log, - &package_json_source, - &mut resolver, - Features::FOLDER, - )?; - let name = lockfile.str(&package.name); - if name.is_empty() { - if manager.options.log_level != LogLevel::Silent { - bun_core::pretty_errorln!( - "error: package.json missing \"name\" in \"{}\"", - BStr::new(package_json_source.path.text), - ); - } - Global::crash(); - } else if !strings::is_npm_package_name(name) { - if manager.options.log_level != LogLevel::Silent { - bun_core::pretty_errorln!( - "error: invalid package.json name \"{}\" in \"{}\"", - BStr::new(name), - BStr::new(package_json_source.path.text), - ); - } - Global::crash(); - } - } - - // Reshaped for borrowck — re-derive `name` here so its - // lifetime is tied only to `lockfile.buffers.string_bytes`, decoupled - // from `package_json_source` (dropped above). + // `name` is a slice into `lockfile.buffers.string_bytes`, decoupled + // from the helper-local package.json source. let name = lockfile.str(&package.name); // Step 2. Setup the global directory - let node_modules: Dir = 'brk: { - bin::Linker::ensure_umask(); - let explicit_global_dir: &[u8] = match &ctx.install { - Some(install_) => install_.global_dir.as_deref().unwrap_or(b""), - None => b"", - }; - manager.global_dir = Some(Dir::from_fd(package_manager_options::open_global_dir( - explicit_global_dir, - )?)); - - setup_global_dir(manager, &&mut *ctx)?; - - match manager - .global_dir - .as_ref() - .unwrap() - .make_open_path(b"node_modules", Default::default()) - { - Ok(d) => break 'brk d, - Err(e) => { - if manager.options.log_level != LogLevel::Silent { - bun_core::pretty_errorln!( - "error: failed to create node_modules in global dir due to error {}", - BStr::new(e.name()), - ); - } - Global::crash(); - } - } - }; + let node_modules: Dir = open_global_node_modules(manager, &mut *ctx)?; // Step 3a. symlink to the node_modules folder { diff --git a/src/runtime/cli/mod.rs b/src/runtime/cli/mod.rs index 292db89d5bfd..03e4dc8f65d5 100644 --- a/src/runtime/cli/mod.rs +++ b/src/runtime/cli/mod.rs @@ -369,6 +369,7 @@ pub(crate) mod pm_why_command; pub mod publish_command; #[path = "remove_command.rs"] pub(crate) mod remove_command; +pub(crate) mod run_processes_shared; #[path = "scan_command.rs"] pub mod scan_command; #[path = "unlink_command.rs"] @@ -379,6 +380,7 @@ pub(crate) mod update_command; pub mod update_interactive_command; #[path = "why_command.rs"] pub mod why_command; +pub(crate) mod workspace_helpers; // ─── crate-local helper for param-table concatenation ──────────────────────── // `bun_clap::parse_param!` is a real proc-macro (const `Param` literal), diff --git a/src/runtime/cli/multi_run.rs b/src/runtime/cli/multi_run.rs index 72c9fd15af29..84ddb9982d44 100644 --- a/src/runtime/cli/multi_run.rs +++ b/src/runtime/cli/multi_run.rs @@ -1,6 +1,6 @@ use core::ffi::{c_char, c_void}; use core::ptr; -use std::sync::atomic::{AtomicBool, Ordering}; +use std::sync::atomic::Ordering; use std::time::Instant; use crate::Error; @@ -14,6 +14,9 @@ use bun_paths::{self as path, PathBuffer}; use bun_resolver::package_json::{IncludeDependencies, IncludeScripts}; use crate::Command; +use crate::cli::run_processes_shared::{ + AbortHandler, SHOULD_ABORT, aggregate_exit_code, buffered_stdio, watch_or_reap, +}; use crate::filter_arg as FilterArg; use crate::run_command::RunCommand; @@ -22,8 +25,7 @@ use crate::run_command::RunCommand; #[cfg(unix)] use crate::api::bun::process::SpawnResultExt as _; use crate::api::bun::process::{ - self as spawn, Process, Rusage, SpawnOptions, SpawnProcessResult, Status, - event_loop_handle_to_ctx, + self as spawn, Process, SpawnOptions, SpawnProcessResult, Status, event_loop_handle_to_ctx, }; use bun_dotenv::Loader as DotEnvLoader; type OutputWriter = bun_core::io::Writer; @@ -270,16 +272,7 @@ impl<'a> ProcessHandle<'a> { ) }); - match process.watch_or_reap() { - Ok(_) => {} - Err(err) => { - if !process.has_exited() { - // SAFETY: all-zero is a valid Rusage (POD C struct) - let rusage = bun_core::ffi::zeroed::(); - process.on_exit(Status::Err(err), &rusage); - } - } - } + watch_or_reap(process); Ok(()) } @@ -590,87 +583,11 @@ impl<'a> State<'a> { } fn finalize(&self) -> u8 { - for handle in self.handles.iter() { - if let Some(proc) = &handle.process { - match &proc.status { - Status::Exited(exited) => { - if exited.code != 0 { - return exited.code; - } - } - Status::Signaled(signal) => { - return bun_sys::SignalCode(*signal).to_exit_code().unwrap_or(1); - } - _ => return 1, - } - } - } - 0 - } -} - -struct AbortHandler; - -static SHOULD_ABORT: AtomicBool = AtomicBool::new(false); - -impl AbortHandler { - #[cfg(unix)] - extern "C" fn posix_signal_handler( - _sig: i32, - _info: *const bun_sys::posix::siginfo_t, - _: *const c_void, - ) { - SHOULD_ABORT.store(true, Ordering::SeqCst); - } - - #[cfg(windows)] - extern "system" fn windows_ctrl_handler( - dw_ctrl_type: bun_sys::windows::DWORD, - ) -> bun_sys::windows::BOOL { - if dw_ctrl_type == bun_sys::windows::CTRL_C_EVENT { - SHOULD_ABORT.store(true, Ordering::SeqCst); - return bun_sys::windows::TRUE; - } - bun_sys::windows::FALSE - } - - fn install() { - #[cfg(unix)] - { - // bun_sys::posix::Sigaction is a re-export of libc::sigaction; construct - // via zeroed() (POD C struct) and populate sa_sigaction/sa_mask/sa_flags. - // SAFETY: all-zero is a valid `libc::sigaction`; sigemptyset/sigaction are - // FFI calls with no extra preconditions beyond valid pointers. - unsafe { - let mut action: bun_sys::posix::Sigaction = bun_core::ffi::zeroed(); - action.sa_sigaction = Self::posix_signal_handler as *const () as usize; - libc::sigemptyset(&raw mut action.sa_mask); - action.sa_flags = (libc::SA_SIGINFO | libc::SA_RESTART | libc::SA_RESETHAND) as _; - bun_sys::posix::sigaction(libc::SIGINT, &raw const action, core::ptr::null_mut()); - } - } - #[cfg(not(unix))] - { - let res = bun_sys::windows::SetConsoleCtrlHandler( - Some(Self::windows_ctrl_handler), - bun_sys::windows::TRUE, - ); - if res == 0 { - if bun_core::env::IS_DEBUG { - bun_core::warn!("Failed to set abort handler\n"); - } - } - } - } - - fn uninstall() { - #[cfg(windows)] - { - let _ = bun_sys::windows::SetConsoleCtrlHandler( - Some(Self::windows_ctrl_handler), - bun_sys::windows::FALSE, - ); - } + aggregate_exit_code( + self.handles + .iter() + .map(|h| h.process.as_ref().map(|p| &p.status)), + ) } } @@ -1227,18 +1144,8 @@ pub(crate) fn run(ctx: &mut Command::ContextData) -> Result(), - ))), - #[cfg(unix)] - stderr: spawn::Stdio::Buffer, - #[cfg(not(unix))] - stderr: spawn::Stdio::Buffer(bun_core::heap::into_raw(Box::new( - bun_core::ffi::zeroed::(), - ))), + stdout: buffered_stdio(), + stderr: buffered_stdio(), cwd: config.cwd.clone(), #[cfg(windows)] windows: spawn::WindowsOptions { diff --git a/src/runtime/cli/outdated_command.rs b/src/runtime/cli/outdated_command.rs index 0e3c7dfe61c7..d6a7a04862c7 100644 --- a/src/runtime/cli/outdated_command.rs +++ b/src/runtime/cli/outdated_command.rs @@ -8,16 +8,12 @@ use bun_core::{Global, Output}; use bun_glob as glob; use bun_install::dependency::{self, Behavior}; use bun_install::lockfile::package::PackageColumns as _; -use bun_install::lockfile::{LoadResult, LoadStep}; -use bun_install::package_manager::{ - LogLevel, ManifestLoad, Subcommand, WorkspaceFilter, populate_manifest_cache, -}; +use bun_install::package_manager::{ManifestLoad, Subcommand, populate_manifest_cache}; use bun_install::{CommandLineArguments, DependencyID, PackageID, PackageManager, resolution}; -use bun_paths::{self as path, PathBuffer}; -use bun_resolver::fs::FileSystem; use bun_wyhash::hash; use crate::Command; +use crate::cli::workspace_helpers; pub(crate) struct OutdatedCommand; @@ -89,68 +85,7 @@ impl OutdatedCommand { original_cwd: &[u8], manager: &mut PackageManager, ) -> crate::Result<()> { - // Reshaped for borrowck — `load_from_cwd` would otherwise alias - // `PackageManager` with its `lockfile` field. Project disjoint - // raw pointers from the singleton first; `load_from_cwd` only reads - // `manager.options` / migration helpers and never re-borrows - // `manager.lockfile` through the `pm` argument. - let pm_ptr: *mut PackageManager = manager; - let not_silent = manager.options.log_level != LogLevel::Silent; - let log_ptr: *mut bun_ast::Log = manager.log; - - // SAFETY: `lockfile` is the owned `Box` field on the singleton; - // no other live `&mut Lockfile` exists at this point. - let lockfile: &mut bun_install::lockfile::Lockfile = unsafe { &mut *(*pm_ptr).lockfile }; - // SAFETY: `manager.log` is set non-null by `PackageManager::init`. - let log = unsafe { &mut *log_ptr }; - match lockfile.load_from_cwd::( - // SAFETY: see comment above — `load_from_cwd` accesses `manager` - // fields disjoint from `lockfile`. - Some(unsafe { &mut *pm_ptr }), - log, - ) { - LoadResult::NotFound => { - if not_silent { - Output::err_generic("missing lockfile, nothing outdated", ()); - } - Global::crash(); - } - LoadResult::Err(cause) => { - if not_silent { - match cause.step { - LoadStep::OpenFile => Output::err_generic( - "failed to open lockfile: {s}", - (cause.value.name(),), - ), - LoadStep::ParseFile => Output::err_generic( - "failed to parse lockfile: {s}", - (cause.value.name(),), - ), - LoadStep::ReadFile => Output::err_generic( - "failed to read lockfile: {s}", - (cause.value.name(),), - ), - LoadStep::Migrating => Output::err_generic( - "failed to migrate lockfile: {s}", - (cause.value.name(),), - ), - } - if ctx.log_ref().has_errors() { - // SAFETY: `log_ptr` aliases `manager.log` which is the - // `*logger.Log` borrowed from `Command::Context`; no - // other `&mut Log` is live here. - let _ = - unsafe { (*log_ptr).print(std::ptr::from_mut(Output::error_writer())) }; - } - } - Global::crash(); - } - LoadResult::Ok(_) => { - // `load_from_cwd(&mut self, ..)` populates the - // lockfile in place, so the `ok.lockfile: &mut Lockfile` reborrow - // is the same storage and no reassignment is needed. - } - } + workspace_helpers::load_lockfile_or_crash(ctx, manager); if Output::enable_ansi_colors_stdout() { Self::outdated_dispatch::(original_cwd, manager) @@ -165,14 +100,15 @@ impl OutdatedCommand { ) -> crate::Result<()> { if !manager.options.filter_patterns.is_empty() { let filters = manager.options.filter_patterns; - let workspace_pkg_ids = Self::find_matching_workspaces(original_cwd, manager, filters); + let workspace_pkg_ids = + workspace_helpers::find_matching_workspaces(original_cwd, manager, filters); populate_manifest_cache::populate_manifest_cache( manager, populate_manifest_cache::Packages::Ids(&workspace_pkg_ids), )?; Self::print_outdated_info_table::(manager, &workspace_pkg_ids, true) } else if manager.options.do_.recursive() { - let all_workspaces = Self::get_all_workspaces(manager); + let all_workspaces = workspace_helpers::get_all_workspaces(manager); populate_manifest_cache::populate_manifest_cache( manager, populate_manifest_cache::Packages::Ids(&all_workspaces), @@ -194,118 +130,6 @@ impl OutdatedCommand { } } - fn get_all_workspaces(manager: &PackageManager) -> Vec { - let lockfile = &manager.lockfile; - let packages = lockfile.packages.slice(); - let pkg_resolutions = packages.items_resolution(); - - let mut workspace_pkg_ids: Vec = Vec::new(); - for (pkg_id, resolution) in pkg_resolutions.iter().enumerate() { - if resolution.tag != resolution::Tag::Workspace - && resolution.tag != resolution::Tag::Root - { - continue; - } - workspace_pkg_ids.push(pkg_id as PackageID); - } - workspace_pkg_ids - } - - fn find_matching_workspaces( - original_cwd: &[u8], - manager: &PackageManager, - filters: &[&[u8]], - ) -> Vec { - let lockfile = &manager.lockfile; - let packages = lockfile.packages.slice(); - let pkg_names = packages.items_name(); - let pkg_resolutions = packages.items_resolution(); - let string_buf = lockfile.buffers.string_bytes.as_slice(); - - let mut workspace_pkg_ids: Vec = Vec::new(); - for (pkg_id, resolution) in pkg_resolutions.iter().enumerate() { - if resolution.tag != resolution::Tag::Workspace - && resolution.tag != resolution::Tag::Root - { - continue; - } - workspace_pkg_ids.push(pkg_id as PackageID); - } - - let mut path_buf = PathBuffer::uninit(); - - let converted_filters: Vec = filters - .iter() - .map(|filter| { - bun_core::handle_oom(WorkspaceFilter::init(filter, original_cwd, &mut path_buf.0)) - }) - .collect(); - // `defer { filter.deinit(allocator); allocator.free(...) }` — implicit via Drop. - - // SAFETY: `FileSystem::init` runs during `PackageManager::init` so the - // process-singleton is populated. - let top_level_dir = FileSystem::get().top_level_dir; - - // move all matched workspaces to front of array - let mut i: usize = 0; - while i < workspace_pkg_ids.len() { - let workspace_pkg_id = workspace_pkg_ids[i]; - - let matched = 'matched: { - for filter in &converted_filters { - match filter { - WorkspaceFilter::Path(pattern) => { - if pattern.is_empty() { - continue; - } - let res = &pkg_resolutions[workspace_pkg_id as usize]; - let res_path: &[u8] = match res.tag { - resolution::Tag::Workspace => { - // Borrow the field in-place so the returned slice (which may - // point into the inline small-string storage) stays valid. - res.workspace().slice(string_buf) - } - resolution::Tag::Root => top_level_dir, - _ => unreachable!(), - }; - - let abs_res_path = path::resolve_path::join_abs_string_buf::< - path::platform::Posix, - >( - top_level_dir, &mut path_buf.0, &[res_path] - ); - - if !glob::r#match( - pattern, - strings::without_trailing_slash(abs_res_path), - ) - .matches() - { - break 'matched false; - } - } - WorkspaceFilter::Name(pattern) => { - let name = pkg_names[workspace_pkg_id as usize].slice(string_buf); - if !glob::r#match(pattern, name).matches() { - break 'matched false; - } - } - WorkspaceFilter::All => {} - } - } - true - }; - - if matched { - i += 1; - } else { - workspace_pkg_ids.swap_remove(i); - } - } - - workspace_pkg_ids - } - fn group_catalog_dependencies( manager: &PackageManager, outdated_items: &[OutdatedInfo], diff --git a/src/runtime/cli/pack_command.rs b/src/runtime/cli/pack_command.rs index 855960f9d37e..5ca2f4b3552c 100644 --- a/src/runtime/cli/pack_command.rs +++ b/src/runtime/cli/pack_command.rs @@ -31,9 +31,7 @@ use bun_core::{ZStr, strings}; use bun_paths::resolve_path; use bun_semver as Semver; use bun_sha_hmac::sha; -use bun_sys::{ - self, CloseOnDrop, Dir, Fd, FdDirExt as _, FdExt as _, File, dir_iterator as DirIterator, -}; +use bun_sys::{self, Dir, Fd, FdDirExt as _, FdExt as _, File, dir_iterator as DirIterator}; // ─────────────────────────────────────────────────────────────────────────── // local shims for upstream-stub gaps @@ -1839,7 +1837,7 @@ fn new_boxed_buffered_file_reader(file: bun_sys::File) -> Box Option<&[u8]> { // pack() // ─────────────────────────────────────────────────────────────────────────── -// Const generics cannot vary the -// return type directly, so both instantiations return an Option that is -// `Some` only when FOR_PUBLISH == true. -pub(crate) type PackReturn<'a, const FOR_PUBLISH: bool> = Option>; - -pub(crate) fn pack( - ctx: &mut Context<'_>, +/// Reads `abs_package_json_path` through the workspace package.json cache; +/// read/parse failures are fatal. Unlike +/// `WorkspacePackageJSONCache::get_with_path_or_exit`, this keeps pack's +/// error wording and ordering (`Output::err` first, then the log printed +/// unconditionally on parse errors), matching the `pack_command.zig` +/// reference. +fn load_package_json_or_exit<'a>( + manager_ptr: *mut PackageManager, abs_package_json_path: &ZStr, -) -> Result, PackError> { - // Raw pointer for the `pm_workspace_cache`/`pm_log` disjoint-field - // projections and the `'static` lifetime extension when returning - // `Publish::Context`. - let manager_ptr: *mut PackageManager = &raw mut *ctx.manager; - let log_level = ctx.manager.options.log_level; - let bump = pack_bump(); +) -> &'a mut WorkspacePackageJSONCache::MapEntry { // Note: `workspace_package_json_cache` and `log` are disjoint fields on // `PackageManager`; route through raw-pointer field projections so the // two `&mut` borrows don't conflict. - let mut json = match pm_workspace_cache(manager_ptr).get_with_path( + match pm_workspace_cache(manager_ptr).get_with_path( pm_log(manager_ptr), abs_package_json_path.as_bytes(), WorkspacePackageJSONCache::GetJSONOptions { @@ -1937,7 +1930,25 @@ pub(crate) fn pack( Global::crash(); } WorkspacePackageJSONCache::GetResult::Entry(entry) => entry, - }; + } +} + +// Const generics cannot vary the +// return type directly, so both instantiations return an Option that is +// `Some` only when FOR_PUBLISH == true. +pub(crate) type PackReturn<'a, const FOR_PUBLISH: bool> = Option>; + +pub(crate) fn pack( + ctx: &mut Context<'_>, + abs_package_json_path: &ZStr, +) -> Result, PackError> { + // Raw pointer for the `pm_workspace_cache`/`pm_log` disjoint-field + // projections and the `'static` lifetime extension when returning + // `Publish::Context`. + let manager_ptr: *mut PackageManager = &raw mut *ctx.manager; + let log_level = ctx.manager.options.log_level; + let bump = pack_bump(); + let mut json = load_package_json_or_exit(manager_ptr, abs_package_json_path); if FOR_PUBLISH { if let Some(config) = json.root.get(b"publishConfig") { @@ -2163,33 +2174,7 @@ pub(crate) fn pack( let _ = pm_workspace_cache(manager_ptr).map.remove(cache_key); // Re-read package.json from disk - json = match pm_workspace_cache(manager_ptr).get_with_path( - pm_log(manager_ptr), - abs_package_json_path.as_bytes(), - WorkspacePackageJSONCache::GetJSONOptions { - guess_indentation: true, - ..Default::default() - }, - ) { - WorkspacePackageJSONCache::GetResult::ReadErr(err) => { - Output::err( - err, - "failed to read package.json: {}", - format_args!("{}", bstr::BStr::new(abs_package_json_path.as_bytes())), - ); - Global::crash(); - } - WorkspacePackageJSONCache::GetResult::ParseErr(err) => { - Output::err( - err, - "failed to parse package.json: {}", - format_args!("{}", bstr::BStr::new(abs_package_json_path.as_bytes())), - ); - let _ = pm_log(manager_ptr).print(std::ptr::from_mut(Output::error_writer())); - Global::crash(); - } - WorkspacePackageJSONCache::GetResult::Entry(entry) => entry, - }; + json = load_package_json_or_exit(manager_ptr, abs_package_json_path); // Re-validate private flag after scripts may have modified it. if FOR_PUBLISH { @@ -2592,11 +2577,9 @@ pub(crate) fn pack( node = Some(progress.start(b"", pack_queue.count() + bundled_pack_queue.count() + 1)); node.as_mut().expect("infallible: progress active").unit = Progress::Unit::Files; } - // Note: the loop bodies' only early exits are `continue` - // and `Global::crash()` (never returns, no - // unwinding). `scopeguard` captures of `&mut node` overlap the inline - // uses below, so call `complete_one()` explicitly at every loop-body - // exit and `end()` once after the loops. + // Note: `scopeguard` captures of `&mut node` would overlap the inline + // uses below, so call `complete_one()` explicitly after each archived + // entry and `end()` once after the queues drain. entry = archive_package_json( ctx, @@ -2613,142 +2596,41 @@ pub(crate) fn pack( .complete_one(); } - while let Some(item) = pack_queue.remove_or_null() { - let file = match bun_sys::openat( - Fd::from_std_dir(&root_dir), - &item.path, - bun_sys::O::RDONLY, - 0, - ) { - Ok(f) => f, - Err(err) => { - if item.optional { - ctx.stats.total_files -= 1; - if log_level.show_progress() { - node.as_mut() - .expect("infallible: progress active") - .complete_one(); - } - continue; - } - Output::err( - err, - "failed to open file: \"{}\"", - format_args!("{}", bstr::BStr::new(item.path.as_bytes())), - ); - Global::crash(); - } - }; - - let fd: Fd = match file - .make_lib_uv_owned_for_syscall(bun_sys::Tag::open, bun_sys::ErrorCase::CloseOnFail) - { - Ok(fd) => fd, - Err(err) => { - Output::err( - err, - "failed to open file: \"{}\"", - format_args!("{}", bstr::BStr::new(item.path.as_bytes())), - ); - Global::crash(); - } - }; - - let _close_fd = CloseOnDrop::new(fd); - - let stat = match bun_sys::sys_uv::fstat(fd) { - Ok(s) => s, - Err(err) => { - Output::err( - err, - "failed to stat file: \"{}\"", - format_args!("{}", bstr::BStr::new(item.path.as_bytes())), - ); - Global::crash(); - } - }; - - pack_list.push(PackListEntry { - subpath: ZBox::from_bytes(item.path.as_bytes()), - size: usize::try_from(stat.st_size).expect("int cast"), - }); - - entry = add_archive_entry( - ctx, - fd, - &stat, - &item.path, - &mut read_buf, - &mut file_reader, - // SAFETY: `archive` is the non-null `*mut Archive` returned by - // `Archive::write_new()` above; only this thread accesses it. - unsafe { &mut *archive }, - entry, - &mut print_buf, - &bins, - )?; - - if log_level.show_progress() { - node.as_mut() - .expect("infallible: progress active") - .complete_one(); - } - } - - while let Some(item) = bundled_pack_queue.remove_or_null() { - let file = match root_dir.open_file(&item.path, bun_sys::O::RDONLY, 0) { - Ok(f) => f, - Err(err) => { - if item.optional { - ctx.stats.total_files -= 1; - if log_level.show_progress() { - node.as_mut() - .expect("infallible: progress active") - .complete_one(); - } - continue; - } - Output::err( - err, - "failed to open file: \"{}\"", - format_args!("{}", bstr::BStr::new(item.path.as_bytes())), - ); - Global::crash(); - } - }; - let stat = match file.stat() { - Ok(s) => s, - Err(err) => { - Output::err( - err, - "failed to stat file: \"{}\"", - format_args!("{}", file.handle), - ); - Global::crash(); - } - }; - - entry = add_archive_entry( - ctx, - file.handle, - &stat, - &item.path, - &mut read_buf, - &mut file_reader, - // SAFETY: `archive` is the non-null `*mut Archive` returned by - // `Archive::write_new()` above; only this thread accesses it. - unsafe { &mut *archive }, - entry, - &mut print_buf, - &bins, - )?; + entry = archive_pack_queue( + ctx, + &mut pack_queue, + PackQueueOpenMode::UvOwnedFd, + &root_dir, + Some(&mut pack_list), + &mut read_buf, + &mut file_reader, + // SAFETY: `archive` is the non-null `*mut Archive` returned by + // `Archive::write_new()` above; only this thread accesses it. + unsafe { &mut *archive }, + entry, + &mut print_buf, + &bins, + log_level, + &mut node, + )?; - if log_level.show_progress() { - node.as_mut() - .expect("infallible: progress active") - .complete_one(); - } - } + entry = archive_pack_queue( + ctx, + &mut bundled_pack_queue, + PackQueueOpenMode::PlainFile, + &root_dir, + None, + &mut read_buf, + &mut file_reader, + // SAFETY: `archive` is the non-null `*mut Archive` returned by + // `Archive::write_new()` above; only this thread accesses it. + unsafe { &mut *archive }, + entry, + &mut print_buf, + &bins, + log_level, + &mut node, + )?; if log_level.show_progress() { if let Some(n) = node.as_mut() { @@ -3192,6 +3074,144 @@ fn archive_package_json( Ok(entry.clear()) } +/// How [`archive_pack_queue`] opens each queued file. +#[derive(Clone, Copy, PartialEq, Eq)] +enum PackQueueOpenMode { + /// Open with `bun_sys::openat` and convert to a libuv-owned descriptor + /// stat'd through `sys_uv`. + UvOwnedFd, + /// Open relative to `root_dir` as a plain `File`. + PlainFile, +} + +/// Drains `queue`, archiving each file via [`add_archive_entry`]; see +/// [`PackQueueOpenMode`] for how each file is opened. Each entry is also +/// appended to `pack_list` when provided. +/// +/// The loop body's only early exits are `continue` and `Global::crash()` +/// (never returns, no unwinding), so `node.complete_one()` is called +/// explicitly at every loop-body exit instead of via a scope guard. +fn archive_pack_queue( + ctx: &mut Context<'_>, + queue: &mut PackQueue, + open_mode: PackQueueOpenMode, + root_dir: &Dir, + mut pack_list: Option<&mut PackList>, + read_buf: &mut [u8], + file_reader: &mut BufferedFileReader, + archive: &mut Archive, + mut entry: *mut ArchiveEntry, + print_buf: &mut Vec, + bins: &[BinInfo], + log_level: LogLevel, + node: &mut Option<&mut Progress::Node>, +) -> Result<*mut ArchiveEntry, AllocError> { + let uv_owned_fd = open_mode == PackQueueOpenMode::UvOwnedFd; + while let Some(item) = queue.remove_or_null() { + let opened = if uv_owned_fd { + bun_sys::openat( + Fd::from_std_dir(root_dir), + &item.path, + bun_sys::O::RDONLY, + 0, + ) + .map(File::from_fd) + } else { + root_dir.open_file(&item.path, bun_sys::O::RDONLY, 0) + }; + let file = match opened { + Ok(f) => f, + Err(err) => { + if item.optional { + ctx.stats.total_files -= 1; + if log_level.show_progress() { + node.as_mut() + .expect("infallible: progress active") + .complete_one(); + } + continue; + } + Output::err( + err, + "failed to open file: \"{}\"", + format_args!("{}", bstr::BStr::new(item.path.as_bytes())), + ); + Global::crash(); + } + }; + + let file = if uv_owned_fd { + match file + .into_raw() + .make_lib_uv_owned_for_syscall(bun_sys::Tag::open, bun_sys::ErrorCase::CloseOnFail) + { + Ok(fd) => File::from_fd(fd), + Err(err) => { + Output::err( + err, + "failed to open file: \"{}\"", + format_args!("{}", bstr::BStr::new(item.path.as_bytes())), + ); + Global::crash(); + } + } + } else { + file + }; + + let stat = match if uv_owned_fd { + bun_sys::sys_uv::fstat(file.handle) + } else { + file.stat() + } { + Ok(s) => s, + Err(err) => { + if uv_owned_fd { + Output::err( + err, + "failed to stat file: \"{}\"", + format_args!("{}", bstr::BStr::new(item.path.as_bytes())), + ); + } else { + Output::err( + err, + "failed to stat file: \"{}\"", + format_args!("{}", file.handle), + ); + } + Global::crash(); + } + }; + + if let Some(pack_list) = pack_list.as_deref_mut() { + pack_list.push(PackListEntry { + subpath: ZBox::from_bytes(item.path.as_bytes()), + size: usize::try_from(stat.st_size).expect("int cast"), + }); + } + + entry = add_archive_entry( + ctx, + file.handle, + &stat, + &item.path, + read_buf, + file_reader, + archive, + entry, + print_buf, + bins, + )?; + + if log_level.show_progress() { + node.as_mut() + .expect("infallible: progress active") + .complete_one(); + } + } + Ok(entry) +} + fn add_archive_entry( ctx: &mut Context<'_>, file: Fd, diff --git a/src/runtime/cli/pm_update_package_json.rs b/src/runtime/cli/pm_update_package_json.rs index 85bca97c3b81..0de2f75c33d1 100644 --- a/src/runtime/cli/pm_update_package_json.rs +++ b/src/runtime/cli/pm_update_package_json.rs @@ -17,7 +17,7 @@ use bun_install::package_manager_real::{Subcommand, update_package_json_and_inst use crate::build_command::BuildCommand; use crate::cli::Cli; -use crate::command::{self, Context, ContextData}; +use crate::command::{Context, ContextData}; pub(crate) fn update_package_json_and_install_catch_error( ctx: Context, @@ -52,96 +52,113 @@ pub(crate) fn update_package_json_and_install( // `parse` requires ``, expand to a `match`. let mut cli = CommandLineArguments::parse(subcommand)?; - // The way this works: - // 1. Run the bundler on source files - // 2. Rewrite positional arguments to act identically to the developer - // typing in the dependency names - // 3. Run the install command if cli.analyze { - // `ctx`/`cli` are stored as raw `*mut` because - // `BuildCommand::exec` holds `command::get()` (the same `ContextData`) across - // the `on_fetch` callback, and `DependenciesScanner.entry_points` owns a copy - // of `cli.positionals[1..]` for the duration of the scan; storing `&mut` here - // would assert exclusivity we don't have. - struct Analyzer { - ctx: *mut ContextData, - cli: *mut CommandLineArguments, - subcommand: Subcommand, - } - impl bun_bundler::bundle_v2::OnDependenciesAnalyze for Analyzer { - fn on_analyze( - &mut self, - result: &mut DependenciesScannerResult<'_, '_>, - ) -> Result<(), bun_bundler::Error> { - let this = self; - // TODO: add separate argument that makes it so positionals[1..] is not done and instead the positionals are passed - // - // Process-lifetime storage for the rewritten positionals — - // `Global::exit(0)` follows immediately. `OnceLock` (not - // leaked). - static OWNED_KEYS: std::sync::OnceLock>> = std::sync::OnceLock::new(); - static POSITIONALS: std::sync::OnceLock> = - std::sync::OnceLock::new(); + return analyze_dependencies_and_install(ctx, &mut cli, b"add", &mut |ctx, cli| { + update_package_json_and_install_and_cli(ctx, subcommand, cli).map_err(Into::into) + }); + } + + update_package_json_and_install_and_cli(ctx, subcommand, cli).map_err(Into::into) +} + +/// Shared body of the `cli.analyze` branch of `bun install` / `bun add`: +/// 1. Run the bundler's dependency scanner over the positional entry points +/// 2. Rewrite the positionals to `[verb, ...discovered dependency names]`, +/// acting identically to the developer typing in the dependency names +/// 3. Re-enter the install path via `install` and exit the process +pub(crate) fn analyze_dependencies_and_install( + ctx: &mut ContextData, + cli: &mut CommandLineArguments, + verb: &'static [u8], + install: &mut dyn FnMut(&mut ContextData, CommandLineArguments) -> Result<(), Error>, +) -> Result<(), Error> { + // `ctx`/`cli` are stored as raw `*mut` because `BuildCommand::exec` holds + // the global `Context` (the same `ContextData`) across the `on_analyze` + // callback, and `DependenciesScanner.entry_points` owns a copy of + // `cli.positionals[1..]` for the duration of the scan; storing `&mut` + // here would assert exclusivity we don't have. + struct Analyzer<'a> { + ctx: *mut ContextData, + cli: *mut CommandLineArguments, + verb: &'static [u8], + install: &'a mut dyn FnMut(&mut ContextData, CommandLineArguments) -> Result<(), Error>, + } + impl bun_bundler::bundle_v2::OnDependenciesAnalyze for Analyzer<'_> { + fn on_analyze( + &mut self, + result: &mut DependenciesScannerResult<'_, '_>, + ) -> Result<(), bun_bundler::Error> { + let this = self; + // TODO: add separate argument that makes it so positionals[1..] is not done and instead the positionals are passed + // + // Process-lifetime storage for the rewritten positionals — + // `Global::exit(0)` follows immediately. + // `OnceLock` (not leaking) per PORTING.md §Forbidden. + static OWNED_KEYS: std::sync::OnceLock>> = std::sync::OnceLock::new(); + static POSITIONALS: std::sync::OnceLock> = + std::sync::OnceLock::new(); - let owned = OWNED_KEYS.get_or_init(|| { - result - .dependencies - .keys() - .iter() - .map(|k| Box::<[u8]>::from(&**k)) - .collect() - }); - let positionals = POSITIONALS.get_or_init(|| { - let mut v: Vec<&'static [u8]> = Vec::with_capacity(owned.len() + 1); - v.push(b"add"); - for k in owned { - v.push(&**k); - } - v - }); + let owned = OWNED_KEYS.get_or_init(|| { + result + .dependencies + .keys() + .iter() + .map(|k| Box::<[u8]>::from(&**k)) + .collect() + }); + let positionals = POSITIONALS.get_or_init(|| { + let mut v: Vec<&'static [u8]> = Vec::with_capacity(owned.len() + 1); + v.push(this.verb); + for k in owned { + v.push(&**k); + } + v + }); - // SAFETY: `this.cli` / `this.ctx` were set from live stack locals in - // `update_package_json_and_install` whose scope encloses the entire - // `BuildCommand::exec` call (and hence this callback). The bundler has - // finished reading `entry_points` before invoking `on_fetch`, and this - // callback never returns (`Global::exit` below), so forming fresh `&mut` - // here is exclusive for the remainder of the process. - let cli = unsafe { &mut *this.cli }; - cli.positionals = positionals.as_slice(); - // SAFETY: `this.ctx` points to the `ctx` stack local in - // `update_package_json_and_install`, whose frame outlives this - // callback; `Global::exit` below makes this `&mut` exclusive for - // the remainder of the process. - let ctx = unsafe { &mut *this.ctx }; + // SAFETY: `this.cli` / `this.ctx` were set from live locals in + // `analyze_dependencies_and_install`'s caller, whose scope + // encloses the entire `BuildCommand::exec` call (and hence this + // callback). The bundler does not touch the global `ContextData` + // between dependency-scan completion and `on_analyze` invocation, + // so forming a fresh `&mut` here is exclusive for the duration of + // the `install` continuation. + let cli = unsafe { &mut *this.cli }; + cli.positionals = positionals.as_slice(); + // SAFETY: see above — same invariant covers `this.ctx`. + let ctx = unsafe { &mut *this.ctx }; - update_package_json_and_install_and_cli(ctx, this.subcommand, cli.clone()) - .map_err(crate::Error::from)?; + (this.install)(ctx, cli.clone()).map_err(bun_bundler::Error::from)?; - Global::exit(0); - } + Global::exit(0); } + } - // Note: `DependenciesScanner.entry_points` is `Box<[Box<[u8]>]>`. - // Clone the argv slices into an owned - // buffer (small one-shot list — no perf concern) so `cli` is not borrowed across - // the `&mut analyzer` setup. - let entry_points: Box<[Box<[u8]>]> = cli.positionals[1..] - .iter() - .map(|s| Box::<[u8]>::from(*s)) - .collect(); - - let mut analyzer = Analyzer { - ctx: std::ptr::from_mut::(ctx), - cli: &raw mut cli, - subcommand, - }; + // `DependenciesScanner.entry_points` is `Box<[Box<[u8]>]>`. Clone the + // argv slices into an owned buffer (small one-shot list — no perf + // concern). Captured *before* raw-ptr aliasing of `cli` below so the + // access goes through the live `&mut cli` borrow. + let entry_points: Box<[Box<[u8]>]> = cli.positionals[1..] + .iter() + .map(|s| Box::<[u8]>::from(*s)) + .collect(); - let fetcher = DependenciesScanner::new(&mut analyzer, entry_points); + // Derive raw pointers from the existing `&mut` borrows; all subsequent + // access to `ctx` / `cli` in this function goes through these. + let ctx_ptr: *mut ContextData = ctx; + let mut analyzer = Analyzer { + ctx: ctx_ptr, + cli, + verb, + install, + }; - // This runs the bundler. - BuildCommand::exec(command::get(), Some(&fetcher))?; - return Ok(()); - } + let fetcher = DependenciesScanner::new(&mut analyzer, entry_points); - update_package_json_and_install_and_cli(ctx, subcommand, cli).map_err(Into::into) + // `Command.get()` resolves to the same `*ContextData` already held in + // `ctx`; reborrow through `ctx_ptr` rather than minting a fresh + // `&'static mut` from the global static (which would alias the + // still-live `ctx` parameter under stacked borrows). + // SAFETY: `ctx_ptr` was just derived from the live `ctx: &mut + // ContextData` parameter; `ctx` is not accessed again in this function. + BuildCommand::exec(unsafe { &mut *ctx_ptr }, Some(&fetcher)) } diff --git a/src/runtime/cli/repl.rs b/src/runtime/cli/repl.rs index 553d90cf2ba1..fb9313e0bdd7 100644 --- a/src/runtime/cli/repl.rs +++ b/src/runtime/cli/repl.rs @@ -580,6 +580,16 @@ enum ReplResult { SkipEval, } +/// How `evaluate_to_value` reports promise rejections and interrupts. +/// Mirrors the difference between repl.zig's evaluateAndPrint (sets `_error` +/// on globalThis, prints a newline on interrupt) and evaluateAndCopy (does +/// neither). +#[derive(Clone, Copy, PartialEq, Eq)] +enum ReportMode { + Print, + Copy, +} + fn cmd_help(repl: &mut Repl, _: &[u8]) -> ReplResult { repl.print(format_args!( "\n{}REPL Commands:{}\n", @@ -1286,19 +1296,20 @@ impl<'a> Repl<'a> { // JavaScript Evaluation // ======================================================================== - fn evaluate_and_print(&mut self, code: &[u8]) { - let Some(global) = self.global else { - return; - }; - let Some(vm) = self.vm else { - return; - }; + /// Run `code` through the interactive REPL pipeline: transform_for_repl, + /// evaluate, await any async IIFE promise (with Ctrl+C signal handling), + /// unwrap the `{ value: expr }` wrapper, then store the result and set `_` + /// on globalThis. Returns `None` when the outcome was already reported + /// (errors, interrupts, or raw-evaluation fallback). + fn evaluate_to_value(&mut self, code: &[u8], mode: ReportMode) -> Option { + let global = self.global?; + let vm = self.vm?; // Transform the code using REPL mode (hoists declarations, wraps result in { value: expr }) let Some(transformed_code) = self.transform_for_repl(code) else { // Transform failed, try evaluating raw code (for syntax errors, etc.) self.evaluate_raw(code); - return; + return None; }; // Evaluate the transformed code @@ -1320,7 +1331,7 @@ impl<'a> Repl<'a> { if !exception.is_undefined() && !exception.is_null() { self.set_last_error(exception); self.print_js_error(exception); - return; + return None; } // Handle async IIFE results - wait for promise to resolve @@ -1346,7 +1357,7 @@ impl<'a> Repl<'a> { global.clear_termination_exception(); self.print(format_args!("\n")); self.disable_signals_during_wait(); - return; + return None; } // SAFETY: `vm.jsc_vm` is the live JSC VM handle for this thread. @@ -1359,18 +1370,22 @@ impl<'a> Repl<'a> { PromiseStatus::Rejected => { let rejection = jsc::JSPromise::opaque_mut(promise).result(jsc_vm_ref); self.set_last_error(rejection); - // Set _error on the global object - let global_this = global.to_js_value(); - global_this.put(global, b"_error", rejection); + if mode == ReportMode::Print { + // Set _error on the global object + let global_this = global.to_js_value(); + global_this.put(global, b"_error", rejection); + } self.print_js_error(rejection); self.disable_signals_during_wait(); - return; + return None; } PromiseStatus::Pending => { // Interrupted by signal or timed out - self.print(format_args!("\n")); + if mode == ReportMode::Print { + self.print(format_args!("\n")); + } self.disable_signals_during_wait(); - return; + return None; } } self.disable_signals_during_wait(); @@ -1390,7 +1405,7 @@ impl<'a> Repl<'a> { self.set_last_error(exc); self.print_js_error(exc); vm.as_mut().tick(); - return; + return None; } }; if let Some(value) = maybe_value { @@ -1398,7 +1413,7 @@ impl<'a> Repl<'a> { } } - // Store and print result + // Store the result self.set_last_result(actual_result); // Set _ to the last result (only if not undefined) @@ -1408,6 +1423,14 @@ impl<'a> Repl<'a> { global_this.put(global, b"_", actual_result); } + Some(actual_result) + } + + fn evaluate_and_print(&mut self, code: &[u8]) { + let Some(actual_result) = self.evaluate_to_value(code, ReportMode::Print) else { + return; + }; + if actual_result.is_undefined() { if self.use_colors { self.print(format_args!("{}undefined{}\n", Color::DIM, Color::RESET)); @@ -1419,7 +1442,9 @@ impl<'a> Repl<'a> { } // Tick the event loop to handle any pending work - vm.as_mut().tick(); + if let Some(vm) = self.vm { + vm.as_mut().tick(); + } } /// Evaluate a script from `bun repl -e/--eval` or `-p/--print` non-interactively. @@ -1601,106 +1626,20 @@ impl<'a> Repl<'a> { /// Evaluate code and copy the result to clipboard instead of printing it fn evaluate_and_copy(&mut self, code: &[u8]) { - let Some(global) = self.global else { - return; - }; - let Some(vm) = self.vm else { - return; - }; - - let Some(transformed_code) = self.transform_for_repl(code) else { - self.evaluate_raw(code); + let Some(actual_result) = self.evaluate_to_value(code, ReportMode::Copy) else { return; }; - let mut exception: JSValue = JSValue::UNDEFINED; - // SAFETY: `global` is a live opaque `JSGlobalObject` handle; slice ptr/len pairs - // are valid for the duration of the call; `exception` is a stack local. - let result = unsafe { - Bun__REPL__evaluate( - global, - transformed_code.as_ptr(), - transformed_code.len(), - b"[repl]".as_ptr(), - b"[repl]".len(), - &raw mut exception, - ) - }; - - if !exception.is_undefined() && !exception.is_null() { - self.set_last_error(exception); - self.print_js_error(exception); - return; - } - - let mut resolved_result = result; - if let Some(promise) = result.as_promise() { - // SAFETY: `promise` is a live JSC heap cell; `vm.jsc_vm` is the - // owning JSC VM handle for this thread. - jsc::JSPromise::opaque_mut(promise).set_handled(); - self.enable_signals_during_wait(); - // Note: reshaped for borrowck — disable_signals_during_wait called on each path - // Interrupted (SIGINT forbids execution) ⇒ handled just below. - let _ = vm - .as_mut() - .wait_for_promise(jsc::AnyPromise::Normal(promise)); - if vm.jsc_vm().execution_forbidden() { - vm.jsc_vm().set_execution_forbidden(false); - global.clear_termination_exception(); - self.print(format_args!("\n")); - self.disable_signals_during_wait(); - return; - } - let jsc_vm_ref = vm.jsc_vm(); - match jsc::JSPromise::opaque_mut(promise).status() { - PromiseStatus::Fulfilled => { - resolved_result = jsc::JSPromise::opaque_mut(promise).result(jsc_vm_ref) - } - PromiseStatus::Rejected => { - let rejection = jsc::JSPromise::opaque_mut(promise).result(jsc_vm_ref); - self.set_last_error(rejection); - self.print_js_error(rejection); - self.disable_signals_during_wait(); - return; - } - PromiseStatus::Pending => { - self.disable_signals_during_wait(); - return; - } - } - self.disable_signals_during_wait(); - } - - let mut actual_result = resolved_result; - if resolved_result.is_object() { - let maybe_value = - match resolved_result.get_own(global, &bun_core::String::static_("value")) { - Ok(v) => v, - Err(err) => { - let exc = global.take_exception(err); - self.set_last_error(exc); - self.print_js_error(exc); - vm.as_mut().tick(); - return; - } - }; - if let Some(value) = maybe_value { - actual_result = value; + if let Err(err) = self.copy_value_to_clipboard(actual_result) { + if let Some(global) = self.global { + let exc = global.take_exception(err); + self.set_last_error(exc); + self.print_js_error(exc); } } - - self.set_last_result(actual_result); - if !actual_result.is_undefined() { - let global_this = global.to_js_value(); - global_this.put(global, b"_", actual_result); - } - - if let Err(err) = self.copy_value_to_clipboard(actual_result) { - let exc = global.take_exception(err); - self.set_last_error(exc); - self.print_js_error(exc); + if let Some(vm) = self.vm { + vm.as_mut().tick(); } - vm.as_mut().tick(); } /// Format a JS value as a string suitable for clipboard. diff --git a/src/runtime/cli/repl_command.rs b/src/runtime/cli/repl_command.rs index 061ac4de2b1a..71675e1d5fd5 100644 --- a/src/runtime/cli/repl_command.rs +++ b/src/runtime/cli/repl_command.rs @@ -98,20 +98,7 @@ impl ReplCommand { // ReplRunner construction to avoid a move-after-borrow. // Configure bundler options - // `BundleOptions.install` is `Option>` so no - // lifetime-extension cast is needed. - let install_ptr = ctx.install.as_deref().map(core::ptr::NonNull::from); - b.options.install = install_ptr; - b.resolver.opts.install = install_ptr; - b.resolver.opts.global_cache = ctx.debug.global_cache; - let offline = ctx - .debug - .offline_mode_setting - .unwrap_or(OfflineMode::Online); - b.resolver.opts.install_preference = offline; - b.options.global_cache = b.resolver.opts.global_cache; - b.options.install_preference = offline; - b.resolver.env_loader = NonNull::new(b.env); + crate::cli::run_command::wire_install_options(b, ctx); b.options.env.behavior = EnvBehavior::LoadAllWithoutInlining; b.options.dead_code_elimination = false; // REPL needs all code @@ -294,4 +281,3 @@ unsafe extern "C" { } use bun_bundler::options::EnvBehavior; -use bun_options_types::offline_mode::OfflineMode; diff --git a/src/runtime/cli/run_command.rs b/src/runtime/cli/run_command.rs index aa99cd121241..d76c950f07e6 100644 --- a/src/runtime/cli/run_command.rs +++ b/src/runtime/cli/run_command.rs @@ -90,6 +90,33 @@ impl Default for ExecCfg { } } +/// Shared ctx→install/global-cache/offline-mode option projection for a +/// transpiler and its resolver, used by the boot paths that run user code +/// (`bun run`, the repl). +pub(crate) fn wire_install_options(b: &mut Transpiler<'_>, ctx: &ContextData) { + use bun_options_types::offline_mode::OfflineMode; + + // `BundleOptions::install` is a raw `NonNull` backref into + // the CLI's `Box` (process-lifetime). + // `as_deref` yields `&BunInstall`, which + // `NonNull::from` converts without the lifetime tie. + let install_ptr = ctx.install.as_deref().map(::core::ptr::NonNull::from); + b.options.install = install_ptr; + b.resolver.opts.install = install_ptr; + b.resolver.opts.global_cache = ctx.debug.global_cache; + let offline = ctx + .debug + .offline_mode_setting + .unwrap_or(OfflineMode::Online); + b.resolver.opts.install_preference = offline; + b.options.global_cache = ctx.debug.global_cache; + b.options.install_preference = offline; + // Stored as `NonNull` (not `&Loader`): `configure_defines()` later + // reborrows the same allocation as `&mut Loader`, which would alias a + // live `&Loader`. The Loader outlives the resolver. + b.resolver.env_loader = ::core::ptr::NonNull::new(b.env); +} + pub(crate) struct RunCommand; impl RunCommand { @@ -773,24 +800,8 @@ Full documentation is available at https://bun.com/docs/cli/run /// [`boot_standalone`]. fn wire_transpiler_from_ctx(b: &mut Transpiler<'_>, ctx: &mut ContextData) { use bun_options_types::context::MacroOptions; - use bun_options_types::offline_mode::OfflineMode; - - // `BundleOptions::install` is a raw `NonNull` backref into - // the CLI's `Box` (process-lifetime). - // `as_deref` yields `&BunInstall`, which - // `NonNull::from` converts without the lifetime tie. - let install_ptr = ctx.install.as_deref().map(::core::ptr::NonNull::from); - b.options.install = install_ptr; - b.resolver.opts.install = install_ptr; - b.resolver.opts.global_cache = ctx.debug.global_cache; - let offline = ctx - .debug - .offline_mode_setting - .unwrap_or(OfflineMode::Online); - b.resolver.opts.install_preference = offline; - b.options.global_cache = ctx.debug.global_cache; - b.options.install_preference = offline; - b.resolver.env_loader = ::core::ptr::NonNull::new(b.env); + + wire_install_options(b, ctx); b.options.minify_identifiers = ctx.bundler_options.minify_identifiers; b.options.minify_whitespace = ctx.bundler_options.minify_whitespace; diff --git a/src/runtime/cli/run_processes_shared.rs b/src/runtime/cli/run_processes_shared.rs new file mode 100644 index 000000000000..d78b2c2b9533 --- /dev/null +++ b/src/runtime/cli/run_processes_shared.rs @@ -0,0 +1,121 @@ +//! Helpers shared by the multi-process script runners: `filter_run` +//! (`bun run --filter`) and `multi_run` (`bun run --parallel`/`--sequential`). + +use std::sync::atomic::{AtomicBool, Ordering}; + +use crate::api::bun::process::{self as spawn, Process, Rusage, Status}; + +/// Set from a signal handler; polled by the run loops. +pub(crate) static SHOULD_ABORT: AtomicBool = AtomicBool::new(false); + +pub(crate) struct AbortHandler; + +impl AbortHandler { + #[cfg(unix)] + extern "C" fn posix_signal_handler( + _sig: i32, + _info: *const bun_sys::posix::siginfo_t, + _: *const core::ffi::c_void, + ) { + SHOULD_ABORT.store(true, Ordering::SeqCst); + } + + #[cfg(windows)] + extern "system" fn windows_ctrl_handler( + dw_ctrl_type: bun_sys::windows::DWORD, + ) -> bun_sys::windows::BOOL { + if dw_ctrl_type == bun_sys::windows::CTRL_C_EVENT { + SHOULD_ABORT.store(true, Ordering::SeqCst); + return bun_sys::windows::TRUE; + } + bun_sys::windows::FALSE + } + + pub(crate) fn install() { + #[cfg(unix)] + { + // SAFETY: all-zero is a valid `libc::sigaction`; sigemptyset/sigaction are + // FFI calls with no extra preconditions beyond valid pointers. + unsafe { + let mut action: bun_sys::posix::Sigaction = bun_core::ffi::zeroed(); + action.sa_sigaction = Self::posix_signal_handler as *const () as usize; + libc::sigemptyset(&raw mut action.sa_mask); + action.sa_flags = (libc::SA_SIGINFO | libc::SA_RESTART | libc::SA_RESETHAND) as _; + bun_sys::posix::sigaction(libc::SIGINT, &raw const action, core::ptr::null_mut()); + } + } + #[cfg(not(unix))] + { + let res = bun_sys::windows::SetConsoleCtrlHandler( + Some(Self::windows_ctrl_handler), + bun_sys::windows::TRUE, + ); + if res == 0 { + if bun_core::env::IS_DEBUG { + bun_core::warn!("Failed to set abort handler\n"); + } + } + } + } + + pub(crate) fn uninstall() { + // only necessary on Windows, as on posix we pass the SA_RESETHAND flag + #[cfg(windows)] + { + // (None, FALSE) only clears the ignore attribute; unregistering the + // handler routine requires passing its address. + let _ = bun_sys::windows::SetConsoleCtrlHandler( + Some(Self::windows_ctrl_handler), + bun_sys::windows::FALSE, + ); + } + } +} + +/// `Process::watch_or_reap` with the shared error fallback: if registration +/// fails and the process has not already exited, synthesize an error exit so +/// the run loop still observes a terminal status. +pub(crate) fn watch_or_reap(process: &mut Process) { + if let Err(err) = process.watch_or_reap() { + if !process.has_exited() { + // SAFETY: all-zero is a valid Rusage (POD C struct) + let rusage = bun_core::ffi::zeroed::(); + process.on_exit(Status::Err(err), &rusage); + } + } +} + +/// First non-zero exit code across all spawned handles; signaled/errored +/// processes map to their signal exit code (or 1). 0 when every spawned +/// process exited cleanly. +pub(crate) fn aggregate_exit_code<'h>(statuses: impl Iterator>) -> u8 { + for status in statuses.flatten() { + match status { + Status::Exited(exited) => { + if exited.code != 0 { + return exited.code; + } + } + Status::Signaled(signal) => { + return bun_sys::SignalCode(*signal).to_exit_code().unwrap_or(1); + } + _ => return 1, + } + } + 0 +} + +/// A `Stdio::Buffer` slot for `SpawnOptions`; on Windows this carries a freshly +/// allocated libuv pipe whose ownership moves into the spawn result. +pub(crate) fn buffered_stdio() -> spawn::Stdio { + #[cfg(unix)] + { + spawn::Stdio::Buffer + } + #[cfg(not(unix))] + { + spawn::Stdio::Buffer(bun_core::heap::into_raw(Box::new(bun_core::ffi::zeroed::< + bun_sys::windows::libuv::Pipe, + >()))) + } +} diff --git a/src/runtime/cli/unlink_command.rs b/src/runtime/cli/unlink_command.rs index cfababcd13c5..55001cc46995 100644 --- a/src/runtime/cli/unlink_command.rs +++ b/src/runtime/cli/unlink_command.rs @@ -5,13 +5,11 @@ use bun_core::{Global, Output}; use bun_paths::{AbsPath, PathBuffer, platform, resolve_path}; use bun_sys::{self as sys, Dir, Fd, FdDirExt}; -use bun_install::Features; use bun_install::bin as stub_bin; use bun_install::bin_real as bin; -use bun_install::lockfile_real::{Lockfile, package::Package}; use bun_install::package_manager_real::{ self as pm, CommandLineArguments, Subcommand, attempt_to_create_package_json, - global_link_dir_path, options::LogLevel, package_manager_options, setup_global_dir, + global_link_dir_path, options::LogLevel, }; use crate::command::ContextData; @@ -50,66 +48,11 @@ fn unlink(ctx: &mut ContextData) -> crate::Result<()> { if manager.options.positionals.len() == 1 { // bun unlink - let mut lockfile = Lockfile::default(); - let mut package = Package::default(); - // Step 1. parse the nearest package.json file - { - let package_json_source = match bun_ast::to_source( - manager.original_package_json_path.as_zstr(), - Default::default(), - ) { - Ok(s) => s, - Err(e) => { - Output::err_generic( - "failed to read \"{}\" for unlinking: {}", - ( - BStr::new(manager.original_package_json_path.as_bytes()), - BStr::new(e.name()), - ), - ); - Global::crash(); - } - }; - lockfile.init_empty(); - - let mut resolver: () = (); - // `log_mut()` returns a borrow decoupled from `&self`; disjoint - // storage from `&mut PackageManager` (owned by the CLI `Context`). - let log = manager.log_mut(); - package.parse::<()>( - &mut lockfile, - manager, - log, - &package_json_source, - &mut resolver, - Features::FOLDER, - )?; - let name = lockfile.str(&package.name); - if name.is_empty() { - if manager.options.log_level != LogLevel::Silent { - bun_core::pretty_errorln!( - "error: package.json missing \"name\" in \"{}\"", - BStr::new(package_json_source.path.text), - ); - } - Global::crash(); - } else if !strings::is_npm_package_name(name) { - if manager.options.log_level != LogLevel::Silent { - bun_core::pretty_errorln!( - "error: invalid package.json name \"{}\" in \"{}\"", - BStr::new(name), - BStr::new(package_json_source.path.text), - ); - } - Global::crash(); - } - } + let (lockfile, package) = super::link_command::load_package_for_link(manager, "unlinking")?; - // Reshaped for borrowck — `name` borrows `lockfile`; re-derive - // it after the parse block so its lifetime is decoupled from - // `package_json_source` (dropped above) while remaining a slice into - // `lockfile.buffers.string_bytes`. + // `name` is a slice into `lockfile.buffers.string_bytes`, decoupled + // from the helper-local package.json source. let name = lockfile.str(&package.name); match sys::lstat(resolve_path::join_abs_string_z::( @@ -135,36 +78,7 @@ fn unlink(ctx: &mut ContextData) -> crate::Result<()> { } // Step 2. Setup the global directory - let node_modules: Dir = 'brk: { - bin::Linker::ensure_umask(); - let explicit_global_dir: &[u8] = match &ctx.install { - Some(install_) => install_.global_dir.as_deref().unwrap_or(b""), - None => b"", - }; - manager.global_dir = Some(Dir::from_fd(package_manager_options::open_global_dir( - explicit_global_dir, - )?)); - - setup_global_dir(manager, &&mut *ctx)?; - - match manager - .global_dir - .as_ref() - .unwrap() - .make_open_path(b"node_modules", Default::default()) - { - Ok(d) => break 'brk d, - Err(e) => { - if manager.options.log_level != LogLevel::Silent { - bun_core::pretty_errorln!( - "error: failed to create node_modules in global dir due to error {}", - BStr::new(e.name()), - ); - } - Global::crash(); - } - } - }; + let node_modules: Dir = super::link_command::open_global_node_modules(manager, &mut *ctx)?; // Step 3b. Link any global bins if package.bin.tag != stub_bin::Tag::None { diff --git a/src/runtime/cli/update_interactive_command.rs b/src/runtime/cli/update_interactive_command.rs index fbab8741e3a3..61d684eaebbe 100644 --- a/src/runtime/cli/update_interactive_command.rs +++ b/src/runtime/cli/update_interactive_command.rs @@ -8,13 +8,10 @@ use bstr::BStr; use bun_alloc::Arena as Bump; use bun_collections::StringHashMap; use bun_core::{Global, Output}; -use bun_glob as glob; use bun_install::dependency::{self, Behavior}; use bun_install::lockfile::package::PackageColumns as _; -use bun_install::lockfile::{LoadResult, LoadStep}; use bun_install::package_manager::{ - LogLevel, ManifestLoad, ROOT_PACKAGE_JSON_PATH, Subcommand, WorkspaceFilter, - install_with_manager, populate_manifest_cache, + ManifestLoad, ROOT_PACKAGE_JSON_PATH, Subcommand, install_with_manager, populate_manifest_cache, }; use bun_install::{ CommandLineArguments, GetJsonOptions, GetJsonResult, INVALID_PACKAGE_ID, PackageID, @@ -36,6 +33,7 @@ use bun_paths::{self as path, PathBuffer}; use bun_semver::{self as semver, SlicedString}; use crate::Command; +use crate::cli::workspace_helpers; pub(crate) struct TerminalHyperlink<'a> { link: &'a [u8], @@ -495,60 +493,13 @@ impl UpdateInteractiveCommand { original_cwd: &[u8], manager: &mut PackageManager, ) -> crate::Result<()> { - // Reshaped for borrowck — capture `log_level` / `ctx.log` - // before borrowing `&mut manager.lockfile`. - let not_silent = manager.options.log_level != LogLevel::Silent; - let ctx_log_ptr: *mut bun_ast::Log = ctx.log; - - match manager.load_lockfile_from_cwd::() { - LoadResult::NotFound => { - if not_silent { - Output::err_generic("missing lockfile, nothing outdated", ()); - } - Global::crash(); - } - LoadResult::Err(cause) => { - if not_silent { - match cause.step { - LoadStep::OpenFile => Output::err_generic( - "failed to open lockfile: {s}", - (cause.value.name(),), - ), - LoadStep::ParseFile => Output::err_generic( - "failed to parse lockfile: {s}", - (cause.value.name(),), - ), - LoadStep::ReadFile => Output::err_generic( - "failed to read lockfile: {s}", - (cause.value.name(),), - ), - LoadStep::Migrating => Output::err_generic( - "failed to migrate lockfile: {s}", - (cause.value.name(),), - ), - } - // SAFETY: `ctx.log` is set by `Command::create_context_data` - // for every subcommand and is non-null for the command's - // lifetime. - if unsafe { (*ctx_log_ptr).has_errors() } { - manager - .log_mut() - .print(std::ptr::from_mut(Output::error_writer()))?; - } - } - Global::crash(); - } - LoadResult::Ok(_) => { - // `load_lockfile_from_cwd` populates `manager.lockfile` (Box) - // in place, so no reassignment is needed. - } - } + workspace_helpers::load_lockfile_or_crash(ctx, manager); let workspace_pkg_ids: Vec = if !manager.options.filter_patterns.is_empty() { let filters = manager.options.filter_patterns; - Self::find_matching_workspaces(original_cwd, manager, filters) + workspace_helpers::find_matching_workspaces(original_cwd, manager, filters) } else if manager.options.do_.recursive() { - Self::get_all_workspaces(manager) + workspace_helpers::get_all_workspaces(manager) } else { let root_pkg_id = manager .root_package_id @@ -731,113 +682,6 @@ impl UpdateInteractiveCommand { Ok(()) } - fn get_all_workspaces(manager: &PackageManager) -> Vec { - let lockfile = &manager.lockfile; - let packages = lockfile.packages.slice(); - let pkg_resolutions = packages.items_resolution(); - - let mut workspace_pkg_ids: Vec = Vec::new(); - for (pkg_id, resolution) in pkg_resolutions.iter().enumerate() { - if resolution.tag != resolution::Tag::Workspace - && resolution.tag != resolution::Tag::Root - { - continue; - } - workspace_pkg_ids.push(pkg_id as PackageID); - } - workspace_pkg_ids - } - - fn find_matching_workspaces( - original_cwd: &[u8], - manager: &PackageManager, - filters: &[&[u8]], - ) -> Vec { - let lockfile = &manager.lockfile; - let packages = lockfile.packages.slice(); - let pkg_names = packages.items_name(); - let pkg_resolutions = packages.items_resolution(); - let string_buf = lockfile.buffers.string_bytes.as_slice(); - - let mut workspace_pkg_ids: Vec = Vec::new(); - for (pkg_id, resolution) in pkg_resolutions.iter().enumerate() { - if resolution.tag != resolution::Tag::Workspace - && resolution.tag != resolution::Tag::Root - { - continue; - } - workspace_pkg_ids.push(pkg_id as PackageID); - } - - let mut path_buf = PathBuffer::uninit(); - - let converted_filters: Vec = filters - .iter() - .map(|filter| { - WorkspaceFilter::init(filter, original_cwd, &mut path_buf.0).expect("OOM") - }) - .collect(); - // `defer { filter.deinit(allocator); allocator.free(...) }` — implicit via Drop. - - // SAFETY: `FileSystem::init` ran during `PackageManager::init`. - let top_level_dir = FileSystem::get().top_level_dir; - - // move all matched workspaces to front of array - let mut i: usize = 0; - while i < workspace_pkg_ids.len() { - let workspace_pkg_id = workspace_pkg_ids[i]; - - let matched = 'matched: { - for filter in &converted_filters { - match filter { - WorkspaceFilter::Path(pattern) => { - if pattern.is_empty() { - continue; - } - let res = &pkg_resolutions[workspace_pkg_id as usize]; - let res_path: &[u8] = match res.tag { - resolution::Tag::Workspace => res.workspace().slice(string_buf), - resolution::Tag::Root => top_level_dir, - _ => unreachable!(), - }; - - let abs_res_path = path::resolve_path::join_abs_string_buf::< - path::platform::Posix, - >( - top_level_dir, &mut path_buf.0, &[res_path] - ); - - if !glob::r#match( - pattern, - strings::without_trailing_slash(abs_res_path), - ) - .matches() - { - break 'matched false; - } - } - WorkspaceFilter::Name(pattern) => { - let name = pkg_names[workspace_pkg_id as usize].slice(string_buf); - if !glob::r#match(pattern, name).matches() { - break 'matched false; - } - } - WorkspaceFilter::All => {} - } - } - true - }; - - if matched { - i += 1; - } else { - workspace_pkg_ids.swap_remove(i); - } - } - - workspace_pkg_ids - } - fn group_catalog_dependencies( packages: Vec, ) -> crate::Result> { diff --git a/src/runtime/cli/workspace_helpers.rs b/src/runtime/cli/workspace_helpers.rs new file mode 100644 index 000000000000..f91ef5041606 --- /dev/null +++ b/src/runtime/cli/workspace_helpers.rs @@ -0,0 +1,160 @@ +//! Workspace/lockfile helpers shared by `bun outdated` and +//! `bun update --interactive`. + +use bun_core::strings; +use bun_core::{Global, Output}; +use bun_glob as glob; +use bun_install::lockfile::package::PackageColumns as _; +use bun_install::lockfile::{LoadResult, LoadStep}; +use bun_install::package_manager::{LogLevel, WorkspaceFilter}; +use bun_install::{PackageID, PackageManager, resolution}; +use bun_paths::{self as path, PathBuffer}; +use bun_resolver::fs::FileSystem; + +use crate::Command; + +/// Load the lockfile from the current directory, reporting errors and exiting +/// the process on failure. +pub(crate) fn load_lockfile_or_crash(ctx: &Command::ContextData, manager: &mut PackageManager) { + let not_silent = manager.options.log_level != LogLevel::Silent; + match manager.load_lockfile_from_cwd::() { + LoadResult::NotFound => { + if not_silent { + Output::err_generic("missing lockfile, nothing outdated", ()); + } + Global::crash(); + } + LoadResult::Err(cause) => { + if not_silent { + match cause.step { + LoadStep::OpenFile => { + Output::err_generic("failed to open lockfile: {s}", (cause.value.name(),)); + } + LoadStep::ParseFile => { + Output::err_generic("failed to parse lockfile: {s}", (cause.value.name(),)); + } + LoadStep::ReadFile => { + Output::err_generic("failed to read lockfile: {s}", (cause.value.name(),)); + } + LoadStep::Migrating => { + Output::err_generic( + "failed to migrate lockfile: {s}", + (cause.value.name(),), + ); + } + } + if ctx.log_ref().has_errors() { + let _ = manager + .log_mut() + .print(std::ptr::from_mut(Output::error_writer())); + } + } + Global::crash(); + } + LoadResult::Ok(_) => { + // `load_from_cwd(&mut self, ..)` populates the lockfile in place, + // so no reassignment is needed. + } + } +} + +/// Collect the package IDs of the root package and every workspace package. +pub(crate) fn get_all_workspaces(manager: &PackageManager) -> Vec { + let lockfile = &manager.lockfile; + let packages = lockfile.packages.slice(); + let pkg_resolutions = packages.items_resolution(); + + let mut workspace_pkg_ids: Vec = Vec::new(); + for (pkg_id, resolution) in pkg_resolutions.iter().enumerate() { + if resolution.tag != resolution::Tag::Workspace && resolution.tag != resolution::Tag::Root { + continue; + } + workspace_pkg_ids.push(pkg_id as PackageID); + } + workspace_pkg_ids +} + +/// Collect the workspace package IDs matching the `--filter` patterns. +pub(crate) fn find_matching_workspaces( + original_cwd: &[u8], + manager: &PackageManager, + filters: &[&[u8]], +) -> Vec { + let lockfile = &manager.lockfile; + let packages = lockfile.packages.slice(); + let pkg_names = packages.items_name(); + let pkg_resolutions = packages.items_resolution(); + let string_buf = lockfile.buffers.string_bytes.as_slice(); + + let mut workspace_pkg_ids = get_all_workspaces(manager); + + let mut path_buf = PathBuffer::uninit(); + + let converted_filters: Vec = filters + .iter() + .map(|filter| { + bun_core::handle_oom(WorkspaceFilter::init(filter, original_cwd, &mut path_buf.0)) + }) + .collect(); + // `defer { filter.deinit(allocator); allocator.free(...) }` — implicit via Drop. + + // SAFETY: `FileSystem::init` runs during `PackageManager::init` so the + // process-singleton is populated. + let top_level_dir = FileSystem::get().top_level_dir; + + // move all matched workspaces to front of array + let mut i: usize = 0; + while i < workspace_pkg_ids.len() { + let workspace_pkg_id = workspace_pkg_ids[i]; + + let matched = 'matched: { + for filter in &converted_filters { + match filter { + WorkspaceFilter::Path(pattern) => { + if pattern.is_empty() { + continue; + } + let res = &pkg_resolutions[workspace_pkg_id as usize]; + let res_path: &[u8] = match res.tag { + resolution::Tag::Workspace => { + // Borrow the field in-place so the returned slice (which may + // point into the inline small-string storage) stays valid. + res.workspace().slice(string_buf) + } + resolution::Tag::Root => top_level_dir, + _ => unreachable!(), + }; + + let abs_res_path = path::resolve_path::join_abs_string_buf::< + path::platform::Posix, + >( + top_level_dir, &mut path_buf.0, &[res_path] + ); + + if !glob::r#match(pattern, strings::without_trailing_slash(abs_res_path)) + .matches() + { + break 'matched false; + } + } + WorkspaceFilter::Name(pattern) => { + let name = pkg_names[workspace_pkg_id as usize].slice(string_buf); + if !glob::r#match(pattern, name).matches() { + break 'matched false; + } + } + WorkspaceFilter::All => {} + } + } + true + }; + + if matched { + i += 1; + } else { + workspace_pkg_ids.swap_remove(i); + } + } + + workspace_pkg_ids +} From 4bdc74735f313a5c1e0176d544547f4f08310fa0 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 10 Jun 2026 03:23:03 +0000 Subject: [PATCH 2/7] test: cover the shared CLI error exits for link, pack, outdated, and update --interactive --- test/cli/install/bun-install-registry.test.ts | 15 ++++++++ test/cli/install/bun-link.test.ts | 34 +++++++++++++++++++ test/cli/install/bun-pack.test.ts | 7 ++++ test/cli/update_interactive_install.test.ts | 21 ++++++++++++ 4 files changed, 77 insertions(+) diff --git a/test/cli/install/bun-install-registry.test.ts b/test/cli/install/bun-install-registry.test.ts index f3d9ddbb6936..e2add5ce2498 100644 --- a/test/cli/install/bun-install-registry.test.ts +++ b/test/cli/install/bun-install-registry.test.ts @@ -8287,6 +8287,21 @@ describe("outdated", () => { expect(rest).toMatchSnapshot(); }); } + test("errors without a lockfile", async () => { + await write(packageJson, JSON.stringify({ name: "no-lockfile", version: "1.0.0" })); + + const { stdout, stderr, exited } = spawn({ + cmd: [bunExe(), "outdated"], + cwd: packageDir, + stdout: "pipe", + stderr: "pipe", + env, + }); + + const [err, _out, exitCode] = await Promise.all([stderr.text(), stdout.text(), exited]); + expect(err).toContain("error: missing lockfile, nothing outdated"); + expect(exitCode).toBe(1); + }); test("in workspace", async () => { await Promise.all([ write( diff --git a/test/cli/install/bun-link.test.ts b/test/cli/install/bun-link.test.ts index 8a937dad63fd..492adf9e33c7 100644 --- a/test/cli/install/bun-link.test.ts +++ b/test/cli/install/bun-link.test.ts @@ -471,3 +471,37 @@ it("should link dependency without crashing", async () => { // This should fail with a non-zero exit code. expect(await exited4).toBe(1); }); + +for (const command of ["link", "unlink"]) { + it(`should error when ${command}ing a package without a name`, async () => { + await writeFile(join(link_dir, "package.json"), JSON.stringify({ version: "0.0.1" })); + + const { stdout, stderr, exited } = spawn({ + cmd: [bunExe(), command], + cwd: link_dir, + stdout: "pipe", + stdin: "ignore", + stderr: "pipe", + env, + }); + const [err, _out, exitCode] = await Promise.all([stderr.text(), stdout.text(), exited]); + expect(err).toContain('error: package.json missing "name"'); + expect(exitCode).toBe(1); + }); +} + +it("should error when linking a package with an invalid name", async () => { + await writeFile(join(link_dir, "package.json"), JSON.stringify({ name: "NOT a valid name!", version: "0.0.1" })); + + const { stdout, stderr, exited } = spawn({ + cmd: [bunExe(), "link"], + cwd: link_dir, + stdout: "pipe", + stdin: "ignore", + stderr: "pipe", + env, + }); + const [err, _out, exitCode] = await Promise.all([stderr.text(), stdout.text(), exited]); + expect(err).toContain('error: invalid package.json name "NOT a valid name!"'); + expect(exitCode).toBe(1); +}); diff --git a/test/cli/install/bun-pack.test.ts b/test/cli/install/bun-pack.test.ts index 8e349f865f35..8d7f686c87b5 100644 --- a/test/cli/install/bun-pack.test.ts +++ b/test/cli/install/bun-pack.test.ts @@ -51,6 +51,13 @@ test("basic", async () => { expect(tarball.entries).toMatchObject([{ "pathname": "package/package.json" }, { "pathname": "package/index.js" }]); }); +test("fails when package.json cannot be parsed", async () => { + await write(join(packageDir, "package.json"), '{"name": "pack-bad-json",'); + + const { err } = await packExpectError(packageDir, bunEnv); + expect(err).toContain(`failed to parse package.json: ${join(packageDir, "package.json")}`); +}); + test("in subdirectory", async () => { await Promise.all([ write( diff --git a/test/cli/update_interactive_install.test.ts b/test/cli/update_interactive_install.test.ts index dad6bbbb6053..513d86ad6de1 100644 --- a/test/cli/update_interactive_install.test.ts +++ b/test/cli/update_interactive_install.test.ts @@ -240,3 +240,24 @@ describe.concurrent("bun update --interactive actually installs packages", () => expect(pkg.dependencies["is-even"]).toBe("0.1.0"); }); }); + +describe.concurrent("bun update --interactive error handling", () => { + test("errors without a lockfile", async () => { + using dir = tempDir("update-interactive-no-lockfile", { + "package.json": JSON.stringify({ name: "no-lockfile", version: "1.0.0" }), + }); + + await using proc = Bun.spawn({ + cmd: [bunExe(), "update", "--interactive"], + cwd: String(dir), + env: bunEnv, + stdin: "ignore", + stdout: "pipe", + stderr: "pipe", + }); + + const [stderr, _stdout, exitCode] = await Promise.all([proc.stderr.text(), proc.stdout.text(), proc.exited]); + expect(stderr).toContain("error: missing lockfile, nothing outdated"); + expect(exitCode).toBe(1); + }); +}); From 5652989ac20092b6255084b4184aae7bfb030d2c Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 10 Jun 2026 03:26:34 +0000 Subject: [PATCH 3/7] Drop stale TODO moved by the analyze-branch consolidation --- src/runtime/cli/pm_update_package_json.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/runtime/cli/pm_update_package_json.rs b/src/runtime/cli/pm_update_package_json.rs index 0de2f75c33d1..8a556d01ca4f 100644 --- a/src/runtime/cli/pm_update_package_json.rs +++ b/src/runtime/cli/pm_update_package_json.rs @@ -89,8 +89,6 @@ pub(crate) fn analyze_dependencies_and_install( result: &mut DependenciesScannerResult<'_, '_>, ) -> Result<(), bun_bundler::Error> { let this = self; - // TODO: add separate argument that makes it so positionals[1..] is not done and instead the positionals are passed - // // Process-lifetime storage for the rewritten positionals — // `Global::exit(0)` follows immediately. // `OnceLock` (not leaking) per PORTING.md §Forbidden. From f6fbddf2e09ab8e07b36bce1c24155af4b289d33 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Tue, 11 Aug 2026 02:07:33 +0000 Subject: [PATCH 4/7] Route catalog updates through replacement_version_literal and pin the extractor edge cases edit_catalogs_after_update carried a third copy of the alias-prefix and pin-style logic that replacement_version_literal already covers for direct dependencies; use the helper there too and cover aliased catalog entries. Add fromUrl cases for the per-host differences the shared bitbucket, gist, and sourcehut extractor is parameterized on: the rejected aux segment, missing user or project, gist's optional user, and whether an undecodable segment is reported as "not hosted" or as an invalid URL. --- .../PackageManager/PackageJSONEditor.rs | 61 +++---------------- test/cli/install/catalogs.test.ts | 50 +++++++++++++++ .../install/hosted-git-info/from-url.test.ts | 39 ++++++++++++ 3 files changed, 96 insertions(+), 54 deletions(-) diff --git a/src/install/PackageManager/PackageJSONEditor.rs b/src/install/PackageManager/PackageJSONEditor.rs index aca597a00b13..5f45ea4894e7 100644 --- a/src/install/PackageManager/PackageJSONEditor.rs +++ b/src/install/PackageManager/PackageJSONEditor.rs @@ -708,60 +708,13 @@ pub(crate) fn edit_catalogs_after_update( } let info = &infos[index]; - let version_fmt = resolution.npm().version.fmt(string_buf); - let new_version: Vec = 'new_version: { - if options.exact_versions { - let mut v = Vec::new(); - write!(&mut v, "{}", version_fmt).expect("infallible: in-memory write"); - break 'new_version v; - } - - let version_literal: &[u8] = 'version_literal: { - if !info.is_alias { - break 'version_literal &info.original_version_literal; - } - if let Some(at_index) = - strings::last_index_of_char(&info.original_version_literal, b'@') - { - break 'version_literal &info.original_version_literal[at_index + 1..]; - } - &info.original_version_literal - }; - - let pinned_version = semver::Version::which_version_is_pinned(version_literal); - let mut v = Vec::new(); - match pinned_version { - semver::PinnedVersion::Patch => { - write!(&mut v, "{}", version_fmt).expect("infallible: in-memory write") - } - semver::PinnedVersion::Minor => { - write!(&mut v, "~{}", version_fmt).expect("infallible: in-memory write") - } - semver::PinnedVersion::Major => { - write!(&mut v, "^{}", version_fmt).expect("infallible: in-memory write") - } - } - v - }; - - new_literals[index] = Some(if info.is_alias { - let dep_literal = &info.original_version_literal; - if let Some(at_index) = strings::last_index_of_char(dep_literal, b'@') { - let mut v = Vec::new(); - write!( - &mut v, - "{}@{}", - bstr::BStr::new(&dep_literal[0..at_index]), - bstr::BStr::new(&new_version) - ) - .expect("infallible: in-memory write"); - v - } else { - new_version - } - } else { - new_version - }); + new_literals[index] = Some(replacement_version_literal( + resolution.npm().version.fmt(string_buf), + &info.original_version_literal, + info.is_alias, + &info.original_version_literal, + options.exact_versions, + )); } let mut changed = false; diff --git a/test/cli/install/catalogs.test.ts b/test/cli/install/catalogs.test.ts index 08cc2b4044db..424c8a1a4aff 100644 --- a/test/cli/install/catalogs.test.ts +++ b/test/cli/install/catalogs.test.ts @@ -360,6 +360,56 @@ describe("update", () => { expect(exitCode).toBe(0); }); + test("--latest keeps the npm: alias prefix and pin style of aliased catalog entries", async () => { + const { packageDir } = await registry.createTestDir(); + await Promise.all([ + write( + join(packageDir, "package.json"), + JSON.stringify({ + name: "catalog-update-alias", + workspaces: { + packages: ["packages/*"], + catalog: { + "aliased": "npm:no-deps@^1.0.0", + }, + catalogs: { + pinned: { + "aliased": "npm:no-deps@1.0.1", + }, + }, + }, + }), + ), + write( + join(packageDir, "packages", "pkg1", "package.json"), + JSON.stringify({ + name: "pkg1", + dependencies: { + "aliased": "catalog:", + }, + }), + ), + write( + join(packageDir, "packages", "pkg2", "package.json"), + JSON.stringify({ + name: "pkg2", + dependencies: { + "aliased": "catalog:pinned", + }, + }), + ), + ]); + await runBunInstall(bunEnv, packageDir); + + const { err, exitCode } = await runUpdate(packageDir, "--latest"); + expect(err).not.toContain("error:"); + + const root = await file(join(packageDir, "package.json")).json(); + expect(root.workspaces.catalog).toEqual({ "aliased": "npm:no-deps@^2.0.0" }); + expect(root.workspaces.catalogs.pinned).toEqual({ "aliased": "npm:no-deps@2.0.0" }); + expect(exitCode).toBe(0); + }); + for (const fromWorkspace of [false, true]) { test(`--latest --dry-run does not modify any package.json (from ${fromWorkspace ? "workspace" : "root"})`, async () => { const { packageDir } = await registry.createTestDir(); diff --git a/test/cli/install/hosted-git-info/from-url.test.ts b/test/cli/install/hosted-git-info/from-url.test.ts index e6cb2ab12c88..048abf55dff6 100644 --- a/test/cli/install/hosted-git-info/from-url.test.ts +++ b/test/cli/install/hosted-git-info/from-url.test.ts @@ -18,6 +18,45 @@ describe("fromUrl", () => { }); }); + // bitbucket, gist, and sourcehut share the `/user/project[/aux]` extractor; + // these pin the per-host differences it is parameterized on. + describe("user/project extractors", () => { + it.each([ + // tarball-ish aux segment rejected per host + "https://bitbucket.org/foo/bar/get/archive.tar.gz", + "https://gist.github.com/foo/feedbeef/raw/fix%2Fbug/", + "https://git.sr.ht/~foo/bar/archive/HEAD.tar.gz", + // missing project (gist: missing both user and project) + "https://bitbucket.org/foo", + "https://git.sr.ht/~foo", + "https://gist.github.com/", + // user is required everywhere except gist + "https://bitbucket.org//bar", + "https://git.sr.ht//bar", + ])("%s is not a hosted git url", url => { + expect(hostedGitInfo.fromUrl(url)).toBeNull(); + }); + + it.each([ + ["https://gist.github.com/feedbeef", null], + ["https://gist.github.com//feedbeef", null], + ["https://gist.github.com/foo/feedbeef", "foo"], + ])("gist %s has user %p", (url, user) => { + expect(hostedGitInfo.fromUrl(url)).toMatchObject({ type: "gist", user, project: "feedbeef" }); + }); + + it.each(["https://gist.github.com/foo/bar%0N", "https://git.sr.ht/~foo/bar%0N"])( + "%s with an undecodable project is not a hosted git url", + url => { + expect(hostedGitInfo.fromUrl(url)).toBeNull(); + }, + ); + + it("bitbucket rejects an undecodable project as an invalid url", () => { + expect(() => hostedGitInfo.fromUrl("https://bitbucket.org/foo/bar%0N")).toThrow("Invalid Git URL: InvalidURL"); + }); + }); + // TODO(markovejnovic): Unskip these tests. describe.skip("invalid urls", () => { describe.each(Object.entries(invalidGitUrls))("%s", (_, urls: (string | null | undefined)[]) => { From 9ae3ff521b1a072783edd7545f8cb95b9da4a880 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Tue, 11 Aug 2026 02:07:33 +0000 Subject: [PATCH 5/7] Narrow get_with_path_or_exit to the crate and refresh two helper comments The helper has no callers outside bun_install. The pack and repl comments described the helpers in terms of code that no longer exists. --- src/install/PackageManager/WorkspacePackageJSONCache.rs | 2 +- src/runtime/cli/pack_command.rs | 5 ++--- src/runtime/cli/repl.rs | 6 +++--- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/install/PackageManager/WorkspacePackageJSONCache.rs b/src/install/PackageManager/WorkspacePackageJSONCache.rs index 8b6d4a75dce6..cc6a5fda8ede 100644 --- a/src/install/PackageManager/WorkspacePackageJSONCache.rs +++ b/src/install/PackageManager/WorkspacePackageJSONCache.rs @@ -209,7 +209,7 @@ impl WorkspacePackageJSONCache { /// `get_with_path`, except read/parse failures are fatal: pending log /// messages and the error are printed to stderr, then the process exits. - pub fn get_with_path_or_exit( + pub(crate) fn get_with_path_or_exit( &mut self, log: &mut Log, abs_package_json_path: &[u8], diff --git a/src/runtime/cli/pack_command.rs b/src/runtime/cli/pack_command.rs index 5ca2f4b3552c..72e260ae6098 100644 --- a/src/runtime/cli/pack_command.rs +++ b/src/runtime/cli/pack_command.rs @@ -1894,9 +1894,8 @@ fn opt_pack_gzip_level(m: &PackageManager) -> Option<&[u8]> { /// Reads `abs_package_json_path` through the workspace package.json cache; /// read/parse failures are fatal. Unlike /// `WorkspacePackageJSONCache::get_with_path_or_exit`, this keeps pack's -/// error wording and ordering (`Output::err` first, then the log printed -/// unconditionally on parse errors), matching the `pack_command.zig` -/// reference. +/// existing error wording and ordering (`Output::err` first, then the log +/// printed unconditionally on parse errors), which bun-pack.test.ts asserts. fn load_package_json_or_exit<'a>( manager_ptr: *mut PackageManager, abs_package_json_path: &ZStr, diff --git a/src/runtime/cli/repl.rs b/src/runtime/cli/repl.rs index fb9313e0bdd7..7f0dd9c518d3 100644 --- a/src/runtime/cli/repl.rs +++ b/src/runtime/cli/repl.rs @@ -581,12 +581,12 @@ enum ReplResult { } /// How `evaluate_to_value` reports promise rejections and interrupts. -/// Mirrors the difference between repl.zig's evaluateAndPrint (sets `_error` -/// on globalThis, prints a newline on interrupt) and evaluateAndCopy (does -/// neither). #[derive(Clone, Copy, PartialEq, Eq)] enum ReportMode { + /// `evaluate_and_print`: a rejection also sets `_error` on globalThis, and + /// a promise still pending after the wait prints a newline. Print, + /// `evaluate_and_copy`: neither of the above. Copy, } From ca15187e329e27cf583bbfca52df9ae53b1f2230 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Tue, 11 Aug 2026 02:51:19 +0000 Subject: [PATCH 6/7] pack: correct the archive_pack_queue exit note and document load_package_json_or_exit's lifetime; pin per-host aux segments in fromUrl tests --- src/runtime/cli/pack_command.rs | 12 +++++++++--- test/cli/install/hosted-git-info/from-url.test.ts | 12 ++++++++++++ 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/src/runtime/cli/pack_command.rs b/src/runtime/cli/pack_command.rs index 72e260ae6098..5fc7cb4bf222 100644 --- a/src/runtime/cli/pack_command.rs +++ b/src/runtime/cli/pack_command.rs @@ -1896,6 +1896,10 @@ fn opt_pack_gzip_level(m: &PackageManager) -> Option<&[u8]> { /// `WorkspacePackageJSONCache::get_with_path_or_exit`, this keeps pack's /// existing error wording and ordering (`Output::err` first, then the log /// printed unconditionally on parse errors), which bun-pack.test.ts asserts. +/// +/// `'a` is unbounded: the entry lives in `workspace_package_json_cache`, so +/// the reference is invalid once that map is mutated (`pack` removes the +/// entry after lifecycle scripts run and immediately reloads it). fn load_package_json_or_exit<'a>( manager_ptr: *mut PackageManager, abs_package_json_path: &ZStr, @@ -3087,9 +3091,11 @@ enum PackQueueOpenMode { /// [`PackQueueOpenMode`] for how each file is opened. Each entry is also /// appended to `pack_list` when provided. /// -/// The loop body's only early exits are `continue` and `Global::crash()` -/// (never returns, no unwinding), so `node.complete_one()` is called -/// explicitly at every loop-body exit instead of via a scope guard. +/// The loop body's early exits are `continue`, `Global::crash()`, and `?` on +/// `AllocError`; the last two never resume (the `AllocError` becomes +/// `PackError::OutOfMemory` and the caller exits via `out_of_memory()`), so +/// `node.complete_one()` is called explicitly before each `continue` instead +/// of via a scope guard. fn archive_pack_queue( ctx: &mut Context<'_>, queue: &mut PackQueue, diff --git a/test/cli/install/hosted-git-info/from-url.test.ts b/test/cli/install/hosted-git-info/from-url.test.ts index 048abf55dff6..9fb773fca772 100644 --- a/test/cli/install/hosted-git-info/from-url.test.ts +++ b/test/cli/install/hosted-git-info/from-url.test.ts @@ -37,6 +37,18 @@ describe("fromUrl", () => { expect(hostedGitInfo.fromUrl(url)).toBeNull(); }); + // The rejected aux segment is per host: another host's segment parses. + it.each([ + ["https://bitbucket.org/foo/bar/raw", { type: "bitbucket", user: "foo", project: "bar" }], + ["https://bitbucket.org/foo/bar/archive", { type: "bitbucket", user: "foo", project: "bar" }], + ["https://gist.github.com/foo/feedbeef/get", { type: "gist", user: "foo", project: "feedbeef" }], + ["https://gist.github.com/foo/feedbeef/archive", { type: "gist", user: "foo", project: "feedbeef" }], + ["https://git.sr.ht/~foo/bar/get", { type: "sourcehut", user: "~foo", project: "bar" }], + ["https://git.sr.ht/~foo/bar/raw", { type: "sourcehut", user: "~foo", project: "bar" }], + ])("%s parses despite the aux segment", (url, expected) => { + expect(hostedGitInfo.fromUrl(url)).toMatchObject(expected); + }); + it.each([ ["https://gist.github.com/feedbeef", null], ["https://gist.github.com//feedbeef", null], From b5b9aaa3dc10e27401e441a6c8071adc37925108 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Tue, 11 Aug 2026 17:19:42 +0000 Subject: [PATCH 7/7] ci: retrigger