From 96ef649f5100b82d89f4c4f3a7106a134423f396 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 15 Aug 2026 00:09:20 +0000 Subject: [PATCH 1/4] install: import pnpm-workspace.yaml without a migratable pnpm-lock.yaml pnpm-workspace.yaml was only read at the end of a successful pnpm-lock.yaml migration. With no lockfile (or one older than lockfileVersion 7) bun resolved the root package.json as-is and a pnpm monorepo installed as a single package with no warning. When no lockfile was loaded and the root package.json has no `workspaces` field, run the same package.json update before the root is parsed, so the workspace globs, catalogs, overrides and patchedDependencies from pnpm-workspace.yaml (and package.json's `pnpm` field) are picked up. While here, in update_package_json_after_migration: - parse the yaml into the function-scoped arena. The quoted and block scalars it holds were printed after the block-local arena that owned them had been destroyed. - re-parse the printed package.json into the cache entry instead of leaving it pointing at store-allocated nodes. bun remove / bun unlink now write back the cache entry rather than the text printed before the install, so the imported fields survive them. --- docs/pm/cli/install.mdx | 4 +- docs/pm/isolated-installs.mdx | 6 +- .../PackageManager/install_with_manager.rs | 7 + .../updatePackageJSONAndInstall.rs | 76 +++--- src/install/pnpm.rs | 171 ++++++------- .../migration/pnpm-lock-migration.test.ts | 235 ++++++++++++++++++ 6 files changed, 364 insertions(+), 135 deletions(-) diff --git a/docs/pm/cli/install.mdx b/docs/pm/cli/install.mdx index 0b219f2b285e..7cdb81abba2c 100644 --- a/docs/pm/cli/install.mdx +++ b/docs/pm/cli/install.mdx @@ -531,7 +531,7 @@ The migration process handles: ### Workspace Configuration -When a `pnpm-workspace.yaml` file exists, Bun migrates workspace settings to your root `package.json`: +When a `pnpm-workspace.yaml` file exists, Bun migrates workspace settings to your root `package.json`. This happens together with the lockfile migration, and also when there is no `pnpm-lock.yaml` to migrate (it was never committed, or it is too old to convert), as long as `bun.lock` does not exist yet and the root `package.json` has no `workspaces` field of its own: ```yaml pnpm-workspace.yaml icon="file-code" packages: @@ -596,7 +596,7 @@ Bun migrates the following pnpm configuration from both `pnpm-lock.yaml` and `pn - All catalog entries referenced by dependencies must exist in the catalogs definition - Every workspace in `pnpm-lock.yaml` must have its `package.json` on disk (in Docker, copy them in before `bun install`) - Relative `link:` dependencies and git dependencies with a sub-directory (`resolution.path`) are not supported -- If migration fails for any of these reasons, Bun prints why and resolves from scratch instead +- If migration fails for any of these reasons, Bun prints why and resolves from scratch instead. The `pnpm-workspace.yaml` settings above are still migrated, so workspaces, catalogs and overrides are not lost After migration, you can safely remove `pnpm-lock.yaml` and `pnpm-workspace.yaml` files. diff --git a/docs/pm/isolated-installs.mdx b/docs/pm/isolated-installs.mdx index 0a148a16fb52..9c0d7f4dc040 100644 --- a/docs/pm/isolated-installs.mdx +++ b/docs/pm/isolated-installs.mdx @@ -207,13 +207,15 @@ bun install --linker isolated Isolated installs are conceptually similar to pnpm, so migration is direct: ```bash terminal icon="terminal" -# Remove pnpm files -rm -rf node_modules pnpm-lock.yaml +# Remove pnpm's node_modules +rm -rf node_modules # Install with Bun's isolated linker bun install --linker isolated ``` +Keep `pnpm-lock.yaml` around for this first install: Bun converts it to `bun.lock`, so you keep the versions pnpm had resolved. The workspace list, catalogs and overrides in `pnpm-workspace.yaml` are moved into the root `package.json` whether or not a `pnpm-lock.yaml` is present. See [pnpm migration](/pm/cli/install#pnpm-migration) for what is converted. + The main difference is that Bun uses symlinks in `node_modules` while pnpm uses a global store with symlinks. ## When to use isolated installs diff --git a/src/install/PackageManager/install_with_manager.rs b/src/install/PackageManager/install_with_manager.rs index 3c134c0cc864..03aa01877530 100644 --- a/src/install/PackageManager/install_with_manager.rs +++ b/src/install/PackageManager/install_with_manager.rs @@ -1937,6 +1937,13 @@ fn create_new_lockfile_and_enqueue( Global::crash(); } + // A loaded lockfile already describes the project (a migrated + // pnpm-lock.yaml imports pnpm-workspace.yaml itself); without one, the + // package.json read below is all the install has to go on. + if !matches!(load_result, lockfile::LoadResult::Ok { .. }) { + crate::pnpm::migrate_pnpm_workspace_config(manager)?; + } + // SAFETY: `manager.log` is a non-null backref to the CLI log set at init(). let root_package_json_entry = match manager.workspace_package_json_cache.get_with_path( manager.log_mut(), diff --git a/src/install/PackageManager/updatePackageJSONAndInstall.rs b/src/install/PackageManager/updatePackageJSONAndInstall.rs index 65ac909a28f0..72e532f142b4 100644 --- a/src/install/PackageManager/updatePackageJSONAndInstall.rs +++ b/src/install/PackageManager/updatePackageJSONAndInstall.rs @@ -486,12 +486,12 @@ fn update_package_json_and_install_with_manager_with_updates( // The Smarter™ approach is you resolve ahead of time and write to disk once! // But, turns out that's slower in any case where more than one package has to be resolved (most of the time!) // Concurrent network requests are faster than doing one and then waiting until the next batch - let new_package_json_source: Vec = package_json_writer - .ctx - .written_without_trailing_zero() - .to_vec(); - // The cache entry (`Cow<'static, [u8]>`) outlives this stack frame, so it needs its own copy. - current_package_json.source.contents = Cow::Owned(new_package_json_source.clone()); + current_package_json.source.contents = Cow::Owned( + package_json_writer + .ctx + .written_without_trailing_zero() + .to_vec(), + ); // The edits above went into a promoted copy // (`current_package_json_root`), so re-parse the // printed source so the cached AST (consumed by `FolderResolver` for workspace @@ -664,40 +664,36 @@ fn update_package_json_and_install_with_manager_with_updates( } if manager.options.do_.contains(Do::WRITE_PACKAGE_JSON) { - let (source, path): (&[u8], &ZStr) = - if matches!(manager.options.patch_features, PatchFeatures::Commit { .. }) { - 'source_and_path: { - let root_package_json_entry = match manager - .workspace_package_json_cache - .get_with_path( - manager.log_mut(), - root_package_json_path.as_bytes(), - GetJSONOptions::default(), - ) - .unwrap() - { - Ok(e) => e, - Err(err) => { - Output::err( - err, - "failed to read/parse package.json at '{s}'", - (BStr::new(root_package_json_path.as_bytes()),), - ); - Global::exit(1); - } - }; - - break 'source_and_path ( - &root_package_json_entry.source.contents, - root_package_json_path, - ); - } - } else { - ( - &new_package_json_source, - manager.original_package_json_path.as_zstr(), - ) - }; + // `bun patch --commit` records the patch in the root package.json even + // when run from a workspace member; everything else edited the cwd's. + let path: &ZStr = if matches!(manager.options.patch_features, PatchFeatures::Commit { .. }) + { + root_package_json_path + } else { + manager.original_package_json_path.as_zstr() + }; + // The cache entry, not the text printed before the install: a pnpm + // migration during `install_with_manager` edits the root entry too. + let entry = match manager + .workspace_package_json_cache + .get_with_path( + manager.log_mut(), + path.as_bytes(), + GetJSONOptions::default(), + ) + .unwrap() + { + Ok(entry) => entry, + Err(err) => { + Output::err( + err, + "failed to read/parse package.json at '{s}'", + (BStr::new(path.as_bytes()),), + ); + Global::exit(1); + } + }; + let source: &[u8] = &entry.source.contents; // Now that we've run the install step // We can save our in-memory package.json to disk diff --git a/src/install/pnpm.rs b/src/install/pnpm.rs index abf7916c98f8..e9b01561d5d0 100644 --- a/src/install/pnpm.rs +++ b/src/install/pnpm.rs @@ -6,7 +6,7 @@ use bun_alloc::AllocError; use bun_collections::StringArrayHashMap; use bun_ast::{self, self as js_ast, E, Expr, ExprData, G}; -use bun_core::strings; +use bun_core::{Global, strings}; use bun_semver as semver; use bun_semver::{ExternalString, String}; use bun_sys::{self as sys, Fd}; @@ -17,6 +17,7 @@ use crate::external_slice::ExternalSlice; use crate::integrity::Integrity; use crate::lockfile::{self, LoadResult, LoadResultOk, Lockfile}; use crate::npm::{self}; +use crate::package_manager_real::update_package_json_and_install::print_package_json_into_cache_entry; use crate::repository::Repository; use crate::resolution::{self, Resolution, TaggedValue}; use crate::{DependencyID, INVALID_PACKAGE_ID, PackageID, PackageManager}; @@ -175,6 +176,25 @@ fn collect_patch_paths( Ok(()) } +fn root_package_json<'a>( + manager: &'a mut PackageManager, + log: &mut bun_ast::Log, +) -> Option<&'a mut crate::WorkspacePackageJsonCacheEntry> { + let mut pkg_json_path = bun_paths::AutoAbsPath::init_top_level_dir(); + let _ = pkg_json_path.append(b"package.json"); // OOM/capacity error is non-actionable here + match manager.workspace_package_json_cache.get_with_path( + log, + pkg_json_path.slice(), + crate::GetJsonOptions { + guess_indentation: true, + ..Default::default() + }, + ) { + crate::GetJsonResult::Entry(entry) => Some(entry), + crate::GetJsonResult::ReadErr(_) | crate::GetJsonResult::ParseErr(_) => None, + } +} + /// Current pnpm records only the patch hash in the lockfile; the patch file path lives in the config. fn read_config_patch_paths( manager: &mut PackageManager, @@ -182,12 +202,7 @@ fn read_config_patch_paths( ) -> Result>, AllocError> { let mut paths: StringArrayHashMap> = StringArrayHashMap::new(); - let mut pkg_json_path = bun_paths::AutoAbsPath::init_top_level_dir(); - let _ = pkg_json_path.append(b"package.json"); - if let crate::GetJsonResult::Entry(pkg_json) = manager - .workspace_package_json_cache - .get_with_path(log, pkg_json_path.slice(), Default::default()) - { + if let Some(pkg_json) = root_package_json(manager, log) { if let Some(patched) = pkg_json .root .get(b"pnpm") @@ -2327,40 +2342,49 @@ fn rewrite_bare_patch_keys( bstr::BStr::new(&**res_str) ) .map_err(|_| AllocError)?; - // Interned into the DATA_STORE backing the cached package.json Expr tree, which outlives this fn. + // Lives in the AST store like the key node below; nothing resets it before the caller prints the tree. let interned: &[u8] = js_ast::data_store_dupe_str(join_buf.as_slice()); prop.key = Some(Expr::init(E::EString::init(interned), bun_ast::Loc::EMPTY)); } Ok(()) } -/// Updates package.json with workspace and catalog information after migration +/// Without a pnpm-lock.yaml to migrate (never committed, or older than +/// `migrate_pnpm_lockfile` accepts) the install resolves from package.json +/// alone, which turns a pnpm monorepo into a single package. A root +/// package.json that already declares `workspaces` is treated as migrated. +pub(crate) fn migrate_pnpm_workspace_config( + manager: &mut PackageManager, +) -> Result<(), AllocError> { + if !sys::exists_z(bun_core::zstr!("pnpm-workspace.yaml")) { + return Ok(()); + } + let log = manager.log_mut(); + let Some(root_pkg_json) = root_package_json(manager, log) else { + return Ok(()); + }; + if root_pkg_json.root.get(b"workspaces").is_some() { + return Ok(()); + } + update_package_json_after_migration(manager, log, Fd::cwd(), &StringArrayHashMap::new()) +} + +/// Moves the configuration pnpm reads from package.json's `pnpm` field and +/// from pnpm-workspace.yaml (workspace globs, catalogs, overrides, patched +/// dependencies) into the package.json fields `bun install` reads. +/// `patches` maps the bare `name` patch keys the lockfile resolved to their +/// versions; it is empty when there is no lockfile. fn update_package_json_after_migration( manager: &mut PackageManager, log: &mut bun_ast::Log, dir: Fd, patches: &StringArrayHashMap>, ) -> Result<(), AllocError> { - let mut pkg_json_path = bun_paths::AutoAbsPath::init_top_level_dir(); - let _ = pkg_json_path.append(b"package.json"); // OOM/capacity error is non-actionable here - let bump = bun_alloc::Arena::new(); let silent = manager.options.log_level.is_silent(); - let root_pkg_json = match manager - .workspace_package_json_cache - .get_with_path( - log, - pkg_json_path.slice(), - crate::GetJsonOptions { - guess_indentation: true, - ..Default::default() - }, - ) - .unwrap() - { - Ok(j) => j, - Err(_) => return Ok(()), + let Some(root_pkg_json) = root_package_json(manager, log) else { + return Ok(()); }; let mut json = root_pkg_json.root; @@ -2506,10 +2530,7 @@ fn update_package_json_after_migration( } } - // Each `&'static [u8]` here is interned into the thread-local `DATA_STORE` - // (see `data_store_dupe_str` below) so it shares the lifetime of the - // `Expr` nodes it ends up backing inside the cached `root_pkg_json.root`. - let mut workspace_paths: Option> = None; + let mut workspace_paths: Option> = None; let mut catalog_obj: Option = None; let mut catalogs_obj: Option = None; let mut workspace_overrides_obj: Option = None; @@ -2517,20 +2538,15 @@ fn update_package_json_after_migration( match sys::File::read_from(Fd::cwd(), b"pnpm-workspace.yaml") { Ok(contents) => 'read_pnpm_workspace_yaml: { - // The `Vec` would drop at the end of this arm while the - // `Expr`s it backs (catalog/catalogs/overrides/patchedDependencies - // below) escape into `json` and the - // `workspace_package_json_cache`. Intern the bytes into the same - // thread-local `DATA_STORE` that owns the surrounding `Expr` - // nodes — arena ownership, not a leak (bulk-freed on - // `Expr::data_store_reset`). - let contents: &'static [u8] = js_ast::data_store_dupe_str(&contents); + // The yaml `Expr`s spliced into `json` below borrow the source text + // (plain scalars) and the parse arena (quoted and block scalars), so + // both live in `bump` until `json` has been printed. + let contents: &[u8] = bump.alloc_slice_copy(&contents); let yaml_source = bun_ast::Source::init_path_string(b"pnpm-workspace.yaml", contents); - let arena = bun_alloc::Arena::new(); let Ok(ws_root) = bun_parsers::yaml::YAML::parse( &yaml_source, log, - &arena, + &bump, bun_parsers::yaml::CyclicAliases::Reject, ) else { break 'read_pnpm_workspace_yaml; @@ -2538,16 +2554,10 @@ fn update_package_json_after_migration( if let Some(packages_expr) = ws_root.get(b"packages") { if let Some(mut packages) = packages_expr.as_array() { - let mut paths: Vec<&'static [u8]> = Vec::new(); + let mut paths: Vec<&[u8]> = Vec::new(); while let Some(package_path) = packages.next() { if let Some(package_path_str) = as_string(&package_path) { - // Intern (vs. the prior `Box<[u8]>`) so the - // `EString` nodes built from these paths below do - // not dangle once this function returns and the - // boxes drop — they are stored into - // `root_pkg_json.root` which is cached in - // `manager.workspace_package_json_cache`. - paths.push(js_ast::data_store_dupe_str(package_path_str)); + paths.push(package_path_str); } } workspace_paths = Some(paths); @@ -2718,50 +2728,29 @@ fn update_package_json_after_migration( } } - if needs_update { - let mut buffer_writer = bun_js_printer::BufferWriter::init(); - buffer_writer.append_newline = !root_pkg_json.source.contents().is_empty() - && root_pkg_json.source.contents()[root_pkg_json.source.contents().len() - 1] == b'\n'; - let mut package_json_writer = bun_js_printer::BufferPrinter::init(buffer_writer); - - if bun_js_printer::print_json( - &mut package_json_writer, - json, - &root_pkg_json.source, - bun_js_printer::PrintJsonOptions { - indent: root_pkg_json.indentation, - mangled_props: None, - ..Default::default() - }, - ) - .is_err() - { - return Ok(()); - } - - if package_json_writer.flush().is_err() { - return Err(AllocError); - } + if !needs_update { + return Ok(()); + } - root_pkg_json.source.contents = std::borrow::Cow::Owned( - package_json_writer - .ctx - .written_without_trailing_zero() - .to_vec(), - ); + // The nodes spliced into `json` above live in `bump` and the thread-local + // AST store; re-parsing the printed source leaves the cache entry owning + // everything it points at, as the rest of the install expects. + print_package_json_into_cache_entry(root_pkg_json, json); + if let Err(err) = root_pkg_json.reparse_root(log) { + bun_core::pretty_errorln!("package.json failed to parse due to error {}", err.name()); + Global::crash(); + } - // Write the updated package.json - if sys::File::write_file( - dir, - bun_core::zstr!("package.json"), - root_pkg_json.source.contents(), - ) - .is_ok() - && !moved.is_empty() - && !silent - { - bun_core::pretty_errorln!("moved {} in package.json", moved.join(", ")); - } + if sys::File::write_file( + dir, + bun_core::zstr!("package.json"), + root_pkg_json.source.contents(), + ) + .is_ok() + && !moved.is_empty() + && !silent + { + bun_core::pretty_errorln!("moved {} in package.json", moved.join(", ")); } Ok(()) @@ -2771,7 +2760,7 @@ fn is_non_empty_object(expr: &Expr) -> bool { matches!(&expr.data, ExprData::EObject(o) if !o.properties.is_empty()) } -fn paths_array(paths: &[&'static [u8]]) -> Expr { +fn paths_array(paths: &[&[u8]]) -> Expr { let mut items = js_ast::ExprNodeList::init_capacity(paths.len()); for path in paths { VecExt::append( diff --git a/test/cli/install/migration/pnpm-lock-migration.test.ts b/test/cli/install/migration/pnpm-lock-migration.test.ts index 039e8727b2fc..c0cd813efefe 100644 --- a/test/cli/install/migration/pnpm-lock-migration.test.ts +++ b/test/cli/install/migration/pnpm-lock-migration.test.ts @@ -402,3 +402,238 @@ snapshots: expect(stderr).not.toContain("migrated lockfile from pnpm-lock.yaml"); }); }); + +// Everything below resolves only workspace packages, so it never touches a registry. +describe.concurrent("pnpm-workspace.yaml is imported into package.json", () => { + const rootPackageJson = { name: "root", private: true }; + const workspaceYaml = `packages:\n - "packages/*"\n`; + const workspacePackages = { + "packages/a/package.json": JSON.stringify({ name: "@w/a", version: "1.2.3" }), + "packages/b/package.json": JSON.stringify({ + name: "@w/b", + version: "0.0.1", + dependencies: { "@w/a": "workspace:*" }, + }), + }; + const movedWorkspaces = "moved pnpm-workspace.yaml to workspaces in package.json"; + + async function runBun(cwd: string, ...args: string[]) { + await using proc = Bun.spawn({ + cmd: [bunExe(), ...args], + cwd, + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + return { stdout, stderr, exitCode }; + } + + function readPackageJson(dir: string) { + return JSON.parse(fs.readFileSync(join(dir, "package.json"), "utf8")); + } + + // `@w/b` can only see `@w/a` if the install knew `packages/*` were workspaces. + async function versionOfAResolvedFromB(dir: string) { + const { stdout, stderr, exitCode } = await runBun( + join(dir, "packages/b"), + "-e", + `console.log(require("@w/a/package.json").version)`, + ); + expect({ stdout, stderr, exitCode }).toEqual({ stdout: "1.2.3\n", stderr: "", exitCode: 0 }); + } + + // Quoted yaml scalars are stored differently from plain ones by the parser; cover both. + const catalogFixture = { + "package.json": JSON.stringify({ ...rootPackageJson, pnpm: { overrides: { "from-package-json": "1.0.0" } } }), + "pnpm-workspace.yaml": `packages: + - 'packages/*' +catalog: + "@w/a": "workspace:*" + left-pad: ^1.3.0 +catalogs: + build: + '@scope/quoted': '~2.0.0' +overrides: + "@scope/quoted": "2.0.1" + plain: 3.0.0 +`, + ...workspacePackages, + "packages/b/package.json": JSON.stringify({ + name: "@w/b", + version: "0.0.1", + dependencies: { "@w/a": "catalog:" }, + }), + }; + const catalogFixtureMoved = + "moved pnpm.overrides to overrides, pnpm-workspace.yaml to workspaces, pnpm-workspace.yaml overrides to overrides in package.json"; + const catalogFixtureImported = { + ...rootPackageJson, + workspaces: { + packages: ["packages/*"], + catalog: { "@w/a": "workspace:*", "left-pad": "^1.3.0" }, + catalogs: { build: { "@scope/quoted": "~2.0.0" } }, + }, + overrides: { "from-package-json": "1.0.0", "@scope/quoted": "2.0.1", plain: "3.0.0" }, + }; + + test("by bun install when there is no lockfile at all", async () => { + await using dir = tempDir("pnpm-workspace-yaml-no-lockfile", { + "package.json": JSON.stringify(rootPackageJson, null, 2) + "\n", + "pnpm-workspace.yaml": workspaceYaml, + ...workspacePackages, + }); + + const first = await runBun(dir, "install"); + expect(first.stderr).toContain(movedWorkspaces); + expect(first.exitCode).toBe(0); + + expect(readPackageJson(dir)).toEqual({ ...rootPackageJson, workspaces: ["packages/*"] }); + const lockfile = fs.readFileSync(join(dir, "bun.lock"), "utf8"); + expect(lockfile).toContain(`"@w/a": ["@w/a@workspace:packages/a"]`); + expect(lockfile).toContain(`"@w/b": ["@w/b@workspace:packages/b"]`); + await versionOfAResolvedFromB(dir); + + // package.json now declares the workspaces and bun.lock exists: nothing left to import. + const second = await runBun(dir, "install"); + expect(second.stderr).not.toContain("moved pnpm"); + expect(second.exitCode).toBe(0); + expect(readPackageJson(dir)).toEqual({ ...rootPackageJson, workspaces: ["packages/*"] }); + }); + + test("with its catalogs and overrides when there is no lockfile", async () => { + await using dir = tempDir("pnpm-workspace-yaml-catalog", catalogFixture); + + const { stderr, exitCode } = await runBun(dir, "install"); + expect(stderr).toContain(catalogFixtureMoved); + expect(exitCode).toBe(0); + + expect(readPackageJson(dir)).toEqual(catalogFixtureImported); + expect(fs.readFileSync(join(dir, "bun.lock"), "utf8")).toContain(`"@w/b": ["@w/b@workspace:packages/b"]`); + await versionOfAResolvedFromB(dir); + }); + + test("with its catalogs and overrides while migrating pnpm-lock.yaml", async () => { + await using dir = tempDir("pnpm-workspace-yaml-catalog-lockfile", { + ...catalogFixture, + "pnpm-lock.yaml": `lockfileVersion: '9.0' + +catalogs: + default: + '@w/a': + specifier: workspace:* + version: link:packages/a + +importers: + + .: {} + + packages/a: {} + + packages/b: + dependencies: + '@w/a': + specifier: 'catalog:' + version: link:../a +`, + }); + + const { stderr, exitCode } = await runBun(dir, "install"); + expect(stderr).toContain(catalogFixtureMoved); + expect(stderr).toContain("migrated lockfile from pnpm-lock.yaml"); + expect(exitCode).toBe(0); + + expect(readPackageJson(dir)).toEqual(catalogFixtureImported); + await versionOfAResolvedFromB(dir); + }); + + test("when pnpm-lock.yaml is too old to migrate", async () => { + await using dir = tempDir("pnpm-workspace-yaml-old-lockfile", { + "package.json": JSON.stringify(rootPackageJson), + "pnpm-workspace.yaml": workspaceYaml, + "pnpm-lock.yaml": `lockfileVersion: '6.0'\n\nimporters:\n\n .: {}\n`, + ...workspacePackages, + }); + + const { stderr, exitCode } = await runBun(dir, "install"); + expect(stderr).toContain("pnpm-lock.yaml is lockfileVersion 6.0, which bun cannot migrate"); + expect(stderr).toContain(movedWorkspaces); + expect(exitCode).toBe(0); + + expect(readPackageJson(dir)).toEqual({ ...rootPackageJson, workspaces: ["packages/*"] }); + await versionOfAResolvedFromB(dir); + }); + + test("by bun add, alongside the added dependency", async () => { + await using dir = tempDir("pnpm-workspace-yaml-add", { + "package.json": JSON.stringify(rootPackageJson), + "pnpm-workspace.yaml": workspaceYaml, + "lib/c/package.json": JSON.stringify({ name: "c", version: "0.0.1" }), + ...workspacePackages, + }); + + const { stderr, exitCode } = await runBun(dir, "add", "file:lib/c"); + expect(stderr).toContain(movedWorkspaces); + expect(exitCode).toBe(0); + + expect(readPackageJson(dir)).toEqual({ + ...rootPackageJson, + dependencies: { c: "file:lib/c" }, + workspaces: ["packages/*"], + }); + await versionOfAResolvedFromB(dir); + }); + + test("by bun remove, surviving its package.json write-back", async () => { + await using dir = tempDir("pnpm-workspace-yaml-remove", { + "package.json": JSON.stringify({ ...rootPackageJson, dependencies: { c: "file:lib/c" } }), + "pnpm-workspace.yaml": workspaceYaml, + "lib/c/package.json": JSON.stringify({ name: "c", version: "0.0.1" }), + ...workspacePackages, + }); + + const { stderr, exitCode } = await runBun(dir, "remove", "c"); + expect(stderr).toContain(movedWorkspaces); + expect(exitCode).toBe(0); + + expect(readPackageJson(dir)).toEqual({ ...rootPackageJson, workspaces: ["packages/*"] }); + await versionOfAResolvedFromB(dir); + }); + + test("not when package.json already declares workspaces", async () => { + const declared = { ...rootPackageJson, workspaces: ["packages/a"] }; + await using dir = tempDir("pnpm-workspace-yaml-declared", { + "package.json": JSON.stringify(declared), + "pnpm-workspace.yaml": workspaceYaml, + ...workspacePackages, + }); + + const { stderr, exitCode } = await runBun(dir, "install"); + expect(stderr).not.toContain("moved pnpm"); + expect(exitCode).toBe(0); + + expect(readPackageJson(dir)).toEqual(declared); + const lockfile = fs.readFileSync(join(dir, "bun.lock"), "utf8"); + expect(lockfile).toContain(`"@w/a": ["@w/a@workspace:packages/a"]`); + expect(lockfile).not.toContain("@w/b"); + }); + + test("not when a bun.lock exists", async () => { + const single = { ...rootPackageJson, dependencies: { c: "file:lib/c" } }; + await using dir = tempDir("pnpm-workspace-yaml-has-bun-lock", { + "package.json": JSON.stringify(single), + "lib/c/package.json": JSON.stringify({ name: "c", version: "0.0.1" }), + ...workspacePackages, + }); + expect((await runBun(dir, "install")).exitCode).toBe(0); + expect(fs.existsSync(join(dir, "bun.lock"))).toBe(true); + + fs.writeFileSync(join(dir, "pnpm-workspace.yaml"), workspaceYaml); + const { stderr, exitCode } = await runBun(dir, "install"); + expect(stderr).not.toContain("moved pnpm"); + expect(exitCode).toBe(0); + + expect(readPackageJson(dir)).toEqual(single); + expect(fs.readFileSync(join(dir, "bun.lock"), "utf8")).not.toContain("@w/a"); + }); +}); From e4baa599859c7d446de772661d1534a32804c9e6 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 15 Aug 2026 00:32:09 +0000 Subject: [PATCH 2/4] install: fail when the migrated package.json cannot be written A failed write of the package.json the pnpm migration produced was ignored: the install went on resolving from the in-memory copy and saved a bun.lock that did not match the package.json on disk. Report the error and exit instead, the same way a failed bun.lock save is handled. --- docs/pm/isolated-installs.mdx | 2 +- src/install/pnpm.rs | 19 ++++---- .../migration/pnpm-lock-migration.test.ts | 43 ++++++++++++++++++- 3 files changed, 54 insertions(+), 10 deletions(-) diff --git a/docs/pm/isolated-installs.mdx b/docs/pm/isolated-installs.mdx index 9c0d7f4dc040..49380c66ad32 100644 --- a/docs/pm/isolated-installs.mdx +++ b/docs/pm/isolated-installs.mdx @@ -214,7 +214,7 @@ rm -rf node_modules bun install --linker isolated ``` -Keep `pnpm-lock.yaml` around for this first install: Bun converts it to `bun.lock`, so you keep the versions pnpm had resolved. The workspace list, catalogs and overrides in `pnpm-workspace.yaml` are moved into the root `package.json` whether or not a `pnpm-lock.yaml` is present. See [pnpm migration](/pm/cli/install#pnpm-migration) for what is converted. +Keep `pnpm-lock.yaml` around for this first install: Bun converts it to `bun.lock`, so you keep the versions pnpm had resolved. The same first install (no `bun.lock` yet, no `workspaces` field in the root `package.json`) also moves the workspace list, catalogs and overrides from `pnpm-workspace.yaml` into the root `package.json`, with or without a `pnpm-lock.yaml`. See [pnpm migration](/pm/cli/install#pnpm-migration) for what is converted. The main difference is that Bun uses symlinks in `node_modules` while pnpm uses a global store with symlinks. diff --git a/src/install/pnpm.rs b/src/install/pnpm.rs index e9b01561d5d0..61a577917902 100644 --- a/src/install/pnpm.rs +++ b/src/install/pnpm.rs @@ -6,7 +6,7 @@ use bun_alloc::AllocError; use bun_collections::StringArrayHashMap; use bun_ast::{self, self as js_ast, E, Expr, ExprData, G}; -use bun_core::{Global, strings}; +use bun_core::{Global, Output, strings}; use bun_semver as semver; use bun_semver::{ExternalString, String}; use bun_sys::{self as sys, Fd}; @@ -2741,16 +2741,19 @@ fn update_package_json_after_migration( Global::crash(); } - if sys::File::write_file( + let moved = moved.join(", "); + // The install that follows resolves from the updated entry, so a bun.lock + // saved after a failed write here would not match the package.json on disk. + if let Err(err) = sys::File::write_file( dir, bun_core::zstr!("package.json"), root_pkg_json.source.contents(), - ) - .is_ok() - && !moved.is_empty() - && !silent - { - bun_core::pretty_errorln!("moved {} in package.json", moved.join(", ")); + ) { + Output::err(err, "failed to move {} in package.json", (&moved,)); + Global::crash(); + } + if !silent { + bun_core::pretty_errorln!("moved {} in package.json", moved); } Ok(()) diff --git a/test/cli/install/migration/pnpm-lock-migration.test.ts b/test/cli/install/migration/pnpm-lock-migration.test.ts index c0cd813efefe..e8c85b56aeb4 100644 --- a/test/cli/install/migration/pnpm-lock-migration.test.ts +++ b/test/cli/install/migration/pnpm-lock-migration.test.ts @@ -1,6 +1,6 @@ import { describe, expect, test } from "bun:test"; import fs from "fs"; -import { bunEnv, bunExe, tempDir } from "harness"; +import { bunEnv, bunExe, isWindows, tempDir } from "harness"; import { join } from "path"; describe("pnpm-lock.yaml migration", () => { @@ -636,4 +636,45 @@ importers: expect(readPackageJson(dir)).toEqual(single); expect(fs.readFileSync(join(dir, "bun.lock"), "utf8")).not.toContain("@w/a"); }); + + // A 0444 package.json does not stop root from writing it, and the mode bits mean something else on Windows. + test.skipIf(isWindows || process.getuid?.() === 0).each([ + ["without a lockfile", {}], + [ + "while migrating pnpm-lock.yaml", + { + "pnpm-lock.yaml": `lockfileVersion: '9.0' + +importers: + + .: {} + + packages/a: {} + + packages/b: + dependencies: + '@w/a': + specifier: workspace:* + version: link:../a +`, + }, + ], + ])("and the install fails when package.json cannot be written back, %s", async (_, lockfile) => { + const original = JSON.stringify(rootPackageJson); + await using dir = tempDir("pnpm-workspace-yaml-readonly", { + "package.json": original, + "pnpm-workspace.yaml": workspaceYaml, + ...workspacePackages, + ...lockfile, + }); + fs.chmodSync(join(dir, "package.json"), 0o444); + + const { stderr, exitCode } = await runBun(dir, "install"); + expect(stderr).toContain("failed to move pnpm-workspace.yaml to workspaces in package.json"); + expect(stderr).not.toContain("moved pnpm"); + expect(exitCode).toBe(1); + + expect(fs.readFileSync(join(dir, "package.json"), "utf8")).toBe(original); + expect(fs.existsSync(join(dir, "bun.lock"))).toBe(false); + }); }); From 03f8b0829e18ce19e780668a7ab5542445642a6e Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 15 Aug 2026 00:41:41 +0000 Subject: [PATCH 3/4] install: shorten the comments added by the pnpm-workspace.yaml import --- .../PackageManager/install_with_manager.rs | 4 +--- .../updatePackageJSONAndInstall.rs | 6 ++--- src/install/pnpm.rs | 22 +++++-------------- 3 files changed, 8 insertions(+), 24 deletions(-) diff --git a/src/install/PackageManager/install_with_manager.rs b/src/install/PackageManager/install_with_manager.rs index 03aa01877530..c534b7ef1040 100644 --- a/src/install/PackageManager/install_with_manager.rs +++ b/src/install/PackageManager/install_with_manager.rs @@ -1937,9 +1937,7 @@ fn create_new_lockfile_and_enqueue( Global::crash(); } - // A loaded lockfile already describes the project (a migrated - // pnpm-lock.yaml imports pnpm-workspace.yaml itself); without one, the - // package.json read below is all the install has to go on. + // A loaded lockfile already describes the project; a migrated pnpm-lock.yaml imports the yaml itself. if !matches!(load_result, lockfile::LoadResult::Ok { .. }) { crate::pnpm::migrate_pnpm_workspace_config(manager)?; } diff --git a/src/install/PackageManager/updatePackageJSONAndInstall.rs b/src/install/PackageManager/updatePackageJSONAndInstall.rs index 72e532f142b4..68939ccefa3b 100644 --- a/src/install/PackageManager/updatePackageJSONAndInstall.rs +++ b/src/install/PackageManager/updatePackageJSONAndInstall.rs @@ -664,16 +664,14 @@ fn update_package_json_and_install_with_manager_with_updates( } if manager.options.do_.contains(Do::WRITE_PACKAGE_JSON) { - // `bun patch --commit` records the patch in the root package.json even - // when run from a workspace member; everything else edited the cwd's. + // `bun patch --commit` records the patch in the root package.json, even when run from a workspace. let path: &ZStr = if matches!(manager.options.patch_features, PatchFeatures::Commit { .. }) { root_package_json_path } else { manager.original_package_json_path.as_zstr() }; - // The cache entry, not the text printed before the install: a pnpm - // migration during `install_with_manager` edits the root entry too. + // Written from the cache entry: a pnpm migration inside `install_with_manager` may have edited it since. let entry = match manager .workspace_package_json_cache .get_with_path( diff --git a/src/install/pnpm.rs b/src/install/pnpm.rs index 61a577917902..376ae501a116 100644 --- a/src/install/pnpm.rs +++ b/src/install/pnpm.rs @@ -2349,10 +2349,7 @@ fn rewrite_bare_patch_keys( Ok(()) } -/// Without a pnpm-lock.yaml to migrate (never committed, or older than -/// `migrate_pnpm_lockfile` accepts) the install resolves from package.json -/// alone, which turns a pnpm monorepo into a single package. A root -/// package.json that already declares `workspaces` is treated as migrated. +/// Imports pnpm-workspace.yaml when no pnpm-lock.yaml was migrated; a root `workspaces` field means it already was. pub(crate) fn migrate_pnpm_workspace_config( manager: &mut PackageManager, ) -> Result<(), AllocError> { @@ -2369,11 +2366,7 @@ pub(crate) fn migrate_pnpm_workspace_config( update_package_json_after_migration(manager, log, Fd::cwd(), &StringArrayHashMap::new()) } -/// Moves the configuration pnpm reads from package.json's `pnpm` field and -/// from pnpm-workspace.yaml (workspace globs, catalogs, overrides, patched -/// dependencies) into the package.json fields `bun install` reads. -/// `patches` maps the bare `name` patch keys the lockfile resolved to their -/// versions; it is empty when there is no lockfile. +/// Moves the settings pnpm reads from package.json `pnpm.*` and pnpm-workspace.yaml into the fields bun reads. fn update_package_json_after_migration( manager: &mut PackageManager, log: &mut bun_ast::Log, @@ -2538,9 +2531,7 @@ fn update_package_json_after_migration( match sys::File::read_from(Fd::cwd(), b"pnpm-workspace.yaml") { Ok(contents) => 'read_pnpm_workspace_yaml: { - // The yaml `Expr`s spliced into `json` below borrow the source text - // (plain scalars) and the parse arena (quoted and block scalars), so - // both live in `bump` until `json` has been printed. + // Quoted and block scalars are copied into the parse arena, so it has to outlive the print below. let contents: &[u8] = bump.alloc_slice_copy(&contents); let yaml_source = bun_ast::Source::init_path_string(b"pnpm-workspace.yaml", contents); let Ok(ws_root) = bun_parsers::yaml::YAML::parse( @@ -2732,9 +2723,7 @@ fn update_package_json_after_migration( return Ok(()); } - // The nodes spliced into `json` above live in `bump` and the thread-local - // AST store; re-parsing the printed source leaves the cache entry owning - // everything it points at, as the rest of the install expects. + // The spliced-in nodes live in `bump` and the AST store; re-parse so the cache entry owns its tree. print_package_json_into_cache_entry(root_pkg_json, json); if let Err(err) = root_pkg_json.reparse_root(log) { bun_core::pretty_errorln!("package.json failed to parse due to error {}", err.name()); @@ -2742,8 +2731,7 @@ fn update_package_json_after_migration( } let moved = moved.join(", "); - // The install that follows resolves from the updated entry, so a bun.lock - // saved after a failed write here would not match the package.json on disk. + // Continuing would save a bun.lock that does not match the package.json left on disk. if let Err(err) = sys::File::write_file( dir, bun_core::zstr!("package.json"), From c8f7a0cd91ce4da7a6b33bc6a573505e8ccae942 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 15 Aug 2026 01:11:55 +0000 Subject: [PATCH 4/4] install: run patch --commit before reading the package.json it edits do_patch_commit loads the lockfile, and migrating a pnpm-lock.yaml there rewrites the cached root package.json and re-parses it, which frees the tree the caller had already taken its Expr from (and the migration can grow the cache map under the entry pointer too). Load first, then read the entry. --- .../updatePackageJSONAndInstall.rs | 42 ++++++------- .../install/migration/pnpm-lock-v9.test.ts | 59 ++++++++++++++++++- 2 files changed, 80 insertions(+), 21 deletions(-) diff --git a/src/install/PackageManager/updatePackageJSONAndInstall.rs b/src/install/PackageManager/updatePackageJSONAndInstall.rs index 68939ccefa3b..94a1a813b237 100644 --- a/src/install/PackageManager/updatePackageJSONAndInstall.rs +++ b/src/install/PackageManager/updatePackageJSONAndInstall.rs @@ -287,9 +287,18 @@ fn update_package_json_and_install_with_manager_with_updates( add_catalog::prepare(manager, &updates); + // Loads the lockfile, and migrating one rewrites the cached root package.json: do it before reading the entry. + let patch_commit: Option = + if matches!(manager.options.patch_features, PatchFeatures::Commit { .. }) { + let mut pathbuf = PathBuffer::uninit(); + patch_package::do_patch_commit(manager, &mut pathbuf, log_level)? + } else { + None + }; + // reshaped for borrowck — `get_with_path` returns `&mut MapEntry` // borrowed from `manager.workspace_package_json_cache`, but we then need - // `&mut *manager` for `PackageJSONEditor::edit` / `do_patch_commit` while still + // `&mut *manager` for `PackageJSONEditor::edit` while still // holding the entry. Demote to `*mut MapEntry` and re- // borrow at point of use. The cache map is not mutated again until the // next `get_with_path` call below, so the pointer remains valid. @@ -329,8 +338,7 @@ fn update_package_json_and_install_with_manager_with_updates( }; // SAFETY: see note above — pointer into `manager.workspace_package_json_cache`, // valid until the next `get_with_path`. No `&mut manager.workspace_package_json_cache` - // is taken across this borrow; `PackageJSONEditor` and `do_patch_commit` touch only - // disjoint manager fields. + // is taken across this borrow; `PackageJSONEditor` touches only disjoint manager fields. let current_package_json: &mut MapEntry = unsafe { &mut *current_package_json_ptr }; let mut current_package_json_root: bun_ast::Expr = current_package_json.root; let current_package_json_indent = current_package_json.indentation; @@ -428,23 +436,17 @@ fn update_package_json_and_install_with_manager_with_updates( } } _ => { - if matches!(manager.options.patch_features, PatchFeatures::Commit { .. }) { - let mut pathbuf = PathBuffer::uninit(); - if let Some(stuff) = - patch_package::do_patch_commit(manager, &mut pathbuf, log_level)? - { - // we're inside a workspace package, we need to edit the - // root json, not the `current_package_json` - if stuff.not_in_workspace_root { - not_in_workspace_root = Some(stuff); - } else { - PackageJSONEditor::edit_patched_dependencies( - manager, - &mut current_package_json_root, - &stuff.patch_key, - &stuff.patchfile_path, - )?; - } + if let Some(stuff) = patch_commit { + // Inside a workspace package the root package.json is edited below, not `current_package_json`. + if stuff.not_in_workspace_root { + not_in_workspace_root = Some(stuff); + } else { + PackageJSONEditor::edit_patched_dependencies( + manager, + &mut current_package_json_root, + &stuff.patch_key, + &stuff.patchfile_path, + )?; } } } diff --git a/test/cli/install/migration/pnpm-lock-v9.test.ts b/test/cli/install/migration/pnpm-lock-v9.test.ts index 9fe4c0434e27..3a865c4643af 100644 --- a/test/cli/install/migration/pnpm-lock-v9.test.ts +++ b/test/cli/install/migration/pnpm-lock-v9.test.ts @@ -1,5 +1,5 @@ import { afterAll, beforeAll, describe, expect, test } from "bun:test"; -import { existsSync, readdirSync, realpathSync, rmSync } from "fs"; +import { appendFileSync, existsSync, readdirSync, realpathSync, rmSync, writeFileSync } from "fs"; import { bunEnv, bunExe, nodeModulesPackages, tempDir, VerdaccioRegistry } from "harness"; import { dirname, join } from "path"; @@ -948,6 +948,63 @@ snapshots: ); }, ); + + // `bun patch --commit` loads the lockfile before it edits package.json, so the migration's rewrite of + // package.json (and of its cache entry) happens underneath the command. + test("bun patch --commit that has to migrate the lockfile keeps the migration's package.json edits", async () => { + const packageJson = { name: "patch-commit-migrates", dependencies: { "no-deps": "^1.0.0" } }; + const { packageDir } = await verdaccio.createTestDir({ + bunfigOpts: { linker: "hoisted" }, + files: { + "package.json": JSON.stringify(packageJson), + "pnpm-workspace.yaml": "packages:\n - 'packages/*'\n", + "packages/a/package.json": JSON.stringify({ name: "a", version: "1.0.0" }), + "pnpm-lock.yaml": `lockfileVersion: '9.0' + +importers: + + .: + dependencies: + no-deps: + specifier: ^1.0.0 + version: 1.0.1 + + packages/a: {} + +packages: + + no-deps@1.0.1: + resolution: {integrity: ${NO_DEPS_1_0_1_INTEGRITY}} + +snapshots: + + no-deps@1.0.1: {} +`, + }, + }); + + // `bun patch` installs and unlinks node_modules/no-deps from the cache copy that `--commit` diffs against; + // the repo is then put back to how it was checked out, so `--commit` is the command that migrates. + expect((await run(packageDir, "patch", "no-deps")).exitCode).toBe(0); + rmSync(join(packageDir, "bun.lock")); + writeFileSync(join(packageDir, "package.json"), JSON.stringify(packageJson)); + appendFileSync(join(packageDir, "node_modules/no-deps/index.js"), "globalThis.patchedAtCommit = true;\n"); + + const { stderr, exitCode } = await run(packageDir, "patch", "--commit", "node_modules/no-deps"); + expect(stderr).toContain("moved pnpm-workspace.yaml to workspaces in package.json"); + expect(stderr).toContain("migrated lockfile from pnpm-lock.yaml"); + expect(exitCode).toBe(0); + + expect(await Bun.file(join(packageDir, "package.json")).json()).toEqual({ + ...packageJson, + workspaces: ["packages/*"], + patchedDependencies: { "no-deps@1.0.1": "patches/no-deps@1.0.1.patch" }, + }); + expect(await Bun.file(join(packageDir, "patches/no-deps@1.0.1.patch")).text()).toContain( + "+globalThis.patchedAtCommit = true;", + ); + expect(await bunLockOf(packageDir)).toContain(`"no-deps@1.0.1": "patches/no-deps@1.0.1.patch"`); + }); }); test("catalog:default is the default catalog", async () => {