diff --git a/src/install/PackageInstall.rs b/src/install/PackageInstall.rs index 43521627fdef..8cc7a2056efe 100644 --- a/src/install/PackageInstall.rs +++ b/src/install/PackageInstall.rs @@ -2278,68 +2278,27 @@ impl<'a> PackageInstall<'a> { let state = manager.get_preinstall_state(package_id); match state { crate::PreinstallState::Done => false, - _ => 'brk: { - if self.patch.is_none() { - let exists = match resolution_tag { - resolution::Tag::Npm => 'package_json_exists: { - // SAFETY: `buf` and `self.cache_dir_subpath` both derive from the - // same thread-local `cached_package_folder_name_buf` raw pointer - // (the debug_assert below checks the subpath aliases this buffer), - // so there is no cross-thread access. No other `&mut` into the - // buffer is created while `buf` is live, and the only writes are - // at indices >= `subpath_len` — past the subpath's contents — with - // the NUL terminator restored by the scopeguard before the borrow - // ends. - let buf: &mut [u8] = unsafe { - (*crate::package_manager::cached_package_folder_name_buf()) - .as_mut_slice() - }; - - debug_assert!(bun_core::is_slice_in_buffer( - self.cache_dir_subpath.as_bytes(), - buf - )); - - let subpath_len = - strings::without_trailing_slash(self.cache_dir_subpath.as_bytes()) - .len(); - buf[subpath_len] = SEP; - // SAFETY: p points into the long-lived cached_package_folder_name_buf; - // subpath_len is in bounds (was the prior NUL position). - let _restore = - scopeguard::guard(buf.as_mut_ptr(), move |p: *mut u8| unsafe { - *p.add(subpath_len) = 0; - }); - buf[subpath_len + 1..subpath_len + 1 + b"package.json\0".len()] - .copy_from_slice(b"package.json\0"); - // SAFETY: NUL written above. - let subpath = - ZStr::from_buf(&buf[..], subpath_len + 1 + b"package.json".len()); - break 'package_json_exists sys::exists_at(self.cache_dir, subpath); - } - _ => sys::directory_exists_at(self.cache_dir, self.cache_dir_subpath) - .unwrap_or(false), - }; - if exists { - manager.set_preinstall_state(package_id, crate::PreinstallState::Done); - } - break 'brk !exists; - } - let idx = strings::last_index_of(self.cache_dir_subpath.as_bytes(), b"_patch_hash=") - .unwrap_or_else(|| { - panic!("Patched dependency cache dir subpath does not have the \"_patch_hash=HASH\" suffix. This is a bug, please file a GitHub issue.") - }); - let cache_dir_subpath_without_patch_hash = - &self.cache_dir_subpath.as_bytes()[..idx]; - // Use a stack PathBuffer (no shared state). - let mut join_buf = PathBuffer::uninit(); - join_buf[..cache_dir_subpath_without_patch_hash.len()] - .copy_from_slice(cache_dir_subpath_without_patch_hash); - join_buf[cache_dir_subpath_without_patch_hash.len()] = 0; - // SAFETY: NUL written above. - let subpath = - ZStr::from_buf(&join_buf[..], cache_dir_subpath_without_patch_hash.len()); - let exists = sys::directory_exists_at(self.cache_dir, subpath).unwrap_or(false); + _ => { + let exists = if self.patch.is_none() { + crate::package_manager::directories::is_package_in_cache_at( + self.cache_dir, + self.cache_dir_subpath, + resolution_tag, + ) + } else { + let idx = + strings::last_index_of(self.cache_dir_subpath.as_bytes(), b"_patch_hash=") + .unwrap_or_else(|| { + panic!("Patched dependency cache dir subpath does not have the \"_patch_hash=HASH\" suffix. This is a bug, please file a GitHub issue.") + }); + let non_patched = + bun_core::ZBox::from_bytes(&self.cache_dir_subpath.as_bytes()[..idx]); + crate::package_manager::directories::is_package_in_cache_at( + self.cache_dir, + &non_patched, + resolution_tag, + ) + }; if exists { manager.set_preinstall_state(package_id, crate::PreinstallState::Done); } diff --git a/src/install/PackageManager/PackageManagerDirectories.rs b/src/install/PackageManager/PackageManagerDirectories.rs index 950f3ef74b54..5a2097bab505 100644 --- a/src/install/PackageManager/PackageManagerDirectories.rs +++ b/src/install/PackageManager/PackageManagerDirectories.rs @@ -754,6 +754,29 @@ pub fn is_folder_in_cache(this: &mut PackageManager, folder_path: &ZStr) -> bool sys::directory_exists_at(get_cache_directory(this), folder_path).unwrap_or(false) } +/// Cache hit for an unpatched entry: npm folders must contain `package.json`, git checkouts the `.bun-tag` written last. +pub fn is_package_in_cache_at(cache_dir: Fd, folder_path: &ZStr, tag: ResolutionTag) -> bool { + let marker: &[u8] = match tag { + ResolutionTag::Npm => b"package.json", + ResolutionTag::Git => b".bun-tag", + _ => return sys::directory_exists_at(cache_dir, folder_path).unwrap_or(false), + }; + let mut buf = PathBuffer::uninit(); + let marker_path = path::resolve_path::join_z_buf::( + &mut buf.0, + &[folder_path.as_bytes(), marker], + ); + sys::exists_at(cache_dir, marker_path) +} + +pub fn is_package_in_cache( + this: &mut PackageManager, + folder_path: &ZStr, + tag: ResolutionTag, +) -> bool { + is_package_in_cache_at(get_cache_directory(this), folder_path, tag) +} + // ─────────────────────────── global directories ─────────────────────────────── pub fn setup_global_dir(manager: &mut PackageManager, ctx: &Command::Context) -> Result<(), Error> { diff --git a/src/install/PackageManager/PackageManagerLifecycle.rs b/src/install/PackageManager/PackageManagerLifecycle.rs index b1845b30b495..63730f9876a4 100644 --- a/src/install/PackageManager/PackageManagerLifecycle.rs +++ b/src/install/PackageManager/PackageManagerLifecycle.rs @@ -158,7 +158,12 @@ impl PackageManager { return PreinstallState::Extract; } - if directories::is_folder_in_cache(self, folder_path) { + let in_cache = if patch_hash.is_some() { + directories::is_folder_in_cache(self, folder_path) + } else { + directories::is_package_in_cache(self, folder_path, pkg.resolution.tag) + }; + if in_cache { self.set_preinstall_state(pkg.meta.id, PreinstallState::Done); return PreinstallState::Done; } @@ -181,7 +186,8 @@ impl PackageManager { }); // Owned NUL-terminated copy. let non_patched_path = ZBox::from_bytes(&folder_path.as_bytes()[..idx]); - if directories::is_folder_in_cache(self, &non_patched_path) { + if directories::is_package_in_cache(self, &non_patched_path, pkg.resolution.tag) + { self.set_preinstall_state(pkg.meta.id, PreinstallState::ApplyPatch); // yay step 1 is already done for us return PreinstallState::ApplyPatch; diff --git a/src/install/PackageManager/patchPackage.rs b/src/install/PackageManager/patchPackage.rs index 6d3a15d1b5ae..49029f7cd401 100644 --- a/src/install/PackageManager/patchPackage.rs +++ b/src/install/PackageManager/patchPackage.rs @@ -159,161 +159,125 @@ pub fn do_patch_commit( }; let mut iterator = tree::Iterator::<{ tree::IteratorPathStyle::NodeModules }>::init(&lockfile); - // reshaped for borrowck — `compute_cache_dir_and_subpath` borrows - // `manager` mutably while the package name/resolution borrow `lockfile` - // (which itself sometimes aliases `manager.lockfile`). Clone the slice/ - // resolution out first, then compute, then assemble the result tuple. - 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()), + let (changes_dir, pkg): (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(); } }; - 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()), + 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 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 json = parsed.root; - 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(); + 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; } - Some(PackageIndexEntry::Id(id)) => *lockfile.packages.get(*id as usize), - Some(PackageIndexEntry::Ids(ids)) => 'brk: { - let mut resolution_label = Vec::new(); - for &id in ids.as_slice() { - let pkg = *lockfile.packages.get(id as usize); - if print_resolution_label( - &mut resolution_label, - &pkg.resolution, - lockfile.buffers.string_bytes.as_slice(), - ) == 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 resolution_clone = actual_package.resolution; - let cache_result = compute_cache_dir_and_subpath( - manager, - &name, - &resolution_clone, - &mut folder_path_buf, - None, + } + bun_core::pretty_error!( + "error: invalid package.json, missing or invalid property \"version\": {}\n", + bstr::BStr::new(package_json_source.path.text()), ); - let cache_dir = cache_result.cache_dir; - let cache_dir_subpath = cache_result.cache_dir_subpath; - - let changes_dir = argument.to_vec(); + Global::crash(); + }; - break 'result (cache_dir, cache_dir_subpath, changes_dir, actual_package); - } - PatchArgKind::NameAndVersion => 'brk: { - let (name, version) = Dependency::split_name_and_maybe_version(argument); - let (pkg_id, node_modules_relative_path) = pkg_info_for_name_and_version( - &lockfile, - &mut iterator, - argument, - name, - version, - ); + 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: { + let mut resolution_label = Vec::new(); + for &id in ids.as_slice() { + let pkg = *lockfile.packages.get(id as usize); + if print_resolution_label( + &mut resolution_label, + &pkg.resolution, + lockfile.buffers.string_bytes.as_slice(), + ) == 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 changes_dir = resolve_path::join_z_buf::( - &mut pathbuf[..], - &[&node_modules_relative_path, name], - ) - .as_bytes() - .to_vec(); - let pkg = *lockfile.packages.get(pkg_id as usize); - - let pkg_name_slice = pkg - .name - .slice(lockfile.buffers.string_bytes.as_slice()) - .to_vec(); - let resolution_clone = pkg.resolution; - let cache_result = compute_cache_dir_and_subpath( - manager, - &pkg_name_slice, - &resolution_clone, - &mut folder_path_buf, - None, - ); - let cache_dir = cache_result.cache_dir; - let cache_dir_subpath = cache_result.cache_dir_subpath; - break 'brk (cache_dir, cache_dir_subpath, changes_dir, pkg); - } - }; + break 'result (argument.to_vec(), actual_package); + } + PatchArgKind::NameAndVersion => 'brk: { + let (name, version) = Dependency::split_name_and_maybe_version(argument); + let (pkg_id, node_modules_relative_path) = + pkg_info_for_name_and_version(&lockfile, &mut iterator, argument, name, version); + + let changes_dir = resolve_path::join_z_buf::( + &mut pathbuf[..], + &[&node_modules_relative_path, name], + ) + .as_bytes() + .to_vec(); + break 'brk (changes_dir, *lockfile.packages.get(pkg_id as usize)); + } + }; - // zls - let cache_dir: Fd = cache_dir; - let cache_dir_subpath: &ZStr = cache_dir_subpath; + // `compute_cache_dir_and_subpath` resolves `pkg.resolution`'s strings against `manager.lockfile`. + manager.lockfile = lockfile; + let name = manager.lockfile.str(&pkg.name).to_vec(); + let cache_result = + compute_cache_dir_and_subpath(manager, &name, &pkg.resolution, &mut folder_path_buf, None); + let cache_dir: Fd = cache_result.cache_dir; + let cache_dir_subpath: &ZStr = cache_result.cache_dir_subpath; let changes_dir: &[u8] = &changes_dir; - let pkg: Package = pkg; + let lockfile: &Lockfile = &manager.lockfile; - let name = pkg.name.slice(lockfile.buffers.string_bytes.as_slice()); + let name = name.as_slice(); let mut patch_key = Vec::new(); write!( &mut patch_key, @@ -653,6 +617,14 @@ fn escape_patch_filename(name: &[u8]) -> Option> { Newline, CarriageReturn, Tab, + // NTFS-reserved; escaped on every OS so a committed patches/ dir checks out on Windows. + Colon, + Question, + Asterisk, + Quote, + LessThan, + GreaterThan, + Pipe, // Dot, Other, } @@ -666,6 +638,13 @@ fn escape_patch_filename(name: &[u8]) -> Option> { EscapeVal::Newline => Some(b"%0A"), EscapeVal::CarriageReturn => Some(b"%0D"), EscapeVal::Tab => Some(b"%09"), + EscapeVal::Colon => Some(b"%3A"), + EscapeVal::Question => Some(b"%3F"), + EscapeVal::Asterisk => Some(b"%2A"), + EscapeVal::Quote => Some(b"%22"), + EscapeVal::LessThan => Some(b"%3C"), + EscapeVal::GreaterThan => Some(b"%3E"), + EscapeVal::Pipe => Some(b"%7C"), // EscapeVal::Dot => Some(b"%2E"), EscapeVal::Other => None, } @@ -681,6 +660,13 @@ fn escape_patch_filename(name: &[u8]) -> Option> { table[b'\n' as usize] = EscapeVal::Newline; table[b'\r' as usize] = EscapeVal::CarriageReturn; table[b'\t' as usize] = EscapeVal::Tab; + table[b':' as usize] = EscapeVal::Colon; + table[b'?' as usize] = EscapeVal::Question; + table[b'*' as usize] = EscapeVal::Asterisk; + table[b'"' as usize] = EscapeVal::Quote; + table[b'<' as usize] = EscapeVal::LessThan; + table[b'>' as usize] = EscapeVal::GreaterThan; + table[b'|' as usize] = EscapeVal::Pipe; table }; let mut count: usize = 0; diff --git a/src/install/isolated_install.rs b/src/install/isolated_install.rs index b406dd64f6cf..9b94cdda2d30 100644 --- a/src/install/isolated_install.rs +++ b/src/install/isolated_install.rs @@ -2302,6 +2302,7 @@ pub(crate) fn install_isolated_packages( continue; } + // Downloads only produce the unpatched folder; `apply_package_patch` derives the rest. // SAFETY: each arm reads the union field that `pkg_res_tag` // (== `pkg_res.tag`) names as active. let cache_subpath_z: &bun_core::ZStr = match pkg_res_tag { @@ -2309,36 +2310,33 @@ pub(crate) fn install_isolated_packages( installer.manager(), pkg_name.slice(string_buf), pkg_res.npm().version, - patch_info.contents_hash(), + None, ), ResolutionTag::Git => package_manager::cached_git_folder_name( installer.manager(), pkg_res.git(), - patch_info.contents_hash(), + None, ), ResolutionTag::Github => package_manager::cached_github_folder_name( installer.manager(), pkg_res.github(), - patch_info.contents_hash(), + None, ), ResolutionTag::LocalTarball => package_manager::cached_tarball_folder_name( installer.manager(), *pkg_res.local_tarball(), - patch_info.contents_hash(), + None, ), ResolutionTag::RemoteTarball => { package_manager::cached_tarball_folder_name( installer.manager(), *pkg_res.remote_tarball(), - patch_info.contents_hash(), + None, ) } _ => unreachable!(), }; - let mut pkg_cache_dir_subpath: AutoRelPath = - AutoRelPath::from(cache_subpath_z.as_bytes()).assume_ok(); - let (cache_dir, cache_dir_path) = installer.manager_mut().get_cache_directory_and_abs_path(); let _ = &cache_dir_path; // dropped at scope exit @@ -2346,38 +2344,18 @@ pub(crate) fn install_isolated_packages( let missing_from_cache = match installer.manager().get_preinstall_state(pkg_id) { install::PreinstallState::Done => false, - _ => 'missing_from_cache: { - if matches!(patch_info, installer::PatchInfo::None) { - let exists = match pkg_res_tag { - ResolutionTag::Npm => { - // Reshaped for borrowck — capture length - // instead of `save()` so the path stays unborrowed. - let cache_dir_path_save = pkg_cache_dir_subpath.len(); - pkg_cache_dir_subpath.append(b"package.json").assume_ok(); - let exists = sys::exists_at( - cache_dir, - pkg_cache_dir_subpath.slice_z(), - ); - pkg_cache_dir_subpath.set_length(cache_dir_path_save); - exists - } - _ => sys::directory_exists_at( - cache_dir, - pkg_cache_dir_subpath.slice_z(), - ) - .unwrap_or(false), - }; - if exists { - installer.manager_mut().set_preinstall_state( - pkg_id, - install::PreinstallState::Done, - ); - } - break 'missing_from_cache !exists; + _ => { + let exists = package_manager::directories::is_package_in_cache_at( + cache_dir, + cache_subpath_z, + pkg_res_tag, + ); + if exists { + installer + .manager_mut() + .set_preinstall_state(pkg_id, install::PreinstallState::Done); } - - // TODO: why does this look like it will never work? - break 'missing_from_cache true; + !exists } }; diff --git a/src/install/isolated_install/Installer.rs b/src/install/isolated_install/Installer.rs index 1984c05fb3a7..edb25516b394 100644 --- a/src/install/isolated_install/Installer.rs +++ b/src/install/isolated_install/Installer.rs @@ -1190,6 +1190,15 @@ impl Task { } } } + + // hardlink/copyfile overlay an existing tree, keeping files a previous patched build added. + let mut previous = AutoPath::init_top_level_dir(); + installer.append_real_store_path( + &mut previous, + self.entry_id, + Which::Final, + ); + let _ = Fd::cwd().delete_tree(previous.slice()); } if uses_global_store { diff --git a/src/install/repository.rs b/src/install/repository.rs index 3e2e9cb3152a..7d86db59cd51 100644 --- a/src/install/repository.rs +++ b/src/install/repository.rs @@ -421,6 +421,77 @@ fn exec(env: &bun_dotenv::Map, argv: &[&[u8]]) -> Result, Error> { Err(crate::Error::InstallFailed) } +/// A cache folder is built under a temporary sibling name and renamed onto `folder_name` once complete. +struct CacheStaging { + cache_dir: bun_sys::Fd, + tmp_name_buf: [u8; 64], + tmp_name_len: usize, +} + +impl CacheStaging { + fn new(cache_dir: bun_sys::Fd) -> Result { + let mut tmp_name_buf = [0u8; 64]; + let tmp_name_len = + Path::fs::FileSystem::tmpname(b"tmp", &mut tmp_name_buf, bun_core::fast_random()) + .map_err(|_| crate::Error::Sys(bun_errno::SystemErrno::ENOSPC))? + .len(); + Ok(Self { + cache_dir, + tmp_name_buf, + tmp_name_len, + }) + } + + fn tmp_name(&self) -> &[u8] { + &self.tmp_name_buf[..self.tmp_name_len] + } + + fn tmp_path(&self) -> &'static [u8] { + Path::resolve_path::join_abs_string::( + &PackageManager::get().cache_directory_path, + &[self.tmp_name()], + ) + } + + fn discard(&self) { + let _ = bun_sys::Dir::borrow(&self.cache_dir).delete_tree(self.tmp_name()); + } + + fn publish( + self, + log: &mut bun_ast::Log, + name: &[u8], + folder_name: &[u8], + ) -> Result { + let renamed = bun_sys::renameat_concurrently_a( + self.cache_dir, + self.tmp_name(), + self.cache_dir, + folder_name, + bun_sys::RenameatConcurrentlyOptions { + move_fallback: false, + }, + ); + // After an exchange the temporary name holds the folder that was replaced. + self.discard(); + if let Err(err) = renamed { + log.add_error_fmt( + None, + bun_ast::Loc::EMPTY, + format_args!( + "moving \"{}\" to cache dir failed: {}", + BStr::new(name), + err + ), + ); + return Err(crate::Error::InstallFailed); + } + bun_sys::Dir::borrow(&self.cache_dir) + .open_at(folder_name) + .map_err(Error::from) + } +} + impl RepositoryExt for Repository { fn parse_append_git(input: &[u8], buf: &mut StringBuf<'_>) -> Result { let mut remain = input; @@ -737,11 +808,7 @@ impl RepositoryExt for Repository { return Err(not_found.into()); } - let target = Path::resolve_path::join_abs_string::( - &PackageManager::get().cache_directory_path, - &[folder_name.as_bytes()], - ); - + let staging = CacheStaging::new(cache_dir)?; if let Err(err) = exec( env, &[ @@ -752,9 +819,10 @@ impl RepositoryExt for Repository { b"--quiet", b"--bare", url, - target, + staging.tmp_path(), ], ) { + staging.discard(); if err == crate::Error::RepositoryNotFound || attempt > 1 { log.add_error_fmt( None, @@ -765,9 +833,7 @@ impl RepositoryExt for Repository { return Err(err); } - bun_sys::Dir::borrow(&cache_dir) - .open_dir_z(folder_name) - .map_err(Into::into) + staging.publish(log, name, folder_name.as_bytes()) } } } @@ -874,97 +940,114 @@ impl RepositoryExt for Repository { ) .as_bytes(); - let package_dir = match bun_sys::Dir::borrow(&cache_dir) - .open_at(folder_name) - .map_err(Error::from) - { - Ok(d) => d, - Err(not_found) => 'brk: { - if not_found != crate::Error::Sys(bun_errno::SystemErrno::ENOENT) { - return Err(not_found); + let package_dir = 'brk: { + match bun_sys::Dir::borrow(&cache_dir).open_at(folder_name) { + Ok(dir) => { + if bun_sys::exists_at(dir.fd(), bun_core::zstr!(".bun-tag")) { + break 'brk dir; + } + dir.close(); } + Err(err) if err.get_errno() == bun_sys::E::ENOENT => {} + Err(err) => return Err(err.into()), + } + let repo_path = bun_sys::get_fd_path( + repo_dir, + // Per-field accessor — disjoint from `folder_name_buf` + // borrow above. See `TlBufs` accessor doc. + TlBufs::final_path_buf(), + )?; - let target = Path::resolve_path::join_abs_string::( - &PackageManager::get().cache_directory_path, - &[folder_name], + let staging = CacheStaging::new(cache_dir)?; + if let Err(err) = exec( + env, + &[ + b"git", + b"clone", + b"-c", + b"core.longpaths=true", + b"--quiet", + b"--no-checkout", + repo_path, + staging.tmp_path(), + ], + ) { + staging.discard(); + log.add_error_fmt( + None, + bun_ast::Loc::EMPTY, + format_args!("\"git clone\" for \"{}\" failed", BStr::new(name)), ); + return Err(err); + } - let repo_path = bun_sys::get_fd_path( - repo_dir, - // Per-field accessor — disjoint from `folder_name_buf` - // borrow above. See `TlBufs` accessor doc. - TlBufs::final_path_buf(), - )?; - - if let Err(err) = exec( - env, - &[ - b"git", - b"clone", - b"-c", - b"core.longpaths=true", - b"--quiet", - b"--no-checkout", - repo_path, - target, - ], - ) { - log.add_error_fmt( - None, - bun_ast::Loc::EMPTY, - format_args!("\"git clone\" for \"{}\" failed", BStr::new(name)), - ); - return Err(err); - } - - let folder = Path::resolve_path::join_abs_string::( - &PackageManager::get().cache_directory_path, - &[folder_name], + if let Err(err) = exec( + env, + // `is_safe_resolved_tag` above rejects a leading `-`, so + // `resolved` cannot be parsed as a git option. + &[ + b"git", + b"-C", + staging.tmp_path(), + b"checkout", + b"--quiet", + resolved, + ], + ) { + staging.discard(); + log.add_error_fmt( + None, + bun_ast::Loc::EMPTY, + format_args!("\"git checkout\" for \"{}\" failed", BStr::new(name)), ); + return Err(err); + } + { + let dir = match bun_sys::Dir::borrow(&cache_dir).open_at(staging.tmp_name()) { + Ok(dir) => dir, + Err(err) => { + staging.discard(); + return Err(err.into()); + } + }; + let _ = dir.delete_tree(b".git"); + // Unlinks a `node_modules` link only; directories are kept (bundleDependencies). + let _ = dir.delete_file_z(bun_core::zstr!("node_modules")); - if let Err(err) = exec( - env, - // `is_safe_resolved_tag` above rejects a leading `-`, so - // `resolved` cannot be parsed as a git option. - &[b"git", b"-C", folder, b"checkout", b"--quiet", resolved], - ) { + // `.bun-tag` is the cache-hit marker, so anything the repository checked in under that name is replaced. + let _ = dir.delete_tree(b".bun-tag"); + let tagged = bun_sys::File::openat( + dir.fd(), + bun_core::zstr!(".bun-tag"), + bun_sys::O::WRONLY + | bun_sys::O::CREAT + | bun_sys::O::EXCL + | if cfg!(windows) { + 0 + } else { + bun_sys::O::NOFOLLOW + }, + 0o664, + ) + .and_then(|f| f.write_all(resolved)); + // Windows cannot rename a directory with an open handle inside it. + dir.close(); + if let Err(err) = tagged { + staging.discard(); log.add_error_fmt( None, bun_ast::Loc::EMPTY, - format_args!("\"git checkout\" for \"{}\" failed", BStr::new(name)), + format_args!( + "writing \".bun-tag\" for \"{}\" failed: {}", + BStr::new(name), + BStr::new(err.name()) + ), ); - return Err(err); - } - let dir = bun_sys::Dir::borrow(&cache_dir) - .open_at(folder_name) - .map_err(Error::from)?; - let _ = dir.delete_tree(b".git"); - // Unlinks a `node_modules` link only; directories are kept (bundleDependencies). - let _ = dir.delete_file_z(bun_core::zstr!("node_modules")); - - if !resolved.is_empty() { - if bun_sys::File::openat( - dir.fd(), - bun_core::zstr!(".bun-tag"), - bun_sys::O::WRONLY - | bun_sys::O::CREAT - | bun_sys::O::TRUNC - | if cfg!(windows) { - 0 - } else { - bun_sys::O::NOFOLLOW - }, - 0o664, - ) - .and_then(|f| f.write_all(resolved)) - .is_err() - { - let _ = dir.delete_file_z(bun_core::zstr!(".bun-tag")); - } + return Err(crate::Error::InstallFailed); } - - break 'brk dir; } + + staging.publish(log, name, folder_name)? }; let (json_file, json_buf) = diff --git a/test/cli/install/bun-install.test.ts b/test/cli/install/bun-install.test.ts index e35753d1be20..0602dec36d5c 100644 --- a/test/cli/install/bun-install.test.ts +++ b/test/cli/install/bun-install.test.ts @@ -5379,9 +5379,11 @@ describe.concurrent("bun-install", () => { expect(err).toContain("Saved lockfile"); expect(out).toContain("1 package installed"); expect(readFileSync(target, "utf8")).toBe("original\n"); - expect(await readdirSorted(join(ctx.package_dir, "node_modules", ".cache", `@G@${sha}`))).toEqual( - isWindows ? [".bun-tag", "package.json"] : ["package.json"], - ); + expect(await readdirSorted(join(ctx.package_dir, "node_modules", ".cache", `@G@${sha}`))).toEqual([ + ".bun-tag", + "package.json", + ]); + expect(await file(join(ctx.package_dir, "node_modules", ".cache", `@G@${sha}`, ".bun-tag")).text()).toBe(sha); expect(await file(join(ctx.package_dir, "node_modules", "has-bun-tag", "package.json")).json()).toEqual({ name: "has-bun-tag", version: "1.0.0", @@ -5391,6 +5393,108 @@ describe.concurrent("bun-install", () => { }); }); + it("replaces a .bun-tag directory checked into a git dependency with the tag", async () => { + await withContext(defaultOpts, async ctx => { + const urls: string[] = []; + setContextHandler(ctx, dummyRegistryForContext(ctx, urls)); + using dir = tempDir("git-dep-bun-tag-dir", { + "work/package.json": JSON.stringify({ name: "has-bun-tag-dir", version: "1.0.0" }), + "work/.bun-tag/nested.txt": "checked in\n", + }); + const sha = await createDumbHttpGitRepo(String(dir), {}); + using server = serveDirectory(String(dir)); + await writeFile( + join(ctx.package_dir, "package.json"), + JSON.stringify({ + name: "foo", + version: "0.0.1", + dependencies: { "has-bun-tag-dir": `git+http://localhost:${server.port}/repo.git` }, + }), + ); + await using proc = spawn({ + cmd: [bunExe(), "install"], + cwd: ctx.package_dir, + stdout: "pipe", + stdin: "ignore", + stderr: "pipe", + env, + }); + const [out, err, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(err).toContain("Saved lockfile"); + expect(out).toContain("1 package installed"); + const cacheFolder = join(ctx.package_dir, "node_modules", ".cache", `@G@${sha}`); + expect(await readdirSorted(cacheFolder)).toEqual([".bun-tag", "package.json"]); + expect(await file(join(cacheFolder, ".bun-tag")).text()).toBe(sha); + expect(urls).toBeEmpty(); + expect(exitCode).toBe(0); + }); + }); + + it("git checkout cache folders appear only once complete and are hit only when tagged", async () => { + await withContext(defaultOpts, async ctx => { + const urls: string[] = []; + setContextHandler(ctx, dummyRegistryForContext(ctx, urls)); + using dir = tempDir("git-dep-checkout-fails", { + "work/package.json": JSON.stringify({ name: "checkout-fails", version: "1.0.0" }), + }); + const sha = await createDumbHttpGitRepo(String(dir), {}); + const treeSha = await git(join(String(dir), "work"), ["rev-parse", "HEAD^{tree}"]); + using server = serveDirectory(String(dir)); + await writeFile( + join(ctx.package_dir, "package.json"), + JSON.stringify({ + name: "foo", + version: "0.0.1", + dependencies: { "checkout-fails": `git+http://localhost:${server.port}/repo.git` }, + }), + ); + async function install() { + await using proc = spawn({ + cmd: [bunExe(), "install"], + cwd: ctx.package_dir, + stdout: "pipe", + stdin: "ignore", + stderr: "pipe", + env, + }); + const [out, err, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + return { out, err, exitCode }; + } + const cache = join(ctx.package_dir, "node_modules", ".cache"); + + expect(await install()).toMatchObject({ exitCode: 0 }); + expect(await readdirSorted(join(cache, `@G@${sha}`))).toEqual([".bun-tag", "package.json"]); + const mirror = (await readdirSorted(cache)).find(entry => entry.endsWith(".git"))!; + + // `git log` during resolution does not need the tree object, but `git checkout` cannot unpack without it. + const treeObject = join(cache, mirror, "objects", treeSha.slice(0, 2), treeSha.slice(2)); + const treeBytes = await file(treeObject).bytes(); + await rm(treeObject); + await rm(join(cache, `@G@${sha}`), { recursive: true }); + await rm(join(ctx.package_dir, "node_modules", "checkout-fails"), { recursive: true }); + const failed = await install(); + expect(failed.err).toContain('"git checkout" for "checkout-fails" failed'); + expect(failed.exitCode).not.toBe(0); + expect(await readdirSorted(cache)).toEqual([mirror]); + + await write(treeObject, treeBytes); + expect(await install()).toMatchObject({ exitCode: 0 }); + expect(await readdirSorted(join(cache, `@G@${sha}`))).toEqual([".bun-tag", "package.json"]); + + // A folder at the cache name without `.bun-tag` (left by older versions) is not a cache hit. + await rm(join(cache, `@G@${sha}`), { recursive: true }); + await mkdir(join(cache, `@G@${sha}`)); + await rm(join(ctx.package_dir, "node_modules", "checkout-fails"), { recursive: true }); + expect(await install()).toMatchObject({ exitCode: 0 }); + expect(await readdirSorted(join(cache, `@G@${sha}`))).toEqual([".bun-tag", "package.json"]); + expect(await readdirSorted(join(ctx.package_dir, "node_modules", "checkout-fails"))).toEqual([ + ".bun-tag", + "package.json", + ]); + expect(urls).toBeEmpty(); + }); + }); + it("should fail on invalid Git URL", async () => { await withContext(defaultOpts, async ctx => { const urls: string[] = []; diff --git a/test/cli/install/bun-patch.test.ts b/test/cli/install/bun-patch.test.ts index 115f4ab54d25..ec6b0b5a061c 100644 --- a/test/cli/install/bun-patch.test.ts +++ b/test/cli/install/bun-patch.test.ts @@ -156,7 +156,7 @@ describe("packages whose label is longer than 1024 bytes", () => { // Committing formats `name@label` before doing anything else. The commit itself cannot // succeed for such a package (the patch file is named after the label, which no file - // system accepts at this length), so what is pinned here is that it fails as an error. + // system accepts at this length), so what is pinned here is that it fails at that step. test.concurrent("bun patch --commit of a long-labeled package exits 1 without recording a patch", async () => { const packageJson = { name: "foo", dependencies: { baz: longSpec("baz-0.0.3.tgz") } }; const packageDir = await createProject("baz-0.0.3.tgz", packageJson); @@ -169,7 +169,7 @@ describe("packages whose label is longer than 1024 bytes", () => { await Bun.write(join(packageDir, "node_modules", "baz", "index.js"), "console.log('patched baz');\n"); const commit = await runBun(packageDir, "patch", "--commit", "node_modules/baz"); - expect(commit.stderr).toContain("error:"); + expect(commit.stderr).toContain("failed renaming patch file to patches dir"); expect(commit.exitCode).toBe(1); expect(await Bun.file(join(packageDir, "package.json")).json()).toEqual(packageJson); }); @@ -1055,3 +1055,180 @@ module.exports = function isOdd() { } }); }); + +// `bun patch --commit` derives the pristine copy's cache folder from the +// package's resolution. For non-registry resolutions (git, github, tarball) +// the resolution strings live in the lockfile's string buffer; resolving them +// against the wrong buffer produced paths like "@GH@@@@1" and the diff step +// failed with "Could not access". +describe.concurrent("bun patch --commit for non-registry dependencies", () => { + async function runBun(cwd: string, env: Record, ...args: string[]) { + await using proc = Bun.spawn({ + cmd: [bunExe(), ...args], + env, + cwd, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + return { stdout, stderr, exitCode }; + } + + async function expectPatchFlowWorks(dir: string, env: Record, commitArg: string) { + { + const { stderr, exitCode } = await runBun(dir, env, "install"); + expect(exitCode, `bun install failed: ${stderr}`).toBe(0); + } + { + const { stderr, exitCode } = await runBun(dir, env, "patch", "pkg-to-patch"); + expect(exitCode, `bun patch failed: ${stderr}`).toBe(0); + } + + await Bun.write(join(dir, "node_modules", "pkg-to-patch", "index.js"), `module.exports = "patched";\n`); + + { + const { stderr, exitCode } = await runBun(dir, env, "patch", "--commit", commitArg); + expect(stderr).not.toContain("Could not access"); + expect(exitCode, `bun patch --commit failed: ${stderr}`).toBe(0); + } + + const pkg = await Bun.file(join(dir, "package.json")).json(); + const entries = Object.entries(pkg.patchedDependencies ?? {}) as [string, string][]; + expect(entries).toHaveLength(1); + const [patchKey, patchPath] = entries[0]; + // the filename must stay valid on Windows (no NTFS-reserved characters) + expect(patchPath).not.toMatch(/[:?*"<>|]/); + const patchContents = await Bun.file(join(dir, patchPath)).text(); + expect(patchContents).toContain('-module.exports = "original";'); + expect(patchContents).toContain('+module.exports = "patched";'); + // the commit flow reinstalls with the patch applied + expect(await Bun.file(join(dir, "node_modules", "pkg-to-patch", "index.js")).text()).toBe( + `module.exports = "patched";\n`, + ); + return patchKey; + } + + test("github dependency", async () => { + await using dir = tempDir("patch-commit-github", { + "package.json": JSON.stringify({ + name: "test-patch-github", + dependencies: { "pkg-to-patch": "github:testowner/testrepo#aaaaaaa" }, + }), + // GitHub API tarballs have an `--` root folder; + // that folder name becomes the `resolved` part of the cache folder name. + "tarball-src": { + "testowner-testrepo-aaaaaaa": { + "package.json": JSON.stringify({ name: "pkg-to-patch", version: "1.0.0" }), + "index.js": `module.exports = "original";\n`, + }, + }, + }); + + await using tarProc = Bun.spawn({ + cmd: [ + "tar", + "-czf", + join(String(dir), "gh.tgz"), + "-C", + join(String(dir), "tarball-src"), + "testowner-testrepo-aaaaaaa", + ], + env: bunEnv, + stdout: "inherit", + stderr: "inherit", + }); + expect(await tarProc.exited).toBe(0); + const tgz = await Bun.file(join(String(dir), "gh.tgz")).bytes(); + + await using server = Bun.serve({ + port: 0, + fetch: () => new Response(tgz, { headers: { "content-type": "application/gzip" } }), + }); + + const env = { + ...bunEnv, + GITHUB_API_URL: `http://localhost:${server.port}`, + BUN_INSTALL_CACHE_DIR: join(String(dir), ".bun-cache"), + }; + + const patchKey = await expectPatchFlowWorks(String(dir), env, "node_modules/pkg-to-patch"); + expect(patchKey).toBe("pkg-to-patch@github:testowner/testrepo#aaaaaaa"); + }); + + test("git dependency", async () => { + await using dir = tempDir("patch-commit-git", { + "gitrepo": { + "package.json": JSON.stringify({ name: "pkg-to-patch", version: "1.0.0" }), + "index.js": `module.exports = "original";\n`, + }, + "project": {}, + }); + const repo = join(String(dir), "gitrepo"); + + // keep git away from the machine's global/system config (autocrlf, gpgsign) + const gitEnv = { ...bunEnv, GIT_CONFIG_NOSYSTEM: "1", GIT_CONFIG_GLOBAL: join(String(dir), "no-gitconfig") }; + for (const args of [ + ["init", "-q"], + ["config", "core.autocrlf", "false"], + ["add", "-A"], + ["-c", "user.email=test@test.test", "-c", "user.name=test", "commit", "-q", "-m", "init"], + // serve the repo over git's dumb HTTP protocol (plain file fetches) + ["update-server-info"], + ]) { + await using proc = Bun.spawn({ cmd: ["git", ...args], cwd: repo, env: gitEnv, stderr: "pipe" }); + const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]); + expect(exitCode, `git ${args.join(" ")} failed: ${stderr}`).toBe(0); + } + + await using server = Bun.serve({ + port: 0, + async fetch(req) { + const pathname = new URL(req.url).pathname; + if (!pathname.startsWith("/repo.git/")) return new Response("not found", { status: 404 }); + const file = Bun.file(join(repo, ".git", ...pathname.slice("/repo.git/".length).split("/"))); + return (await file.exists()) ? new Response(file) : new Response("not found", { status: 404 }); + }, + }); + + const project = join(String(dir), "project"); + const depUrl = `git+http://localhost:${server.port}/repo.git`; + await Bun.write( + join(project, "package.json"), + JSON.stringify({ name: "test-patch-git", dependencies: { "pkg-to-patch": depUrl } }), + ); + + const env = { ...bunEnv, BUN_INSTALL_CACHE_DIR: join(String(dir), ".bun-cache") }; + + const patchKey = await expectPatchFlowWorks(project, env, "node_modules/pkg-to-patch"); + expect(patchKey).toStartWith(`pkg-to-patch@${depUrl}#`); + }); + + test("local tarball dependency", async () => { + await using dir = tempDir("patch-commit-tarball", { + "package.json": JSON.stringify({ + name: "test-patch-tarball", + dependencies: { "pkg-to-patch": "file:./dep.tgz" }, + }), + "tarball-src": { + "package": { + "package.json": JSON.stringify({ name: "pkg-to-patch", version: "1.0.0" }), + "index.js": `module.exports = "original";\n`, + }, + }, + }); + + await using tarProc = Bun.spawn({ + cmd: ["tar", "-czf", join(String(dir), "dep.tgz"), "-C", join(String(dir), "tarball-src"), "package"], + env: bunEnv, + stdout: "inherit", + stderr: "inherit", + }); + expect(await tarProc.exited).toBe(0); + + const env = { ...bunEnv, BUN_INSTALL_CACHE_DIR: join(String(dir), ".bun-cache") }; + + // name-only argument exercises the name-and-version lookup path + const patchKey = await expectPatchFlowWorks(String(dir), env, "pkg-to-patch"); + expect(patchKey).toBe("pkg-to-patch@./dep.tgz"); + }); +}); diff --git a/test/cli/install/isolated-install.test.ts b/test/cli/install/isolated-install.test.ts index 3d3b6a5428ad..a4ff2a70903f 100644 --- a/test/cli/install/isolated-install.test.ts +++ b/test/cli/install/isolated-install.test.ts @@ -724,6 +724,337 @@ index 0000000000000000000000000000000000000000..3b18e512dba79e4c8300dd08aeb37f8e await checkInstall(); }); +test("adding, removing and re-adding a patch for an npm dependency", async () => { + const { packageJson, packageDir } = await registry.createTestDir({ bunfigOpts: { linker: "isolated" } }); + const rootPackageJson = { name: "npm-patch-cycle", dependencies: { "no-deps": "1.0.0" } }; + const patched = { + ...rootPackageJson, + patchedDependencies: { "no-deps@1.0.0": "patches/no-deps@1.0.0.patch" }, + }; + await write( + join(packageDir, "patches", "no-deps@1.0.0.patch"), + `diff --git a/patched.txt b/patched.txt +new file mode 100644 +index 0000000000000000000000000000000000000000..3b18e512dba79e4c8300dd08aeb37f8e728b8dad +--- /dev/null ++++ b/patched.txt +@@ -0,0 +1 @@ ++hello world +`, + ); + const patchedFile = join(packageDir, "node_modules", "no-deps", "patched.txt"); + + const steps = [ + [rootPackageJson, false], + [patched, true], + [rootPackageJson, false], + [patched, true], + ] as const; + for (const [step, [manifest, expectPatched]] of steps.entries()) { + await write(packageJson, JSON.stringify(manifest)); + // hardlink (the Linux default) installs into the store entry in place; clonefile replaces it. + await using proc = spawn({ + cmd: [bunExe(), "install", "--backend", "hardlink"], + cwd: packageDir, + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [err, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]); + expect(err).not.toContain("error:"); + expect(exitCode).toBe(0); + expect({ step, patched: existsSync(patchedFile) }).toEqual({ step, patched: expectPatched }); + } +}); + +// Adding a patchedDependencies entry for a github: dependency of a workspace +// member deadlocked `bun install` forever with the isolated linker: +// re-resolution re-downloaded the github tarball, and the install phase then +// re-enqueued the same tarball task and parked the store entry on the +// completed task's already-drained callback list, so the pending-task count +// never reached zero. +test("adding and removing a patch for a github dependency in a workspace completes", async () => { + const { packageJson, packageDir } = await registry.createTestDir({ bunfigOpts: { linker: "isolated" } }); + + // Minimal gzipped tarball shaped like a github codeload tarball: a single + // root directory wrapping the package contents. + function tarHeader(name: string, size: number, isDir: boolean): Uint8Array { + const header = new Uint8Array(512); + const encoder = new TextEncoder(); + header.set(encoder.encode(name), 0); + header.set(encoder.encode(isDir ? "0000755 " : "0000644 "), 100); + header.set(encoder.encode("0000000 "), 108); + header.set(encoder.encode("0000000 "), 116); + header.set(encoder.encode(size.toString(8).padStart(11, "0") + " "), 124); + header.set(encoder.encode("00000000000 "), 136); + header.set(encoder.encode(" "), 148); + header[156] = (isDir ? "5" : "0").charCodeAt(0); + header.set(encoder.encode("ustar"), 257); + header.set(encoder.encode("00"), 263); + let checksum = 0; + for (const byte of header) checksum += byte; + header.set(encoder.encode(checksum.toString(8).padStart(6, "0") + "\0 "), 148); + return header; + } + const blocks: Uint8Array[] = []; + blocks.push(tarHeader("testowner-testrepo-aaaaaaa/", 0, true)); + for (const [name, contents] of [ + ["package.json", JSON.stringify({ name: "gh-dep", version: "1.0.0" })], + ["index.js", 'console.log("original");\n'], + ]) { + const bytes = new TextEncoder().encode(contents); + blocks.push(tarHeader(`testowner-testrepo-aaaaaaa/${name}`, bytes.length, false)); + blocks.push(bytes); + if (bytes.length % 512 !== 0) blocks.push(new Uint8Array(512 - (bytes.length % 512))); + } + blocks.push(new Uint8Array(1024)); + const tarball = Bun.gzipSync(Buffer.concat(blocks)); + + using server = Bun.serve({ + port: 0, + fetch: () => new Response(tarball, { headers: { "Content-Type": "application/gzip" } }), + }); + + const env = { + ...bunEnv, + GITHUB_API_URL: `http://localhost:${server.port}`, + // CI exports BUN_INSTALL_CACHE_DIR; pin it so this test's cache state is + // its own. + BUN_INSTALL_CACHE_DIR: join(packageDir, ".bun-cache"), + }; + + async function install() { + await using proc = spawn({ + cmd: [bunExe(), "install"], + cwd: packageDir, + env, + stdout: "pipe", + stderr: "pipe", + }); + const [err, exitCode] = await Promise.all([proc.stderr.text(), proc.exited, proc.stdout.text()]); + expect(err).not.toContain("error:"); + expect(exitCode).toBe(0); + } + + const rootPackageJson = { + name: "patched-github-workspace", + workspaces: ["packages/*"], + }; + await write(packageJson, JSON.stringify(rootPackageJson)); + await write( + join(packageDir, "packages", "member", "package.json"), + JSON.stringify({ + name: "member", + version: "1.0.0", + dependencies: { + "gh-dep": "github:testowner/testrepo#aaaaaaa", + }, + }), + ); + await write( + join(packageDir, "patches", "gh-dep.patch"), + `diff --git a/index.js b/index.js +index 1f0e8b9f1f9a56799cdbc1a5a2f8cf9f9a3b2f1c..2f0e8b9f1f9a56799cdbc1a5a2f8cf9f9a3b2f1d 100644 +--- a/index.js ++++ b/index.js +@@ -1 +1 @@ +-console.log("original"); ++console.log("patched"); +`, + ); + + const installedIndexJs = file(join(packageDir, "packages", "member", "node_modules", "gh-dep", "index.js")); + + await install(); + expect(await installedIndexJs.text()).toBe('console.log("original");\n'); + + // Adding the patch triggers a re-resolution; this install hung forever + // before the fix. + await write( + packageJson, + JSON.stringify({ + ...rootPackageJson, + patchedDependencies: { + "gh-dep@github:testowner/testrepo#aaaaaaa": "patches/gh-dep.patch", + }, + }), + ); + await install(); + expect(await installedIndexJs.text()).toBe('console.log("patched");\n'); + + // Cold cache with the patch still in the lockfile: the install phase itself + // downloads the tarball and applies the patch after extraction. + await rm(join(packageDir, ".bun-cache"), { recursive: true, force: true }); + await rm(join(packageDir, "node_modules"), { recursive: true, force: true }); + await rm(join(packageDir, "packages", "member", "node_modules"), { recursive: true, force: true }); + await install(); + expect(await installedIndexJs.text()).toBe('console.log("patched");\n'); + + // Removing the patch re-resolves again and rebuilds the store entry from + // the unpatched cache folder (the PatchInfo::Remove path, which hung the + // same way). + await write(packageJson, JSON.stringify(rootPackageJson)); + await install(); + expect(await installedIndexJs.text()).toBe('console.log("original");\n'); + + // Re-adding the same patch must patch again. + await write( + packageJson, + JSON.stringify({ + ...rootPackageJson, + patchedDependencies: { + "gh-dep@github:testowner/testrepo#aaaaaaa": "patches/gh-dep.patch", + }, + }), + ); + await install(); + expect(await installedIndexJs.text()).toBe('console.log("patched");\n'); +}); + +// Same deadlock through the git: task-id space (clone + checkout tasks +// instead of a tarball download). The repo is served over git's dumb HTTP +// protocol: after `git update-server-info`, a bare repo is plain static +// files. Requires the git executable to build the fixture repository. +const gitExecutable = Bun.which("git"); +test.skipIf(!gitExecutable)("adding and removing a patch for a git dependency in a workspace completes", async () => { + const { packageJson, packageDir } = await registry.createTestDir({ bunfigOpts: { linker: "isolated" } }); + + const srcDir = join(packageDir, "git-src"); + const bareDir = join(packageDir, "repo.git"); + // Isolate git from system/global config (e.g. core.autocrlf on Windows + // would rewrite the checked-out file contents this test asserts on). + const gitConfigEnv = { + GIT_CONFIG_NOSYSTEM: "1", + GIT_CONFIG_GLOBAL: join(packageDir, "gitconfig"), + }; + const gitEnv = { + ...bunEnv, + ...gitConfigEnv, + GIT_AUTHOR_NAME: "bun-test", + GIT_AUTHOR_EMAIL: "test@bun.sh", + GIT_COMMITTER_NAME: "bun-test", + GIT_COMMITTER_EMAIL: "test@bun.sh", + }; + async function git(args: string[], cwd: string): Promise { + await using proc = spawn({ cmd: [gitExecutable!, ...args], cwd, env: gitEnv, stdout: "pipe", stderr: "pipe" }); + const [out, err, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(err).not.toContain("fatal:"); + expect(exitCode).toBe(0); + return out; + } + + await write(join(packageDir, "gitconfig"), "[core]\n\tautocrlf = false\n"); + await write(join(srcDir, "package.json"), JSON.stringify({ name: "git-dep", version: "1.0.0" })); + await write(join(srcDir, "index.js"), 'console.log("original");\n'); + await git(["init", "-q"], srcDir); + await git(["add", "-A"], srcDir); + await git(["commit", "-qm", "init"], srcDir); + const sha = (await git(["rev-parse", "HEAD"], srcDir)).trim(); + await git(["clone", "-q", "--bare", srcDir, bareDir], packageDir); + await git(["update-server-info"], bareDir); + + using server = Bun.serve({ + port: 0, + async fetch(req) { + const { pathname } = new URL(req.url); + if (!pathname.startsWith("/repo.git/")) return new Response("not found", { status: 404 }); + const f = file(join(bareDir, pathname.slice("/repo.git/".length))); + return (await f.exists()) ? new Response(f) : new Response("not found", { status: 404 }); + }, + }); + const repoUrl = `git+http://127.0.0.1:${server.port}/repo.git`; + + const env = { + ...bunEnv, + ...gitConfigEnv, + BUN_INSTALL_CACHE_DIR: join(packageDir, ".bun-cache"), + }; + + async function install() { + await using proc = spawn({ + cmd: [bunExe(), "install"], + cwd: packageDir, + env, + stdout: "pipe", + stderr: "pipe", + }); + const [err, exitCode] = await Promise.all([proc.stderr.text(), proc.exited, proc.stdout.text()]); + expect(err).not.toContain("error:"); + expect(exitCode).toBe(0); + } + + const rootPackageJson = { + name: "patched-git-workspace", + workspaces: ["packages/*"], + }; + await write(packageJson, JSON.stringify(rootPackageJson)); + await write( + join(packageDir, "packages", "member", "package.json"), + JSON.stringify({ + name: "member", + version: "1.0.0", + dependencies: { + "git-dep": repoUrl, + }, + }), + ); + await write( + join(packageDir, "patches", "git-dep.patch"), + `diff --git a/index.js b/index.js +index 1f0e8b9f1f9a56799cdbc1a5a2f8cf9f9a3b2f1c..2f0e8b9f1f9a56799cdbc1a5a2f8cf9f9a3b2f1d 100644 +--- a/index.js ++++ b/index.js +@@ -1 +1 @@ +-console.log("original"); ++console.log("patched"); +`, + ); + + const installedIndexJs = file(join(packageDir, "packages", "member", "node_modules", "git-dep", "index.js")); + + await install(); + expect(await installedIndexJs.text()).toBe('console.log("original");\n'); + + // The patchedDependencies key must carry the resolved commit; a key without + // it is silently ignored, which the patched-content assertion would catch. + await write( + packageJson, + JSON.stringify({ + ...rootPackageJson, + patchedDependencies: { + [`git-dep@${repoUrl}#${sha}`]: "patches/git-dep.patch", + }, + }), + ); + await install(); + expect(await installedIndexJs.text()).toBe('console.log("patched");\n'); + + // Cold cache with the patch still in the lockfile: the install phase + // clones and checks out itself, applying the patch after the checkout. + await rm(join(packageDir, ".bun-cache"), { recursive: true, force: true }); + await rm(join(packageDir, "node_modules"), { recursive: true, force: true }); + await rm(join(packageDir, "packages", "member", "node_modules"), { recursive: true, force: true }); + await install(); + expect(await installedIndexJs.text()).toBe('console.log("patched");\n'); + + await write(packageJson, JSON.stringify(rootPackageJson)); + await install(); + expect(await installedIndexJs.text()).toBe('console.log("original");\n'); + + // Re-adding the same patch must patch again. + await write( + packageJson, + JSON.stringify({ + ...rootPackageJson, + patchedDependencies: { + [`git-dep@${repoUrl}#${sha}`]: "patches/git-dep.patch", + }, + }), + ); + await install(); + expect(await installedIndexJs.text()).toBe('console.log("patched");\n'); +}); + for (const backend of ["clonefile", "hardlink", "copyfile"]) { test(`isolated install with backend: ${backend}`, async () => { const { packageJson, packageDir } = await registry.createTestDir({ bunfigOpts: { linker: "isolated" } });