diff --git a/docs/pm/cli/install.mdx b/docs/pm/cli/install.mdx index 0b219f2b285..59baa27e635 100644 --- a/docs/pm/cli/install.mdx +++ b/docs/pm/cli/install.mdx @@ -523,6 +523,7 @@ The migration process handles: - Converts `pnpm-lock.yaml` (lockfile versions 7–9, including pnpm 11's multi-document files) to `bun.lock` - Preserves resolved versions and integrity hashes - Preserves peer dependency ranges and `peerDependenciesMeta`, so the next `bun install` leaves the migrated lockfile unchanged +- Keeps `catalog:` and `workspace:` specifiers as written and records each workspace package's version, so `bun install --frozen-lockfile` accepts the migrated lockfile and the versions pnpm locked for each workspace are the ones installed - Migrates git, GitHub, tarball URL, `file:`, and `npm:` alias dependencies, including transitive ones - Resolves pnpm named registries (`name@registry:version`) via `namedRegistries` in `pnpm-workspace.yaml` - Converts injected workspace packages (`dependenciesMeta.*.injected`) to ordinary workspace dependencies diff --git a/src/install/lockfile/bun.lock.rs b/src/install/lockfile/bun.lock.rs index 963a06ebbcb..4bafd9ed6e9 100644 --- a/src/install/lockfile/bun.lock.rs +++ b/src/install/lockfile/bun.lock.rs @@ -3442,22 +3442,30 @@ fn map_dep_to_pkg( resolutions[dep_id as usize] = pkg_id; if text_lockfile_version != Version::V0 { - let res = &pkg_resolutions[pkg_id as usize]; - if res.tag == ResolutionTag::Workspace { - // Whole-struct assign so `DependencyVersion::Drop` frees any prior - // npm chain. SAFETY: `res.tag == Workspace` checked above. - let literal = dep.version.literal; - dep.version = DependencyVersion { - tag: DependencyVersionTag::Workspace, - literal, - value: DependencyVersionValue { - workspace: *res.workspace(), - }, - }; - } + adopt_workspace_resolution(dep, &pkg_resolutions[pkg_id as usize]); } } +/// An edge bound to a workspace member takes the shape `Package::parse_dependency` +/// gives it (`workspace` tag carrying the member's path, literal kept), so the +/// differ compares equal against a fresh package.json parse. Other targets leave +/// the edge as parsed. +pub(crate) fn adopt_workspace_resolution(dep: &mut Dependency, res: &Resolution) { + if res.tag != ResolutionTag::Workspace { + return; + } + // Whole-struct assign so `DependencyVersion::Drop` frees any prior + // npm chain. SAFETY: `res.tag == Workspace` checked above. + let literal = dep.version.literal; + dep.version = DependencyVersion { + tag: DependencyVersionTag::Workspace, + literal, + value: DependencyVersionValue { + workspace: *res.workspace(), + }, + }; +} + fn dependency_resolution_failure( dep: &Dependency, pkg_path: Option<&[u8]>, diff --git a/src/install/pnpm.rs b/src/install/pnpm.rs index abf7916c98f..86f9ad989a1 100644 --- a/src/install/pnpm.rs +++ b/src/install/pnpm.rs @@ -8,6 +8,7 @@ use bun_collections::StringArrayHashMap; use bun_ast::{self, self as js_ast, E, Expr, ExprData, G}; use bun_core::strings; use bun_semver as semver; +use bun_semver::query::token::Wildcard; use bun_semver::{ExternalString, String}; use bun_sys::{self as sys, Fd}; @@ -45,6 +46,21 @@ macro_rules! string_bytes { }; } +// Binds a root or workspace edge the way bun.lock's reader does. `workspace:*` rows and ranges +// pnpm linked to a member arrive here parsed from their literal, while the package.json rows the +// differ compares them against carry the member's path. A macro so callers can keep slices of +// `string_bytes` alive across the call. +macro_rules! bind_importer_dependency { + ($lockfile:expr, $dep_id:expr, $pkg_id:expr) => {{ + let pkg_id: PackageID = $pkg_id; + $lockfile.buffers.resolutions[$dep_id as usize] = pkg_id; + lockfile::bun_lock::adopt_workspace_resolution( + &mut $lockfile.buffers.dependencies[$dep_id as usize], + &$lockfile.packages.items_resolution()[pkg_id as usize], + ); + }}; +} + /// returns (peers_index, patch_hash_index) /// https://github.com/pnpm/pnpm/blob/102d5a01ddabda1184b88119adccfbe956d30579/packages/dependency-path/src/index.ts#L9-L31 fn index_of_dep_path_suffix(path: &[u8]) -> (Option, Option) { @@ -780,20 +796,16 @@ pub(crate) fn migrate_pnpm_lockfile<'a>( let path_str = sbuf!(lockfile).append(importer_path)?; lockfile.workspace_paths.put(name_hash, path_str)?; - if let Some(version_expr) = value.get(b"version") { - let Some(version_raw) = as_string(&version_expr) else { - return Err(invalid_pnpm_lockfile()); - }; + // Same rule as the `workspaces` array parser: a version that does not parse, or has a + // wildcard, leaves the member unversioned. + if let Some((version_raw, _)) = get_string(workspace_root, b"version") { let version_str = sbuf!(lockfile).append(version_raw)?; - let parsed = semver::Version::parse(version_str.sliced(string_bytes!(lockfile))); - if !parsed.valid { - return Err(invalid_pnpm_lockfile()); + if parsed.valid && parsed.wildcard == Wildcard::None { + lockfile + .workspace_versions + .put(name_hash, parsed.version.min())?; } - - lockfile - .workspace_versions - .put(name_hash, parsed.version.min())?; } } @@ -1440,14 +1452,14 @@ pub(crate) fn migrate_pnpm_lockfile<'a>( let mut path_buf = bun_paths::AutoAbsPath::init_top_level_dir(); let _ = path_buf.join(&[workspace_path]); // path-buffer overflow unreachable for bounded inputs if let Some(workspace_pkg_id) = pkg_map.get(path_buf.slice()) { - lockfile.buffers.resolutions[dep_id as usize] = *workspace_pkg_id; + bind_importer_dependency!(lockfile, dep_id, *workspace_pkg_id); continue; } } let dep_name = dep.name.slice(string_buf); if let Some(peer_pkg_id) = resolve_peer_like_bun_lock(lockfile, &dep) { - lockfile.buffers.resolutions[dep_id as usize] = peer_pkg_id; + bind_importer_dependency!(lockfile, dep_id, peer_pkg_id); continue; } let Some(mut version_maybe_alias) = importer_versions.get(dep_name).map(|v| &**v) @@ -1476,7 +1488,7 @@ pub(crate) fn migrate_pnpm_lockfile<'a>( let mut path_buf = bun_paths::AutoAbsPath::init_top_level_dir(); let _ = path_buf.join(&[maybe_symlink_or_folder_or_workspace_path]); // path-buffer overflow unreachable for bounded inputs if let Some(pkg_id) = pkg_map.get(path_buf.slice()) { - lockfile.buffers.resolutions[dep_id as usize] = *pkg_id; + bind_importer_dependency!(lockfile, dep_id, *pkg_id); continue; } } @@ -1492,7 +1504,7 @@ pub(crate) fn migrate_pnpm_lockfile<'a>( )); }; - lockfile.buffers.resolutions[dep_id as usize] = *pkg_id; + bind_importer_dependency!(lockfile, dep_id, *pkg_id); } } @@ -1514,7 +1526,7 @@ pub(crate) fn migrate_pnpm_lockfile<'a>( let string_buf = string_bytes!(lockfile); let dep_name = dep.name.slice(string_buf); if let Some(peer_pkg_id) = resolve_peer_like_bun_lock(lockfile, &dep) { - lockfile.buffers.resolutions[dep_id as usize] = peer_pkg_id; + bind_importer_dependency!(lockfile, dep_id, peer_pkg_id); continue; } let Some(mut version_maybe_alias) = importer_versions.get(dep_name).map(|v| &**v) @@ -1544,7 +1556,7 @@ pub(crate) fn migrate_pnpm_lockfile<'a>( let mut path_buf = bun_paths::AutoAbsPath::init_top_level_dir(); let _ = path_buf.join(&[workspace_path, maybe_symlink_or_folder_or_workspace_path]); // path-buffer overflow unreachable for bounded inputs if let Some(link_pkg_id) = pkg_map.get(path_buf.slice()) { - lockfile.buffers.resolutions[dep_id as usize] = *link_pkg_id; + bind_importer_dependency!(lockfile, dep_id, *link_pkg_id); continue; } } @@ -1560,7 +1572,7 @@ pub(crate) fn migrate_pnpm_lockfile<'a>( )); }; - lockfile.buffers.resolutions[dep_id as usize] = *res_pkg_id; + bind_importer_dependency!(lockfile, dep_id, *res_pkg_id); } } @@ -1983,36 +1995,29 @@ fn append_importer_dependency( specifier_str: &[u8], behavior: dependency::Behavior, ) -> Result<(), ParseAppendDependenciesError> { - if strings::has_prefix(specifier_str, b"catalog:") { - let name_hash = semver::string::Builder::string_hash(name_str); - let name = sbuf!(lockfile).append_external_with_hash(name_str, name_hash)?; - let mut catalog_group_name_str = specifier_str[b"catalog:".len()..].trim_ascii(); - if catalog_group_name_str == b"default" { - catalog_group_name_str = b""; - } - let catalog_group_name = sbuf!(lockfile).append(catalog_group_name_str)?; - // `CatalogMap::get` borrows `&self` and the whole lockfile, so move catalogs out for the call. - let catalogs = core::mem::take(&mut lockfile.catalogs); - let dep_result = catalogs.get(lockfile, catalog_group_name, name.value); - lockfile.catalogs = catalogs; - let Some(mut dep) = dep_result else { - // catalog is missing an entry in the "catalogs" object in the lockfile + // The row keeps its `catalog:` reference, as the package.json parser and the bun.lock reader + // keep theirs; the importer's own `version:` field binds it below. Only the entry's existence + // is checked here. + if let Some(catalog_name) = + strings::without_prefix_if_possible_comptime(specifier_str, b"catalog:") + { + let catalog_name = catalog_name.trim_ascii(); + if lockfile + .catalogs + .find(string_bytes!(lockfile), catalog_name, name_str) + .is_none() + { log.add_error_fmt( None, bun_ast::Loc::EMPTY, format_args!( "pnpm-lock.yaml catalog '{}' missing entry for dependency '{}'", - bstr::BStr::new(specifier_str[b"catalog:".len()..].trim_ascii()), + bstr::BStr::new(catalog_name), bstr::BStr::new(name_str) ), ); return Err(ParseAppendDependenciesError::PnpmLockfileMissingCatalogEntry); - }; - - dep.behavior = behavior; - - lockfile.buffers.dependencies.push(dep); - return Ok(()); + } } append_manifest_dependency(lockfile, log, name_str, specifier_str, behavior) diff --git a/test/cli/install/migration/__snapshots__/pnpm-lock-migration.test.ts.snap b/test/cli/install/migration/__snapshots__/pnpm-lock-migration.test.ts.snap index b57fba25cfe..ebd0f9da8e1 100644 --- a/test/cli/install/migration/__snapshots__/pnpm-lock-migration.test.ts.snap +++ b/test/cli/install/migration/__snapshots__/pnpm-lock-migration.test.ts.snap @@ -32,6 +32,7 @@ exports[`pnpm-lock.yaml migration pnpm workspace lockfile migration: workspace-p }, "apps/web": { "name": "@repo/web", + "version": "1.0.0", "dependencies": { "@repo/ui": "workspace:*", "@repo/utils": "workspace:*", @@ -40,12 +41,14 @@ exports[`pnpm-lock.yaml migration pnpm workspace lockfile migration: workspace-p }, "packages/ui": { "name": "@repo/ui", + "version": "1.0.0", "dependencies": { "react": "^18.2.0", }, }, "packages/utils": { "name": "@repo/utils", + "version": "1.0.0", "dependencies": { "lodash": "^4.17.21", }, diff --git a/test/cli/install/migration/__snapshots__/pnpm-migration-complete.test.ts.snap b/test/cli/install/migration/__snapshots__/pnpm-migration-complete.test.ts.snap index 58de333615b..14ee44574b4 100644 --- a/test/cli/install/migration/__snapshots__/pnpm-migration-complete.test.ts.snap +++ b/test/cli/install/migration/__snapshots__/pnpm-migration-complete.test.ts.snap @@ -248,8 +248,8 @@ exports[`PNPM Migration Complete Test Suite comprehensive PNPM migration with al "": { "name": "catalogs-test", "dependencies": { - "lodash": "4.17.21", - "react": "18.2.0", + "lodash": "catalog:tools", + "react": "catalog:", }, }, }, @@ -363,6 +363,7 @@ exports[`PNPM Migration Complete Test Suite comprehensive PNPM migration with al }, "packages/pkg1": { "name": "@workspace/pkg1", + "version": "1.0.0", "dependencies": { "@workspace/pkg2": "workspace:*", "lodash": "^4.17.21", @@ -370,12 +371,14 @@ exports[`PNPM Migration Complete Test Suite comprehensive PNPM migration with al }, "packages/pkg2": { "name": "@workspace/pkg2", + "version": "1.0.0", "dependencies": { "@workspace/pkg3": "workspace:*", }, }, "packages/pkg3": { "name": "@workspace/pkg3", + "version": "1.0.0", "dependencies": { "@workspace/pkg1": "workspace:*", }, diff --git a/test/cli/install/migration/pnpm-lock-v9.test.ts b/test/cli/install/migration/pnpm-lock-v9.test.ts index 9fe4c0434e2..7480148b05f 100644 --- a/test/cli/install/migration/pnpm-lock-v9.test.ts +++ b/test/cli/install/migration/pnpm-lock-v9.test.ts @@ -2631,6 +2631,294 @@ importers: expect(await bunLockOf(String(formatted))).toBe(await bunLockOf(String(plain))); }); + // The install after a migration diffs each importer's rows against a fresh package.json parse. A row + // in a different shape re-resolves the whole importer, and a range with two versions in the lockfile + // then moves: below, the members lock `no-deps` at 1.0.0 while `one-dep` brings in 1.0.1. + describe("importer rows match a package.json parse", () => { + const catalogFiles = { + "pnpm-workspace.yaml": `packages: + - packages/* + +catalog: + a-dep: ^1.0.1 + +catalogs: + tooling: + one-dep: ^1.0.0 +`, + "package.json": JSON.stringify({ + name: "catalog-rows", + private: true, + devDependencies: { "one-dep": "catalog:tooling" }, + }), + "packages/app/package.json": JSON.stringify({ + name: "app", + version: "1.0.0", + dependencies: { "a-dep": "catalog:", "no-deps": "^1.0.0" }, + }), + "pnpm-lock.yaml": `lockfileVersion: '9.0' + +catalogs: + default: + a-dep: + specifier: ^1.0.1 + version: 1.0.1 + tooling: + one-dep: + specifier: ^1.0.0 + version: 1.0.0 + +importers: + + .: + devDependencies: + one-dep: + specifier: catalog:tooling + version: 1.0.0 + + packages/app: + dependencies: + a-dep: + specifier: 'catalog:' + version: 1.0.1 + no-deps: + specifier: ^1.0.0 + version: 1.0.0 + +packages: + + a-dep@1.0.1: + resolution: {integrity: ${A_DEP_1_0_1_INTEGRITY}} + + no-deps@1.0.0: + resolution: {integrity: ${NO_DEPS_1_0_0_INTEGRITY}} + + no-deps@1.0.1: + resolution: {integrity: ${NO_DEPS_1_0_1_INTEGRITY}} + + one-dep@1.0.0: + resolution: {integrity: ${ONE_DEP_1_0_0_INTEGRITY}} + +snapshots: + + a-dep@1.0.1: {} + + no-deps@1.0.0: {} + + no-deps@1.0.1: {} + + one-dep@1.0.0: + dependencies: + no-deps: 1.0.1 +`, + }; + + const workspaceRowFiles = { + "pnpm-workspace.yaml": `packages: + - packages/* +`, + "package.json": JSON.stringify({ + name: "workspace-rows", + private: true, + devDependencies: { "one-dep": "^1.0.0" }, + }), + "packages/lib/package.json": JSON.stringify({ name: "lib", version: "1.2.3" }), + "packages/app/package.json": JSON.stringify({ + name: "app", + version: "1.0.0", + dependencies: { lib: "workspace:*", "no-deps": "^1.0.0" }, + }), + // link-workspace-packages shape: a plain range pnpm linked to the member; the version is not semver + "packages/cli/package.json": JSON.stringify({ + name: "cli", + version: "unversioned", + dependencies: { lib: "^1.0.0", "no-deps": "^1.0.0" }, + }), + "pnpm-lock.yaml": `lockfileVersion: '9.0' + +importers: + + .: + devDependencies: + one-dep: + specifier: ^1.0.0 + version: 1.0.0 + + packages/app: + dependencies: + lib: + specifier: workspace:* + version: link:../lib + no-deps: + specifier: ^1.0.0 + version: 1.0.0 + + packages/cli: + dependencies: + lib: + specifier: ^1.0.0 + version: link:../lib + no-deps: + specifier: ^1.0.0 + version: 1.0.0 + + packages/lib: {} + +packages: + + no-deps@1.0.0: + resolution: {integrity: ${NO_DEPS_1_0_0_INTEGRITY}} + + no-deps@1.0.1: + resolution: {integrity: ${NO_DEPS_1_0_1_INTEGRITY}} + + one-dep@1.0.0: + resolution: {integrity: ${ONE_DEP_1_0_0_INTEGRITY}} + +snapshots: + + no-deps@1.0.0: {} + + no-deps@1.0.1: {} + + one-dep@1.0.0: + dependencies: + no-deps: 1.0.1 +`, + }; + + const noDeps100 = { name: "no-deps", version: "1.0.0" }; + + // `bun install` on a pnpm checkout migrates in-process and installs from the migrated rows directly. + async function installStraightFromPnpmLock(files: Record) { + const { packageDir: viaMigrate } = await verdaccio.createTestDir({ bunfigOpts: { linker: "hoisted" }, files }); + const { packageDir: viaInstall } = await verdaccio.createTestDir({ bunfigOpts: { linker: "hoisted" }, files }); + + const [migrated, install] = await Promise.all([migrate(viaMigrate), run(viaInstall, "install")]); + + expect(migrated.stderr).toContain("migrated lockfile from pnpm-lock.yaml"); + expect(migrated.exitCode).toBe(0); + expect(install.stderr).toContain("migrated lockfile from pnpm-lock.yaml"); + expect(install.stderr).not.toContain("error:"); + expect(install.exitCode).toBe(0); + expect(await bunLockOf(viaInstall)).toBe(await bunLockOf(viaMigrate)); + + return viaInstall; + } + + test("catalog: rows keep the reference and a frozen install accepts bun pm migrate's lockfile", async () => { + const { packageDir } = await verdaccio.createTestDir({ bunfigOpts: { linker: "hoisted" }, files: catalogFiles }); + + const { stderr, exitCode } = await migrate(packageDir); + + expect(stderr).toContain("migrated lockfile from pnpm-lock.yaml"); + expect(exitCode).toBe(0); + + const migrated = await bunLockOf(packageDir); + expect(workspacesSection(migrated)).toMatchInlineSnapshot(` + " "workspaces": { + "": { + "name": "catalog-rows", + "devDependencies": { + "one-dep": "catalog:tooling", + }, + }, + "packages/app": { + "name": "app", + "version": "1.0.0", + "dependencies": { + "a-dep": "catalog:", + "no-deps": "^1.0.0", + }, + }, + }, + "catalog": { + "a-dep": "^1.0.1", + }, + "catalogs": { + "tooling": { + "one-dep": "^1.0.0", + }, + }, + " + `); + + const install = await run(packageDir, "install", "--frozen-lockfile"); + + expect(install.stderr).not.toContain("error:"); + expect(install.exitCode).toBe(0); + expect(await bunLockOf(packageDir)).toBe(migrated); + expect(await installedPackageJson(packageDir, "packages/app", "no-deps")).toStrictEqual(noDeps100); + }); + + test("bun install straight from pnpm-lock.yaml keeps the versions locked for a catalog: user", async () => { + const dir = await installStraightFromPnpmLock(catalogFiles); + + expect(await installedPackageJson(dir, "packages/app", "no-deps")).toStrictEqual(noDeps100); + expect(await installedPackageJson(dir, "packages/app", "a-dep")).toStrictEqual({ + name: "a-dep", + version: "1.0.1", + }); + }); + + test("workspace: and linked rows keep their specifiers; members get their package.json version", async () => { + const { packageDir } = await verdaccio.createTestDir({ + bunfigOpts: { linker: "hoisted" }, + files: workspaceRowFiles, + }); + + const { stderr, exitCode } = await migrate(packageDir); + + expect(stderr).toContain("migrated lockfile from pnpm-lock.yaml"); + expect(exitCode).toBe(0); + + const bunLock = await bunLockOf(packageDir); + expect(workspacesSection(bunLock)).toMatchInlineSnapshot(` + " "workspaces": { + "": { + "name": "workspace-rows", + "devDependencies": { + "one-dep": "^1.0.0", + }, + }, + "packages/app": { + "name": "app", + "version": "1.0.0", + "dependencies": { + "lib": "workspace:*", + "no-deps": "^1.0.0", + }, + }, + "packages/cli": { + "name": "cli", + "dependencies": { + "lib": "^1.0.0", + "no-deps": "^1.0.0", + }, + }, + "packages/lib": { + "name": "lib", + "version": "1.2.3", + }, + }, + " + `); + expect(bunLock).toContain(`"lib": ["lib@workspace:packages/lib"]`); + expect(bunLock).toContain(`"no-deps@1.0.0"`); + expect(bunLock).toContain(`"no-deps@1.0.1"`); + expect(bunLock).not.toContain("link:"); + }); + + test("bun install straight from pnpm-lock.yaml keeps the versions locked for workspace: and linked users", async () => { + const dir = await installStraightFromPnpmLock(workspaceRowFiles); + + for (const member of ["packages/app", "packages/cli"]) { + expect(await installedPackageJson(dir, member, "no-deps")).toStrictEqual(noDeps100); + expect(await installedPackageJson(dir, member, "lib")).toStrictEqual({ name: "lib", version: "1.2.3" }); + } + }); + }); + describe("catalogs", () => { // pnpm/pnpm#10551: pruned Docker contexts ship the lockfile without pnpm-workspace.yaml test("lockfile catalogs: section is enough without pnpm-workspace.yaml", async () => { diff --git a/test/cli/install/migration/pnpm-migration-complete.test.ts b/test/cli/install/migration/pnpm-migration-complete.test.ts index 117b7465751..33a056385e9 100644 --- a/test/cli/install/migration/pnpm-migration-complete.test.ts +++ b/test/cli/install/migration/pnpm-migration-complete.test.ts @@ -714,9 +714,9 @@ snapshots: expect(catalogsExitCode).toBe(0); const catalogsLockfile = fs.readFileSync(join(catalogsTest, "bun.lock"), "utf8"); - // Catalogs are resolved to actual versions during migration - expect(catalogsLockfile).toContain('"react": "18.2.0"'); - expect(catalogsLockfile).toContain('"lodash": "4.17.21"'); + // Importer rows keep their catalog references, as package.json spells them + expect(catalogsLockfile).toContain('"react": "catalog:"'); + expect(catalogsLockfile).toContain('"lodash": "catalog:tools"'); // The actual packages should be in the lockfile expect(catalogsLockfile).toContain('"react@18.2.0"'); expect(catalogsLockfile).toContain('"lodash@4.17.21"');