From 41e89df105b8e3d6d37f3d1ac9f04d53c23ccce3 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 7 Aug 2026 14:34:04 +0000 Subject: [PATCH 01/11] install: reuse lockfile-resolved git packages for branch and bare refs when re-resolving The text lockfile writes the resolved commit in the committish position of a git/github resolution string (git+url#), so reloading it parses the sha as the committish and the original ref is lost. Re-resolution (any edit that re-parses a workspace member's dependency list) then re-enqueues the dependency, and the in-memory lookup compares the fresh committish ("main", "v1", or empty) against the loaded sha, misses, and fetches the remote again on every install. Reuse the package an identical dependency literal is already bound to in the loaded lockfile instead. Skipped for bun update targets, which must keep re-resolving the ref against the remote. --- .../PackageManager/PackageManagerEnqueue.rs | 93 +++++++ test/cli/install/bun-lock.test.ts | 245 ++++++++++++++++++ 2 files changed, 338 insertions(+) diff --git a/src/install/PackageManager/PackageManagerEnqueue.rs b/src/install/PackageManager/PackageManagerEnqueue.rs index f0c91da56b53..84980e9fb192 100644 --- a/src/install/PackageManager/PackageManagerEnqueue.rs +++ b/src/install/PackageManager/PackageManagerEnqueue.rs @@ -1182,6 +1182,16 @@ pub fn enqueue_dependency_with_main_and_success_fn( return Ok(()); } + // Second: reuse the package an identical dependency is already + // bound to. A branch, tag, or bare committish can never match the + // lookup above after a lockfile round-trip. + if let Some(pkg_id) = + find_locked_git_package(this, id, dependency, &dep, ResolutionTag::Git) + { + success_fn(this, id, pkg_id); + return Ok(()); + } + // reshaped for borrowck — `alias`/`url` borrow // `this.lockfile.buffers.string_bytes`; detach the slice // lifetimes so the `&mut PackageManager` reborrows for the @@ -1292,6 +1302,16 @@ pub fn enqueue_dependency_with_main_and_success_fn( return Ok(()); } + // Second: reuse the package an identical dependency is already + // bound to. A branch, tag, or bare committish can never match the + // lookup above after a lockfile round-trip. + if let Some(pkg_id) = + find_locked_git_package(this, id, dependency, dep, ResolutionTag::Github) + { + success_fn(this, id, pkg_id); + return Ok(()); + } + let url = this.alloc_github_url(dep); // url is Box<[u8]>; dropped at scope end let task_id = Task::Id::for_tarball(&url); @@ -1933,6 +1953,79 @@ fn update_name_and_name_hash_from_version_replacement( } } +/// Finds the package an identical, already-resolved dependency is bound to, so +/// a git/github dependency re-enqueued during re-resolution (most commonly a +/// workspace member edit re-parsing the member's whole dependency list) reuses +/// it instead of fetching the remote again. +/// +/// The `get_package_id` lookup in the git/github arms can only match a +/// committish equal to the loaded one, and the text lockfile writes the +/// *resolved* commit in the committish position of the resolution string, so a +/// branch, tag, or bare ref never matches again after a lockfile round-trip. +/// Dependency entries keep their original version literals, so an identical +/// literal bound to a package of the same repository is the lossless record of +/// what this dependency previously resolved to. +fn find_locked_git_package( + this: &PackageManager, + id: DependencyID, + dependency: &Dependency, + repo: &Repository, + resolution_tag: ResolutionTag, +) -> Option { + if this.lockfile.buffers.resolutions[id as usize] != invalid_package_id { + return None; + } + + // `bun update` re-resolves git refs against the remote; reusing the locked + // commit here would turn the update into a no-op. Same update-target test + // as `Diff::generate` (an empty request list is a bare `bun update`). + if this.to_update + && (this.update_requests.is_empty() + || this + .update_requests + .iter() + .any(|request| request.name_hash == dependency.name_hash)) + { + return None; + } + + let buf = this.lockfile.buffers.string_bytes.as_slice(); + let package_resolutions = this.lockfile.packages.items_resolution(); + let dependencies = this.lockfile.buffers.dependencies.as_slice(); + let resolutions = this.lockfile.buffers.resolutions.as_slice(); + + for (other, &package_id) in dependencies.iter().zip(resolutions) { + if package_id == invalid_package_id || (package_id as usize) >= package_resolutions.len() { + continue; + } + if other.name_hash != dependency.name_hash + || other.version.tag != dependency.version.tag + || !other + .version + .literal + .eql(dependency.version.literal, buf, buf) + { + continue; + } + // Overrides and catalogs replace the version after parsing, so an + // identical literal does not guarantee an identical resolution; the + // resolved repository must match the effective one too. (A changed + // override/catalog also invalidates the old binding before + // re-enqueueing, so a stale binding is never reachable here.) + let resolution = &package_resolutions[package_id as usize]; + if resolution.tag != resolution_tag { + continue; + } + let locked = resolution.repository(); + if !locked.repo.eql(repo.repo, buf, buf) || !locked.owner.eql(repo.owner, buf, buf) { + continue; + } + return Some(package_id); + } + + None +} + pub(crate) enum ResolvedPackageTask { /// Pending network task to schedule NetworkTask(*mut NetworkTask), diff --git a/test/cli/install/bun-lock.test.ts b/test/cli/install/bun-lock.test.ts index 8180f837292a..93750d1a5203 100644 --- a/test/cli/install/bun-lock.test.ts +++ b/test/cli/install/bun-lock.test.ts @@ -1012,3 +1012,248 @@ it("optional peer with a non-wildcard range is idempotent with two versions of t await rm(join(packageDir, "node_modules"), { recursive: true, force: true }); await run(["install", "--frozen-lockfile"]); }); + +// Minimal gzipped tarball with a single root folder wrapping the files, the +// shape of both github codeload tarballs and npm pack tarballs. +function makeTarball(rootDir: string, files: Record): Uint8Array { + function tarHeader(name: string, size: number, isDir: boolean): Uint8Array { + const header = new Uint8Array(512); + const encoder = new TextEncoder(); + header.set(encoder.encode(name), 0); + header.set(encoder.encode(isDir ? "0000755 " : "0000644 "), 100); + header.set(encoder.encode("0000000 "), 108); + header.set(encoder.encode("0000000 "), 116); + header.set(encoder.encode(size.toString(8).padStart(11, "0") + " "), 124); + header.set(encoder.encode("00000000000 "), 136); + header.set(encoder.encode(" "), 148); + header[156] = (isDir ? "5" : "0").charCodeAt(0); + header.set(encoder.encode("ustar"), 257); + header.set(encoder.encode("00"), 263); + let checksum = 0; + for (const byte of header) checksum += byte; + header.set(encoder.encode(checksum.toString(8).padStart(6, "0") + "\0 "), 148); + return header; + } + const blocks: Uint8Array[] = []; + blocks.push(tarHeader(`${rootDir}/`, 0, true)); + for (const [name, contents] of Object.entries(files)) { + const bytes = new TextEncoder().encode(contents); + blocks.push(tarHeader(`${rootDir}/${name}`, bytes.length, false)); + blocks.push(bytes); + if (bytes.length % 512 !== 0) blocks.push(new Uint8Array(512 - (bytes.length % 512))); + } + blocks.push(new Uint8Array(1024)); + return Bun.gzipSync(Buffer.concat(blocks)); +} + +// The text lockfile writes the resolved commit in the committish position of a +// git/github resolution string ("git+url#"), so after a lockfile round +// trip a dependency naming a branch or tag (or no ref at all) never matches the +// loaded committish again. Re-resolving (any edit that re-parses a workspace +// member's dependency list) then fetched every such dependency from the remote +// on every install. The identical dependency literal already bound in the +// loaded lockfile must be reused instead. +it("re-resolving reuses branch and bare ref git dependencies from the lockfile instead of re-fetching", async () => { + const { packageDir, packageJson } = await registry.createTestDir(); + + // Isolate git from system/global config (e.g. core.autocrlf on Windows). + const gitEnv = { + ...env, + GIT_CONFIG_NOSYSTEM: "1", + GIT_CONFIG_GLOBAL: join(packageDir, "gitconfig"), + GIT_AUTHOR_NAME: "bun-test", + GIT_AUTHOR_EMAIL: "test@bun.sh", + GIT_COMMITTER_NAME: "bun-test", + GIT_COMMITTER_EMAIL: "test@bun.sh", + }; + async function git(args: string[], cwd: string): Promise { + await using proc = spawn({ cmd: ["git", ...args], cwd, env: gitEnv, stdout: "pipe", stderr: "pipe" }); + const [out, err, code] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(err).not.toContain("fatal:"); + expect(code).toBe(0); + return out; + } + await write(join(packageDir, "gitconfig"), "[core]\n\tautocrlf = false\n"); + + // Bare repos served over git's dumb HTTP protocol: after + // `git update-server-info`, a bare repo is plain static files. + async function makeBareRepo(name: string): Promise { + const srcDir = join(packageDir, `${name}-src`); + await write(join(srcDir, "package.json"), JSON.stringify({ name, version: "1.0.0" })); + await write(join(srcDir, "index.js"), `module.exports = '${name}';\n`); + await git(["init", "-q", "-b", "main"], srcDir); + await git(["add", "-A"], srcDir); + await git(["commit", "-qm", "init"], srcDir); + const sha = (await git(["rev-parse", "HEAD"], srcDir)).trim(); + await git(["clone", "-q", "--bare", srcDir, join(packageDir, `${name}.git`)], packageDir); + await git(["update-server-info"], join(packageDir, `${name}.git`)); + return sha; + } + const bareSha = await makeBareRepo("bare-dep"); + const branchSha = await makeBareRepo("branch-dep"); + + let gitRequests = 0; + await using gitServer = Bun.serve({ + port: 0, + async fetch(req) { + gitRequests++; + const { pathname } = new URL(req.url); + const match = pathname.match(/^\/((?:bare|branch)-dep\.git)\/(.+)$/); + if (!match) return new Response("not found", { status: 404 }); + const f = file(join(packageDir, match[1], match[2])); + return (await f.exists()) ? new Response(f) : new Response("not found", { status: 404 }); + }, + }); + + const ghTarball = makeTarball("testowner-testrepo-aaaaaaa", { + "package.json": JSON.stringify({ name: "gh-dep", version: "1.0.0" }), + "index.js": "module.exports = 'gh';\n", + }); + let githubDownloads = 0; + await using ghServer = Bun.serve({ + port: 0, + fetch() { + githubDownloads++; + return new Response(ghTarball, { headers: { "Content-Type": "application/gzip" } }); + }, + }); + + const installEnv = { + ...gitEnv, + GITHUB_API_URL: `http://localhost:${ghServer.port}`, + // CI exports BUN_INSTALL_CACHE_DIR; pin it so this test's cache is its own. + BUN_INSTALL_CACHE_DIR: join(packageDir, ".bun-cache"), + }; + async function install() { + await using proc = spawn({ + cmd: [bunExe(), "install"], + cwd: packageDir, + env: installEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [err, code] = await Promise.all([proc.stderr.text(), proc.exited, proc.stdout.text()]); + expect(err).not.toContain("error:"); + expect(code).toBe(0); + } + + await write(packageJson, JSON.stringify({ name: "ws-root", workspaces: ["packages/*"] })); + const memberPackageJson = join(packageDir, "packages", "member", "package.json"); + const memberDeps: Record = { + "bare-dep": `git+http://127.0.0.1:${gitServer.port}/bare-dep.git`, + "branch-dep": `git+http://127.0.0.1:${gitServer.port}/branch-dep.git#main`, + "gh-dep": "github:testowner/testrepo#main", + }; + await write(memberPackageJson, JSON.stringify({ name: "member", version: "1.0.0", dependencies: memberDeps })); + await write( + join(packageDir, "packages", "member", "dummy", "package.json"), + JSON.stringify({ name: "dummy", version: "1.0.0" }), + ); + + await install(); + expect(gitRequests).toBeGreaterThan(0); + expect(githubDownloads).toBe(1); + const lock = await file(join(packageDir, "bun.lock")).text(); + expect(lock).toContain(`bare-dep@git+http://127.0.0.1:${gitServer.port}/bare-dep.git#${bareSha}`); + expect(lock).toContain(`branch-dep@git+http://127.0.0.1:${gitServer.port}/branch-dep.git#${branchSha}`); + + // Dirty the lockfile with a change that re-resolves the member's unchanged + // dependencies (a workspace member edit re-parses its whole dependency list). + memberDeps["dummy"] = "file:./dummy"; + await write(memberPackageJson, JSON.stringify({ name: "member", version: "1.0.0", dependencies: memberDeps })); + const requestsAfterFirstInstall = gitRequests; + await install(); + expect({ gitRequests, githubDownloads }).toEqual({ + gitRequests: requestsAfterFirstInstall, + githubDownloads: 1, + }); + + // The locked commits did not move. + const lockAfter = await file(join(packageDir, "bun.lock")).text(); + expect(lockAfter).toContain(`bare-dep@git+http://127.0.0.1:${gitServer.port}/bare-dep.git#${bareSha}`); + expect(lockAfter).toContain(`branch-dep@git+http://127.0.0.1:${gitServer.port}/branch-dep.git#${branchSha}`); + expect(await file(join(packageDir, "node_modules", "bare-dep", "index.js")).text()).toBe( + "module.exports = 'bare-dep';\n", + ); + expect(await file(join(packageDir, "node_modules", "branch-dep", "index.js")).text()).toBe( + "module.exports = 'branch-dep';\n", + ); + expect(await file(join(packageDir, "node_modules", "gh-dep", "index.js")).text()).toBe("module.exports = 'gh';\n"); +}); + +// `bun update` must keep going to the remote for a branch-tracking ref: the +// reuse above is explicitly skipped for update targets. +it("`bun update` still re-resolves a branch ref git dependency against the remote", async () => { + const { packageDir, packageJson } = await registry.createTestDir(); + + const gitEnv = { + ...env, + GIT_CONFIG_NOSYSTEM: "1", + GIT_CONFIG_GLOBAL: join(packageDir, "gitconfig"), + GIT_AUTHOR_NAME: "bun-test", + GIT_AUTHOR_EMAIL: "test@bun.sh", + GIT_COMMITTER_NAME: "bun-test", + GIT_COMMITTER_EMAIL: "test@bun.sh", + }; + async function git(args: string[], cwd: string): Promise { + await using proc = spawn({ cmd: ["git", ...args], cwd, env: gitEnv, stdout: "pipe", stderr: "pipe" }); + const [out, err, code] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(err).not.toContain("fatal:"); + expect(code).toBe(0); + return out; + } + await write(join(packageDir, "gitconfig"), "[core]\n\tautocrlf = false\n"); + + const srcDir = join(packageDir, "git-src"); + const bareDir = join(packageDir, "repo.git"); + await write(join(srcDir, "package.json"), JSON.stringify({ name: "git-dep", version: "1.0.0" })); + await git(["init", "-q", "-b", "main"], srcDir); + await git(["add", "-A"], srcDir); + await git(["commit", "-qm", "init"], srcDir); + await git(["clone", "-q", "--bare", srcDir, bareDir], packageDir); + await git(["update-server-info"], bareDir); + + let gitRequests = 0; + await using server = Bun.serve({ + port: 0, + async fetch(req) { + gitRequests++; + const { pathname } = new URL(req.url); + if (!pathname.startsWith("/repo.git/")) return new Response("not found", { status: 404 }); + const f = file(join(bareDir, pathname.slice("/repo.git/".length))); + return (await f.exists()) ? new Response(f) : new Response("not found", { status: 404 }); + }, + }); + + const installEnv = { + ...gitEnv, + BUN_INSTALL_CACHE_DIR: join(packageDir, ".bun-cache"), + }; + async function run(args: string[]) { + await using proc = spawn({ + cmd: [bunExe(), ...args], + cwd: packageDir, + env: installEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [err, code] = await Promise.all([proc.stderr.text(), proc.exited, proc.stdout.text()]); + expect(err).not.toContain("error:"); + expect(code).toBe(0); + } + + await write( + packageJson, + JSON.stringify({ + name: "git-update-root", + dependencies: { "git-dep": `git+http://127.0.0.1:${server.port}/repo.git#main` }, + }), + ); + + await run(["install"]); + const requestsAfterInstall = gitRequests; + expect(requestsAfterInstall).toBeGreaterThan(0); + + await run(["update", "git-dep"]); + expect(gitRequests).toBeGreaterThan(requestsAfterInstall); +}); From 01f22dacd91e8ea006acbbec8cf0d1ffba9834ab Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 7 Aug 2026 14:53:15 +0000 Subject: [PATCH 02/11] test: tolerate external SIGKILL of git children in CI setup installs --- test/cli/install/bun-lock.test.ts | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/test/cli/install/bun-lock.test.ts b/test/cli/install/bun-lock.test.ts index 93750d1a5203..957f9888f5a3 100644 --- a/test/cli/install/bun-lock.test.ts +++ b/test/cli/install/bun-lock.test.ts @@ -1124,7 +1124,7 @@ it("re-resolving reuses branch and bare ref git dependencies from the lockfile i // CI exports BUN_INSTALL_CACHE_DIR; pin it so this test's cache is its own. BUN_INSTALL_CACHE_DIR: join(packageDir, ".bun-cache"), }; - async function install() { + async function install(retries = 1) { await using proc = spawn({ cmd: [bunExe(), "install"], cwd: packageDir, @@ -1133,6 +1133,13 @@ it("re-resolving reuses branch and bare ref git dependencies from the lockfile i stderr: "pipe", }); const [err, code] = await Promise.all([proc.stderr.text(), proc.exited, proc.stdout.text()]); + // Loaded CI machines can OOM-kill a spawned git child (SIGKILL); that is + // environmental, not the behavior under test. The request-count + // assertions below only ever compare values captured after a completed + // install, so a retried attempt cannot mask the re-fetch bug. + if (retries > 0 && err.includes("signal 9")) { + return install(retries - 1); + } expect(err).not.toContain("error:"); expect(code).toBe(0); } @@ -1152,7 +1159,7 @@ it("re-resolving reuses branch and bare ref git dependencies from the lockfile i await install(); expect(gitRequests).toBeGreaterThan(0); - expect(githubDownloads).toBe(1); + expect(githubDownloads).toBeGreaterThan(0); const lock = await file(join(packageDir, "bun.lock")).text(); expect(lock).toContain(`bare-dep@git+http://127.0.0.1:${gitServer.port}/bare-dep.git#${bareSha}`); expect(lock).toContain(`branch-dep@git+http://127.0.0.1:${gitServer.port}/branch-dep.git#${branchSha}`); @@ -1162,10 +1169,11 @@ it("re-resolving reuses branch and bare ref git dependencies from the lockfile i memberDeps["dummy"] = "file:./dummy"; await write(memberPackageJson, JSON.stringify({ name: "member", version: "1.0.0", dependencies: memberDeps })); const requestsAfterFirstInstall = gitRequests; + const downloadsAfterFirstInstall = githubDownloads; await install(); expect({ gitRequests, githubDownloads }).toEqual({ gitRequests: requestsAfterFirstInstall, - githubDownloads: 1, + githubDownloads: downloadsAfterFirstInstall, }); // The locked commits did not move. @@ -1229,7 +1237,7 @@ it("`bun update` still re-resolves a branch ref git dependency against the remot ...gitEnv, BUN_INSTALL_CACHE_DIR: join(packageDir, ".bun-cache"), }; - async function run(args: string[]) { + async function run(args: string[], retries = 1) { await using proc = spawn({ cmd: [bunExe(), ...args], cwd: packageDir, @@ -1238,6 +1246,11 @@ it("`bun update` still re-resolves a branch ref git dependency against the remot stderr: "pipe", }); const [err, code] = await Promise.all([proc.stderr.text(), proc.exited, proc.stdout.text()]); + // Loaded CI machines can OOM-kill a spawned git child (SIGKILL); retrying + // only ever adds requests, so the requests-increase assertion holds. + if (retries > 0 && err.includes("signal 9")) { + return run(args, retries - 1); + } expect(err).not.toContain("error:"); expect(code).toBe(0); } From 184ddc01b9c12738090d378d17a9bcd04a7cfc4a Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 7 Aug 2026 14:54:52 +0000 Subject: [PATCH 03/11] trim comments in the locked-git-package reuse path --- .../PackageManager/PackageManagerEnqueue.rs | 39 +++++++------------ 1 file changed, 13 insertions(+), 26 deletions(-) diff --git a/src/install/PackageManager/PackageManagerEnqueue.rs b/src/install/PackageManager/PackageManagerEnqueue.rs index 84980e9fb192..1c76f4ff11c4 100644 --- a/src/install/PackageManager/PackageManagerEnqueue.rs +++ b/src/install/PackageManager/PackageManagerEnqueue.rs @@ -1182,9 +1182,7 @@ pub fn enqueue_dependency_with_main_and_success_fn( return Ok(()); } - // Second: reuse the package an identical dependency is already - // bound to. A branch, tag, or bare committish can never match the - // lookup above after a lockfile round-trip. + // Second: the package an identical dependency literal resolved to if let Some(pkg_id) = find_locked_git_package(this, id, dependency, &dep, ResolutionTag::Git) { @@ -1302,9 +1300,7 @@ pub fn enqueue_dependency_with_main_and_success_fn( return Ok(()); } - // Second: reuse the package an identical dependency is already - // bound to. A branch, tag, or bare committish can never match the - // lookup above after a lockfile round-trip. + // Second: the package an identical dependency literal resolved to if let Some(pkg_id) = find_locked_git_package(this, id, dependency, dep, ResolutionTag::Github) { @@ -1953,18 +1949,12 @@ fn update_name_and_name_hash_from_version_replacement( } } -/// Finds the package an identical, already-resolved dependency is bound to, so -/// a git/github dependency re-enqueued during re-resolution (most commonly a -/// workspace member edit re-parsing the member's whole dependency list) reuses -/// it instead of fetching the remote again. -/// -/// The `get_package_id` lookup in the git/github arms can only match a -/// committish equal to the loaded one, and the text lockfile writes the -/// *resolved* commit in the committish position of the resolution string, so a -/// branch, tag, or bare ref never matches again after a lockfile round-trip. -/// Dependency entries keep their original version literals, so an identical -/// literal bound to a package of the same repository is the lossless record of -/// what this dependency previously resolved to. +/// The package an identical git/github dependency (same name and version +/// literal) is already bound to. `bun.lock` writes the resolved commit in the +/// committish position of the resolution string, so after a reload a branch, +/// tag, or bare ref never matches `get_package_id`'s committish comparison; +/// the unchanged dependency literal is the lossless record of the previous +/// resolution, and reusing its binding keeps re-resolution off the network. fn find_locked_git_package( this: &PackageManager, id: DependencyID, @@ -1976,9 +1966,8 @@ fn find_locked_git_package( return None; } - // `bun update` re-resolves git refs against the remote; reusing the locked - // commit here would turn the update into a no-op. Same update-target test - // as `Diff::generate` (an empty request list is a bare `bun update`). + // An update target must re-resolve against the remote (same test as + // `Diff::generate`; an empty request list is a bare `bun update`). if this.to_update && (this.update_requests.is_empty() || this @@ -2007,11 +1996,9 @@ fn find_locked_git_package( { continue; } - // Overrides and catalogs replace the version after parsing, so an - // identical literal does not guarantee an identical resolution; the - // resolved repository must match the effective one too. (A changed - // override/catalog also invalidates the old binding before - // re-enqueueing, so a stale binding is never reachable here.) + // Overrides/catalogs replace the version after parsing, so the bound + // package must also match the effective repository (changed overrides + // and catalogs invalidate old bindings before re-enqueueing). let resolution = &package_resolutions[package_id as usize]; if resolution.tag != resolution_tag { continue; From d2dc988059a143c828ecb21117267d71fe594c0a Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 7 Aug 2026 14:59:34 +0000 Subject: [PATCH 04/11] test: dedupe git fixture, narrow retry to the git-child kill, snapshot counters across retries --- test/cli/install/bun-lock.test.ts | 65 +++++++++++++------------------ 1 file changed, 28 insertions(+), 37 deletions(-) diff --git a/test/cli/install/bun-lock.test.ts b/test/cli/install/bun-lock.test.ts index 957f9888f5a3..2c39518bb5be 100644 --- a/test/cli/install/bun-lock.test.ts +++ b/test/cli/install/bun-lock.test.ts @@ -1046,17 +1046,8 @@ function makeTarball(rootDir: string, files: Record): Uint8Array return Bun.gzipSync(Buffer.concat(blocks)); } -// The text lockfile writes the resolved commit in the committish position of a -// git/github resolution string ("git+url#"), so after a lockfile round -// trip a dependency naming a branch or tag (or no ref at all) never matches the -// loaded committish again. Re-resolving (any edit that re-parses a workspace -// member's dependency list) then fetched every such dependency from the remote -// on every install. The identical dependency literal already bound in the -// loaded lockfile must be reused instead. -it("re-resolving reuses branch and bare ref git dependencies from the lockfile instead of re-fetching", async () => { - const { packageDir, packageJson } = await registry.createTestDir(); - - // Isolate git from system/global config (e.g. core.autocrlf on Windows). +// Isolate git from system/global config (e.g. core.autocrlf on Windows). +async function makeGitFixture(packageDir: string) { const gitEnv = { ...env, GIT_CONFIG_NOSYSTEM: "1", @@ -1074,6 +1065,19 @@ it("re-resolving reuses branch and bare ref git dependencies from the lockfile i return out; } await write(join(packageDir, "gitconfig"), "[core]\n\tautocrlf = false\n"); + return { gitEnv, git }; +} + +// The text lockfile writes the resolved commit in the committish position of a +// git/github resolution string ("git+url#"), so after a lockfile round +// trip a dependency naming a branch or tag (or no ref at all) never matches the +// loaded committish again. Re-resolving (any edit that re-parses a workspace +// member's dependency list) then fetched every such dependency from the remote +// on every install. The identical dependency literal already bound in the +// loaded lockfile must be reused instead. +it("re-resolving reuses branch and bare ref git dependencies from the lockfile instead of re-fetching", async () => { + const { packageDir, packageJson } = await registry.createTestDir(); + const { gitEnv, git } = await makeGitFixture(packageDir); // Bare repos served over git's dumb HTTP protocol: after // `git update-server-info`, a bare repo is plain static files. @@ -1125,6 +1129,8 @@ it("re-resolving reuses branch and bare ref git dependencies from the lockfile i BUN_INSTALL_CACHE_DIR: join(packageDir, ".bun-cache"), }; async function install(retries = 1) { + const gitRequestsBefore = gitRequests; + const githubDownloadsBefore = githubDownloads; await using proc = spawn({ cmd: [bunExe(), "install"], cwd: packageDir, @@ -1133,11 +1139,12 @@ it("re-resolving reuses branch and bare ref git dependencies from the lockfile i stderr: "pipe", }); const [err, code] = await Promise.all([proc.stderr.text(), proc.exited, proc.stdout.text()]); - // Loaded CI machines can OOM-kill a spawned git child (SIGKILL); that is - // environmental, not the behavior under test. The request-count - // assertions below only ever compare values captured after a completed - // install, so a retried attempt cannot mask the re-fetch bug. - if (retries > 0 && err.includes("signal 9")) { + // A loaded CI machine can OOM-kill the spawned git child (SIGKILL); that + // is environmental, not the behavior under test. Restore the counters so + // the retried attempt starts from the aborted attempt's baseline. + if (retries > 0 && err.includes("git failed with signal 9")) { + gitRequests = gitRequestsBefore; + githubDownloads = githubDownloadsBefore; return install(retries - 1); } expect(err).not.toContain("error:"); @@ -1193,24 +1200,7 @@ it("re-resolving reuses branch and bare ref git dependencies from the lockfile i // reuse above is explicitly skipped for update targets. it("`bun update` still re-resolves a branch ref git dependency against the remote", async () => { const { packageDir, packageJson } = await registry.createTestDir(); - - const gitEnv = { - ...env, - GIT_CONFIG_NOSYSTEM: "1", - GIT_CONFIG_GLOBAL: join(packageDir, "gitconfig"), - GIT_AUTHOR_NAME: "bun-test", - GIT_AUTHOR_EMAIL: "test@bun.sh", - GIT_COMMITTER_NAME: "bun-test", - GIT_COMMITTER_EMAIL: "test@bun.sh", - }; - async function git(args: string[], cwd: string): Promise { - await using proc = spawn({ cmd: ["git", ...args], cwd, env: gitEnv, stdout: "pipe", stderr: "pipe" }); - const [out, err, code] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); - expect(err).not.toContain("fatal:"); - expect(code).toBe(0); - return out; - } - await write(join(packageDir, "gitconfig"), "[core]\n\tautocrlf = false\n"); + const { gitEnv, git } = await makeGitFixture(packageDir); const srcDir = join(packageDir, "git-src"); const bareDir = join(packageDir, "repo.git"); @@ -1246,9 +1236,10 @@ it("`bun update` still re-resolves a branch ref git dependency against the remot stderr: "pipe", }); const [err, code] = await Promise.all([proc.stderr.text(), proc.exited, proc.stdout.text()]); - // Loaded CI machines can OOM-kill a spawned git child (SIGKILL); retrying - // only ever adds requests, so the requests-increase assertion holds. - if (retries > 0 && err.includes("signal 9")) { + // A loaded CI machine can OOM-kill the spawned git child (SIGKILL); + // retrying only ever adds requests, so the requests-increase assertion + // holds. + if (retries > 0 && err.includes("git failed with signal 9")) { return run(args, retries - 1); } expect(err).not.toContain("error:"); From e91879aafcd914f59b6b77e79b38f5425a7aaac0 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 7 Aug 2026 15:16:12 +0000 Subject: [PATCH 05/11] install: invalidate all override/catalog bindings before re-enqueueing, match ssh-prefixed repos in locked reuse A changed override or catalog cleared and re-enqueued affected dependencies one at a time, so with several dependencies sharing a name the first re-resolution could rebind to a sibling's not-yet-cleared binding from the old override. Clear every affected slot first. Also match a reloaded resolution whose scp-like repo gained the writer's ssh:// prefix against the freshly parsed dependency, and retry the CI installs from a clean cache (a killed git checkout leaves a half-created folder the next attempt trusts). --- .../PackageManager/PackageManagerEnqueue.rs | 14 ++- .../PackageManager/install_with_manager.rs | 25 ++++- test/cli/install/bun-lock.test.ts | 101 +++++++++++++++++- 3 files changed, 130 insertions(+), 10 deletions(-) diff --git a/src/install/PackageManager/PackageManagerEnqueue.rs b/src/install/PackageManager/PackageManagerEnqueue.rs index 1c76f4ff11c4..24fee3f83567 100644 --- a/src/install/PackageManager/PackageManagerEnqueue.rs +++ b/src/install/PackageManager/PackageManagerEnqueue.rs @@ -2004,7 +2004,19 @@ fn find_locked_git_package( continue; } let locked = resolution.repository(); - if !locked.repo.eql(repo.repo, buf, buf) || !locked.owner.eql(repo.owner, buf, buf) { + if !locked.owner.eql(repo.owner, buf, buf) { + continue; + } + // The lockfile writer prefixes an scp-like repo ("git@host:path") with + // "ssh://", so a reloaded resolution carries the prefix while the + // freshly parsed dependency does not. + let locked_repo = locked.repo.slice(buf); + let dep_repo = repo.repo.slice(buf); + let repo_matches = strings::eql(locked_repo, dep_repo) + || (strings::has_prefix_comptime(locked_repo, b"ssh://") + && dependency::is_scp_like_path(dep_repo) + && strings::eql(&locked_repo[b"ssh://".len()..], dep_repo)); + if !repo_matches { continue; } return Some(package_id); diff --git a/src/install/PackageManager/install_with_manager.rs b/src/install/PackageManager/install_with_manager.rs index a06d651a152c..91f9abfdc7cb 100644 --- a/src/install/PackageManager/install_with_manager.rs +++ b/src/install/PackageManager/install_with_manager.rs @@ -484,12 +484,23 @@ pub fn install_with_manager( // ever reads through a pointer into the old backing storage. if manager.summary.overrides_changed && !all_name_hashes.is_empty() { let dependencies_len = manager.lockfile.buffers.dependencies.len(); + // Invalidate every affected resolution before enqueueing + // any of them: re-resolution consults existing bindings + // (`find_locked_git_package`), and a not-yet-cleared + // sibling would rebind the dependency to the package the + // old override resolved to. + for dependency_i in 0..dependencies_len { + if all_name_hashes.contains( + &manager.lockfile.buffers.dependencies[dependency_i].name_hash, + ) { + manager.lockfile.buffers.resolutions[dependency_i] = + invalid_package_id; + } + } for dependency_i in 0..dependencies_len { let dependency = manager.lockfile.buffers.dependencies[dependency_i].clone(); if all_name_hashes.contains(&dependency.name_hash) { - manager.lockfile.buffers.resolutions[dependency_i] = - invalid_package_id; if let Err(err) = enqueue_dependency_with_main( manager, dependency_i as u32, @@ -505,6 +516,14 @@ pub fn install_with_manager( if manager.summary.catalogs_changed { let dependencies_len = manager.lockfile.buffers.dependencies.len(); + // Same two-pass shape as the overrides loop above. + for dep_i in 0..dependencies_len { + if manager.lockfile.buffers.dependencies[dep_i].version.tag + == DependencyVersionTag::Catalog + { + manager.lockfile.buffers.resolutions[dep_i] = invalid_package_id; + } + } for _dep_id in 0..dependencies_len { let dep_id: DependencyID = u32::try_from(_dep_id).expect("int cast"); let dep = @@ -513,8 +532,6 @@ pub fn install_with_manager( continue; } - manager.lockfile.buffers.resolutions[dep_id as usize] = - invalid_package_id; if let Err(err) = enqueue_dependency_with_main( manager, dep_id, diff --git a/test/cli/install/bun-lock.test.ts b/test/cli/install/bun-lock.test.ts index 2c39518bb5be..f0ca84799c89 100644 --- a/test/cli/install/bun-lock.test.ts +++ b/test/cli/install/bun-lock.test.ts @@ -1140,9 +1140,12 @@ it("re-resolving reuses branch and bare ref git dependencies from the lockfile i }); const [err, code] = await Promise.all([proc.stderr.text(), proc.exited, proc.stdout.text()]); // A loaded CI machine can OOM-kill the spawned git child (SIGKILL); that - // is environmental, not the behavior under test. Restore the counters so - // the retried attempt starts from the aborted attempt's baseline. + // is environmental, not the behavior under test. A kill between git clone + // and git checkout leaves a half-created cache folder the next attempt + // would trust, so retry from a clean cache, with the counters restored to + // the aborted attempt's baseline. if (retries > 0 && err.includes("git failed with signal 9")) { + await rm(installEnv.BUN_INSTALL_CACHE_DIR, { recursive: true, force: true }); gitRequests = gitRequestsBefore; githubDownloads = githubDownloadsBefore; return install(retries - 1); @@ -1236,10 +1239,12 @@ it("`bun update` still re-resolves a branch ref git dependency against the remot stderr: "pipe", }); const [err, code] = await Promise.all([proc.stderr.text(), proc.exited, proc.stdout.text()]); - // A loaded CI machine can OOM-kill the spawned git child (SIGKILL); - // retrying only ever adds requests, so the requests-increase assertion - // holds. + // A loaded CI machine can OOM-kill the spawned git child (SIGKILL); retry + // from a clean cache (a kill between clone and checkout leaves a + // half-created folder the next attempt would trust). Retrying only ever + // adds requests, so the requests-increase assertion holds. if (retries > 0 && err.includes("git failed with signal 9")) { + await rm(installEnv.BUN_INSTALL_CACHE_DIR, { recursive: true, force: true }); return run(args, retries - 1); } expect(err).not.toContain("error:"); @@ -1261,3 +1266,89 @@ it("`bun update` still re-resolves a branch ref git dependency against the remot await run(["update", "git-dep"]); expect(gitRequests).toBeGreaterThan(requestsAfterInstall); }); + +// A changed override that moves a git dependency to a different ref of the +// same repository must re-resolve every dependent. Several dependencies +// sharing one name is the interesting case: the reuse above must not rebind a +// cleared dependency to a sibling's not-yet-cleared binding from the old +// override. +it("a changed git override re-resolves every dependent instead of reusing the old pin", async () => { + const { packageDir, packageJson } = await registry.createTestDir(); + const { gitEnv, git } = await makeGitFixture(packageDir); + + const srcDir = join(packageDir, "git-src"); + const bareDir = join(packageDir, "repo.git"); + await write(join(srcDir, "package.json"), JSON.stringify({ name: "over-dep", version: "1.0.0" })); + await write(join(srcDir, "index.js"), "module.exports = 'V1';\n"); + await git(["init", "-q", "-b", "main"], srcDir); + await git(["add", "-A"], srcDir); + await git(["commit", "-qm", "v1"], srcDir); + await git(["tag", "v1"], srcDir); + const sha1 = (await git(["rev-parse", "HEAD"], srcDir)).trim(); + await write(join(srcDir, "index.js"), "module.exports = 'V2';\n"); + await git(["add", "-A"], srcDir); + await git(["commit", "-qm", "v2"], srcDir); + await git(["tag", "v2"], srcDir); + const sha2 = (await git(["rev-parse", "HEAD"], srcDir)).trim(); + await git(["clone", "-q", "--bare", srcDir, bareDir], packageDir); + await git(["update-server-info"], bareDir); + + await using server = Bun.serve({ + port: 0, + async fetch(req) { + const { pathname } = new URL(req.url); + if (!pathname.startsWith("/repo.git/")) return new Response("not found", { status: 404 }); + const f = file(join(bareDir, pathname.slice("/repo.git/".length))); + return (await f.exists()) ? new Response(f) : new Response("not found", { status: 404 }); + }, + }); + + const installEnv = { + ...gitEnv, + BUN_INSTALL_CACHE_DIR: join(packageDir, ".bun-cache"), + }; + async function install(retries = 1) { + await using proc = spawn({ + cmd: [bunExe(), "install"], + cwd: packageDir, + env: installEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [err, code] = await Promise.all([proc.stderr.text(), proc.exited, proc.stdout.text()]); + // See the retry note in the re-resolving test above. + if (retries > 0 && err.includes("git failed with signal 9")) { + await rm(installEnv.BUN_INSTALL_CACHE_DIR, { recursive: true, force: true }); + return install(retries - 1); + } + expect(err).not.toContain("error:"); + expect(code).toBe(0); + } + + const repoUrl = `git+http://127.0.0.1:${server.port}/repo.git`; + function rootPackageJson(ref: string) { + return JSON.stringify({ + name: "ws-root", + workspaces: ["packages/*"], + overrides: { "over-dep": `${repoUrl}#${ref}` }, + }); + } + await write(packageJson, rootPackageJson("v1")); + for (const member of ["member-a", "member-b"]) { + await write( + join(packageDir, "packages", member, "package.json"), + JSON.stringify({ name: member, version: "1.0.0", dependencies: { "over-dep": "^1.0.0" } }), + ); + } + + await install(); + expect(await file(join(packageDir, "bun.lock")).text()).toContain(`#${sha1}`); + expect(await file(join(packageDir, "node_modules", "over-dep", "index.js")).text()).toBe("module.exports = 'V1';\n"); + + await write(packageJson, rootPackageJson("v2")); + await install(); + const lockAfter = await file(join(packageDir, "bun.lock")).text(); + expect(lockAfter).toContain(`#${sha2}`); + expect(lockAfter).not.toContain(`#${sha1}`); + expect(await file(join(packageDir, "node_modules", "over-dep", "index.js")).text()).toBe("module.exports = 'V2';\n"); +}); From 9f88a2410466c016a756d29d4f1b4551bda0d7d5 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 7 Aug 2026 15:19:05 +0000 Subject: [PATCH 06/11] ci: retrigger From 909a97bddf68c13b60193dd8f543874fd7b5a556 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 7 Aug 2026 15:48:38 +0000 Subject: [PATCH 07/11] test: stage the git reuse installs so only one clone runs at a time Loaded ASAN CI machines reliably OOM-kill one of two concurrent git children; staging the dependency additions keeps a single clone chain in flight per install and tightens the assertions to per-repo request counters. --- test/cli/install/bun-lock.test.ts | 53 ++++++++++++++++++++----------- 1 file changed, 34 insertions(+), 19 deletions(-) diff --git a/test/cli/install/bun-lock.test.ts b/test/cli/install/bun-lock.test.ts index f0ca84799c89..333358d8e5a6 100644 --- a/test/cli/install/bun-lock.test.ts +++ b/test/cli/install/bun-lock.test.ts @@ -1096,14 +1096,16 @@ it("re-resolving reuses branch and bare ref git dependencies from the lockfile i const bareSha = await makeBareRepo("bare-dep"); const branchSha = await makeBareRepo("branch-dep"); - let gitRequests = 0; + let bareRequests = 0; + let branchRequests = 0; await using gitServer = Bun.serve({ port: 0, async fetch(req) { - gitRequests++; const { pathname } = new URL(req.url); const match = pathname.match(/^\/((?:bare|branch)-dep\.git)\/(.+)$/); if (!match) return new Response("not found", { status: 404 }); + if (match[1] === "bare-dep.git") bareRequests++; + else branchRequests++; const f = file(join(packageDir, match[1], match[2])); return (await f.exists()) ? new Response(f) : new Response("not found", { status: 404 }); }, @@ -1129,7 +1131,8 @@ it("re-resolving reuses branch and bare ref git dependencies from the lockfile i BUN_INSTALL_CACHE_DIR: join(packageDir, ".bun-cache"), }; async function install(retries = 1) { - const gitRequestsBefore = gitRequests; + const bareRequestsBefore = bareRequests; + const branchRequestsBefore = branchRequests; const githubDownloadsBefore = githubDownloads; await using proc = spawn({ cmd: [bunExe(), "install"], @@ -1146,7 +1149,8 @@ it("re-resolving reuses branch and bare ref git dependencies from the lockfile i // the aborted attempt's baseline. if (retries > 0 && err.includes("git failed with signal 9")) { await rm(installEnv.BUN_INSTALL_CACHE_DIR, { recursive: true, force: true }); - gitRequests = gitRequestsBefore; + bareRequests = bareRequestsBefore; + branchRequests = branchRequestsBefore; githubDownloads = githubDownloadsBefore; return install(retries - 1); } @@ -1154,37 +1158,48 @@ it("re-resolving reuses branch and bare ref git dependencies from the lockfile i expect(code).toBe(0); } + // The installs are staged so at most one git clone runs at a time: loaded + // CI machines reliably OOM-kill one of two concurrent git children under an + // ASAN build. Each stage edits the member, which re-parses its whole + // dependency list and re-resolves the dependencies added by earlier stages. await write(packageJson, JSON.stringify({ name: "ws-root", workspaces: ["packages/*"] })); const memberPackageJson = join(packageDir, "packages", "member", "package.json"); const memberDeps: Record = { "bare-dep": `git+http://127.0.0.1:${gitServer.port}/bare-dep.git`, - "branch-dep": `git+http://127.0.0.1:${gitServer.port}/branch-dep.git#main`, - "gh-dep": "github:testowner/testrepo#main", }; - await write(memberPackageJson, JSON.stringify({ name: "member", version: "1.0.0", dependencies: memberDeps })); + async function writeMember() { + await write(memberPackageJson, JSON.stringify({ name: "member", version: "1.0.0", dependencies: memberDeps })); + } + await writeMember(); await write( join(packageDir, "packages", "member", "dummy", "package.json"), JSON.stringify({ name: "dummy", version: "1.0.0" }), ); await install(); - expect(gitRequests).toBeGreaterThan(0); - expect(githubDownloads).toBeGreaterThan(0); + expect(bareRequests).toBeGreaterThan(0); const lock = await file(join(packageDir, "bun.lock")).text(); expect(lock).toContain(`bare-dep@git+http://127.0.0.1:${gitServer.port}/bare-dep.git#${bareSha}`); - expect(lock).toContain(`branch-dep@git+http://127.0.0.1:${gitServer.port}/branch-dep.git#${branchSha}`); - // Dirty the lockfile with a change that re-resolves the member's unchanged - // dependencies (a workspace member edit re-parses its whole dependency list). + // Adding dependencies to the member re-resolves bare-dep; its bare ref must + // bind to the loaded package without contacting the remote again. + memberDeps["branch-dep"] = `git+http://127.0.0.1:${gitServer.port}/branch-dep.git#main`; + memberDeps["gh-dep"] = "github:testowner/testrepo#main"; + await writeMember(); + const bareRequestsAfterFirstInstall = bareRequests; + await install(); + expect(bareRequests).toBe(bareRequestsAfterFirstInstall); + expect(branchRequests).toBeGreaterThan(0); + expect(githubDownloads).toBeGreaterThan(0); + const lockSecond = await file(join(packageDir, "bun.lock")).text(); + expect(lockSecond).toContain(`branch-dep@git+http://127.0.0.1:${gitServer.port}/branch-dep.git#${branchSha}`); + + // Another member edit re-resolves all three; none may go to the network. memberDeps["dummy"] = "file:./dummy"; - await write(memberPackageJson, JSON.stringify({ name: "member", version: "1.0.0", dependencies: memberDeps })); - const requestsAfterFirstInstall = gitRequests; - const downloadsAfterFirstInstall = githubDownloads; + await writeMember(); + const snapshot = { bareRequests, branchRequests, githubDownloads }; await install(); - expect({ gitRequests, githubDownloads }).toEqual({ - gitRequests: requestsAfterFirstInstall, - githubDownloads: downloadsAfterFirstInstall, - }); + expect({ bareRequests, branchRequests, githubDownloads }).toEqual(snapshot); // The locked commits did not move. const lockAfter = await file(join(packageDir, "bun.lock")).text(); From 3b5a1405f15eb6b9ef32462637ab312c56e52f9b Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 7 Aug 2026 16:37:34 +0000 Subject: [PATCH 08/11] install: check out the bound package's locked commit when re-enqueueing a resolved git dependency The isolated linker's store waits on the locked commit's checkout id. The resolve-phase re-enqueue after a cold-cache clone derived the checkout from find_commit on the dependency's committish, which follows the ref: with a moved branch head the locked checkout never ran and the install starved forever (and before the reuse, the pin silently floated to the new head instead). Prefer the bound package's resolved commit, matching how the clone-failure drain and the hoisted installer already key it. Covers the changed-catalog variant of the two-pass invalidation with a test, and extends the reuse test with the changed-literal boundary and a cold-cache leg under the isolated linker. --- .../PackageManager/PackageManagerEnqueue.rs | 43 +++++++-- test/cli/install/bun-lock.test.ts | 96 ++++++++++++++----- 2 files changed, 109 insertions(+), 30 deletions(-) diff --git a/src/install/PackageManager/PackageManagerEnqueue.rs b/src/install/PackageManager/PackageManagerEnqueue.rs index 24fee3f83567..239ff5ee304f 100644 --- a/src/install/PackageManager/PackageManagerEnqueue.rs +++ b/src/install/PackageManager/PackageManagerEnqueue.rs @@ -1218,14 +1218,41 @@ pub fn enqueue_dependency_with_main_and_success_fn( } if let Some(repo_fd) = this.git_repositories.get(&clone_id).copied() { - let resolved = Repository::find_commit( - this.env_mut(), - this.log_mut(), - repo_fd, - alias, - this.lockfile.str(&dep.committish), - clone_id, - )?; + // A dependency already bound to a package checks out that + // package's locked commit. `find_commit` follows the ref, + // which may have moved since it was locked, and the isolated + // installer's store entry waits on the locked checkout id + // (the clone-failure drain in runTasks.rs keys the same way). + let bound_resolved: Option> = { + let bound = this.lockfile.buffers.resolutions[id as usize]; + if bound != invalid_package_id + && (bound as usize) < this.lockfile.packages.len() + { + let bound_res = &this.lockfile.packages.items_resolution()[bound as usize]; + if bound_res.tag == ResolutionTag::Git { + let locked = bound_res + .git() + .resolved + .slice(this.lockfile.buffers.string_bytes.as_slice()); + (!locked.is_empty()).then(|| locked.to_vec()) + } else { + None + } + } else { + None + } + }; + let resolved = match bound_resolved { + Some(resolved) => resolved, + None => Repository::find_commit( + this.env_mut(), + this.log_mut(), + repo_fd, + alias, + this.lockfile.str(&dep.committish), + clone_id, + )?, + }; let checkout_id = Task::Id::for_git_checkout(url, &resolved); let needs_ctx = diff --git a/test/cli/install/bun-lock.test.ts b/test/cli/install/bun-lock.test.ts index 333358d8e5a6..96519aad68e8 100644 --- a/test/cli/install/bun-lock.test.ts +++ b/test/cli/install/bun-lock.test.ts @@ -1135,7 +1135,10 @@ it("re-resolving reuses branch and bare ref git dependencies from the lockfile i const branchRequestsBefore = branchRequests; const githubDownloadsBefore = githubDownloads; await using proc = spawn({ - cmd: [bunExe(), "install"], + // Explicit linker: the cold-cache stage below regresses only under the + // isolated linker's store (its entry waits on the locked commit's + // checkout id). + cmd: [bunExe(), "install", "--linker", "isolated"], cwd: packageDir, env: installEnv, stdout: "pipe", @@ -1205,14 +1208,49 @@ it("re-resolving reuses branch and bare ref git dependencies from the lockfile i const lockAfter = await file(join(packageDir, "bun.lock")).text(); expect(lockAfter).toContain(`bare-dep@git+http://127.0.0.1:${gitServer.port}/bare-dep.git#${bareSha}`); expect(lockAfter).toContain(`branch-dep@git+http://127.0.0.1:${gitServer.port}/branch-dep.git#${branchSha}`); - expect(await file(join(packageDir, "node_modules", "bare-dep", "index.js")).text()).toBe( - "module.exports = 'bare-dep';\n", - ); - expect(await file(join(packageDir, "node_modules", "branch-dep", "index.js")).text()).toBe( - "module.exports = 'branch-dep';\n", - ); - expect(await file(join(packageDir, "node_modules", "gh-dep", "index.js")).text()).toBe("module.exports = 'gh';\n"); -}); + const memberModules = join(packageDir, "packages", "member", "node_modules"); + expect(await file(join(memberModules, "bare-dep", "index.js")).text()).toBe("module.exports = 'bare-dep';\n"); + expect(await file(join(memberModules, "branch-dep", "index.js")).text()).toBe("module.exports = 'branch-dep';\n"); + expect(await file(join(memberModules, "gh-dep", "index.js")).text()).toBe("module.exports = 'gh';\n"); + + // Changing a dependency's ref is the boundary the reuse must not cross: a + // different literal consults the remote again (here the same commit wins, + // so only the request counter moves). + memberDeps["branch-dep"] = `git+http://127.0.0.1:${gitServer.port}/branch-dep.git`; + await writeMember(); + const beforeRefChange = { bareRequests, branchRequests, githubDownloads }; + await install(); + expect(branchRequests).toBeGreaterThan(beforeRefChange.branchRequests); + expect({ bareRequests, githubDownloads }).toEqual({ + bareRequests: beforeRefChange.bareRequests, + githubDownloads: beforeRefChange.githubDownloads, + }); + expect(await file(join(packageDir, "bun.lock")).text()).toContain(`branch-dep.git#${branchSha}`); + + // Cold cache with a moved branch head: the lockfile pin must win and the + // install must terminate. The isolated store waits on the locked commit's + // checkout; a re-enqueued dependency that follows the moved ref instead of + // the bound package's commit starves it forever (and without the reuse the + // pin silently floats to the new head). + const branchSrc = join(packageDir, "branch-dep-src"); + await write(join(branchSrc, "index.js"), "module.exports = 'branch-dep-v2';\n"); + await git(["add", "-A"], branchSrc); + await git(["commit", "-qm", "move"], branchSrc); + const movedSha = (await git(["rev-parse", "HEAD"], branchSrc)).trim(); + await git(["fetch", "-q", branchSrc, "+refs/heads/*:refs/heads/*"], join(packageDir, "branch-dep.git")); + await git(["update-server-info"], join(packageDir, "branch-dep.git")); + + delete memberDeps["dummy"]; + await writeMember(); + await rm(installEnv.BUN_INSTALL_CACHE_DIR, { recursive: true, force: true }); + await rm(join(packageDir, "node_modules"), { recursive: true, force: true }); + await rm(memberModules, { recursive: true, force: true }); + await install(); + const lockCold = await file(join(packageDir, "bun.lock")).text(); + expect(lockCold).toContain(`branch-dep.git#${branchSha}`); + expect(lockCold).not.toContain(movedSha); + expect(await file(join(memberModules, "branch-dep", "index.js")).text()).toBe("module.exports = 'branch-dep';\n"); +}, 30_000); // `bun update` must keep going to the remote for a branch-tracking ref: the // reuse above is explicitly skipped for update targets. @@ -1282,12 +1320,12 @@ it("`bun update` still re-resolves a branch ref git dependency against the remot expect(gitRequests).toBeGreaterThan(requestsAfterInstall); }); -// A changed override that moves a git dependency to a different ref of the -// same repository must re-resolve every dependent. Several dependencies -// sharing one name is the interesting case: the reuse above must not rebind a -// cleared dependency to a sibling's not-yet-cleared binding from the old -// override. -it("a changed git override re-resolves every dependent instead of reusing the old pin", async () => { +// A changed override or catalog entry that moves a git dependency to a +// different ref of the same repository must re-resolve every dependent. +// Several dependencies sharing one name is the interesting case: the reuse +// above must not rebind a cleared dependency to a sibling's not-yet-cleared +// binding from the old entry. +async function changedEntryReResolves(mode: "overrides" | "catalog") { const { packageDir, packageJson } = await registry.createTestDir(); const { gitEnv, git } = await makeGitFixture(packageDir); @@ -1342,17 +1380,25 @@ it("a changed git override re-resolves every dependent instead of reusing the ol const repoUrl = `git+http://127.0.0.1:${server.port}/repo.git`; function rootPackageJson(ref: string) { - return JSON.stringify({ - name: "ws-root", - workspaces: ["packages/*"], - overrides: { "over-dep": `${repoUrl}#${ref}` }, - }); + return JSON.stringify( + mode === "overrides" + ? { + name: "ws-root", + workspaces: ["packages/*"], + overrides: { "over-dep": `${repoUrl}#${ref}` }, + } + : { + name: "ws-root", + workspaces: { packages: ["packages/*"], catalog: { "over-dep": `${repoUrl}#${ref}` } }, + }, + ); } + const memberSpec = mode === "overrides" ? "^1.0.0" : "catalog:"; await write(packageJson, rootPackageJson("v1")); for (const member of ["member-a", "member-b"]) { await write( join(packageDir, "packages", member, "package.json"), - JSON.stringify({ name: member, version: "1.0.0", dependencies: { "over-dep": "^1.0.0" } }), + JSON.stringify({ name: member, version: "1.0.0", dependencies: { "over-dep": memberSpec } }), ); } @@ -1366,4 +1412,10 @@ it("a changed git override re-resolves every dependent instead of reusing the ol expect(lockAfter).toContain(`#${sha2}`); expect(lockAfter).not.toContain(`#${sha1}`); expect(await file(join(packageDir, "node_modules", "over-dep", "index.js")).text()).toBe("module.exports = 'V2';\n"); -}); +} + +it("a changed git override re-resolves every dependent instead of reusing the old pin", () => + changedEntryReResolves("overrides")); + +it("a changed git catalog entry re-resolves every dependent instead of reusing the old pin", () => + changedEntryReResolves("catalog")); From f98470acaaf5418a5fcc1dae3c409aa1e4a697df Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 7 Aug 2026 17:02:42 +0000 Subject: [PATCH 09/11] test: raise the staged reuse test's timeout for the kill-retry worst case --- test/cli/install/bun-lock.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/cli/install/bun-lock.test.ts b/test/cli/install/bun-lock.test.ts index 96519aad68e8..517d4077af32 100644 --- a/test/cli/install/bun-lock.test.ts +++ b/test/cli/install/bun-lock.test.ts @@ -1250,7 +1250,9 @@ it("re-resolving reuses branch and bare ref git dependencies from the lockfile i expect(lockCold).toContain(`branch-dep.git#${branchSha}`); expect(lockCold).not.toContain(movedSha); expect(await file(join(memberModules, "branch-dep", "index.js")).text()).toBe("module.exports = 'branch-dep';\n"); -}, 30_000); + // Five staged installs plus a git-child-kill retry exceed the 5s default; + // this also bounds the cold-cache starvation mode to a fast failure. +}, 90_000); // `bun update` must keep going to the remote for a branch-tracking ref: the // reuse above is explicitly skipped for update targets. From 7c7be7c771abfa7c751017b29da870f579881342 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 7 Aug 2026 17:50:50 +0000 Subject: [PATCH 10/11] install: restrict locked reuse to byte-equal repos, raise git test timeouts Binding an scp-like dependency (git@host:path) to a package serialized with the writer's ssh:// prefix would key the resolve-phase clone and checkout task ids on different repo bytes than the isolated store's waiter, stranding it; scp dependencies keep the pre-reuse fetch path. --- .../PackageManager/PackageManagerEnqueue.rs | 19 ++++++------------- test/cli/install/bun-lock.test.ts | 18 ++++++++++++------ 2 files changed, 18 insertions(+), 19 deletions(-) diff --git a/src/install/PackageManager/PackageManagerEnqueue.rs b/src/install/PackageManager/PackageManagerEnqueue.rs index 239ff5ee304f..fc52c6926b10 100644 --- a/src/install/PackageManager/PackageManagerEnqueue.rs +++ b/src/install/PackageManager/PackageManagerEnqueue.rs @@ -2030,20 +2030,13 @@ fn find_locked_git_package( if resolution.tag != resolution_tag { continue; } + // Byte-equal repos only. An scp-like repo ("git@host:path") is + // serialized with an "ssh://" prefix the dependency parse lacks, and + // every git task id downstream keys on these exact bytes, so binding + // across the two spellings would strand the isolated store's checkout + // waiter; scp dependencies keep the pre-reuse fetch path instead. let locked = resolution.repository(); - if !locked.owner.eql(repo.owner, buf, buf) { - continue; - } - // The lockfile writer prefixes an scp-like repo ("git@host:path") with - // "ssh://", so a reloaded resolution carries the prefix while the - // freshly parsed dependency does not. - let locked_repo = locked.repo.slice(buf); - let dep_repo = repo.repo.slice(buf); - let repo_matches = strings::eql(locked_repo, dep_repo) - || (strings::has_prefix_comptime(locked_repo, b"ssh://") - && dependency::is_scp_like_path(dep_repo) - && strings::eql(&locked_repo[b"ssh://".len()..], dep_repo)); - if !repo_matches { + if !locked.repo.eql(repo.repo, buf, buf) || !locked.owner.eql(repo.owner, buf, buf) { continue; } return Some(package_id); diff --git a/test/cli/install/bun-lock.test.ts b/test/cli/install/bun-lock.test.ts index 517d4077af32..f807943df71f 100644 --- a/test/cli/install/bun-lock.test.ts +++ b/test/cli/install/bun-lock.test.ts @@ -1320,7 +1320,7 @@ it("`bun update` still re-resolves a branch ref git dependency against the remot await run(["update", "git-dep"]); expect(gitRequests).toBeGreaterThan(requestsAfterInstall); -}); +}, 60_000); // A changed override or catalog entry that moves a git dependency to a // different ref of the same repository must re-resolve every dependent. @@ -1416,8 +1416,14 @@ async function changedEntryReResolves(mode: "overrides" | "catalog") { expect(await file(join(packageDir, "node_modules", "over-dep", "index.js")).text()).toBe("module.exports = 'V2';\n"); } -it("a changed git override re-resolves every dependent instead of reusing the old pin", () => - changedEntryReResolves("overrides")); - -it("a changed git catalog entry re-resolves every dependent instead of reusing the old pin", () => - changedEntryReResolves("catalog")); +it( + "a changed git override re-resolves every dependent instead of reusing the old pin", + () => changedEntryReResolves("overrides"), + 60_000, +); + +it( + "a changed git catalog entry re-resolves every dependent instead of reusing the old pin", + () => changedEntryReResolves("catalog"), + 60_000, +); From 2fb4cbf9763d55bf2f56b25c22abdfd14fc8e4e0 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 7 Aug 2026 18:23:44 +0000 Subject: [PATCH 11/11] test: keep the cold-cache stage to a single git clone --- test/cli/install/bun-lock.test.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/cli/install/bun-lock.test.ts b/test/cli/install/bun-lock.test.ts index f807943df71f..f1694317a66e 100644 --- a/test/cli/install/bun-lock.test.ts +++ b/test/cli/install/bun-lock.test.ts @@ -1240,7 +1240,10 @@ it("re-resolving reuses branch and bare ref git dependencies from the lockfile i await git(["fetch", "-q", branchSrc, "+refs/heads/*:refs/heads/*"], join(packageDir, "branch-dep.git")); await git(["update-server-info"], join(packageDir, "branch-dep.git")); + // Drop every git dependency except the one under test so the cold install + // runs a single clone chain (see the staging note above). delete memberDeps["dummy"]; + delete memberDeps["bare-dep"]; await writeMember(); await rm(installEnv.BUN_INSTALL_CACHE_DIR, { recursive: true, force: true }); await rm(join(packageDir, "node_modules"), { recursive: true, force: true });