bun patch --commit: restore the node_modules/<pkg> link under the isolated linker

src/install/PackageManager.rs

@@ -461,6 +461,9 @@ pub struct PackageManager {
     pub(crate) patched_dependencies_to_remove:
         ArrayHashMap<PackageNameAndVersionHash, () /* , ArrayIdentityContext::U64, false */>,
 
+    // bun patch --commit: the folder that was diffed; the isolated linker puts its link back.
+    pub(crate) committed_patch: Option<patch_package::CommittedPatch>,
+
     pub(crate) active_lifecycle_scripts: crate::lifecycle_script_runner::List<'static>,
     pub(crate) last_reported_slow_lifecycle_script_at: u64,
     pub(crate) cached_tick_for_slow_lifecycle_script_logging: u64,
@@ -2147,6 +2150,7 @@ pub fn init(
         wr!(edited_package_jsons, Vec::new());
         wr!(catalog_add, add_catalog::State::default());
         wr!(patched_dependencies_to_remove, ArrayHashMap::default());
+        wr!(committed_patch, None);
         wr!(last_reported_slow_lifecycle_script_at, 0);
         wr!(cached_tick_for_slow_lifecycle_script_logging, 0);
     }
@@ -2615,6 +2619,7 @@ fn init_with_runtime_once(
         wr!(edited_package_jsons, Vec::new());
         wr!(catalog_add, add_catalog::State::default());
         wr!(patched_dependencies_to_remove, ArrayHashMap::default());
+        wr!(committed_patch, None);
         wr!(last_reported_slow_lifecycle_script_at, 0);
         wr!(cached_tick_for_slow_lifecycle_script_logging, 0);
     }

src/install/PackageManager/patchPackage.rs

@@ -53,6 +53,37 @@ pub struct PatchCommitResult {
     pub(crate) not_in_workspace_root: bool,
 }
 
+/// The folder that `bun patch --commit` diffed. The isolated linker puts its link back.
+pub struct CommittedPatch {
+    real_path: Box<[u8]>,
+    /// False when the diff was empty: the folder equals the package, and no patch is recorded.
+    pub(crate) has_changes: bool,
+}
+
+impl CommittedPatch {
+    fn new(folder: &[u8]) -> sys::Result<CommittedPatch> {
+        let mut buf = bun_paths::path_buffer_pool::get();
+        Ok(CommittedPatch {
+            real_path: Box::from(real_path_of_folder(folder, &mut buf)?),
+            has_changes: true,
+        })
+    }
+
+    pub(crate) fn is_folder(&self, path: &[u8]) -> bool {
+        // The name rules out most paths without a syscall.
+        if bun_paths::basename(path) != bun_paths::basename(&self.real_path) {
+            return false;
+        }
+        let mut buf = bun_paths::path_buffer_pool::get();
+        real_path_of_folder(path, &mut buf).is_ok_and(|real_path| real_path == &*self.real_path)
+    }
+}
+
+fn real_path_of_folder<'a>(folder: &[u8], buf: &'a mut PathBuffer) -> sys::Result<&'a [u8]> {
+    let dir = Dir::cwd().open_dir(folder, sys::OpenDirOptions::default())?;
+    dir.get_fd_path(buf).map(|real_path| &*real_path)
+}
+
 /// - Arg is the dir containing the package with changes OR name and version
 /// - Get the patch file contents by running git diff on the temp dir and the original package dir
 /// - Write the patch file to $PATCHES_DIR/$PKG_NAME_AND_VERSION.patch
@@ -267,6 +298,30 @@ pub fn do_patch_commit(
         }
     };
 
+    // `git diff` records a link as `new file mode 120000`, and no install can apply that.
+    if !is_real_dir_not_symlink(&changes_dir) {
+        bun_core::pretty_errorln!(
+            "<r><red>error<r>: <b>{}<r> is not a folder that bun patch prepared",
+            bstr::BStr::new(&changes_dir),
+        );
+        bun_core::note!(
+            "Run `<cyan>bun patch {}<r>` first",
+            bstr::BStr::new(manager.options.positionals[1]),
+        );
+        Global::crash();
+    }
+    manager.committed_patch = match CommittedPatch::new(&changes_dir) {
+        Ok(committed) => Some(committed),
+        Err(e) => {
+            Output::err(
+                e,
+                "failed to open directory <b>{s}<r>",
+                (bstr::BStr::new(&changes_dir),),
+            );
+            Global::crash();
+        }
+    };
+
     // `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();
@@ -353,6 +408,10 @@ pub fn do_patch_commit(
 
             break 'has_nested_node_modules true;
         };
+        // The diff leaves that folder out. It can hold the `bun patch` copy of a nested package.
+        if has_nested_node_modules {
+            manager.committed_patch = None;
+        }
 
         let patch_tag_tmpname = match bun_paths::fs::FileSystem::tmpname(
             b"patch_tmp",
@@ -537,9 +596,23 @@ pub fn do_patch_commit(
             );
             Output::flush();
             drop(contents);
+            if let Some(committed) = &mut manager.committed_patch {
+                committed.has_changes = false;
+            }
             return Ok(None);
         }
 
