Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 39 additions & 4 deletions src/install/isolated_install/Installer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>",
Expand Down Expand Up @@ -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),
}
Expand All @@ -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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -2304,7 +2329,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;

Expand All @@ -2331,6 +2359,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(
Expand Down Expand Up @@ -2427,11 +2456,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
Expand Down
73 changes: 72 additions & 1 deletion test/cli/install/isolated-install.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand Down Expand Up @@ -367,6 +367,77 @@ test("can install folder dependencies on root package", async () => {
]);
});

test("a dependency whose bins fail to link does not stop its siblings' bins 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" } });
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",
"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"),
},
});

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 linkedBin = (dir: string, name: string) =>
existsSync(join(dir, "node_modules", ".bin", isWindows ? `${name}.bunx` : name));
expect({
"root a-cli": linkedBin(packageDir, "a-cli"),
"root z-cli": linkedBin(packageDir, "z-cli"),
"ws z-cli": linkedBin(join(packageDir, "packages", "ws"), "z-cli"),
}).toEqual({
"root a-cli": true,
"root z-cli": true,
"ws z-cli": true,
});

// Every failing dependency is reported, attributed to the 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",
"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@",
]);
expect(exitCode).toBe(1);
});

describe("isolated workspaces", () => {
test("basic", async () => {
const { packageJson, packageDir } = await registry.createTestDir({ bunfigOpts: { linker: "isolated" } });
Expand Down
Loading