diff --git a/src/install/PackageManager.rs b/src/install/PackageManager.rs index e1524675ffb9..9594ec991990 100644 --- a/src/install/PackageManager.rs +++ b/src/install/PackageManager.rs @@ -584,10 +584,23 @@ pub struct PackageUpdateInfo { pub(crate) original_version_literal: Box<[u8]>, // set by the post-install write-back; the install summary still needs the entry pub(crate) written_back: bool, + /// Registered by `package_json_editor::record_catalog_originals`: the name's `catalog:` rows carry the move, so the install summary reports it through them. + pub(crate) catalog_entry: bool, pub(crate) original_version_string_buf: Box<[u8]>, pub(crate) original_version: Option, } +impl PackageUpdateInfo { + /// `version`'s tag strings live in `buf` (a lockfile string buffer that cleaning rebuilds), so the original keeps its own copy of them. + pub(crate) fn set_original_version(&mut self, version: Semver::Version, buf: &[u8]) { + let mut tag_buf = + vec![0u8; version.tag.pre.len() + version.tag.build.len()].into_boxed_slice(); + let mut cursor: &mut [u8] = &mut tag_buf; + self.original_version = Some(version.clone_into(buf, &mut cursor)); + self.original_version_string_buf = tag_buf; + } +} + pub struct CatalogUpdateInfo { /// Catalog group name; empty for the default catalog. pub catalog_name: Box<[u8]>, diff --git a/src/install/PackageManager/PackageJSONEditor.rs b/src/install/PackageManager/PackageJSONEditor.rs index 88773115f6a9..ded777e0f465 100644 --- a/src/install/PackageManager/PackageJSONEditor.rs +++ b/src/install/PackageManager/PackageJSONEditor.rs @@ -511,9 +511,7 @@ fn edit_update_entries( *entry.value_ptr = PackageUpdateInfo { original_version_literal: version_literal_owned, - written_back: false, - original_version_string_buf: Box::default(), - original_version: None, + ..Default::default() }; if update_to_latest { @@ -755,6 +753,62 @@ pub(crate) fn edit_catalogs_before_update( Ok(!manager.updating_catalogs.is_empty()) } +/// Runs on the loaded lockfile, before the differ: every `catalog:` row of an entry recorded by `edit_catalogs_before_update` registers its name in `updating_packages` with the row's locked version as the original, the way the cwd's own dependency lists register theirs, so the install summary prints the entry's move as an update row; a name those lists already registered keeps their original. +pub(crate) fn record_catalog_originals( + manager: &mut PackageManager, +) -> Result<(), bun_alloc::AllocError> { + let infos: &[CatalogUpdateInfo] = &manager.updating_catalogs; + if infos.is_empty() { + return Ok(()); + } + let by_name = CatalogInfoIndex::init(infos)?; + let lockfile: &Lockfile = &manager.lockfile; + let updating_packages = &mut manager.updating_packages; + let string_buf = lockfile.buffers.string_bytes.as_slice(); + let package_resolutions = lockfile.packages.items_resolution(); + + for (dep, &package_id) in lockfile + .buffers + .dependencies + .iter() + .zip(lockfile.buffers.resolutions.iter()) + { + if dep.version.tag != dependency::Tag::Catalog + || (package_id as usize) >= package_resolutions.len() + { + continue; + } + let resolution = &package_resolutions[package_id as usize]; + if resolution.tag != resolution::Tag::Npm { + continue; + } + let dep_name = dep.name.slice(string_buf); + let catalog_name = dep.version.catalog().slice(string_buf); + let Some(info) = by_name + .candidates(dep_name) + .and_then(|candidates| CatalogInfoIndex::pick(candidates, infos, catalog_name)) + .map(|i| &infos[i]) + else { + continue; + }; + let entry = updating_packages.get_or_put(dep_name)?; + if entry.found_existing { + continue; + } + *entry.value_ptr = PackageUpdateInfo { + original_version_literal: info.original_version_literal.clone(), + // The entry is written by `edit_catalogs_after_update`; `edit_update_entries` has nothing of it to write into the cwd's dependency lists. + written_back: true, + catalog_entry: true, + ..Default::default() + }; + entry + .value_ptr + .set_original_version(resolution.npm().version, string_buf); + } + Ok(()) +} + /// Writes each recorded catalog entry's resolved literal (unresolved ones are restored) into the root AST; returns `changed`. pub(crate) fn edit_catalogs_after_update( manager: &mut PackageManager, @@ -1045,9 +1099,7 @@ pub(crate) fn edit( *entry.value_ptr = PackageUpdateInfo { original_version_literal: version_literal_owned, - written_back: false, - original_version_string_buf: Box::default(), - original_version: None, + ..Default::default() }; } } diff --git a/src/install/PackageManager/install_with_manager.rs b/src/install/PackageManager/install_with_manager.rs index 3c134c0cc864..0a26d837c18f 100644 --- a/src/install/PackageManager/install_with_manager.rs +++ b/src/install/PackageManager/install_with_manager.rs @@ -23,7 +23,7 @@ use crate::{ }; // Bring the typed `items_()` column accessors into scope for // `MultiArrayList` / `Slice`. -use super::Command; +use super::{Command, PackageJSONEditor}; use crate::PackageManager; use crate::config_version::ConfigVersion; use crate::hoisted_install::install_hoisted_packages; @@ -1880,24 +1880,13 @@ fn record_updating_package_versions(manager: &mut PackageManager) { if original_resolution.tag != ResolutionTag::Npm { continue; } - - let mut original = original_resolution.npm().version; - let tag_total = original.tag.pre.len() + original.tag.build.len(); - if tag_total > 0 { - let mut tag_buf = vec![0u8; tag_total].into_boxed_slice(); - let mut ptr = &mut tag_buf[..]; - original.tag = original_resolution - .npm() - .version - .tag - .clone_into(&lockfile.buffers.string_bytes, &mut ptr); - - entry_ptr.original_version_string_buf = tag_buf; - } - - entry_ptr.original_version = Some(original); + entry_ptr.set_original_version( + original_resolution.npm().version, + &lockfile.buffers.string_bytes, + ); } } + PackageJSONEditor::record_catalog_originals(manager).unwrap_or_oom(); } #[cold] diff --git a/src/install/lockfile/printer/tree_printer.rs b/src/install/lockfile/printer/tree_printer.rs index 4c7385b7748c..3bf736768cb6 100644 --- a/src/install/lockfile/printer/tree_printer.rs +++ b/src/install/lockfile/printer/tree_printer.rs @@ -7,8 +7,8 @@ use crate::package_manager_real::TrackInstalledBin; use bun_core::fmt::PathSep; use bun_install::lockfile::{Printer, package::Meta as PackageMeta}; use bun_install::{ - self as install, Bin, Dependency, DependencyID, INVALID_PACKAGE_ID, PackageID, PackageManager, - PackageNameHash, Resolution, Subcommand, bin, resolution, + self as install, Bin, Dependency, DependencyID, DependencyVersionTag, INVALID_PACKAGE_ID, + PackageID, PackageManager, PackageNameHash, Resolution, Subcommand, bin, resolution, }; use bun_sys::Fd; @@ -27,6 +27,8 @@ fn print_installed_workspace_section< printed_new_install: &mut bool, id_map: Option<&mut [DependencyID]>, update_owners: &[PackageID], + // The summary has no other sections, so `print_catalog_entry_updates` runs here. + sole_section: bool, ) -> Result<(), crate::Error> where W: Write, @@ -114,6 +116,19 @@ where } if !PRINT_SECTION_HEADER { + if sole_section + && print_catalog_entry_updates::( + this, + manager, + installed, + pkg_metas, + &mut update_dedupe, + writer, + )? + { + *printed_new_install = true; + printed_update = true; + } if print_transitive_updates::( this, manager, @@ -375,6 +390,53 @@ where Ok(()) } +/// A bare `bun update` moves the root's catalog entries for every importer at once, but the summary's one section only walks the rows of `update_owners` (above, hence the shared `update_dedupe`), so the entries consumed elsewhere are reported through whichever `catalog:` row names them. The verbose summary prints a section per importer, each reporting its own `catalog:` rows, and skips this. Like a direct dependency's row, a row prints whether or not its new version had to be installed. +fn print_catalog_entry_updates( + this: &Printer, + manager: &mut PackageManager, + installed: &Bitset, + pkg_metas: &[PackageMeta], + update_dedupe: &mut HashMap, + writer: &mut W, +) -> Result +where + W: Write, +{ + if !manager + .updating_packages + .values() + .iter() + .any(|info| info.catalog_entry) + { + return Ok(false); + } + let string_buf = this.lockfile.buffers.string_bytes.as_slice(); + let dependencies = this.lockfile.buffers.dependencies.as_slice(); + let mut printed = false; + for (dep_id, dep) in dependencies.iter().enumerate() { + if dep.version.tag != DependencyVersionTag::Catalog + || !manager + .updating_packages + .get(dep.name.slice(string_buf)) + .is_some_and(|info| info.catalog_entry) + { + continue; + } + let dep_id = DependencyID::try_from(dep_id).expect("int cast"); + let ShouldPrintPackageInstallResult::Update(update_info) = + should_print_package_install(this, manager, dep_id, installed, None, pkg_metas) + else { + continue; + }; + if update_dedupe.get_or_put(dep.name_hash)?.found_existing { + continue; + } + print_updated_package::(this, manager, &update_info, writer)?; + printed = true; + } + Ok(printed) +} + /// Packages registered by the transitive half of `bun update` are not rows of the walked workspaces, so the walk above never reaches them; the walked workspaces' own targets stay with them. fn print_transitive_updates( this: &Printer, @@ -629,6 +691,7 @@ where &mut had_printed_new_install, None, &[0], + false, )?; for &workspace_dep_id in &workspaces_to_print { @@ -642,6 +705,7 @@ where &mut had_printed_new_install, None, &[workspace_package_id], + false, )?; } } else { @@ -693,6 +757,7 @@ where &mut had_printed_new_install, Some(&mut id_map), &update_owners, + true, )?; } } else { diff --git a/src/install/update_transitive.rs b/src/install/update_transitive.rs index 5f77a9b96e02..aea5cb8eb668 100644 --- a/src/install/update_transitive.rs +++ b/src/install/update_transitive.rs @@ -577,16 +577,8 @@ pub(crate) fn register_moved( continue; } } - let mut tag_buf = - vec![0u8; current.tag.pre.len() + current.tag.build.len()].into_boxed_slice(); - let mut cursor: &mut [u8] = &mut tag_buf; - let original = current.clone_into(buf, &mut cursor); - *entry.value_ptr = PackageUpdateInfo { - original_version_literal: Box::default(), - written_back: false, - original_version_string_buf: tag_buf, - original_version: Some(original), - }; + *entry.value_ptr = PackageUpdateInfo::default(); + entry.value_ptr.set_original_version(current, buf); } Ok(()) } diff --git a/test/cli/install/catalogs.test.ts b/test/cli/install/catalogs.test.ts index 159c6aabe4aa..45cbc5772fe1 100644 --- a/test/cli/install/catalogs.test.ts +++ b/test/cli/install/catalogs.test.ts @@ -352,9 +352,14 @@ describe("update", () => { await createUpdateMonorepo(packageDir, `catalog-update-latest-${label.replace(/\W+/g, "-")}`, isTopLevel); await runBunInstall(bunEnv, packageDir); - const { err, exitCode } = await runUpdate(packageDir, ...flags); + const { out, err, exitCode } = await runUpdate(packageDir, ...flags); expect(err).not.toContain("error:"); + // The moved entry is reported like a direct dependency of the root, even though only pkg1 depends on it. + // a-dep still resolves to 1.0.10 (only its literal changes), so it gets no row. + expect(out.match(/^.*no-deps.*$/gm)).toStrictEqual(["^ no-deps 1.1.0 -> 2.0.0"]); + expect(out.match(/^.*a-dep.*$/gm)).toBeNull(); + // catalog entries are updated, preserving the pinning style const root = await file(join(packageDir, "package.json")).json(); const { catalog, catalogs } = isTopLevel ? root : root.workspaces; @@ -405,9 +410,12 @@ describe("update", () => { await createUpdateMonorepo(packageDir, "catalog-update-in-workspace"); await runBunInstall(bunEnv, packageDir); - const { err, exitCode } = await runUpdate(join(packageDir, "packages", "pkg1"), "--latest"); + const { out, err, exitCode } = await runUpdate(join(packageDir, "packages", "pkg1"), "--latest"); expect(err).not.toContain("error:"); + // pkg1's own `catalog:` row is an update row, not a `+ no-deps@2.0.0` install row. + expect(out.match(/^.*no-deps.*$/gm)).toStrictEqual(["^ no-deps 1.1.0 -> 2.0.0"]); + const root = await file(join(packageDir, "package.json")).json(); expect(root.workspaces.catalog).toEqual({ "no-deps": "^2.0.0" }); expect(root.workspaces.catalogs).toEqual({ a: { "a-dep": "~1.0.10" } }); @@ -419,6 +427,73 @@ describe("update", () => { expect(exitCode).toBe(0); }); + test("--latest reports a catalog entry the root itself depends on once", async () => { + const { packageDir } = await registry.createTestDir(); + await Promise.all([ + write( + join(packageDir, "package.json"), + JSON.stringify({ + name: "catalog-update-root-consumer", + workspaces: { packages: ["packages/*"], catalog: { "no-deps": "^1.0.0" } }, + dependencies: { "no-deps": "catalog:" }, + }), + ), + write( + join(packageDir, "packages", "pkg1", "package.json"), + JSON.stringify({ name: "pkg1", dependencies: { "no-deps": "catalog:" } }), + ), + ]); + await runBunInstall(bunEnv, packageDir); + + const { out, err, exitCode } = await runUpdate(packageDir, "--latest"); + expect(err).not.toContain("error:"); + expect(out.match(/^.*no-deps.*$/gm)).toStrictEqual(["^ no-deps 1.1.0 -> 2.0.0"]); + + const root = await file(join(packageDir, "package.json")).json(); + expect(root.workspaces.catalog).toEqual({ "no-deps": "^2.0.0" }); + expect(root.dependencies).toEqual({ "no-deps": "catalog:" }); + expect(exitCode).toBe(0); + }); + + test("--latest --verbose reports a catalog entry once, under the workspace that depends on it", async () => { + const { packageDir } = await registry.createTestDir(); + await createUpdateMonorepo(packageDir, "catalog-update-verbose"); + await runBunInstall(bunEnv, packageDir); + + const { out, err, exitCode } = await runUpdate(packageDir, "--latest", "--verbose"); + expect(err).not.toContain("error:"); + const lines = out.split(/\r?\n/); + expect(lines.filter(line => line.includes("no-deps"))).toStrictEqual(["^ no-deps 1.1.0 -> 2.0.0"]); + expect(lines[lines.indexOf("pkg1:") + 1]).toBe("^ no-deps 1.1.0 -> 2.0.0"); + expect(exitCode).toBe(0); + }); + + test("--latest reports a catalog entry whose new version needs no install (isolated store already has it)", async () => { + const { packageDir } = await registry.createTestDir({ bunfigOpts: { linker: "isolated" } }); + await createUpdateMonorepo(packageDir, "catalog-update-isolated-rerun"); + await runBunInstall(bunEnv, packageDir); + const rootBefore = await file(join(packageDir, "package.json")).text(); + const lockBefore = await file(join(packageDir, "bun.lock")).text(); + + const first = await runUpdate(packageDir, "--latest"); + expect(first.err).not.toContain("error:"); + expect(first.out.match(/^.*no-deps.*$/gm)).toStrictEqual(["^ no-deps 1.1.0 -> 2.0.0"]); + expect(first.exitCode).toBe(0); + + // Like `git checkout .` followed by `bun install`: the project is back on 1.1.0 while node_modules/.bun keeps no-deps@2.0.0. + await Promise.all([ + write(join(packageDir, "package.json"), rootBefore), + write(join(packageDir, "bun.lock"), lockBefore), + ]); + await runBunInstall(bunEnv, packageDir, { savesLockfile: false }); + + const second = await runUpdate(packageDir, "--latest"); + expect(second.err).not.toContain("error:"); + expect(second.out.match(/^.*no-deps.*$/gm)).toStrictEqual(["^ no-deps 1.1.0 -> 2.0.0"]); + expect((await file(join(packageDir, "package.json")).json()).workspaces.catalog).toEqual({ "no-deps": "^2.0.0" }); + expect(second.exitCode).toBe(0); + }); + test("--latest updates the same package independently per catalog", async () => { const { packageDir } = await registry.createTestDir(); await Promise.all([ @@ -542,8 +617,9 @@ describe("update", () => { ), ]); - const { err, exitCode } = await runUpdate(packageDir, ...args); + const { out, err, exitCode } = await runUpdate(packageDir, ...args); expect(err).not.toContain("error:"); + expect(out.match(/^.*no-deps.*$/gm)).toStrictEqual(["^ no-deps 1.0.0 -> 1.1.0 (v2.0.0 available)"]); const root = await file(join(packageDir, "package.json")).json(); expect(root.workspaces.catalog).toEqual({ "no-deps": "^1.1.0" });