From 3174d03f00cf60f8817d65e2c048695b6c1d9cdd Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 2 Oct 2026 07:46:55 +0000 Subject: [PATCH 1/8] install: a git clone waiter names the package to check out The install phase parks a waiter on the clone of a git repository that is not in the cache. The waiter was a dependency, and when the clone ended the checkout was derived from it: the hoisted linker read the package the dependency resolves to, the isolated linker sent the dependency through the resolver again. The isolated linker keeps one dependency per store entry, and that one can be a peer dependency, or a dependency that an override redirects. Neither is a git dependency, and a peer dependency can resolve to another package. No checkout started for the entry and the install never exited. The waiter is now TaskCallbackContext::GitCheckout with the package id. Both linkers start, or fail, the checkout of that package when the clone ends. --- src/install/PackageInstaller.rs | 2 +- .../PackageManager/PackageManagerEnqueue.rs | 29 +- .../PackageManager/processDependencyList.rs | 5 + src/install/PackageManager/runTasks.rs | 86 ++---- src/install/PackageManagerTask.rs | 1 - src/install/isolated_install.rs | 2 +- src/install/lib.rs | 12 + test/cli/install/bun-install-git-deps.test.ts | 288 +++++++++++++++++- 8 files changed, 349 insertions(+), 76 deletions(-) diff --git a/src/install/PackageInstaller.rs b/src/install/PackageInstaller.rs index a73884b9ccba..f2a3373474bd 100644 --- a/src/install/PackageInstaller.rs +++ b/src/install/PackageInstaller.rs @@ -1609,8 +1609,8 @@ impl<'a> PackageInstaller<'a> { if package_manager::enqueue_git_for_checkout( self.manager_mut(), dependency_id, + package_id, alias.slice(string_buf!()), - resolution, context, download_patch_hash, ) == package_manager::GitEnqueueResult::OfflineMiss diff --git a/src/install/PackageManager/PackageManagerEnqueue.rs b/src/install/PackageManager/PackageManagerEnqueue.rs index ea9ec05aa566..dea833acb63f 100644 --- a/src/install/PackageManager/PackageManagerEnqueue.rs +++ b/src/install/PackageManager/PackageManagerEnqueue.rs @@ -261,17 +261,21 @@ pub enum GitEnqueueResult { OfflineMiss, } +/// Install phase: fetches the git package `package_id` into the cache, then +/// runs `task_context`. `dependency_id` is the dependency the linker placed the +/// package under; it need not resolve to `package_id` (see +/// `TaskCallbackContext::GitCheckout`). pub fn enqueue_git_for_checkout( this: &mut PackageManager, dependency_id: DependencyID, + package_id: PackageID, alias: &[u8], - resolution: &Resolution, task_context: TaskCallbackContext, patch_name_and_version_hash: Option, ) -> GitEnqueueResult { - // SAFETY: caller passes `resolution.tag == Git`; the `git` arm is the - // active union field. Copy out so the value no longer borrows - // `*resolution` while `*this` is mutably reborrowed below. + // The caller passes a package whose `resolution.tag == Git`. Copy out so + // the value does not borrow `*this` while it is mutably reborrowed below. + let resolution: Resolution = this.lockfile.packages.items_resolution()[package_id as usize]; let repository: Repository = *resolution.git(); // reshaped for borrowck — `url`/`resolved` borrow // `this.lockfile.buffers.string_bytes`; detach the slice lifetimes so the @@ -314,7 +318,7 @@ pub fn enqueue_git_for_checkout( repo_fd, dependency_id, alias, - resolution, + &resolution, resolved, patch_name_and_version_hash, ); @@ -327,14 +331,17 @@ pub fn enqueue_git_for_checkout( clone_queue .value_ptr - .push(TaskCallbackContext::Dependency(dependency_id)); + .push(TaskCallbackContext::GitCheckout { + dependency_id, + package_id, + }); if clone_queue.found_existing { return GitEnqueueResult::Queued; } let dep = this.lockfile.buffers.dependencies[dependency_id as usize].clone(); - let task = enqueue_git_clone(this, clone_id, alias, &repository, &dep, resolution, None); + let task = enqueue_git_clone(this, clone_id, alias, &repository, &dep, None); this.enqueue_git_task(task); } GitEnqueueResult::Queued @@ -1504,7 +1511,7 @@ pub fn enqueue_dependency_with_main_and_success_fn( return Ok(()); } - let task = enqueue_git_clone(this, clone_id, alias, &dep, dependency, &res, None); + let task = enqueue_git_clone(this, clone_id, alias, &dep, dependency, None); this.enqueue_git_task(task); } Ok(()) @@ -1924,7 +1931,6 @@ fn enqueue_git_clone( name: &[u8], repository: &Repository, dependency: &Dependency, - res: &Resolution, // if patched then we need to do apply step after network task is done patch_name_and_version_hash: Option, ) -> NonNull> { @@ -1966,7 +1972,6 @@ fn enqueue_git_clone( &mut crate::network_task::filename_store_appender(), ) .expect("unreachable"), - res: *res, }), }, id: task_id, @@ -3235,16 +3240,16 @@ impl PackageManager { pub(crate) fn enqueue_git_for_checkout( &mut self, dependency_id: DependencyID, + package_id: PackageID, alias: &[u8], - resolution: &Resolution, task_context: TaskCallbackContext, patch_name_and_version_hash: Option, ) -> GitEnqueueResult { enqueue_git_for_checkout( self, dependency_id, + package_id, alias, - resolution, task_context, patch_name_and_version_hash, ) diff --git a/src/install/PackageManager/processDependencyList.rs b/src/install/PackageManager/processDependencyList.rs index 7fdd1f654dac..922028868ed7 100644 --- a/src/install/PackageManager/processDependencyList.rs +++ b/src/install/PackageManager/processDependencyList.rs @@ -395,6 +395,11 @@ impl PackageManager { } } } + // Only a linker parks this, and its `run_tasks` starts the + // checkout when the clone ends. Here it would be dropped. + TaskCallbackContext::GitCheckout { .. } => { + debug_assert!(false, "git checkout waiter outside the install phase"); + } _ => {} } Ok(()) diff --git a/src/install/PackageManager/runTasks.rs b/src/install/PackageManager/runTasks.rs index 3feca04e87af..bdbc35140357 100644 --- a/src/install/PackageManager/runTasks.rs +++ b/src/install/PackageManager/runTasks.rs @@ -1331,63 +1331,35 @@ fn run_tasks_erased( } else if cb.has_on_package_download_error && cb.is_store_installer { // The isolated installer queued its entry contexts // under `checkout_id`, not `clone_id`. A failed clone - // never reaches checkout, so drain every waiting - // checkout for this repo or the install loop blocks - // forever on the entry's pending-task slot. - let mut drained_any = false; + // never reaches checkout, so fail the checkout of + // every waiter or the install loop blocks forever on + // the entry's pending-task slot. if let Some(waiters) = manager.task_queue.remove(&task.id) { - let pkg_resolutions = manager.lockfile.packages.items_resolution(); for waiter in waiters.iter() { - let dep_id = match waiter { - bun_install::TaskCallbackContext::Dependency(id) => *id, - _ => continue, - }; - let pkg_id = manager.lockfile.buffers.resolutions[dep_id as usize]; - if pkg_id == INVALID_PACKAGE_ID { - continue; - } - let res = &pkg_resolutions[pkg_id as usize]; - if res.tag != bun_install::ResolutionTag::Git { + let &bun_install::TaskCallbackContext::GitCheckout { + package_id, + .. + } = waiter + else { continue; - } - // SAFETY: `res.tag == Git` checked just above — - // `value.git` is the active union arm. - let res_git = res.git(); + }; + let res = manager.lockfile.packages.items_resolution() + [package_id as usize]; + let git = res.git(); let checkout_id = Task::Id::for_git_checkout( - manager.lockfile.str(&res_git.repo), - manager.lockfile.str(&res_git.resolved), + manager.lockfile.str(&git.repo), + manager.lockfile.str(&git.resolved), ); - drained_any = true; (cb.on_package_download_error_store)( extract_ctx, checkout_id, name, - res, + &res, err, url, ); } } - if !drained_any { - // No clone waiters recorded (or all were skipped - // above) — fall back to the clone task's own - // resolution so the originating entry is still - // released. - // SAFETY: `clone.res.tag == Git` — git-clone tasks are - // only enqueued for git resolutions; `value.git` is - // the active union arm. - let resolved = &clone.res.git().resolved; - let checkout_id = - Task::Id::for_git_checkout(url, manager.lockfile.str(resolved)); - (cb.on_package_download_error_store)( - extract_ctx, - checkout_id, - name, - &clone.res, - err, - url, - ); - } } else if log_level != Options::LogLevel::Silent { bun_ast::add_error_pretty!( manager.log_mut(), @@ -1403,16 +1375,21 @@ fn run_tasks_erased( manager.git_repositories.insert(task.id, repo_fd); - if cb.has_on_extract && cb.is_package_installer { - // Installing! The clone task is shared by every dependency on - // this repo URL; enqueue a checkout per waiter, not just one. + if cb.has_on_extract { + // Installing! The clone task is shared by every package of + // this repo URL; enqueue the checkout of each waiter. The + // waiter names its package: the dependency it was placed + // under can resolve to another one. let Some(waiters) = manager.task_queue.remove(&task.id) else { continue; }; for waiter in waiters.iter() { - let dep_id = match waiter { - bun_install::TaskCallbackContext::Dependency(id) => *id, - _ => continue, + let &bun_install::TaskCallbackContext::GitCheckout { + dependency_id: dep_id, + package_id, + } = waiter + else { + continue; }; // reshaped for borrowck — copy the small `String` handles // so the `&manager.lockfile` borrow doesn't extend across @@ -1421,16 +1398,7 @@ fn run_tasks_erased( let dep = &manager.lockfile.buffers.dependencies[dep_id as usize]; (dep.name, dep.behavior.is_required()) }; - let pkg_id = manager.lockfile.buffers.resolutions[dep_id as usize]; - if pkg_id == INVALID_PACKAGE_ID { - continue; - } - let res = manager.lockfile.packages.items_resolution()[pkg_id as usize]; - if res.tag != bun_install::ResolutionTag::Git { - continue; - } - // SAFETY: `res.tag == Git` checked just above — - // `value.git` is the active union arm. + let res = manager.lockfile.packages.items_resolution()[package_id as usize]; let git = *res.git(); // SAFETY: `string_bytes` lives as long as `manager.lockfile` // and is not reallocated while resolve tasks are draining. diff --git a/src/install/PackageManagerTask.rs b/src/install/PackageManagerTask.rs index e35688d9a7cf..17f4d7c81e99 100644 --- a/src/install/PackageManagerTask.rs +++ b/src/install/PackageManagerTask.rs @@ -572,7 +572,6 @@ pub struct ExtractRequest<'a> { pub struct GitCloneRequest { pub(crate) name: StringOrTinyString, pub(crate) url: StringOrTinyString, - pub(crate) res: Resolution, } pub struct GitCommitRequest { diff --git a/src/install/isolated_install.rs b/src/install/isolated_install.rs index 756bfd98712f..919ba258b509 100644 --- a/src/install/isolated_install.rs +++ b/src/install/isolated_install.rs @@ -2465,8 +2465,8 @@ pub(crate) fn install_isolated_packages( ResolutionTag::Git => { if installer.manager_mut().enqueue_git_for_checkout( dep_id, + pkg_id, dep.name.slice(string_buf), - &pkg_res, ctx, patch_info.name_and_version_hash(), ) == crate::package_manager::GitEnqueueResult::OfflineMiss diff --git a/src/install/lib.rs b/src/install/lib.rs index e4038faccc6e..cc635ce1cf4e 100644 --- a/src/install/lib.rs +++ b/src/install/lib.rs @@ -1090,8 +1090,20 @@ pub enum TaskCallbackContext { IsolatedPackageInstallContext(isolated_install::EntryId), RootDependency(DependencyID), RootRequestId(PackageID), + /// Waits on a git clone in the install phase: the package to check out + /// once the repository is there. `dependency_id` is the dependency the + /// linker placed the package under. It gives the alias and whether the + /// package is required, and can resolve to a different package (a peer + /// dependency in the isolated store). + GitCheckout { + dependency_id: DependencyID, + package_id: PackageID, + }, } +// One per waiter in `task_queue`. +const _: () = assert!(core::mem::size_of::() == 32); + // We can't know all the packages we need until we've downloaded all the packages // The easy way would be: // 1. Download all packages, parsing their dependencies and enqueuing all dependencies for resolution diff --git a/test/cli/install/bun-install-git-deps.test.ts b/test/cli/install/bun-install-git-deps.test.ts index e63c751009a6..36361311beb4 100644 --- a/test/cli/install/bun-install-git-deps.test.ts +++ b/test/cli/install/bun-install-git-deps.test.ts @@ -5,9 +5,9 @@ // a bare repo on disk (served over git's dumb HTTP protocol by Bun.serve // when an http URL is needed) or tarballs built in memory. import { afterAll, beforeAll, expect, test } from "bun:test"; -import { existsSync, mkdirSync, readFileSync, rmSync, writeFileSync } from "fs"; +import { existsSync, mkdirSync, readFileSync, realpathSync, renameSync, rmSync, writeFileSync } from "fs"; import { bunEnv, bunExe, isLinux, isWindows, normalizeBunSnapshot, tempDir } from "harness"; -import { join } from "path"; +import { dirname, join } from "path"; import { pathToFileURL } from "url"; const gitEnv = { @@ -565,6 +565,290 @@ for (const linker of ["hoisted", "isolated"] as const) { ); } +// A registry that has version 1.0.0 of each package: name -> the rest of its +// package.json. The marker its index.js exports is `-from-registry`. +async function serveRegistry(packages: Record) { + const tarballs = new Map(); + for (const [name, fields] of Object.entries(packages)) { + const files = { + "package.json": JSON.stringify({ name, version: "1.0.0", ...fields }), + "index.js": indexJs(`${name}-from-registry`), + }; + tarballs.set(name, await tarballOf("package", files)); + } + return Bun.serve({ + port: 0, + fetch(req) { + const { origin, pathname } = new URL(req.url); + const name = pathname.slice(1).replace(/\.tgz$/, ""); + const tarball = tarballs.get(name); + if (!tarball) return new Response("not found", { status: 404 }); + if (pathname.endsWith(".tgz")) return new Response(tarball); + const dist = { tarball: `${origin}/${name}.tgz`, integrity: integrityOf(tarball) }; + return Response.json({ + name, + "dist-tags": { latest: "1.0.0" }, + versions: { "1.0.0": { name, version: "1.0.0", ...packages[name], dist } }, + }); + }, + }); +} + +// bun.lock's `packages` section as key -> `@`. +async function lockedResolutions(project: string): Promise> { + return Object.fromEntries( + Object.entries(await lockedPackages(project)).map(([key, [resolution]]) => [key, resolution]), + ); +} + +// Where the isolated linker installed the `name` that `dir` resolves: the +// store entry, whose node_modules has the package and what it resolves. +function storeEntryOf(dir: string, name: string) { + return dirname(dirname(realpathSync(join(dir, "node_modules", name)))); +} + +// `/project`, a workspace root with no dependencies of its own, and its +// one member `packages/m`. bun links a workspace with the isolated linker. +function writeWorkspace(root: string, dependencies: Record) { + const project = join(root, "project"); + const member = join(project, "packages", "m"); + mkdirSync(member, { recursive: true }); + writeFileSync(join(project, "package.json"), JSON.stringify({ name: "project", workspaces: ["packages/*"] })); + writeFileSync(join(member, "package.json"), JSON.stringify({ name: "m", version: "1.0.0", dependencies })); + return { project, member }; +} + +// The isolated linker installs a package once for all the dependencies on it +// and keeps one of them with it: the first that its depth-first, name-ordered +// walk reaches. In the tests below that is, for the git package `b` of the +// workspace member `m`, the peer dependency `b: "*"` of `a`, which is walked +// first. It is not a git dependency, and bun.lock resolves a required one to +// the registry's `b` (`a/b`). Once the repository was cloned, the install took +// the checkout to start from that dependency: it started none and never exited. +for (const peer of ["required", "optional"] as const) { + test.concurrent( + `installs a workspace's git dependency that is the ${peer} peer of an earlier package from a cold cache`, + async () => { + using dir = tempDir(`git-dep-${peer}-peer`, {}); + const root = String(dir); + const bare = await makeSharedRepo(root, [{ name: "b", branch: "main" }], "b.git"); + const repoUrl = `git+${pathToFileURL(bare)}`; + // A required peer is also resolved on its own, from the registry. + await using registry = await serveRegistry( + peer === "required" + ? { a: { peerDependencies: { b: "*" } }, b: {} } + : { a: { peerDependencies: { b: "*" }, peerDependenciesMeta: { b: { optional: true } } } }, + ); + const env = { BUN_CONFIG_REGISTRY: registry.url.href }; + const { project, member } = writeWorkspace(root, { a: "1.0.0", b: `${repoUrl}#main` }); + const locked = { + a: "a@1.0.0", + b: `b@${repoUrl}#${branchCommits(bare).main}`, + m: "m@workspace:packages/m", + ...(peer === "required" && { "a/b": "b@1.0.0" }), + }; + // What `m` resolves, and the `b` that `a` resolves: the git one. + const installed = async () => ({ + ...(await installedVersions(member, ["a", "b"])), + "a > b": await installedVersionOf(storeEntryOf(member, "a"), "b"), + }); + const expected = { a: "a-from-registry", b: "main", "a > b": "main" }; + const output = [expect.stringContaining("bun install v"), "", "2 packages installed"]; + + // fresh install to produce a complete lockfile + { + const { stdout, stderr, exitCode } = await runInstall(project, join(root, "cache-warm"), env); + expect(stderr).toContain("Saved lockfile"); + expect(installOutput(stdout)).toEqual(output); + expect(await installed()).toEqual(expected); + expect(await lockedResolutions(project)).toEqual(locked); + expect(exitCode).toBe(0); + } + + // simulate a fresh machine: keep bun.lock, drop node_modules + cache + rmSync(join(project, "node_modules"), { recursive: true }); + rmSync(join(member, "node_modules"), { recursive: true }); + const { stdout, stderr, exitCode } = await runInstall( + project, + join(root, "cache-cold"), + env, + "--frozen-lockfile", + ); + expect(stderr).toBe(""); + expect(installOutput(stdout)).toEqual(output); + expect(await installed()).toEqual(expected); + expect(exitCode).toBe(0); + }, + 30_000, + ); +} + +// One clone serves every package of a repository. Three wait for it here, and +// the one that a's peer dependency stands for is neither the first nor the last. +test.concurrent( + "checks out every package of a repository when a peer dependency stands for one of them", + async () => { + using dir = tempDir("git-dep-peer-shared-repo", {}); + const root = String(dir); + const names = ["aa", "b", "c"]; + const bare = await makeSharedRepo( + root, + names.map(name => ({ name, branch: name })), + ); + const repoUrl = `git+${pathToFileURL(bare)}`; + const commits = branchCommits(bare); + await using registry = await serveRegistry({ a: { peerDependencies: { b: "*" } }, b: {} }); + const env = { BUN_CONFIG_REGISTRY: registry.url.href }; + const { project, member } = writeWorkspace(root, { + a: "1.0.0", + ...Object.fromEntries(names.map(name => [name, `${repoUrl}#${name}`])), + }); + const locked = { + a: "a@1.0.0", + ...Object.fromEntries(names.map(name => [name, `${name}@${repoUrl}#${commits[name]}`])), + m: "m@workspace:packages/m", + "a/b": "b@1.0.0", + }; + const installed = async () => ({ + ...(await installedVersions(member, ["a", ...names])), + "a > b": await installedVersionOf(storeEntryOf(member, "a"), "b"), + }); + const expected = { a: "a-from-registry", aa: "aa", b: "b", c: "c", "a > b": "b" }; + const output = [expect.stringContaining("bun install v"), "", "4 packages installed"]; + + // fresh install to produce a complete lockfile + { + const { stdout, stderr, exitCode } = await runInstall(project, join(root, "cache-warm"), env); + expect(stderr).toContain("Saved lockfile"); + expect(installOutput(stdout)).toEqual(output); + expect(await installed()).toEqual(expected); + expect(await lockedResolutions(project)).toEqual(locked); + expect(exitCode).toBe(0); + } + + // simulate a fresh machine: keep bun.lock, drop node_modules + cache + rmSync(join(project, "node_modules"), { recursive: true }); + rmSync(join(member, "node_modules"), { recursive: true }); + const { stdout, stderr, exitCode } = await runInstall(project, join(root, "cache-cold"), env, "--frozen-lockfile"); + expect(stderr).toBe(""); + expect(installOutput(stdout)).toEqual(output); + expect(await installed()).toEqual(expected); + expect(exitCode).toBe(0); + }, + 30_000, +); + +// The repository is gone when the cold install clones it. The failed clone +// fails every package that waits for it. The one that a's peer dependency +// stands for was left waiting, and the install never exited. +test.concurrent( + "fails every package of a repository that cannot be cloned when a peer dependency stands for one of them", + async () => { + using dir = tempDir("git-dep-peer-clone-fails", {}); + const root = String(dir); + const names = ["b", "c"]; + const bare = await makeSharedRepo( + root, + names.map(name => ({ name, branch: name })), + ); + const repoUrl = `git+${pathToFileURL(bare)}`; + await using registry = await serveRegistry({ a: { peerDependencies: { b: "*" } }, b: {} }); + const env = { BUN_CONFIG_REGISTRY: registry.url.href }; + const { project, member } = writeWorkspace(root, { + a: "1.0.0", + ...Object.fromEntries(names.map(name => [name, `${repoUrl}#${name}`])), + }); + + // fresh install to produce a complete lockfile + { + const { stderr, exitCode } = await runInstall(project, join(root, "cache-warm"), env); + expect(stderr).toContain("Saved lockfile"); + expect(await installedVersions(member, ["a", ...names])).toEqual({ a: "a-from-registry", b: "b", c: "c" }); + expect((await lockedResolutions(project))["a/b"]).toBe("b@1.0.0"); + expect(exitCode).toBe(0); + } + + // a fresh machine that cannot reach the repository + rmSync(join(project, "node_modules"), { recursive: true }); + rmSync(join(member, "node_modules"), { recursive: true }); + renameSync(bare, `${bare}.gone`); + const { stdout, stderr, exitCode } = await runInstall(project, join(root, "cache-cold"), env, "--frozen-lockfile"); + const failed = "error: failed to download "; + expect( + stderr + .split(/\r?\n/) + .filter(line => line.startsWith(failed)) + .map(line => line.slice(failed.length, line.indexOf("@"))), + ).toEqual(names); + expect(stdout).toContain("Failed to install 2 packages"); + expect(exitCode).toBe(1); + }, + 30_000, +); + +// `overrides` puts a git commit in place of a's registry dependency `b`. In +// bun.lock that dependency keeps its range `^1.0.0` and resolves to the git +// package, so it is no git dependency either. +test.concurrent( + "isolated linker installs a git package that an override puts in place of a registry dependency, from a cold cache", + async () => { + using dir = tempDir("git-dep-override", {}); + const root = String(dir); + const bare = await makeSharedRepo(root, [{ name: "b", branch: "main" }], "b.git"); + const repoUrl = `git+${pathToFileURL(bare)}`; + const sha = branchCommits(bare).main; + await using registry = await serveRegistry({ a: { dependencies: { b: "^1.0.0" } } }); + const env = { BUN_CONFIG_REGISTRY: registry.url.href }; + const project = join(root, "project"); + mkdirSync(project); + writeFileSync( + join(project, "package.json"), + JSON.stringify({ + name: "project", + version: "1.0.0", + dependencies: { a: "1.0.0" }, + overrides: { b: `${repoUrl}#${sha}` }, + }), + ); + const installed = async () => ({ + a: await installedVersionOf(project, "a"), + "a > b": await installedVersionOf(storeEntryOf(project, "a"), "b"), + }); + const expected = { a: "a-from-registry", "a > b": "main" }; + const output = [expect.stringContaining("bun install v"), "", "+ a@1.0.0", "", "2 packages installed"]; + + // fresh install to produce a complete lockfile + { + const { stdout, stderr, exitCode } = await runInstall( + project, + join(root, "cache-warm"), + env, + "--linker=isolated", + ); + expect(stderr).toContain("Saved lockfile"); + expect(installOutput(stdout)).toEqual(output); + expect(await installed()).toEqual(expected); + expect(await lockedResolutions(project)).toEqual({ a: "a@1.0.0", b: `b@${repoUrl}#${sha}` }); + expect(exitCode).toBe(0); + } + + // simulate a fresh machine: keep bun.lock, drop node_modules + cache + rmSync(join(project, "node_modules"), { recursive: true }); + const { stdout, stderr, exitCode } = await runInstall( + project, + join(root, "cache-cold"), + env, + "--frozen-lockfile", + "--linker=isolated", + ); + expect(stderr).toBe(""); + expect(installOutput(stdout)).toEqual(output); + expect(await installed()).toEqual(expected); + expect(exitCode).toBe(0); + }, + 30_000, +); + // issue #35420 bug 3: `git+file://` dependencies never cloned at all — the // clone task recognized neither an https nor an ssh URL and finished without // running git, leaving a poisoned repo handle behind. From be3036ba2e0111acd04814c19f6172624bfd5383 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 2 Oct 2026 09:23:32 +0000 Subject: [PATCH 2/8] install: patch a git package by its package id after the checkout enqueue_git_checkout looked up the package to patch by the name of the dependency. An alias does not share its name with the package, so the lookup found nothing: panic: Package not found. When two packages share a name, it took the first one. enqueue_git_for_checkout now passes the package id with the patch hash. --- .../PackageManager/PackageManagerEnqueue.rs | 23 +++----- test/cli/install/bun-install-git-deps.test.ts | 52 +++++++++++++++++++ 2 files changed, 59 insertions(+), 16 deletions(-) diff --git a/src/install/PackageManager/PackageManagerEnqueue.rs b/src/install/PackageManager/PackageManagerEnqueue.rs index dea833acb63f..4df4cabb43c1 100644 --- a/src/install/PackageManager/PackageManagerEnqueue.rs +++ b/src/install/PackageManager/PackageManagerEnqueue.rs @@ -320,7 +320,7 @@ pub fn enqueue_git_for_checkout( alias, &resolution, resolved, - patch_name_and_version_hash, + patch_name_and_version_hash.map(|hash| (package_id, hash)), ); this.enqueue_git_task(task); } else { @@ -2044,16 +2044,18 @@ pub fn enqueue_git_checkout( name: &[u8], resolution: &Resolution, resolved: &[u8], - // if patched then we need to do apply step after network task is done - patch_name_and_version_hash: Option, + // if patched then we need to do apply step after network task is done: + // the package to patch and its name-and-version hash + patch: Option<(PackageID, u64)>, ) -> NonNull> { // The patched-dependency entry can be missing (or its hash not yet // computed) when install state went stale — e.g. the patch was removed // from package.json, leaving the hash only in // `patched_dependencies_to_remove`. Install the package unpatched instead // of panicking. - let patch = patch_name_and_version_hash.and_then(|h| { + let patch = patch.and_then(|(pkg_id, h)| { Some(( + pkg_id, h, this.lockfile .patched_dependencies @@ -2092,18 +2094,7 @@ pub fn enqueue_git_checkout( .expect("unreachable"), }), }, - apply_patch_task: if let Some((h, patch_hash)) = patch { - let dep_name_hash = - this.lockfile.buffers.dependencies[dependency_id as usize].name_hash; - let pkg_id = match this - .lockfile - .package_index - .get(&dep_name_hash) - .unwrap_or_else(|| panic!("Package not found")) - { - PackageIndexEntry::Id(p) => *p, - PackageIndexEntry::Ids(ps) => ps[0], // TODO is this correct - }; + apply_patch_task: if let Some((pkg_id, h, patch_hash)) = patch { let mut pt = PatchTask::new_apply_patch_hash(this, pkg_id, patch_hash, h); pt.callback.apply_mut().task_id = Some(task_id); Some(pt) diff --git a/test/cli/install/bun-install-git-deps.test.ts b/test/cli/install/bun-install-git-deps.test.ts index 36361311beb4..4d720e46664b 100644 --- a/test/cli/install/bun-install-git-deps.test.ts +++ b/test/cli/install/bun-install-git-deps.test.ts @@ -849,6 +849,58 @@ test.concurrent( 30_000, ); +// A patch is applied to a git package right after its checkout when the +// repository was cloned earlier in the same process, here for the `c` that +// `bun install ` adds. The package to patch was looked up by the name of +// the dependency, which an alias does not share with it: `panic: Package not found`. +test.concurrent( + "patches a git package under an alias after another dependency cloned its repository", + async () => { + using dir = tempDir("git-dep-alias-patch", { + "project/patches/b.patch": `diff --git a/index.js b/index.js +index 0000000000000000000000000000000000000000..1111111111111111111111111111111111111111 100644 +--- a/index.js ++++ b/index.js +@@ -1 +1 @@ +-module.exports = "b"; ++module.exports = "b-patched"; +`, + }); + const root = String(dir); + const project = join(root, "project"); + const bare = await makeSharedRepo( + root, + ["b", "c"].map(name => ({ name, branch: name })), + ); + const repoUrl = `git+${pathToFileURL(bare)}`; + writeFileSync( + join(project, "package.json"), + JSON.stringify({ + name: "project", + version: "1.0.0", + dependencies: { "b-alias": `${repoUrl}#b` }, + patchedDependencies: { [`b@${repoUrl}#${branchCommits(bare).b}`]: "patches/b.patch" }, + }), + ); + + // fresh install to produce a complete lockfile + { + const { stderr, exitCode } = await runInstall(project, join(root, "cache-warm"), {}); + expect(stderr).toContain("Saved lockfile"); + expect(await installedVersions(project, ["b-alias"])).toEqual({ "b-alias": "b-patched" }); + expect(exitCode).toBe(0); + } + + // a fresh machine: keep bun.lock, drop node_modules + cache, and add `c` + rmSync(join(project, "node_modules"), { recursive: true }); + const { stderr, exitCode } = await runInstall(project, join(root, "cache-cold"), {}, `${repoUrl}#c`); + expect(stderr).toContain("Saved lockfile"); + expect(await installedVersions(project, ["b-alias", "c"])).toEqual({ "b-alias": "b-patched", c: "c" }); + expect(exitCode).toBe(0); + }, + 30_000, +); + // issue #35420 bug 3: `git+file://` dependencies never cloned at all — the // clone task recognized neither an https nor an ssh URL and finished without // running git, leaving a poisoned repo handle behind. From cc182a9726476de694f2bac82a50db51602f7ae4 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 2 Oct 2026 10:12:42 +0000 Subject: [PATCH 3/8] install: remove the pinned commit from the resolver's git arm It existed for the isolated linker, which sent a resolved dependency through the resolver again when a clone ended. The linker no longer does that, and no resolve-phase caller reaches the arm with a resolved git dependency. --- .../PackageManager/PackageManagerEnqueue.rs | 72 +++++++------------ 1 file changed, 26 insertions(+), 46 deletions(-) diff --git a/src/install/PackageManager/PackageManagerEnqueue.rs b/src/install/PackageManager/PackageManagerEnqueue.rs index 4df4cabb43c1..e510e7beb9e8 100644 --- a/src/install/PackageManager/PackageManagerEnqueue.rs +++ b/src/install/PackageManager/PackageManagerEnqueue.rs @@ -1393,56 +1393,36 @@ pub fn enqueue_dependency_with_main_and_success_fn( let needs_ctx = this.lockfile.buffers.resolutions[id as usize] == invalid_package_id; - // An already-resolved dependency (install-phase re-enqueue - // after the shared clone finished) is pinned: its install - // context is keyed on the stored SHA, and a branch - // committish's current tip may differ. - let pinned: Option> = if needs_ctx { - None - } else { - let pkg_id = this.lockfile.buffers.resolutions[id as usize]; - let pkg_res = this.lockfile.packages.items_resolution()[pkg_id as usize]; - // SAFETY: tag checked — `value.git` is the active union arm. - (pkg_res.tag == ResolutionTag::Git) - .then(|| this.lockfile.str(&pkg_res.git().resolved).to_vec()) - }; - let resolved = match pinned { - Some(resolved) => resolved, + // Waits on a `git log` task; `run_tasks` fills `git_commits` and re-enters here. + let committish = this.lockfile.str_detached(&dep.committish); + let commit_id = Task::Id::for_git_commit(url, committish); + let resolved = match this.git_commits.get(&commit_id) { + Some(resolved) => resolved.clone(), None => { - // Waits on a `git log` task; `run_tasks` fills `git_commits` and re-enters here. - let committish = this.lockfile.str_detached(&dep.committish); - let commit_id = Task::Id::for_git_commit(url, committish); - match this.git_commits.get(&commit_id) { - Some(resolved) => resolved.clone(), - None => { - let entry = this - .task_queue - .get_or_put_context(commit_id, ()) - .expect("unreachable"); - if !entry.found_existing { - *entry.value_ptr = TaskCallbackList::default(); - } - entry.value_ptr.push(ctx); - - if dependency.behavior.is_peer() && !install_peer { - this.peer_dependencies.write_item(id)?; - return Ok(()); - } + let entry = this + .task_queue + .get_or_put_context(commit_id, ()) + .expect("unreachable"); + if !entry.found_existing { + *entry.value_ptr = TaskCallbackList::default(); + } + entry.value_ptr.push(ctx); - if this.has_created_network_task( - commit_id, - dependency.behavior.is_required(), - ) { - return Ok(()); - } + if dependency.behavior.is_peer() && !install_peer { + this.peer_dependencies.write_item(id)?; + return Ok(()); + } - let task = enqueue_git_commit( - this, commit_id, clone_id, alias, url, committish, - ); - this.enqueue_git_task(task); - return Ok(()); - } + if this + .has_created_network_task(commit_id, dependency.behavior.is_required()) + { + return Ok(()); } + + let task = + enqueue_git_commit(this, commit_id, clone_id, alias, url, committish); + this.enqueue_git_task(task); + return Ok(()); } }; let checkout_id = Task::Id::for_git_checkout(url, &resolved); From 7cd143ca2cf3289d2d80c0532555758c9660cfe8 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 2 Oct 2026 10:59:34 +0000 Subject: [PATCH 4/8] test(install): a git package whose URL bun.lock spells differently from the dependency The isolated linker cloned the repository a second time, under the dependency's spelling, and left the package waiting. The typed clone waiter starts the checkout of the package in bun.lock. --- test/cli/install/bun-install-git-deps.test.ts | 39 ++++++++++++++++++- 1 file changed, 38 insertions(+), 1 deletion(-) diff --git a/test/cli/install/bun-install-git-deps.test.ts b/test/cli/install/bun-install-git-deps.test.ts index 4d720e46664b..417f7dc21d95 100644 --- a/test/cli/install/bun-install-git-deps.test.ts +++ b/test/cli/install/bun-install-git-deps.test.ts @@ -5,7 +5,7 @@ // a bare repo on disk (served over git's dumb HTTP protocol by Bun.serve // when an http URL is needed) or tarballs built in memory. import { afterAll, beforeAll, expect, test } from "bun:test"; -import { existsSync, mkdirSync, readFileSync, realpathSync, renameSync, rmSync, writeFileSync } from "fs"; +import { existsSync, mkdirSync, readdirSync, readFileSync, realpathSync, renameSync, rmSync, writeFileSync } from "fs"; import { bunEnv, bunExe, isLinux, isWindows, normalizeBunSnapshot, tempDir } from "harness"; import { dirname, join } from "path"; import { pathToFileURL } from "url"; @@ -849,6 +849,43 @@ test.concurrent( 30_000, ); +// bun.lock can spell a repository's URL differently from the dependency: an +// scp-form URL comes back from it with `ssh://` in front. Here the locked URL +// has a `.` segment. The install clones and checks out what bun.lock names. It +// used to go by the dependency once the clone ended: it cloned a second time +// and left the package waiting. +test.concurrent( + "isolated linker checks out a git package whose URL bun.lock spells differently from the dependency", + async () => { + using dir = tempDir("git-dep-url-spelling", {}); + const root = String(dir); + const name = nameOf("b"); + const repoUrl = `git+${pathToFileURL(sharedBare)}`; + const lockedUrl = repoUrl.replace(/\/shared-repo\.git$/, "/./shared-repo.git"); + const sha = sharedCommits["pkg-b"]; + const project = writeProject(root, { [name]: `${repoUrl}#pkg-b` }); + writeFileSync( + join(project, "bun.lock"), + JSON.stringify({ + lockfileVersion: 1, + configVersion: 1, + workspaces: { "": { name: "project", dependencies: { [name]: `${repoUrl}#pkg-b` } } }, + packages: { [name]: [`${name}@${lockedUrl}#${sha}`, {}, sha] }, + }), + ); + + const cache = join(root, "cache"); + const { stdout, stderr, exitCode } = await runInstall(project, cache, {}, "--frozen-lockfile", "--linker=isolated"); + expect(stderr).toBe(""); + expectInstalled(stdout, { [name]: `${lockedUrl}#${sha}` }); + expect(await installedVersions(project, [name])).toEqual(markers(["b"])); + // one bare clone of the repository + expect(readdirSync(cache).filter(entry => entry.endsWith(".git"))).toHaveLength(1); + expect(exitCode).toBe(0); + }, + 30_000, +); + // A patch is applied to a git package right after its checkout when the // repository was cloned earlier in the same process, here for the `c` that // `bun install ` adds. The package to patch was looked up by the name of From 0ae1745515eb42265003a6e23a99e27cb6bbb8f3 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 2 Oct 2026 15:51:12 +0000 Subject: [PATCH 5/8] test(install): a patched git package under an alias when package.json adds another package of its repository A plain bun install hits the same panic as bun add: the resolve phase clones the repository for the new dependency, and the patched package then takes the checkout that applies the patch. --- test/cli/install/bun-install-git-deps.test.ts | 87 ++++++++++--------- 1 file changed, 47 insertions(+), 40 deletions(-) diff --git a/test/cli/install/bun-install-git-deps.test.ts b/test/cli/install/bun-install-git-deps.test.ts index 417f7dc21d95..b45f27982bf7 100644 --- a/test/cli/install/bun-install-git-deps.test.ts +++ b/test/cli/install/bun-install-git-deps.test.ts @@ -887,14 +887,15 @@ test.concurrent( ); // A patch is applied to a git package right after its checkout when the -// repository was cloned earlier in the same process, here for the `c` that -// `bun install ` adds. The package to patch was looked up by the name of -// the dependency, which an alias does not share with it: `panic: Package not found`. -test.concurrent( - "patches a git package under an alias after another dependency cloned its repository", - async () => { - using dir = tempDir("git-dep-alias-patch", { - "project/patches/b.patch": `diff --git a/index.js b/index.js +// repository was cloned earlier in the same process, here to resolve the new +// dependency `c`. The package to patch was looked up by the name of the +// dependency, which an alias does not share with it: `panic: Package not found`. +for (const added of ["`bun install `", "an edit of package.json"] as const) { + test.concurrent( + `patches a git package under an alias when ${added} adds another package of its repository`, + async () => { + using dir = tempDir("git-dep-alias-patch", { + "project/patches/b.patch": `diff --git a/index.js b/index.js index 0000000000000000000000000000000000000000..1111111111111111111111111111111111111111 100644 --- a/index.js +++ b/index.js @@ -902,41 +903,47 @@ index 0000000000000000000000000000000000000000..11111111111111111111111111111111 -module.exports = "b"; +module.exports = "b-patched"; `, - }); - const root = String(dir); - const project = join(root, "project"); - const bare = await makeSharedRepo( - root, - ["b", "c"].map(name => ({ name, branch: name })), - ); - const repoUrl = `git+${pathToFileURL(bare)}`; - writeFileSync( - join(project, "package.json"), - JSON.stringify({ - name: "project", - version: "1.0.0", - dependencies: { "b-alias": `${repoUrl}#b` }, - patchedDependencies: { [`b@${repoUrl}#${branchCommits(bare).b}`]: "patches/b.patch" }, - }), - ); + }); + const root = String(dir); + const project = join(root, "project"); + const bare = await makeSharedRepo( + root, + ["b", "c"].map(name => ({ name, branch: name })), + ); + const repoUrl = `git+${pathToFileURL(bare)}`; + const writePackageJson = (dependencies: Record) => + writeFileSync( + join(project, "package.json"), + JSON.stringify({ + name: "project", + version: "1.0.0", + dependencies, + patchedDependencies: { [`b@${repoUrl}#${branchCommits(bare).b}`]: "patches/b.patch" }, + }), + ); + writePackageJson({ "b-alias": `${repoUrl}#b` }); - // fresh install to produce a complete lockfile - { - const { stderr, exitCode } = await runInstall(project, join(root, "cache-warm"), {}); + // fresh install to produce a complete lockfile + { + const { stderr, exitCode } = await runInstall(project, join(root, "cache-warm"), {}); + expect(stderr).toContain("Saved lockfile"); + expect(await installedVersions(project, ["b-alias"])).toEqual({ "b-alias": "b-patched" }); + expect(exitCode).toBe(0); + } + + // a fresh machine: keep bun.lock, drop node_modules + cache, and add `c` + rmSync(join(project, "node_modules"), { recursive: true }); + const args: string[] = []; + if (added === "an edit of package.json") writePackageJson({ "b-alias": `${repoUrl}#b`, c: `${repoUrl}#c` }); + else args.push(`${repoUrl}#c`); + const { stderr, exitCode } = await runInstall(project, join(root, "cache-cold"), {}, ...args); expect(stderr).toContain("Saved lockfile"); - expect(await installedVersions(project, ["b-alias"])).toEqual({ "b-alias": "b-patched" }); + expect(await installedVersions(project, ["b-alias", "c"])).toEqual({ "b-alias": "b-patched", c: "c" }); expect(exitCode).toBe(0); - } - - // a fresh machine: keep bun.lock, drop node_modules + cache, and add `c` - rmSync(join(project, "node_modules"), { recursive: true }); - const { stderr, exitCode } = await runInstall(project, join(root, "cache-cold"), {}, `${repoUrl}#c`); - expect(stderr).toContain("Saved lockfile"); - expect(await installedVersions(project, ["b-alias", "c"])).toEqual({ "b-alias": "b-patched", c: "c" }); - expect(exitCode).toBe(0); - }, - 30_000, -); + }, + 30_000, + ); +} // issue #35420 bug 3: `git+file://` dependencies never cloned at all — the // clone task recognized neither an https nor an ssh URL and finished without From 6e1e66012654f87c4d994a609ab4f488ae3d9e89 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 2 Oct 2026 16:35:26 +0000 Subject: [PATCH 6/8] install: shorten the comments of the git checkout waiter --- src/install/PackageManager/PackageManagerEnqueue.rs | 10 ++-------- src/install/PackageManager/processDependencyList.rs | 3 +-- src/install/PackageManager/runTasks.rs | 12 +++++------- src/install/lib.rs | 6 +----- 4 files changed, 9 insertions(+), 22 deletions(-) diff --git a/src/install/PackageManager/PackageManagerEnqueue.rs b/src/install/PackageManager/PackageManagerEnqueue.rs index e510e7beb9e8..59a120f90b14 100644 --- a/src/install/PackageManager/PackageManagerEnqueue.rs +++ b/src/install/PackageManager/PackageManagerEnqueue.rs @@ -261,10 +261,7 @@ pub enum GitEnqueueResult { OfflineMiss, } -/// Install phase: fetches the git package `package_id` into the cache, then -/// runs `task_context`. `dependency_id` is the dependency the linker placed the -/// package under; it need not resolve to `package_id` (see -/// `TaskCallbackContext::GitCheckout`). +/// `dependency_id` need not resolve to `package_id`: see `TaskCallbackContext::GitCheckout`. pub fn enqueue_git_for_checkout( this: &mut PackageManager, dependency_id: DependencyID, @@ -273,8 +270,6 @@ pub fn enqueue_git_for_checkout( task_context: TaskCallbackContext, patch_name_and_version_hash: Option, ) -> GitEnqueueResult { - // The caller passes a package whose `resolution.tag == Git`. Copy out so - // the value does not borrow `*this` while it is mutably reborrowed below. let resolution: Resolution = this.lockfile.packages.items_resolution()[package_id as usize]; let repository: Repository = *resolution.git(); // reshaped for borrowck — `url`/`resolved` borrow @@ -2024,8 +2019,7 @@ pub fn enqueue_git_checkout( name: &[u8], resolution: &Resolution, resolved: &[u8], - // if patched then we need to do apply step after network task is done: - // the package to patch and its name-and-version hash + // the package to patch and its name-and-version hash, if an apply step follows the checkout patch: Option<(PackageID, u64)>, ) -> NonNull> { // The patched-dependency entry can be missing (or its hash not yet diff --git a/src/install/PackageManager/processDependencyList.rs b/src/install/PackageManager/processDependencyList.rs index 922028868ed7..04d5e03e6842 100644 --- a/src/install/PackageManager/processDependencyList.rs +++ b/src/install/PackageManager/processDependencyList.rs @@ -395,8 +395,7 @@ impl PackageManager { } } } - // Only a linker parks this, and its `run_tasks` starts the - // checkout when the clone ends. Here it would be dropped. + // A linker's `run_tasks` starts this checkout. Here it would be dropped. TaskCallbackContext::GitCheckout { .. } => { debug_assert!(false, "git checkout waiter outside the install phase"); } diff --git a/src/install/PackageManager/runTasks.rs b/src/install/PackageManager/runTasks.rs index bdbc35140357..080ca06edeb8 100644 --- a/src/install/PackageManager/runTasks.rs +++ b/src/install/PackageManager/runTasks.rs @@ -1331,9 +1331,9 @@ fn run_tasks_erased( } else if cb.has_on_package_download_error && cb.is_store_installer { // The isolated installer queued its entry contexts // under `checkout_id`, not `clone_id`. A failed clone - // never reaches checkout, so fail the checkout of - // every waiter or the install loop blocks forever on - // the entry's pending-task slot. + // never reaches checkout, so drain every waiting + // checkout for this repo or the install loop blocks + // forever on the entry's pending-task slot. if let Some(waiters) = manager.task_queue.remove(&task.id) { for waiter in waiters.iter() { let &bun_install::TaskCallbackContext::GitCheckout { @@ -1376,10 +1376,8 @@ fn run_tasks_erased( manager.git_repositories.insert(task.id, repo_fd); if cb.has_on_extract { - // Installing! The clone task is shared by every package of - // this repo URL; enqueue the checkout of each waiter. The - // waiter names its package: the dependency it was placed - // under can resolve to another one. + // Installing! The clone task is shared by every dependency on + // this repo URL; enqueue a checkout per waiter, not just one. let Some(waiters) = manager.task_queue.remove(&task.id) else { continue; }; diff --git a/src/install/lib.rs b/src/install/lib.rs index cc635ce1cf4e..59354d28857f 100644 --- a/src/install/lib.rs +++ b/src/install/lib.rs @@ -1090,11 +1090,7 @@ pub enum TaskCallbackContext { IsolatedPackageInstallContext(isolated_install::EntryId), RootDependency(DependencyID), RootRequestId(PackageID), - /// Waits on a git clone in the install phase: the package to check out - /// once the repository is there. `dependency_id` is the dependency the - /// linker placed the package under. It gives the alias and whether the - /// package is required, and can resolve to a different package (a peer - /// dependency in the isolated store). + /// Install phase, parked on a git clone. `dependency_id` may resolve to another package. GitCheckout { dependency_id: DependencyID, package_id: PackageID, From b43b06d09201372ae9243f18a52e0dbff040e8c5 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 3 Oct 2026 04:21:05 +0000 Subject: [PATCH 7/8] test(install): correct the comments of the git dependency tests The override test said the dependency was no git dependency. The install hung because the resolver found the package of the pinned commit already loaded. Also guard the URL spelling test against a replace that matches nothing. --- test/cli/install/bun-install-git-deps.test.ts | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/test/cli/install/bun-install-git-deps.test.ts b/test/cli/install/bun-install-git-deps.test.ts index b45f27982bf7..1551d3d78f1a 100644 --- a/test/cli/install/bun-install-git-deps.test.ts +++ b/test/cli/install/bun-install-git-deps.test.ts @@ -493,8 +493,8 @@ test.concurrent( 30_000, ); -// With the isolated linker, a cold-cache frozen install re-enqueues each -// dependency after the shared clone completes; the checkout id was derived +// With the isolated linker, a cold-cache frozen install re-enqueued each +// dependency after the shared clone completed; the checkout id was derived // from the branch committish's current tip instead of the lockfile's pinned // SHA, so a branch that moved after the lockfile was written installed the // wrong commit and stranded the install context. The hoisted linker had the @@ -608,7 +608,7 @@ function storeEntryOf(dir: string, name: string) { } // `/project`, a workspace root with no dependencies of its own, and its -// one member `packages/m`. bun links a workspace with the isolated linker. +// one member `packages/m`. A new workspace gets the isolated linker. function writeWorkspace(root: string, dependencies: Record) { const project = join(root, "project"); const member = join(project, "packages", "m"); @@ -786,9 +786,9 @@ test.concurrent( 30_000, ); -// `overrides` puts a git commit in place of a's registry dependency `b`. In -// bun.lock that dependency keeps its range `^1.0.0` and resolves to the git -// package, so it is no git dependency either. +// `overrides` puts a git commit in place of a's registry dependency `b`. When +// the clone ended, the install sent that dependency through the resolver, which +// found the package of the pinned commit already loaded and started no checkout. test.concurrent( "isolated linker installs a git package that an override puts in place of a registry dependency, from a cold cache", async () => { @@ -862,6 +862,7 @@ test.concurrent( const name = nameOf("b"); const repoUrl = `git+${pathToFileURL(sharedBare)}`; const lockedUrl = repoUrl.replace(/\/shared-repo\.git$/, "/./shared-repo.git"); + expect(lockedUrl).not.toBe(repoUrl); const sha = sharedCommits["pkg-b"]; const project = writeProject(root, { [name]: `${repoUrl}#pkg-b` }); writeFileSync( @@ -890,7 +891,10 @@ test.concurrent( // repository was cloned earlier in the same process, here to resolve the new // dependency `c`. The package to patch was looked up by the name of the // dependency, which an alias does not share with it: `panic: Package not found`. -for (const added of ["`bun install `", "an edit of package.json"] as const) { +for (const [added, editsPackageJson] of [ + ["`bun install `", false], + ["an edit of package.json", true], +] as const) { test.concurrent( `patches a git package under an alias when ${added} adds another package of its repository`, async () => { @@ -934,7 +938,7 @@ index 0000000000000000000000000000000000000000..11111111111111111111111111111111 // a fresh machine: keep bun.lock, drop node_modules + cache, and add `c` rmSync(join(project, "node_modules"), { recursive: true }); const args: string[] = []; - if (added === "an edit of package.json") writePackageJson({ "b-alias": `${repoUrl}#b`, c: `${repoUrl}#c` }); + if (editsPackageJson) writePackageJson({ "b-alias": `${repoUrl}#b`, c: `${repoUrl}#c` }); else args.push(`${repoUrl}#c`); const { stderr, exitCode } = await runInstall(project, join(root, "cache-cold"), {}, ...args); expect(stderr).toContain("Saved lockfile"); From e1a7d185df76c4bb913ab3696853bd751d135e83 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 3 Oct 2026 07:28:02 +0000 Subject: [PATCH 8/8] test(install): use the default timeout in the new git dependency tests Each case runs two installs and takes under a second on a debug build. The explicit 30 s also lowered the limit that the CI runner sets. --- test/cli/install/bun-install-git-deps.test.ts | 97 +++++++++---------- 1 file changed, 44 insertions(+), 53 deletions(-) diff --git a/test/cli/install/bun-install-git-deps.test.ts b/test/cli/install/bun-install-git-deps.test.ts index 1551d3d78f1a..c817384e60d0 100644 --- a/test/cli/install/bun-install-git-deps.test.ts +++ b/test/cli/install/bun-install-git-deps.test.ts @@ -679,64 +679,59 @@ for (const peer of ["required", "optional"] as const) { expect(await installed()).toEqual(expected); expect(exitCode).toBe(0); }, - 30_000, ); } // One clone serves every package of a repository. Three wait for it here, and // the one that a's peer dependency stands for is neither the first nor the last. -test.concurrent( - "checks out every package of a repository when a peer dependency stands for one of them", - async () => { - using dir = tempDir("git-dep-peer-shared-repo", {}); - const root = String(dir); - const names = ["aa", "b", "c"]; - const bare = await makeSharedRepo( - root, - names.map(name => ({ name, branch: name })), - ); - const repoUrl = `git+${pathToFileURL(bare)}`; - const commits = branchCommits(bare); - await using registry = await serveRegistry({ a: { peerDependencies: { b: "*" } }, b: {} }); - const env = { BUN_CONFIG_REGISTRY: registry.url.href }; - const { project, member } = writeWorkspace(root, { - a: "1.0.0", - ...Object.fromEntries(names.map(name => [name, `${repoUrl}#${name}`])), - }); - const locked = { - a: "a@1.0.0", - ...Object.fromEntries(names.map(name => [name, `${name}@${repoUrl}#${commits[name]}`])), - m: "m@workspace:packages/m", - "a/b": "b@1.0.0", - }; - const installed = async () => ({ - ...(await installedVersions(member, ["a", ...names])), - "a > b": await installedVersionOf(storeEntryOf(member, "a"), "b"), - }); - const expected = { a: "a-from-registry", aa: "aa", b: "b", c: "c", "a > b": "b" }; - const output = [expect.stringContaining("bun install v"), "", "4 packages installed"]; - - // fresh install to produce a complete lockfile - { - const { stdout, stderr, exitCode } = await runInstall(project, join(root, "cache-warm"), env); - expect(stderr).toContain("Saved lockfile"); - expect(installOutput(stdout)).toEqual(output); - expect(await installed()).toEqual(expected); - expect(await lockedResolutions(project)).toEqual(locked); - expect(exitCode).toBe(0); - } +test.concurrent("checks out every package of a repository when a peer dependency stands for one of them", async () => { + using dir = tempDir("git-dep-peer-shared-repo", {}); + const root = String(dir); + const names = ["aa", "b", "c"]; + const bare = await makeSharedRepo( + root, + names.map(name => ({ name, branch: name })), + ); + const repoUrl = `git+${pathToFileURL(bare)}`; + const commits = branchCommits(bare); + await using registry = await serveRegistry({ a: { peerDependencies: { b: "*" } }, b: {} }); + const env = { BUN_CONFIG_REGISTRY: registry.url.href }; + const { project, member } = writeWorkspace(root, { + a: "1.0.0", + ...Object.fromEntries(names.map(name => [name, `${repoUrl}#${name}`])), + }); + const locked = { + a: "a@1.0.0", + ...Object.fromEntries(names.map(name => [name, `${name}@${repoUrl}#${commits[name]}`])), + m: "m@workspace:packages/m", + "a/b": "b@1.0.0", + }; + const installed = async () => ({ + ...(await installedVersions(member, ["a", ...names])), + "a > b": await installedVersionOf(storeEntryOf(member, "a"), "b"), + }); + const expected = { a: "a-from-registry", aa: "aa", b: "b", c: "c", "a > b": "b" }; + const output = [expect.stringContaining("bun install v"), "", "4 packages installed"]; - // simulate a fresh machine: keep bun.lock, drop node_modules + cache - rmSync(join(project, "node_modules"), { recursive: true }); - rmSync(join(member, "node_modules"), { recursive: true }); - const { stdout, stderr, exitCode } = await runInstall(project, join(root, "cache-cold"), env, "--frozen-lockfile"); - expect(stderr).toBe(""); + // fresh install to produce a complete lockfile + { + const { stdout, stderr, exitCode } = await runInstall(project, join(root, "cache-warm"), env); + expect(stderr).toContain("Saved lockfile"); expect(installOutput(stdout)).toEqual(output); expect(await installed()).toEqual(expected); + expect(await lockedResolutions(project)).toEqual(locked); expect(exitCode).toBe(0); - }, - 30_000, -); + } + + // simulate a fresh machine: keep bun.lock, drop node_modules + cache + rmSync(join(project, "node_modules"), { recursive: true }); + rmSync(join(member, "node_modules"), { recursive: true }); + const { stdout, stderr, exitCode } = await runInstall(project, join(root, "cache-cold"), env, "--frozen-lockfile"); + expect(stderr).toBe(""); + expect(installOutput(stdout)).toEqual(output); + expect(await installed()).toEqual(expected); + expect(exitCode).toBe(0); +}); // The repository is gone when the cold install clones it. The failed clone // fails every package that waits for it. The one that a's peer dependency @@ -783,7 +778,6 @@ test.concurrent( expect(stdout).toContain("Failed to install 2 packages"); expect(exitCode).toBe(1); }, - 30_000, ); // `overrides` puts a git commit in place of a's registry dependency `b`. When @@ -846,7 +840,6 @@ test.concurrent( expect(await installed()).toEqual(expected); expect(exitCode).toBe(0); }, - 30_000, ); // bun.lock can spell a repository's URL differently from the dependency: an @@ -884,7 +877,6 @@ test.concurrent( expect(readdirSync(cache).filter(entry => entry.endsWith(".git"))).toHaveLength(1); expect(exitCode).toBe(0); }, - 30_000, ); // A patch is applied to a git package right after its checkout when the @@ -945,7 +937,6 @@ index 0000000000000000000000000000000000000000..11111111111111111111111111111111 expect(await installedVersions(project, ["b-alias", "c"])).toEqual({ "b-alias": "b-patched", c: "c" }); expect(exitCode).toBe(0); }, - 30_000, ); }