diff --git a/src/install/PackageManager/PackageManagerDirectories.rs b/src/install/PackageManager/PackageManagerDirectories.rs index e787e9fac145..5c070ec10612 100644 --- a/src/install/PackageManager/PackageManagerDirectories.rs +++ b/src/install/PackageManager/PackageManagerDirectories.rs @@ -313,50 +313,61 @@ fn get_temporary_directory_run(manager: &mut PackageManager) -> TemporaryDirecto #[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::( - 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!( - "error: bun is unable to write files: {}", + "error: 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 { + bun_core::pretty_errorln!( + "warn: 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::( + 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!( + "error: bun is unable to write files: {}", + bun_fmt::s(err.name()) + ); + Global::crash(); } } } @@ -364,6 +375,8 @@ unsafe fn ensure_cache_directory(this: *mut PackageManager) -> Dir { pub struct CacheDir { pub path: Vec, 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 { @@ -371,6 +384,7 @@ pub fn fetch_cache_directory_path(env: &mut DotEnvLoader, options: Option<&Optio return CacheDir { path: FileSystem::instance().abs(&[dir]).to_vec(), is_node_modules: false, + is_explicit: true, }; } @@ -379,6 +393,7 @@ pub fn fetch_cache_directory_path(env: &mut DotEnvLoader, options: Option<&Optio return CacheDir { path: FileSystem::instance().abs(&[opts.cache_directory]).to_vec(), is_node_modules: false, + is_explicit: true, }; } } @@ -388,6 +403,7 @@ pub fn fetch_cache_directory_path(env: &mut DotEnvLoader, options: Option<&Optio return CacheDir { path: FileSystem::instance().abs(&parts).to_vec(), is_node_modules: false, + is_explicit: false, }; } @@ -396,6 +412,7 @@ pub fn fetch_cache_directory_path(env: &mut DotEnvLoader, options: Option<&Optio return CacheDir { path: FileSystem::instance().abs(&parts).to_vec(), is_node_modules: false, + is_explicit: false, }; } @@ -404,12 +421,14 @@ pub fn fetch_cache_directory_path(env: &mut DotEnvLoader, options: Option<&Optio 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(), } } diff --git a/src/paths/component_iterator.rs b/src/paths/component_iterator.rs index 61612cffadbc..1528449771e1 100644 --- a/src/paths/component_iterator.rs +++ b/src/paths/component_iterator.rs @@ -193,8 +193,8 @@ pub enum MakePathStep { 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. NotFound(E), } @@ -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), MakePathStep::NotFound(e) => { comp = match it.previous() { Some(c) => c, @@ -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::(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"[..]]); + } } diff --git a/test/cli/install/bun-install-cache-dir.test.ts b/test/cli/install/bun-install-cache-dir.test.ts new file mode 100644 index 000000000000..7218806ed574 --- /dev/null +++ b/test/cli/install/bun-install-cache-dir.test.ts @@ -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); + }); + + 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); + }); +});