install: keep linking the remaining bins when one bin or one dependency's bins fail to link

src/install/bin.rs

@@ -831,6 +831,8 @@
     pub abs_dest_buf: &'a mut [u8],
     pub rel_buf: &'a mut [u8],
 
+    /// First error hit while linking this package's bins. The bins after the
+    /// failing one are still linked.
     pub err: Option<Error>,
     pub skipped_due_to_missing_bin: bool,
 }
@@ -925,29 +927,38 @@
 
         bun_core::analytics::Features::binlinks_inc();
 
+        #[cfg(windows)]
+        let target = match sys::File::openat(Fd::cwd(), abs_target, sys::O::RDONLY, 0) {
+            Ok(f) => f,
+            Err(err) => {
+                let err: crate::Error = err.into();
+                if err != crate::Error::Sys(bun_errno::SystemErrno::EISDIR) {
+                    // ignore directories, creating a shim for one won't do anything
+                    self.err = Some(err);
+                }
+                return;
+            }
+        };
+
+        // Only this bin's outcome decides whether the link just made is kept.
+        let prior_err = self.err.take();
         #[cfg(not(windows))]
         {
             self.create_symlink(abs_target, abs_dest, global);
         }
         #[cfg(windows)]
         {
-            let target = match sys::File::openat(Fd::cwd(), abs_target, sys::O::RDONLY, 0) {
-                Ok(f) => f,
-                Err(err) => {
-                    let err: crate::Error = err.into();
-                    if err != crate::Error::Sys(bun_errno::SystemErrno::EISDIR) {
-                        // ignore directories, creating a shim for one won't do anything
-                        self.err = Some(err);
-                    }
-                    return;
-                }
-            };
             self.create_windows_shim(&target, abs_target, abs_dest, global);
         }
+        let err = self.err;
+        self.err = prior_err.or(err);
 
-        if self.err.is_some() {
+        if err.is_some() {
             // cleanup on error just in case
             Self::unlink_bin_or_shim(abs_dest);
+            if let Some(seen) = self.seen.as_deref_mut() {
+                seen.remove(abs_dest.as_bytes());
+            }
             return;
         }
 

src/install/isolated_install/Installer.rs

@@ -375,6 +375,23 @@ impl<'a> Installer<'a> {
                     ),
                 );
             }