+        // The patch parser drops such a file, so the folder has edits that the patch lacks.
+        if strings::split(&contents, b"\n")
+            .any(|line| line.starts_with(b"Binary files ") && line.ends_with(b" differ"))
+        {
+            bun_core::warn!(
+                "git cannot diff binary files as text. The patch does not include the changes to them in <b>{}<r>",
+                bstr::BStr::new(new_folder),
+            );
+            manager.committed_patch = None;
+        }
+
         break 'brk contents;
     };
 

src/install/isolated_install.rs

@@ -2619,6 +2619,8 @@ pub(crate) fn install_isolated_packages(
             }
         }
 
+        installer.relink_committed_patch_after_tasks();
+
         if installer.manager().options.log_level.show_progress() {
             progress.root.end();
             *progress = Progress::default();

src/install/isolated_install/Installer.rs

@@ -37,6 +37,7 @@ use crate::bun_fs;
 use crate::lockfile_real::package::PackageColumns as _;
 use crate::package_manager_real::directories;
 use crate::package_manager_real::package_manager_options::Do;
+use crate::package_manager_real::patch_package::CommittedPatch;
 
 /// The enum lives at module level in `crate::resolution`.
 type ResolutionTag = resolution::Tag;
@@ -1488,12 +1489,13 @@ impl Task {
                         symlinker::Strategy::ExpectMissing
                     };
 
-                    let changed = match installer.symlink_dependencies(self.entry_id, strategy) {
-                        sys::Result::Ok(changed) => changed,
-                        sys::Result::Err(err) => {
-                            return Ok(Yield::failure(TaskError::SymlinkDependencies(err)));
-                        }
-                    };
+                    let changed =
+                        match installer.symlink_dependencies(self.entry_id, strategy, None) {
+                            sys::Result::Ok(changed) => changed,
+                            sys::Result::Err(err) => {
+                                return Ok(Yield::failure(TaskError::SymlinkDependencies(err)));
+                            }
+                        };
 
                     if relinking {
                         if !changed {
@@ -1536,6 +1538,13 @@ impl Task {
 
                 Step::SymlinkDependencyBinaries => {
                     let current_step = Step::SymlinkDependencyBinaries;
+                    if matches!(pkg_res.tag, ResolutionTag::Root | ResolutionTag::Workspace) {
+                        if let sys::Result::Err(err) =
+                            installer.relink_committed_patch(self.entry_id)
+                        {
+                            return Ok(Yield::failure(TaskError::SymlinkDependencies(err)));
+                        }
+                    }
                     if let Err(err) = installer.link_dependency_bins(self.entry_id) {
                         return Ok(Yield::failure(TaskError::Binaries(err)));
                     }
@@ -2235,10 +2244,12 @@ impl<'a> Installer<'a> {
     }
 
     /// Ok(true) when at least one dependency link of the entry was written.
+    /// `only_committed` limits the links to the folder of that commit.
     fn symlink_dependencies(
         &self,
         entry_id: StoreEntryId,
         strategy: symlinker::Strategy,
+        only_committed: Option<&CommittedPatch>,
     ) -> sys::Result<bool> {
         let lockfile = self.lockfile();
         let string_buf = lockfile.buffers.string_bytes.as_slice();
@@ -2279,6 +2290,14 @@ impl<'a> Installer<'a> {
                 self.append_store_path(&mut dep_store_path, dep.entry_id);
             }
 
+            if let Some(committed) = only_committed {
+                if !committed.is_folder(dest.slice())
+                    || !self.store_holds_commit(committed, dep.entry_id, &mut dep_store_path)
+                {
+                    continue;
+                }
+            }
+
             let dest_len = dest.len();
             dest.undo(1);
             let target = dest.relative(&dep_store_path);
@@ -2298,6 +2317,72 @@ impl<'a> Installer<'a> {
         Ok(changed)
     }
 
+    /// `store_path` has the tag file of the committed patch, or exists when the diff was empty.
+    fn store_holds_commit(
+        &self,
+        committed: &CommittedPatch,
+        entry_id: StoreEntryId,
+        store_path: &mut AutoAbsPath,
+    ) -> bool {
+        if !committed.has_changes {
+            return sys::directory_exists_at(Fd::cwd(), store_path.slice_z()).unwrap_or(false);
+        }
+
+        let node_id = self.store.entries.items_node_id()[entry_id.get() as usize];
+        let pkg_id = self.store.nodes.items_pkg_id()[node_id.get() as usize];
+        let pkgs = self.lockfile().packages.slice();
+        let Ok(PatchInfo::Patch(patch)) = self.package_patch_info(
+            pkgs.items_name()[pkg_id as usize],
+            pkgs.items_name_hash()[pkg_id as usize],
+            &pkgs.items_resolution()[pkg_id as usize],
+        ) else {
+            return false;
+        };
+
+        let mut tag_buf: install::BuntagHashBuf = Default::default();
+        let tag = install::buntaghashbuf_make(&mut tag_buf, patch.contents_hash);
+        let store_path_len = store_path.len();
+        store_path.append(&*tag).assume_ok();
+        let has_tag = sys::exists_z(store_path.slice_z());
+        store_path.set_length(store_path_len);
+        has_tag
+    }
+
+    /// For a root or workspace entry whose dependencies are installed, before its scripts run.
+    fn relink_committed_patch(&self, entry_id: StoreEntryId) -> sys::Result<()> {
+        let Some(committed) = self.manager().committed_patch.as_ref() else {
+            return Ok(());
+        };
+        self.symlink_dependencies(
+            entry_id,
+            symlinker::Strategy::ReplaceDirectory,
+            Some(committed),
+        )?;
+        Ok(())
+    }
+
+    /// An entry in a dependency cycle does not wait for its dependencies, so its task can be early.
+    pub(crate) fn relink_committed_patch_after_tasks(&mut self) {
+        if self.manager().committed_patch.is_none() {
+            return;
+        }
+        let pkg_resolutions = self.lockfile().packages.items_resolution();
+        let node_pkg_ids = self.store.nodes.items_pkg_id();
+
+        for (entry_id, node_id) in self.store.entries.items_node_id().iter().enumerate() {
+            let pkg_res = &pkg_resolutions[node_pkg_ids[node_id.get() as usize] as usize];
+            if !matches!(pkg_res.tag, ResolutionTag::Root | ResolutionTag::Workspace) {
+                continue;
+            }
+            let entry_id = StoreEntryId::from(u32::try_from(entry_id).expect("int cast"));
+            if let sys::Result::Err(err) = self.relink_committed_patch(entry_id) {
+                Output::err(err, "failed to link the patched package again", ());
+                Output::flush();
+                self.summary.fail += 1;
+            }
+        }
+    }
+
     pub(crate) fn link_dependency_bins(&self, parent_entry_id: StoreEntryId) -> crate::Result<()> {
         let lockfile = self.lockfile();
         let store = self.store;

src/install/isolated_install/Symlinker.rs

@@ -1,5 +1,6 @@
 use bun_core::strings;
 use bun_paths;
+use bun_paths::path_options::AssumeOk as _;
 use bun_sys::{self, Errno, Fd, FdDirExt, FdExt};
 
 pub(crate) struct Symlinker {
@@ -31,6 +32,28 @@ impl Symlinker {
         }
     }
 
+    /// The directory moves aside first, so a failure never leaves a partly deleted copy at `dest`.
+    fn replace_directory(&mut self) -> bun_sys::Result<()> {
+        let mut aside =
+            bun_paths::Path::<u8>::from(self.dest.dirname().unwrap_or(b".")).assume_ok();
+        aside
+            .append_fmt(format_args!(
+                ".{}.old-{:x}",
+                bstr::BStr::new(self.dest.basename()),
+                bun_core::fast_random(),
+            ))
+            .assume_ok();
+
+        bun_sys::renameat(Fd::cwd(), self.dest.slice_z(), Fd::cwd(), aside.slice_z())?;
+        if let Err(err) = self.symlink() {
+            // When the copy cannot move back, this error names where it is.
+            bun_sys::renameat(Fd::cwd(), aside.slice_z(), Fd::cwd(), self.dest.slice_z())?;
+            return Err(err);
+        }
+        let _ = Fd::cwd().delete_tree(aside.slice_z());
+        Ok(())
+    }
+
     // Ok(true) when a link was written.
     pub(crate) fn ensure_symlink(&mut self, strategy: Strategy) -> bun_sys::Result<bool> {
         match strategy {
@@ -54,7 +77,7 @@ impl Symlinker {
                     },
                 };
             }
-            Strategy::ExpectExisting => {
+            Strategy::ExpectExisting | Strategy::ReplaceDirectory => {
                 let mut current_link_buf = bun_paths::path_buffer_pool::get();
                 let current_link_len =
                     match bun_sys::readlink(self.dest.slice_z(), &mut current_link_buf) {
@@ -100,7 +123,10 @@ impl Symlinker {
                                         false
                                     };
                                     if is_dir {
-                                        return Ok(false);
+                                        if !matches!(strategy, Strategy::ReplaceDirectory) {
+                                            return Ok(false);
+                                        }
+                                        return self.replace_directory().map(|()| true);
                                     }
                                     let _ = bun_sys::unlink(self.dest.slice_z());
                                     return self.symlink().map(|()| true);
@@ -153,4 +179,6 @@ impl Symlinker {
 pub enum Strategy {
     ExpectExisting,
     ExpectMissing,
+    /// `ExpectExisting`, but the link replaces a real directory that `bun patch --commit` diffed.
+    ReplaceDirectory,
 }