Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
91 changes: 55 additions & 36 deletions src/install/PackageManager/PackageManagerDirectories.rs
Original file line number Diff line number Diff line change
Expand Up @@ -313,64 +313,78 @@
#[cold]
#[inline(never)]
unsafe fn ensure_cache_directory(this: *mut PackageManager) -> Dir {
loop {
// SAFETY: field projections through the caller-provided provenance
// root; see fn safety contract. Project `enable` narrowly so callers
// may hold borrows into disjoint `options` sub-fields.
if unsafe { (*this).options.enable.contains(Enable::CACHE) } {
// SAFETY: caller-provided provenance root; `env_mut()` itself
// encapsulates the BackRef deref + singleton-liveness invariant.
let env = unsafe { &*this }.env_mut();
// SAFETY: shared read of `options`; disjoint from `cache_directory_path`.
let cache_dir = fetch_cache_directory_path(env, Some(unsafe { &(*this).options }));
// SAFETY: see fn safety contract.
unsafe { (*this).cache_directory_path = ZBox::from_bytes(&cache_dir.path) };

match Dir::cwd().make_open_path(&cache_dir.path, Default::default()) {
Ok(d) => return d,
Err(_) => {
// SAFETY: narrow `&mut enable` projection; disjoint from
// any `&options.{registries,scope}` the caller may hold.
unsafe { (*this).options.enable.set(Enable::CACHE, false) };
// SAFETY: see fn safety contract.
unsafe { (*this).cache_directory_path = ZBox::from_bytes(b"") };
continue;
}
}
}

// SAFETY: field projections through the caller-provided provenance
// root; see fn safety contract. Project `enable` narrowly so callers
// may hold borrows into disjoint `options` sub-fields.
if unsafe { (*this).options.enable.contains(Enable::CACHE) } {
// SAFETY: caller-provided provenance root; `env_mut()` itself
// encapsulates the BackRef deref + singleton-liveness invariant.
let env = unsafe { &*this }.env_mut();
// SAFETY: shared read of `options`; disjoint from `cache_directory_path`.
let cache_dir = fetch_cache_directory_path(env, Some(unsafe { &(*this).options }));
// SAFETY: see fn safety contract.
unsafe {
(*this).cache_directory_path =
ZBox::from_bytes(path::resolve_path::join_abs_string::<path::platform::Auto>(
FileSystem::instance().top_level_dir(),
&[b"node_modules", b".cache"],
))
};
unsafe { (*this).cache_directory_path = ZBox::from_bytes(&cache_dir.path) };

match Dir::cwd().make_open_path(b"node_modules/.cache", Default::default()) {
match Dir::cwd().make_open_path(&cache_dir.path, Default::default()) {
Ok(d) => return d,
Err(err) => {
Err(err) if cache_dir.is_explicit => {
bun_core::pretty_errorln!(
"<r><red>error<r>: bun is unable to write files: {}",
"<r><red>error<r>: cache directory {} is not creatable: {}",
bun_fmt::quote(&cache_dir.path),
bun_fmt::s(err.name())
);
Global::crash();
}
Err(err) => {
// SAFETY: shared read of `options.log_level`; see fn safety contract.
if unsafe { (*this).options.log_level } != LogLevel::Silent {

Check warning on line 340 in src/install/PackageManager/PackageManagerDirectories.rs

View check run for this annotation

Claude / Claude Code Review

ensure_cache_directory # Safety doc omits newly-read options.log_level

Nit: the fn-level `# Safety` doc on `ensure_cache_directory` enumerates the exact `options` sub-fields projected ("only `options.enable`, `options.cache_directory` (read), `env`, and `cache_directory_path` are touched"), but 5456aa19 added a read of `(*this).options.log_level` here whose inline SAFETY comment says "see fn safety contract" — pointing at a contract that doesn't list `log_level`. Practically harmless (line 324 already takes `&(*this).options` as a whole shared borrow, so no caller
Comment thread
robobun marked this conversation as resolved.
bun_core::pretty_errorln!(
"<r><yellow>warn<r>: cache directory {} is not creatable: {}, falling back to node_modules/.cache",
bun_fmt::quote(&cache_dir.path),
bun_fmt::s(err.name())
);
}
// SAFETY: narrow `&mut enable` projection; disjoint from
// any `&options.{registries,scope}` the caller may hold.
unsafe { (*this).options.enable.set(Enable::CACHE, false) };
}
}
}

// SAFETY: see fn safety contract.
unsafe {
(*this).cache_directory_path =
ZBox::from_bytes(path::resolve_path::join_abs_string::<path::platform::Auto>(
FileSystem::instance().top_level_dir(),
&[b"node_modules", b".cache"],
))
};

match Dir::cwd().make_open_path(b"node_modules/.cache", Default::default()) {
Ok(d) => d,
Err(err) => {
bun_core::pretty_errorln!(
"<r><red>error<r>: bun is unable to write files: {}",
bun_fmt::s(err.name())
);
Global::crash();
}
}
}

pub struct CacheDir {
pub path: Vec<u8>,
pub is_node_modules: bool,
/// `BUN_INSTALL_CACHE_DIR`, `--cache-dir`, or bunfig `install.cache.dir`.
pub is_explicit: bool,
}

pub fn fetch_cache_directory_path(env: &mut DotEnvLoader, options: Option<&Options>) -> CacheDir {
if let Some(dir) = env.get(b"BUN_INSTALL_CACHE_DIR") {
return CacheDir {
path: FileSystem::instance().abs(&[dir]).to_vec(),
is_node_modules: false,
is_explicit: true,
};
}

Expand All @@ -379,6 +393,7 @@
return CacheDir {
path: FileSystem::instance().abs(&[opts.cache_directory]).to_vec(),
is_node_modules: false,
is_explicit: true,
};
}
}
Expand All @@ -388,6 +403,7 @@
return CacheDir {
path: FileSystem::instance().abs(&parts).to_vec(),
is_node_modules: false,
is_explicit: false,
};
}

Expand All @@ -396,6 +412,7 @@
return CacheDir {
path: FileSystem::instance().abs(&parts).to_vec(),
is_node_modules: false,
is_explicit: false,
};
}

