Skip to content
Open
Show file tree
Hide file tree
Changes from 4 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
5 changes: 4 additions & 1 deletion src/jsc/NodeCompileCache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -158,8 +158,11 @@ macro_rules! cclog {
};
}

/// `ENOENT`, not the bare `NOENT` that `<&str>::from(E)` yields on Windows.
fn errno_name(e: &sys::Error) -> &'static str {
<&'static str>::from(e.get_errno())
e.get_error_code_tag_name()
.map(|(name, _)| name)
.unwrap_or("UNKNOWN")
}

/// Read-log tail for an I/O error; ENOENT uses Node's exact wording.
Expand Down
92 changes: 89 additions & 3 deletions src/paths/component_iterator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -183,8 +183,9 @@ pub enum MakePathStep<E> {
Created,
/// Directory already exists (`EEXIST`). Walk advances forward.
Exists,
/// A parent is missing (`ENOENT`). Walk steps back one component;
/// if there is no previous component the carried error is returned.
/// A parent is missing (`ENOENT`). Walk steps back one component; if
/// there is none, or the walk already advanced forward, the carried error
/// is returned.
Comment thread
robobun marked this conversation as resolved.
Resolved
NotFound(E),
}

Expand All @@ -195,7 +196,9 @@ pub enum MakePathStep<E> {
/// Starts at `it.last()`; on `Created`/`Exists` advances via `next()`
/// (returning `Ok(())` when there is none), on `NotFound(e)` steps back via
/// `previous()` (returning `Err(e)` when there is none — i.e. the very first
/// component's parent does not exist).
/// component's parent does not exist). After the walk has advanced, a
/// `NotFound` returns `Err(e)` at once: the parent exists but cannot hold
/// children (a dangling symlink, procfs), so stepping back cannot make progress.
Comment thread
robobun marked this conversation as resolved.
Resolved
///
/// `mkdir` is invoked with `component.path`: a borrowed prefix slice into the
/// original input, never NUL-terminated. Callers that need a sentinel must
Expand All @@ -207,14 +210,17 @@ pub fn make_path_with<'a, T: PathChar, E>(
let Some(mut comp) = it.last() else {
return Ok(());
};
let mut advanced = false;
loop {
match mkdir(comp.path)? {
MakePathStep::Created | MakePathStep::Exists => {
advanced = true;
comp = match it.next() {
Some(c) => c,
None => return Ok(()),
};
}
MakePathStep::NotFound(e) if advanced => return Err(e),
MakePathStep::NotFound(e) => {
comp = match it.previous() {
Some(c) => c,
Expand Down Expand Up @@ -376,4 +382,84 @@ mod tests {
assert_eq!(it.next().unwrap().name, b"c");
assert!(it.next().is_none());
}

#[test]
fn make_path_creates_missing_parents() {
// Only the cwd exists: the walk backs up to `a`, then creates forward.
let it = ComponentIterator::init(&b"a/b/c"[..], PathFormat::Posix).unwrap();
let mut created: Vec<&[u8]> = vec![];
let mut attempts: Vec<&[u8]> = vec![];
let r = make_path_with::<u8, ()>(it, |p| {
attempts.push(p);
let parent: &[u8] = match p {
b"a" => b"",
b"a/b" => b"a",
b"a/b/c" => b"a/b",
_ => unreachable!(),
};
if parent.is_empty() || created.contains(&parent) {
created.push(p);
Ok(MakePathStep::Created)
} else {
Ok(MakePathStep::NotFound(()))
}
});
assert_eq!(r, Ok(()));
assert_eq!(
attempts,
vec![
&b"a/b/c"[..],
&b"a/b"[..],
&b"a"[..],
&b"a/b"[..],
&b"a/b/c"[..]
]
);
assert_eq!(created, vec![&b"a"[..], &b"a/b"[..], &b"a/b/c"[..]]);
}

#[test]
fn make_path_stops_when_parent_exists_but_child_is_not_found() {
// `/a/b` is a dangling symlink: EEXIST for itself, ENOENT below it.
let it = ComponentIterator::init(&b"/a/b/c"[..], PathFormat::Posix).unwrap();
let mut attempts: Vec<&[u8]> = vec![];
let r = make_path_with(it, |p| {
attempts.push(p);
assert!(attempts.len() < 100, "runaway loop");
match p {
b"/a" | b"/a/b" => Ok(MakePathStep::Exists),
b"/a/b/c" => Ok(MakePathStep::NotFound(p)),
_ => unreachable!(),
}
});
assert_eq!(r, Err(&b"/a/b/c"[..]));
assert_eq!(attempts, vec![&b"/a/b/c"[..], &b"/a/b"[..], &b"/a/b/c"[..]]);
}

#[test]
fn make_path_stops_when_open_if_parent_succeeds_but_child_is_not_found() {
// `NtCreateFile(FILE_OPEN_IF)` reports `Created` for an existing
// directory too, so the guard must not depend on `Exists`.
Comment thread
robobun marked this conversation as resolved.
Resolved
let it = ComponentIterator::init(&b"x\\link\\out"[..], PathFormat::Windows).unwrap();
let mut attempts = 0u32;
let r = make_path_with(it, |p| {
attempts += 1;
assert!(attempts < 100, "runaway loop");
match p {
b"x" | b"x\\link" => Ok(MakePathStep::Created),
b"x\\link\\out" => Ok(MakePathStep::NotFound(())),
_ => unreachable!(),
}
});
assert_eq!(r, Err(()));
assert_eq!(attempts, 3);
}

#[test]
fn make_path_reports_missing_root_parent() {
// Nothing under the cwd exists: the first component's error comes back.
let it = ComponentIterator::init(&b"a/b"[..], PathFormat::Posix).unwrap();
let r = make_path_with(it, |p| Ok(MakePathStep::NotFound(p)));
assert_eq!(r, Err(&b"a"[..]));
}
}
25 changes: 25 additions & 0 deletions test/bundler/bun-build-api.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -527,6 +527,31 @@ describe("Bun.build", () => {
Bun.gc(true);
});

test("outdir under a dangling symlink fails with ENOENT instead of never settling", async () => {
// mkdir on the link reports EEXIST, mkdir below it reports ENOENT. The
// recursive mkdir used to retry that pair forever and the build promise
// never settled. Run in a child so the spin cannot take the runner down.
using dir = tempDir("build-outdir-dangling", {
"index.js": `console.log("built");`,
"build.js": `
const r = await Bun.build({ entrypoints: ["./index.js"], outdir: "./dangling/out", throw: false });
console.log(r.success, r.logs.map(l => l.message).join("\\n"));
`,
});
symlinkSync(join(String(dir), "does-not-exist"), join(String(dir), "dangling"));

await using proc = Bun.spawn({
cmd: [bunExe(), "build.js"],
env: bunEnv,
cwd: String(dir),
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect(stdout).toContain("false Failed to create output directory ENOENT");
expect(stderr).toBe("");
expect(exitCode).toBe(0);
});

test("BuildArtifact properties", async () => {
Bun.gc(true);
const outdir = tempDirWithFiles("build-artifact-properties", {
Expand Down
20 changes: 20 additions & 0 deletions test/bundler/cli.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -443,6 +443,26 @@ test("--outdir build succeeds when the output directory already exists with prio
expect(out).not.toContain("stale");
});

// mkdir on a dangling symlink reports EEXIST, mkdir below it reports ENOENT.
// The recursive mkdir behind --outdir used to retry that pair forever.
test("--outdir fails with ENOENT when a parent of the output directory is a dangling symlink", async () => {
using dir = tempDir("build-outdir-dangling-symlink", {
"entry.ts": `console.log("built");`,
});
fs.symlinkSync(path.join(String(dir), "does-not-exist"), path.join(String(dir), "dangling"));

await using proc = Bun.spawn({
cmd: [bunExe(), "build", "entry.ts", "--outdir", path.join("dangling", "out")],
env: bunEnv,
cwd: String(dir),
stdout: "ignore",
stderr: "pipe",
});
const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]);
expect(stderr).toContain("Failed to create output directory ENOENT");
expect(exitCode).toBe(1);
});
Comment thread
robobun marked this conversation as resolved.
Resolved

test("multi-entry build writes each entry point into the output directory", async () => {
using dir = tempDir("build-multi-entry-outdir", {
"a.ts": `export const a: number = 1;\nconsole.log("A" + a);`,
Expand Down
31 changes: 31 additions & 0 deletions test/cli/install/bun-install-cache-dir.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import { expect, test } from "bun:test";
import { bunEnv, bunExe, tempDir } from "harness";
import { symlinkSync } from "node:fs";
import { join } from "node:path";

// mkdir on a dangling symlink reports EEXIST, mkdir below it reports ENOENT.
// The recursive mkdir behind the install cache directory used to retry that
// pair forever, so `bun install` sat at 100% CPU before resolving anything.
// The folder dependency keeps the install off the network.
test("install completes when BUN_INSTALL_CACHE_DIR is under a dangling symlink", async () => {
Comment thread
robobun marked this conversation as resolved.
Resolved
using dir = tempDir("install-cache-dir-dangling", {
"proj/package.json": JSON.stringify({
name: "x",
version: "1.0.0",
dependencies: { localdep: "file:./localdep" },
}),
"proj/localdep/package.json": JSON.stringify({ name: "localdep", version: "1.2.3" }),
});
symlinkSync(join(String(dir), "does-not-exist"), join(String(dir), "dangling"));

await using proc = Bun.spawn({
cmd: [bunExe(), "install"],
cwd: join(String(dir), "proj"),
env: { ...bunEnv, BUN_INSTALL_CACHE_DIR: join(String(dir), "dangling", "cache") },
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect({ stdout, stderr }).toMatchObject({ stdout: expect.stringContaining("1 package installed") });
Comment thread
robobun marked this conversation as resolved.
Resolved
expect(exitCode).toBe(0);
});
44 changes: 44 additions & 0 deletions test/js/node/module/node-module-module.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,50 @@ console.log("survived", require("./late.js"));`,
},
);

test("compile cache under a dangling symlink is reported as FAILED, not retried forever", async () => {
// mkdir on the link reports EEXIST, mkdir below it reports ENOENT. The
// recursive mkdir used to bounce between the two forever, so a bad
// NODE_COMPILE_CACHE hung every process before any user code ran.
using dir = tempDir("compile-cache-dangling", {
"main.js": `
const Module = require("node:module");
const r = Module.enableCompileCache(process.env.CACHE_DIR);
console.log(r.status === Module.constants.compileCacheStatus.FAILED, r.message);
`,
});
const link = path.join(String(dir), "dangling");
fs.symlinkSync(path.join(String(dir), "does-not-exist"), link);
const cacheDir = path.join(link, "cc");
const env = { ...bunEnv };
delete env.NODE_COMPILE_CACHE;

{
await using proc = Bun.spawn({
cmd: [bunExe(), "-e", "console.log('started')"],
env: { ...env, NODE_COMPILE_CACHE: cacheDir },
cwd: String(dir),
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect(stdout).toBe("started\n");
expect(stderr).toBe("");
expect(exitCode).toBe(0);
}

{
await using proc = Bun.spawn({
cmd: [bunExe(), "main.js"],
env: { ...env, CACHE_DIR: cacheDir },
cwd: String(dir),
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect(stdout).toBe("true Cannot create cache directory: ENOENT\n");
expect(stderr).toBe("");
expect(exitCode).toBe(0);
}
});

test.skipIf(!isWindows)("enableCompileCache default dir prefers TEMP over TMP like os.tmpdir", async () => {
using dir = tempDir("compile-cache-tmporder", {});
const temp = path.join(String(dir), "from-temp");
Expand Down
Loading