diff --git a/src/install/PackageManager/PopulateManifestCache.rs b/src/install/PackageManager/PopulateManifestCache.rs index 8716968ad97f..9223672d79e2 100644 --- a/src/install/PackageManager/PopulateManifestCache.rs +++ b/src/install/PackageManager/PopulateManifestCache.rs @@ -116,6 +116,7 @@ impl RunTasksCallbacks for ManifestsOnlyCallbacks { /// Populate the manifest cache for packages included from `root_pkg_ids`. Only manifests of /// direct dependencies of the `root_pkg_ids` are populated. If `root_pkg_ids` has length 0 /// all packages in the lockfile will have their manifests fetched if necessary. +/// A dependency that is queued before or during the call resolves inside it. pub fn populate_manifest_cache( manager: &mut PackageManager, packages: Packages<'_>, @@ -337,6 +338,11 @@ pub fn populate_manifest_cache( } } + // `run_tasks` names the bar when a manifest download completes, and `start_manifest_task` starts none when every name is cached or already requested by a queued row. + if log_level.show_progress() { + manager.start_progress_bar_if_none(); + } + // Derive the raw provenance root first so both `sleep_until` and the // closure body's `&mut *run_closure.manager` share the same SRW tag. let mgr: *mut PackageManager = manager; diff --git a/src/install/PackageManager/runTasks.rs b/src/install/PackageManager/runTasks.rs index 3feca04e87af..b10a47fd016a 100644 --- a/src/install/PackageManager/runTasks.rs +++ b/src/install/PackageManager/runTasks.rs @@ -659,25 +659,21 @@ fn run_tasks_erased( ); } - if cb.manifests_only { - continue; + // A task that `populate_manifest_cache` started has no entry. + if let Some(dependency_list_entry) = + manager.task_queue.get_mut(&task.task_id) + { + let dependency_list = core::mem::take(dependency_list_entry); + + process_dependency_list_for_ctx( + cb, + manager, + dependency_list, + extract_ctx, + install_peer, + )?; } - let dependency_list_entry = manager - .task_queue - .get_mut(&task.task_id) - .expect("infallible: task queued"); - - let dependency_list = core::mem::take(dependency_list_entry); - - process_dependency_list_for_ctx( - cb, - manager, - dependency_list, - extract_ctx, - install_peer, - )?; - continue; } } @@ -1060,30 +1056,25 @@ fn run_tasks_erased( m }; let name_hash = manifest.pkg.name.hash; - let progress_name: Option> = (!cb.manifests_only - && log_level.show_progress() - && !has_updated_this_run.get()) - .then(|| manifest.name().to_vec()); + let progress_name: Option> = (log_level.show_progress() + && !has_updated_this_run.get() + && !cb.manifests_only) + .then(|| manifest.name().to_vec()); manager.manifests.insert(name_hash, manifest)?; - if cb.manifests_only { - continue; - } - - let dependency_list_entry = manager - .task_queue - .get_mut(&task.id) - .expect("infallible: task queued"); - let dependency_list = core::mem::take(dependency_list_entry); + // A task that `populate_manifest_cache` started has no entry. + if let Some(dependency_list_entry) = manager.task_queue.get_mut(&task.id) { + let dependency_list = core::mem::take(dependency_list_entry); - process_dependency_list_for_ctx( - cb, - manager, - dependency_list, - extract_ctx, - install_peer, - )?; + process_dependency_list_for_ctx( + cb, + manager, + dependency_list, + extract_ctx, + install_peer, + )?; + } if let Some(name) = progress_name { manager.set_node_name::( diff --git a/src/install/update_transitive.rs b/src/install/update_transitive.rs index eaac11d84443..1316013c4249 100644 --- a/src/install/update_transitive.rs +++ b/src/install/update_transitive.rs @@ -462,13 +462,17 @@ pub(crate) fn enqueue_peer_rows( register_moved(manager, &from) } -/// Pending log lines go to stderr; `--silent` drops everything but errors. +/// Pending log lines go to stderr; `--silent` drops everything but errors. An error among them fails the install here, because the reset below hides it from `install_with_manager`. fn print_log(manager: &PackageManager) -> crate::Result<()> { let log = manager.log_mut(); - if manager.options.log_level != LogLevel::Silent || log.has_errors() { + let failed = log.has_errors(); + if manager.options.log_level != LogLevel::Silent || failed { log.print(core::ptr::from_mut(Output::error_writer()))?; } log.reset(); + if failed { + return Err(crate::Error::InstallFailed); + } Ok(()) } diff --git a/test/cli/install/bun-update-transitive.test.ts b/test/cli/install/bun-update-transitive.test.ts index 7607ca55ef05..6eea04449b18 100644 --- a/test/cli/install/bun-update-transitive.test.ts +++ b/test/cli/install/bun-update-transitive.test.ts @@ -1231,7 +1231,10 @@ test.concurrent("in a workspace, `bun update` from one member also re-points a s expect(exitCode).toBe(0); }); -type Manifests = Record }>>; +type Manifests = Record< + string, + Record; peerDependencies?: Record }> +>; type Tags = Record>; // Serves one manifest per name from memory; verdaccio has no parent whose newer version keeps a range on the same child, and its dist-tags cannot move mid-test. `tags` is read per request, so a test can move a tag after installing. @@ -1342,6 +1345,266 @@ test.concurrent("`bun update ` leaves the named package's own dependencies expect(bare.exitCode).toBe(0); }); +// util's peer `core` is auto-installed and nothing depends on it outright, so `bun update ` re-resolves it after a manifest fetch of its own. Every other entry that still waits for its manifest at that point has to resolve too. +const PEER_ONLY: Manifests = { + util: { "1.0.0": { peerDependencies: { core: "^1.0.0" } }, "1.1.0": { peerDependencies: { core: "^1.0.0" } } }, + core: { "1.0.0": {}, "1.1.0": {} }, + app: { "1.0.0": { dependencies: { util: "^1.0.0" } } }, + extra: { "1.0.0": {}, "1.1.0": {} }, +}; +const CORE_NEEDS_UTIL = { dependencies: { util: "^1.0.0" } }; + +type NamedPeerCase = { + manifests?: Manifests; + // The first install pins util and core to 1.0.0; the second drops the pins and leaves both parked. + pinned: Json; + stale: Json; + // Written without an install in between, so the update is the first to see them. + edited?: Json; + files?: Record; + args: string[]; + packageJson: Json; + versions: Record; + rows: string[]; +}; + +const DIRECT = { pinned: pkgJson({ util: "1.0.0", core: "1.0.0" }), stale: pkgJson({ util: "^1.0.0" }) }; +const UTIL_ROW = movedRow("util", "1.0.0", "1.1.0"); +const CORE_ROW = movedRow("core", "1.0.0", "1.1.0"); +const BOTH_MOVED = { + packageJson: pkgJson({ util: "^1.1.0" }), + versions: { util: "1.1.0", core: "1.1.0" }, + rows: [UTIL_ROW, CORE_ROW], +}; +const ONLY_CORE_MOVED = { args: ["core"], rows: [CORE_ROW] }; + +const NAMED_PEER_CASES: Record = { + "after the package that declares it": { ...DIRECT, args: ["util", "core"], ...BOTH_MOVED }, + "before the package that declares it": { ...DIRECT, args: ["core", "util"], ...BOTH_MOVED }, + "by a pattern next to the package that declares it": { ...DIRECT, args: ["u*", "core"], ...BOTH_MOVED }, + "by `*`": { ...DIRECT, args: ["*"], ...BOTH_MOVED }, + "with --latest": { ...DIRECT, args: ["--latest", "util", "core"], ...BOTH_MOVED }, + "next to a transitive dependency": { + pinned: pkgJson({ app: "1.0.0", util: "1.0.0", core: "1.0.0" }), + stale: pkgJson({ app: "1.0.0" }), + args: ["util", "core"], + packageJson: pkgJson({ app: "1.0.0" }), + versions: { app: "1.0.0", util: "1.1.0", core: "1.1.0" }, + rows: [UTIL_ROW, CORE_ROW], + }, + "next to an optional dependency": { + pinned: { name: "foo", dependencies: { core: "1.0.0" }, optionalDependencies: { util: "1.0.0" } }, + stale: { name: "foo", optionalDependencies: { util: "^1.0.0" } }, + args: ["util", "core"], + packageJson: { name: "foo", optionalDependencies: { util: "^1.1.0" } }, + versions: { util: "1.1.0", core: "1.1.0" }, + rows: [UTIL_ROW, CORE_ROW], + }, + "when it depends on the package that declares it": { + manifests: { ...PEER_ONLY, core: { "1.0.0": CORE_NEEDS_UTIL, "1.1.0": CORE_NEEDS_UTIL } }, + ...DIRECT, + args: ["util", "core"], + ...BOTH_MOVED, + }, + "after package.json gained a dependency": { + ...DIRECT, + edited: pkgJson({ util: "^1.0.0", extra: "^1.0.0" }), + ...ONLY_CORE_MOVED, + packageJson: pkgJson({ util: "^1.0.0", extra: "^1.0.0" }), + versions: { util: "1.0.0", core: "1.1.0", extra: "1.1.0" }, + }, + "after package.json gained an optional dependency": { + ...DIRECT, + edited: pkgJson({ util: "^1.0.0" }, { optionalDependencies: { extra: "^1.0.0" } }), + ...ONLY_CORE_MOVED, + packageJson: pkgJson({ util: "^1.0.0" }, { optionalDependencies: { extra: "^1.0.0" } }), + versions: { util: "1.0.0", core: "1.1.0", extra: "1.1.0" }, + }, + "after package.json gained the peer itself": { + ...DIRECT, + edited: pkgJson({ util: "^1.0.0", core: "^1.0.0" }), + args: ["core"], + rows: [], + packageJson: pkgJson({ util: "^1.0.0", core: "^1.1.0" }), + versions: { util: "1.0.0", core: "1.1.0" }, + }, + "after package.json gained an override": { + pinned: pkgJson({ util: "1.0.0", core: "1.0.0", extra: "^1.0.0" }), + stale: pkgJson({ util: "^1.0.0", extra: "^1.0.0" }), + edited: pkgJson({ util: "^1.0.0", extra: "^1.0.0" }, { overrides: { extra: "1.0.0" } }), + ...ONLY_CORE_MOVED, + packageJson: pkgJson({ util: "^1.0.0", extra: "^1.0.0" }, { overrides: { extra: "1.0.0" } }), + versions: { util: "1.0.0", core: "1.1.0", extra: "1.0.0" }, + }, + "after package.json gained a folder with a registry dependency": { + ...DIRECT, + edited: pkgJson({ util: "^1.0.0", local: "file:./local" }), + files: { "local/package.json": stringify({ name: "local", version: "1.0.0", dependencies: { extra: "^1.0.0" } }) }, + ...ONLY_CORE_MOVED, + packageJson: pkgJson({ util: "^1.0.0", local: "file:./local" }), + versions: { util: "1.0.0", core: "1.1.0", extra: "1.1.0" }, + }, +}; + +test.concurrent.each(Object.entries(NAMED_PEER_CASES))( + "`bun update` resolves every entry when an auto-installed peer is named %s", + async (_, { manifests = PEER_ONLY, pinned, stale, edited, files = {}, args, packageJson, versions, rows }) => { + using server = await serveRegistry(manifests); + const dir = await setupServed(server, "update-named-peer-", pinned, stale); + expect(await lockedVersions(dir, "util")).toStrictEqual(["1.0.0"]); + expect(await lockedVersions(dir, "core")).toStrictEqual(["1.0.0"]); + if (edited) await write(join(dir, "package.json"), stringify(edited)); + for (const [path, text] of Object.entries(files)) await write(join(dir, path), text); + + const { stdout, exitCode } = await run(dir, "update", ...args); + expect(movedRows(stdout)).toStrictEqual(rows); + expect(await packageJsonOf(dir)).toStrictEqual(packageJson); + for (const [name, version] of Object.entries(versions)) { + expect(await lockedVersions(dir, name)).toStrictEqual([version]); + expect(await installedVersion(dir, name)).toBe(version); + } + await frozen(dir); + expect(exitCode).toBe(0); + }, +); + +test.concurrent( + "`bun update ` from a workspace member resolves its entry next to an auto-installed peer", + async () => { + using server = await serveRegistry(PEER_ONLY); + const dir = String( + tempDir("update-named-peer-member-", { + "package.json": stringify(ROOT), + "packages/pkg1/package.json": stringify(member("pkg1", { util: "1.0.0", core: "1.0.0" })), + }), + ); + await servedBunfig(server, dir); + await install(dir); + await reinstall(dir, member("pkg1", { util: "^1.0.0" }), {}, "packages/pkg1"); + expect(await lockedVersions(dir, "util")).toStrictEqual(["1.0.0"]); + expect(await lockedVersions(dir, "core")).toStrictEqual(["1.0.0"]); + + const { stdout, exitCode } = await runIn(dir, "packages/pkg1", "update", "util", "core"); + expect(movedRows(stdout)).toStrictEqual([UTIL_ROW, CORE_ROW]); + expect(await packageJsonOf(dir, "packages/pkg1")).toStrictEqual(member("pkg1", { util: "^1.1.0" })); + expect(await lockedVersions(dir, "util")).toStrictEqual(["1.1.0"]); + expect(await lockedVersions(dir, "core")).toStrictEqual(["1.1.0"]); + expect(await installedVersion(dir, "util")).toBe("1.1.0"); + expect(await installedVersion(dir, "core")).toBe("1.1.0"); + await frozen(dir); + expect(exitCode).toBe(0); + }, +); + +// With the peer's manifest on disk, --prefer-offline fetches nothing for it; the only request of the update is for util's manifest. +test.concurrent( + "`bun update --prefer-offline ` resolves an entry whose manifest is not cached next to an auto-installed peer", + async () => { + using server = await serveRegistry(PEER_ONLY); + const dir = await setupServed(server, "update-named-peer-offline-", DIRECT.pinned, DIRECT.stale); + const cache = join(dir, ".bun-cache"); + const cached = await Array.fromAsync(new Bun.Glob("*.npm").scan({ cwd: cache })); + const ofUtil: string[] = []; + for (const name of cached) { + if ((await file(join(cache, name)).text()).includes("/util-1.0.0.tgz")) ofUtil.push(name); + } + expect({ cached: cached.length, ofUtil: ofUtil.length }).toStrictEqual({ cached: 2, ofUtil: 1 }); + await rm(join(cache, ofUtil[0])); + + const { stdout, exitCode } = await run(dir, "update", "--prefer-offline", "util", "core"); + expect(movedRows(stdout)).toStrictEqual([UTIL_ROW, CORE_ROW]); + expect(await packageJsonOf(dir)).toStrictEqual(pkgJson({ util: "^1.1.0" })); + expect(await lockedVersions(dir, "util")).toStrictEqual(["1.1.0"]); + expect(await lockedVersions(dir, "core")).toStrictEqual(["1.1.0"]); + await frozen(dir); + expect(exitCode).toBe(0); + }, +); + +const UTIL_PATCHED = { patchedDependencies: { "util@1.0.0": "patches/util@1.0.0.patch" } }; + +test.concurrent( + "`bun update ` holds a patched dependency and moves the auto-installed peer named next to it", + async () => { + using server = await serveRegistry(PEER_ONLY); + const dir = String( + tempDir("update-named-peer-patched-", { + "package.json": stringify(pkgJson({ util: "1.0.0", core: "1.0.0" }, UTIL_PATCHED)), + "patches/util@1.0.0.patch": NO_DEPS_PATCH, + }), + ); + await servedBunfig(server, dir); + await install(dir); + const packageJson = pkgJson({ util: "^1.0.0" }, UTIL_PATCHED); + await reinstall(dir, packageJson); + const patched = file(join(dir, "node_modules", "util", "patched.txt")); + expect(await patched.exists()).toBeTrue(); + + const { stdout, exitCode } = await run(dir, "update", "util", "core"); + expectSummary(stdout, CORE_ROW, keptPatched("util", "1.0.0", "1.1.0"), "", installed(1)); + expect(await packageJsonOf(dir)).toStrictEqual(packageJson); + expect(await lockedVersions(dir, "util")).toStrictEqual(["1.0.0"]); + expect(await lockedVersions(dir, "core")).toStrictEqual(["1.1.0"]); + expect(await installedVersion(dir, "core")).toBe("1.1.0"); + expect(await patched.exists()).toBeTrue(); + await frozen(dir); + expect(exitCode).toBe(0); + }, +); + +// `extra` is resolved while the peer's manifest is fetched, and its tarball fails there. +test.concurrent( + "a failed download for an entry that resolves next to a named auto-installed peer fails the update and writes nothing", + async () => { + const knobs: RegistryKnobs = { status: {} }; + using server = await serveRegistry(PEER_ONLY, {}, knobs); + const dir = await setupServed(server, "update-named-peer-failed-", DIRECT.pinned, DIRECT.stale); + await write(join(dir, "package.json"), stringify(pkgJson({ util: "^1.0.0", extra: "^1.0.0" }))); + const before = { packageJson: await packageJsonText(dir), lock: await lockText(dir) }; + + knobs.status!["extra-1.1.0.tgz"] = 404; + const { stderr, exitCode } = await run(dir, "update", "core"); + expect(errorLines(stderr)).toStrictEqual([`error: GET ${server.url.origin}/extra-1.1.0.tgz - 404`]); + expect(stderr).not.toContain("Saved lockfile"); + expect(await packageJsonText(dir)).toBe(before.packageJson); + expect(await lockText(dir)).toBe(before.lock); + expect(exitCode).toBe(1); + }, +); + +// `extra` fails in the wave that resolves parent. The pass that then moves parent's own dependencies prints the log, and the error has to outlive that. +test.concurrent( + "a failed download next to `bun update --latest ` fails the update and writes nothing", + async () => { + const knobs: RegistryKnobs = { status: {} }; + using server = await serveRegistry( + { + parent: { "1.0.0": { dependencies: { leaf: "^1.0.0" } }, "1.1.0": { dependencies: { leaf: "^1.0.0" } } }, + leaf: { "1.0.0": {}, "1.1.0": {} }, + extra: { "1.0.0": {}, "1.1.0": {} }, + }, + {}, + knobs, + ); + const dir = await setupServed( + server, + "update-latest-failed-", + pkgJson({ parent: "1.0.0", leaf: "1.0.0" }), + pkgJson({ parent: "^1.0.0" }), + ); + await write(join(dir, "package.json"), stringify(pkgJson({ parent: "^1.0.0", extra: "^1.0.0" }))); + const before = { packageJson: await packageJsonText(dir), lock: await lockText(dir) }; + + knobs.status!["extra-1.1.0.tgz"] = 404; + const { stderr, exitCode } = await run(dir, "update", "--latest", "parent"); + expect(errorLines(stderr)).toStrictEqual([`error: GET ${server.url.origin}/extra-1.1.0.tgz - 404`]); + expect(stderr).not.toContain("Saved lockfile"); + expect(await packageJsonText(dir)).toBe(before.packageJson); + expect(await lockText(dir)).toBe(before.lock); + expect(exitCode).toBe(1); + }, +); + // parent and other are already at their newest; the leaf under each is parked one release behind by the dropped root pins. const STALE_CHILDREN: Manifests = { parent: { "1.0.0": { dependencies: { leaf: "^1.0.0" } } }, diff --git a/test/cli/install/migration/yarn-lock-migration.test.ts b/test/cli/install/migration/yarn-lock-migration.test.ts index f32863412586..19022c84a3b0 100644 --- a/test/cli/install/migration/yarn-lock-migration.test.ts +++ b/test/cli/install/migration/yarn-lock-migration.test.ts @@ -1716,3 +1716,88 @@ fsevents@^2.3.2: expect(bunLockContent).toContain("@esbuild/darwin-arm64"); }); }); + +// The migration fetches the manifest of every package and stops at the first request it cannot start (the registry of @z is not http), after it started the request for `good`. The install that follows completes that request. +test("bun install completes a manifest request that the yarn.lock migration left running", async () => { + const names = ["good", "@z/bad"]; + const tarballPath = (name: string) => `/${name}/-/${name.split("/").at(-1)}-1.0.0.tgz`; + const tarballs = new Map(); + const sha1: Record = {}; + for (const name of names) { + const archive = new Bun.Archive( + { "package/package.json": JSON.stringify({ name, version: "1.0.0" }) }, + { compress: "gzip" }, + ); + const bytes = await archive.bytes(); + tarballs.set(tarballPath(name), bytes); + sha1[name] = new Bun.CryptoHasher("sha1").update(bytes).digest("hex"); + } + using server = Bun.serve({ + port: 0, + fetch(request) { + const { origin, pathname } = new URL(request.url); + const path = decodeURIComponent(pathname); + const tarball = tarballs.get(path); + if (tarball) return new Response(tarball); + const name = path.slice(1); + if (!names.includes(name)) return new Response("not found", { status: 404 }); + const dist = { tarball: origin + tarballPath(name) }; + return Response.json({ + name, + "dist-tags": { latest: "1.0.0" }, + versions: { "1.0.0": { name, version: "1.0.0", dist } }, + }); + }, + }); + const origin = server.url.origin; + + using dir = tempDir("yarn-migration-unusable-scope-registry", { + "package.json": JSON.stringify({ + name: "foo", + version: "1.0.0", + dependencies: { good: "^1.0.0", "@z/bad": "^1.0.0" }, + }), + "yarn.lock": `# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY. +# yarn lockfile v1 + + +"@z/bad@^1.0.0": + version "1.0.0" + resolved "${origin}${tarballPath("@z/bad")}#${sha1["@z/bad"]}" + +good@^1.0.0: + version "1.0.0" + resolved "${origin}${tarballPath("good")}#${sha1.good}" +`, + "bunfig.toml": `[install] +registry = "${origin}/" +saveTextLockfile = true + +[install.scopes] +"@z" = "ftp://registry.invalid/" +`, + }); + + await using proc = Bun.spawn({ + cmd: [bunExe(), "install"], + cwd: String(dir), + env: { ...bunEnv, BUN_INSTALL_CACHE_DIR: join(String(dir), ".bun-cache") }, + stdout: "pipe", + stderr: "pipe", + stdin: "ignore", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + + expect(stderr).toContain("migrated lockfile from yarn.lock"); + expect(stdout).toContain("2 packages installed"); + const installed = (name: string) => + JSON.parse(fs.readFileSync(join(String(dir), "node_modules", name, "package.json"), "utf8")); + expect([installed("good"), installed("@z/bad")]).toEqual([ + { name: "good", version: "1.0.0" }, + { name: "@z/bad", version: "1.0.0" }, + ]); + const lock = fs.readFileSync(join(String(dir), "bun.lock"), "utf8"); + expect(lock).toContain(`"good": ["good@1.0.0"`); + expect(lock).toContain(`"@z/bad": ["@z/bad@1.0.0"`); + expect(exitCode).toBe(0); +});