Expand All @@ -404,12 +421,14 @@
return CacheDir {
path: FileSystem::instance().abs(&parts).to_vec(),
is_node_modules: false,
is_explicit: false,
};
}

let fallback_parts: [&[u8]; 1] = [b"node_modules/.bun-cache"];
CacheDir {
is_node_modules: true,
is_explicit: false,
path: FileSystem::instance().abs(&fallback_parts).to_vec(),
}
}
Expand Down
53 changes: 51 additions & 2 deletions src/paths/component_iterator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -193,8 +193,8 @@ 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`). Returns the error if the walk has
/// already advanced or there is no previous component; else steps back.
Comment thread
robobun marked this conversation as resolved.
NotFound(E),
}

Expand All @@ -217,14 +217,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),
Comment thread
robobun marked this conversation as resolved.
MakePathStep::NotFound(e) => {
comp = match it.previous() {
Some(c) => c,
Expand Down Expand Up @@ -389,4 +392,50 @@ mod tests {
assert_eq!(it.next().unwrap().name, b"c");
assert!(it.next().is_none());
}

#[test]
fn make_path_terminates_when_parent_exists_but_child_is_enoent() {
// `/a/b` is a dangling symlink: EEXIST itself, ENOENT for any child.
let it = ComponentIterator::init(&b"/a/b/c"[..], PathFormat::Posix).unwrap();
let mut calls = 0u32;
let r = make_path_with(it, |p| {
calls += 1;
assert!(calls < 100, "runaway loop");
match p {
b"/a" | b"/a/b" => Ok(MakePathStep::Exists),
b"/a/b/c" => Ok(MakePathStep::NotFound(())),
_ => unreachable!(),
}
});
assert!(r.is_err());
// leaf ENOENT → parent EEXIST → leaf ENOENT → stop.
assert_eq!(calls, 3);
}

#[test]
fn make_path_walks_back_then_forward() {
// `/a` exists, `/a/b` and `/a/b/c` do not.
let it = ComponentIterator::init(&b"/a/b/c"[..], PathFormat::Posix).unwrap();
let mut created: Vec<&[u8]> = vec![];
let mut calls = 0u32;
let r = make_path_with::<u8, ()>(it, |p| {
calls += 1;
assert!(calls < 100, "runaway loop");
if p == b"/a" {
return Ok(MakePathStep::Exists);
}
if created.iter().any(|c| *c == p) {
return Ok(MakePathStep::Exists);
}
let parent = &p[..p.iter().rposition(|b| *b == b'/').unwrap()];
if parent == b"/a" || created.iter().any(|c| *c == parent) {
created.push(p);
Ok(MakePathStep::Created)
} else {
Ok(MakePathStep::NotFound(()))
}
});
assert!(r.is_ok());
assert_eq!(created, vec![&b"/a/b"[..], &b"/a/b/c"[..]]);
}
}
131 changes: 131 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,131 @@
import { describe, expect, test } from "bun:test";
import { bunEnv, bunExe, isWindows, tempDir } from "harness";
import { existsSync, symlinkSync } from "node:fs";
import { join } from "node:path";

describe.skipIf(isWindows)("install cache directory inside a dangling symlink", () => {
function setup() {
const dir = tempDir("cache-dir-dangling", {
"proj/package.json": JSON.stringify({
name: "x",
version: "1.0.0",
dependencies: { "any-pkg": "1.0.0" },
}),
});
const root = String(dir);
symlinkSync(join(root, "does-not-exist"), join(root, "dangling"));
return { dir, root, dangling: join(root, "dangling"), cwd: join(root, "proj") };
}

function explicitEnv(cacheDir: string) {
return {
...bunEnv,
BUN_INSTALL_CACHE_DIR: cacheDir,
BUN_CONFIG_REGISTRY: "http://127.0.0.1:1/",
};
}

test.concurrent("bun install exits with a cache-directory error instead of spinning in mkdirat", async () => {
const ctx = setup();
using _dir = ctx.dir;
const cacheDir = join(ctx.dangling, "cache");
await using proc = Bun.spawn({
cmd: [bunExe(), "install"],
cwd: ctx.cwd,
env: explicitEnv(cacheDir),
stdout: "pipe",
stderr: "pipe",
timeout: 15_000,
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);

expect({ signalCode: proc.signalCode, stdout, stderr }).toMatchObject({
signalCode: null,
stderr: expect.stringContaining(`error: cache directory "${cacheDir}" is not creatable: ENOENT`),
});
expect(stderr).not.toContain("ConnectionRefused");
expect(exitCode).not.toBe(0);
});

test.concurrent("bunfig install.cache.dir exits with a cache-directory error instead of spinning", async () => {
using dir = tempDir("cache-dir-dangling-bunfig", {
"proj/package.json": JSON.stringify({ name: "x", version: "1.0.0", dependencies: { "any-pkg": "1.0.0" } }),
});
const root = String(dir);
symlinkSync(join(root, "does-not-exist"), join(root, "dangling"));
const cacheDir = join(root, "dangling", "cache");
await Bun.write(
join(root, "proj", "bunfig.toml"),
`[install]\nregistry = "http://127.0.0.1:1/"\n[install.cache]\ndir = ${JSON.stringify(cacheDir)}\n`,
);
const env = { ...bunEnv };
delete env.BUN_INSTALL_CACHE_DIR;

await using proc = Bun.spawn({
cmd: [bunExe(), "install"],
cwd: join(root, "proj"),
env,
stdout: "pipe",
stderr: "pipe",
timeout: 15_000,
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);

expect({ signalCode: proc.signalCode, stdout, stderr }).toMatchObject({
signalCode: null,
stderr: expect.stringContaining(`error: cache directory "${cacheDir}" is not creatable: ENOENT`),
});
expect(stderr).not.toContain("ConnectionRefused");
expect(exitCode).not.toBe(0);
});
Comment thread
robobun marked this conversation as resolved.

test.concurrent("runtime auto-install exits with a cache-directory error instead of spinning", async () => {
const ctx = setup();
using _dir = ctx.dir;
const cacheDir = join(ctx.dangling, "cache");
await using proc = Bun.spawn({
cmd: [bunExe(), "-e", `require("any-pkg")`],
cwd: ctx.cwd,
env: explicitEnv(cacheDir),
stdout: "pipe",
stderr: "pipe",
timeout: 15_000,
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);

expect({ signalCode: proc.signalCode, stdout, stderr }).toMatchObject({
signalCode: null,
stderr: expect.stringContaining(`error: cache directory "${cacheDir}" is not creatable: ENOENT`),
});
expect(exitCode).not.toBe(0);
});

test.concurrent("implicit $HOME-derived cache dir warns and falls back to node_modules/.cache", async () => {
const ctx = setup();
using _dir = ctx.dir;
const env = { ...bunEnv, BUN_CONFIG_REGISTRY: "http://127.0.0.1:1/" };
delete env.BUN_INSTALL_CACHE_DIR;
delete env.BUN_INSTALL;
delete env.XDG_CACHE_HOME;
env.HOME = ctx.dangling;

await using proc = Bun.spawn({
cmd: [bunExe(), "install"],
cwd: ctx.cwd,
env,
stdout: "pipe",
stderr: "pipe",
timeout: 15_000,
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);

expect({ signalCode: proc.signalCode, stdout, stderr }).toMatchObject({
signalCode: null,
stderr: expect.stringContaining(`is not creatable: ENOENT, falling back to node_modules/.cache`),
});
expect(stderr).toContain("warn: cache directory");
expect(stderr).toContain("ConnectionRefused");
expect(existsSync(join(ctx.cwd, "node_modules", ".cache"))).toBe(true);
expect(exitCode).not.toBe(0);
});
});
Loading