+            TaskError::DependencyBinaries(dep_errs) => {
+                for dep_err in dep_errs.iter() {
+                    let dep_node_id = entry_node_ids[dep_err.dep_entry_id.get() as usize];
+                    let dep_pkg_id = node_pkg_ids[dep_node_id.get() as usize];
+                    Output::err(
+                        dep_err.err,
+                        "failed to link binaries of dependency {}@{} for package: {}@{}",
+                        (
+                            bstr::BStr::new(pkg_names[dep_pkg_id as usize].slice(string_buf)),
+                            pkg_resolutions[dep_pkg_id as usize]
+                                .fmt(string_buf, bun_core::fmt::PathSep::Auto),
+                            bstr::BStr::new(pkg_name.slice(string_buf)),
+                            pkg_res.fmt(string_buf, bun_core::fmt::PathSep::Auto),
+                        ),
+                    );
+                }
+            }
             TaskError::Download(dl) => {
                 Output::err_generic(
                     "failed to download <b>{}@{}<r>: {}\n  <d>{}<r>",
@@ -684,11 +701,18 @@ pub struct DownloadError {
     pub(crate) url: Box<[u8]>,
 }
 
+#[derive(Clone, Copy)]
+pub struct DependencyBinariesError {
+    pub(crate) dep_entry_id: StoreEntryId,
+    pub(crate) err: crate::Error,
+}
+
 pub enum TaskError {
     LinkPackage(sys::Error),
     SymlinkDependencies(sys::Error),
     RunScripts(crate::Error),
     Binaries(crate::Error),
+    DependencyBinaries(Box<[DependencyBinariesError]>),
     Patching(Log),
     Download(DownloadError),
 }
@@ -699,6 +723,7 @@ impl TaskError {
             TaskError::LinkPackage(err) => TaskError::LinkPackage(err.clone()),
             TaskError::SymlinkDependencies(err) => TaskError::SymlinkDependencies(err.clone()),
             TaskError::Binaries(err) => TaskError::Binaries(*err),
+            TaskError::DependencyBinaries(errs) => TaskError::DependencyBinaries(errs.clone()),
             TaskError::RunScripts(err) => TaskError::RunScripts(*err),
             TaskError::Patching(_log) => {
                 // `bun_ast::Log` is non-Clone; the only caller of
@@ -1555,7 +1580,7 @@ impl Task {
                 Step::SymlinkDependencyBinaries => {
                     let current_step = Step::SymlinkDependencyBinaries;
                     if let Err(err) = installer.link_dependency_bins(self.entry_id) {
-                        return Ok(Yield::failure(TaskError::Binaries(err)));
+                        return Ok(Yield::failure(err));
                     }
 
                     match pkg_res.tag {
@@ -1853,6 +1878,8 @@ impl Task {
                         bin_linker.target_node_modules_path = bin_linker.node_modules_path;
                         bin_linker.target_package_name =
                             strings::StringOrTinyString::init(dep_name);
+                        bin_linker.err = None;
+                        bin_linker.skipped_due_to_missing_bin = false;
 
                         if manager_ref.options.log_level.is_verbose() {
                             bun_core::pretty_errorln!(
@@ -2304,7 +2331,10 @@ impl<'a> Installer<'a> {
         Ok(changed)
     }
 
-    pub(crate) fn link_dependency_bins(&self, parent_entry_id: StoreEntryId) -> crate::Result<()> {
+    pub(crate) fn link_dependency_bins(
+        &self,
+        parent_entry_id: StoreEntryId,
+    ) -> core::result::Result<(), TaskError> {
         let lockfile = self.lockfile();
         let store = self.store;
 
@@ -2331,6 +2361,7 @@ impl<'a> Installer<'a> {
         let mut link_rel_buf = paths::path_buffer_pool::get();
 
         let mut seen: StringHashMap<()> = StringHashMap::default();
+        let mut failed: Vec<DependencyBinariesError> = Vec::new();
 
         let mut node_modules_path = DefaultAbsPath::init_top_level_dir();
         self.append_real_store_node_modules_path(
@@ -2414,6 +2445,8 @@ impl<'a> Installer<'a> {
             {
                 bin_linker.target_node_modules_path = bin_linker.node_modules_path;
                 bin_linker.target_package_name = package_name;
+                bin_linker.err = None;
+                bin_linker.skipped_due_to_missing_bin = false;
 
                 if self.manager().options.log_level.is_verbose() {
                     bun_core::pretty_errorln!(
@@ -2427,11 +2460,17 @@ impl<'a> Installer<'a> {
             }
 
             if let Some(err) = bin_linker.err {
-                return Err(err);
+                failed.push(DependencyBinariesError {
+                    dep_entry_id: dep.entry_id,
+                    err,
+                });
             }
         }
 
-        Ok(())
+        if failed.is_empty() {
+            return Ok(());
+        }
+        Err(TaskError::DependencyBinaries(failed.into_boxed_slice()))
     }
 
     /// True when this entry should live in the shared global virtual store

test/cli/install/bun-install-registry.test.ts

@@ -2521,6 +2521,40 @@ test("--production without a lockfile will install and not save lockfile", async
 });
 
 describe("binaries", () => {
+  test("a bin that fails to link does not stop the remaining bins of the package from being linked", async () => {
+    // Longer than a file name may be, so creating the link itself fails. It is
+    // the first of the package's bins, both as declared and sorted.
+    const longBinName = Buffer.alloc(300, "a").toString();
+    await Promise.all([
+      write(packageJson, JSON.stringify({ name: "foo", dependencies: { "multi-bin": "file:./multi-bin" } })),
+      write(
+        join(packageDir, "multi-bin", "package.json"),
+        JSON.stringify({
+          name: "multi-bin",
+          version: "1.0.0",
+          bin: { [longBinName]: "cli.js", "multi-b": "cli.js", "multi-c": "cli.js" },
+        }),
+      ),
+      write(join(packageDir, "multi-bin", "cli.js"), "#!/usr/bin/env node\nconsole.log('multi');\n"),
+    ]);
+
+    await using proc = spawn({
+      cmd: [bunExe(), "install"],
+      cwd: packageDir,
+      env,
+      stdout: "pipe",
+      stderr: "pipe",
+    });
+    const [, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+
+    expect(await readdirSorted(join(packageDir, "node_modules", ".bin"))).toEqual(
+      isWindows ? ["multi-b.bunx", "multi-b.exe", "multi-c.bunx", "multi-c.exe"] : ["multi-b", "multi-c"],
+    );
+    expect(join(packageDir, "node_modules", ".bin", "multi-b")).toBeValidBin(join("..", "multi-bin", "cli.js"));
+    expect(stderr).toContain(`error: Failed to link multi-bin: ${isWindows ? "ENOENT" : "ENAMETOOLONG"}`);
+    expect(exitCode).toBe(1);
+  });
+
   for (const global of [false, true]) {
     describe(`existing destinations${global ? " (global)" : ""}`, () => {
       test("existing non-symlink", async () => {

test/cli/install/isolated-install.test.ts

@@ -2,7 +2,7 @@ import { file, spawn, write } from "bun";
 import { afterAll, beforeAll, describe, expect, test } from "bun:test";
 import { existsSync, lstatSync, readlinkSync, statSync } from "fs";
 import { mkdir, readlink, rm, symlink } from "fs/promises";
-import { VerdaccioRegistry, bunEnv, bunExe, readdirSorted, runBunInstall, tempDir } from "harness";
+import { VerdaccioRegistry, bunEnv, bunExe, isWindows, readdirSorted, runBunInstall, tempDir } from "harness";
 import { createRequire } from "module";
 import { dirname, join } from "path";
 
@@ -367,6 +367,96 @@ test("can install folder dependencies on root package", async () => {
   ]);
 });
 
+test("a bin that fails to link does not stop the remaining bins of the package or of its siblings from being linked", async () => {
+  // `directories.bin` names a file, so opening it as a directory fails with
+  // ENOTDIR (a missing directory would be skipped silently).
+  const badBinDir = (name: string) => JSON.stringify({ name, version: "1.0.0", directories: { bin: "package.json" } });
+  // Longer than a file name may be, so creating the link itself fails. It is
+  // the first of the package's bins, both as declared and sorted.
+  const longBinName = Buffer.alloc(300, "a").toString();
+  const longBinNameError = isWindows ? "ENOENT" : "ENAMETOOLONG";
+  const { packageDir } = await registry.createTestDir({
+    bunfigOpts: { linker: "isolated" },
+    files: {
+      "package.json": JSON.stringify({
+        name: "test-pkg-bin-link-errors",
+        workspaces: ["packages/*"],
+        dependencies: {
+          "a-bin": "file:./deps/a-bin",
+          "bad-bin-dir-1": "file:./deps/bad-bin-dir-1",
+          "bad-bin-dir-2": "file:./deps/bad-bin-dir-2",
+          "multi-bin": "file:./deps/multi-bin",
+          "z-bin": "file:./deps/z-bin",
+        },
+      }),
+      "packages/ws/package.json": JSON.stringify({
+        name: "ws",
+        dependencies: {
+          "bad-bin-dir-1": "file:../../deps/bad-bin-dir-1",
+          "z-bin": "file:../../deps/z-bin",
+        },
+      }),
+      "deps/a-bin/package.json": JSON.stringify({ name: "a-bin", version: "1.0.0", bin: { "a-cli": "cli.js" } }),
+      "deps/a-bin/cli.js": "#!/usr/bin/env node\nconsole.log('a');\n",
+      "deps/z-bin/package.json": JSON.stringify({ name: "z-bin", version: "1.0.0", bin: { "z-cli": "cli.js" } }),
+      "deps/z-bin/cli.js": "#!/usr/bin/env node\nconsole.log('z');\n",
+      "deps/bad-bin-dir-1/package.json": badBinDir("bad-bin-dir-1"),
+      "deps/bad-bin-dir-2/package.json": badBinDir("bad-bin-dir-2"),
+      "deps/multi-bin/package.json": JSON.stringify({
+        name: "multi-bin",
+        version: "1.0.0",
+        bin: { [longBinName]: "cli.js", "multi-b": "cli.js", "multi-c": "cli.js" },
+      }),
+      "deps/multi-bin/cli.js": "#!/usr/bin/env node\nconsole.log('multi');\n",
+    },
+  });
+
+  await using proc = spawn({
+    cmd: [bunExe(), "install"],
+    cwd: packageDir,
+    env: bunEnv,
+    stdout: "pipe",
+    stderr: "pipe",
+  });
+  const [, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+
+  const binDir = async (dir: string) => {
+    const path = join(dir, "node_modules", ".bin");
+    return existsSync(path) ? await readdirSorted(path) : [];
+  };
+  const bins = (...names: string[]) => (isWindows ? names.flatMap(n => [`${n}.bunx`, `${n}.exe`]) : names).sort();
+  expect({
+    root: await binDir(packageDir),
+    ws: await binDir(join(packageDir, "packages", "ws")),
+    "multi-bin's own": await binDir(join(packageDir, "node_modules", ".bun", "multi-bin@file+deps+multi-bin")),
+  }).toEqual({
+    root: bins("a-cli", "multi-b", "multi-c", "z-cli"),
+    ws: bins("z-cli"),
+    "multi-bin's own": bins("multi-b", "multi-c"),
+  });
+
+  // Every failing package is reported once for its own `.bin` and once per
+  // package whose `.bin` it could not be linked into. Tasks run in parallel,
+  // so sort.
+  const binErrors = stderr
+    .split("\n")
+    .filter(line => line.includes("failed to link binaries"))
+    .map(line => line.replaceAll("\\", "/"))
+    .sort();
+  expect(binErrors).toEqual(
+    [
+      "ENOTDIR: failed to link binaries for package: bad-bin-dir-1@deps/bad-bin-dir-1",
+      "ENOTDIR: failed to link binaries for package: bad-bin-dir-2@deps/bad-bin-dir-2",
+      `${longBinNameError}: failed to link binaries for package: multi-bin@deps/multi-bin`,
+      "ENOTDIR: failed to link binaries of dependency bad-bin-dir-1@deps/bad-bin-dir-1 for package: test-pkg-bin-link-errors@",
+      "ENOTDIR: failed to link binaries of dependency bad-bin-dir-1@deps/bad-bin-dir-1 for package: ws@workspace:packages/ws",
+      "ENOTDIR: failed to link binaries of dependency bad-bin-dir-2@deps/bad-bin-dir-2 for package: test-pkg-bin-link-errors@",
+      `${longBinNameError}: failed to link binaries of dependency multi-bin@deps/multi-bin for package: test-pkg-bin-link-errors@`,
+    ].sort(),
+  );
+  expect(exitCode).toBe(1);
+});
+
 describe("isolated workspaces", () => {
   test("basic", async () => {
     const { packageJson, packageDir } = await registry.createTestDir({ bunfigOpts: { linker: "isolated" } });