diff --git a/docs/pm/cli/install.mdx b/docs/pm/cli/install.mdx index ebb8bafd96bf..c01f35940c51 100644 --- a/docs/pm/cli/install.mdx +++ b/docs/pm/cli/install.mdx @@ -174,6 +174,8 @@ bun install --frozen-lockfile Bun does not enable `--frozen-lockfile` automatically in CI; pass the flag or use `bun ci`. If there is no lockfile at all, `--frozen-lockfile` installs from `package.json` without writing one. +If the project has a `package-lock.json`, `yarn.lock`, or `pnpm-lock.yaml` but no `bun.lock`, `--frozen-lockfile` migrates that lockfile in memory and installs from it. The install fails if the migrated lockfile does not match `package.json`. Bun writes nothing in this case: no `bun.lock`, and none of the `package.json` edits a [pnpm migration](#pnpm-migration) makes. It prints a `note:` instead. To finish the migration, run `bun install` without the flag and commit the files it writes. The `bun.lockb` to `bun.lock` conversion described in [lockfile](/pm/lockfile) is the one write `--frozen-lockfile` still performs. + `--frozen-lockfile` works on a pruned monorepo checkout (e.g. `turbo prune` output, or a Docker context with only some workspace folders copied in). If a workspace listed in `bun.lock` is missing its `package.json` on disk, Bun skips it with a `note:` and does not install its exclusive dependencies. If a remaining workspace depends on a skipped one, the install fails. To validate the lockfile without installing, use `bun install --frozen-lockfile --dry-run`. @@ -516,6 +518,8 @@ bun install Migration only runs when `bun.lock` is absent. There is currently no opt-out flag for pnpm migration. +Bun writes the migrated `bun.lock` and the `package.json` edits described below together, and only when the command saves a lockfile. `bun install`, `bun add`, `bun remove`, `bun update`, `bun pm migrate`, and `bun pm trust` write both files. `bun install --frozen-lockfile` (and `bun ci`), `bun install --dry-run`, `bun install --no-save`, and commands that only read the lockfile, such as `bun outdated` and `bun pm why`, migrate in memory and leave both files untouched. + The migration process handles: ### Lockfile Migration diff --git a/docs/pm/lockfile.mdx b/docs/pm/lockfile.mdx index 5bec794a3ad5..3048e8d65d04 100644 --- a/docs/pm/lockfile.mdx +++ b/docs/pm/lockfile.mdx @@ -11,7 +11,7 @@ Yes #### Generate a lockfile without installing? -To generate a lockfile without installing to `node_modules`, use the `--lockfile-only` flag. Bun always saves the lockfile to disk, even if it is already up to date with your project's `package.json`(s). The exception is when `--frozen-lockfile` (or `--production`) is set. +To generate a lockfile without installing to `node_modules`, use the `--lockfile-only` flag. Bun always saves the lockfile to disk, even if it is already up to date with your project's `package.json`(s). The exceptions are `--frozen-lockfile` (or `--production`), `--dry-run`, and `--no-save`, which save nothing. ```bash terminal icon="terminal" bun install --lockfile-only @@ -65,3 +65,5 @@ When you run `bun install` in a project without a `bun.lock`, Bun automatically Bun does not migrate a `package-lock.json` from npm 6 or older (`lockfileVersion` 1); it prints a warning and resolves from `package.json` instead. Bun preserves the original lockfile. You can remove it manually after verification. + +Bun writes the migrated `bun.lock` only when the command saves a lockfile. `bun install --frozen-lockfile` (and `bun ci`), `--dry-run`, `--no-save`, and read-only commands such as `bun outdated` use the migrated lockfile in memory and write nothing. Under `--frozen-lockfile`, Bun prints a `note:` asking you to run `bun install` and commit the result. To migrate without installing, run `bun pm migrate` or `bun install --lockfile-only`. diff --git a/src/install/PackageManager.rs b/src/install/PackageManager.rs index e1524675ffb9..89b797d6d6c3 100644 --- a/src/install/PackageManager.rs +++ b/src/install/PackageManager.rs @@ -447,6 +447,10 @@ pub struct PackageManager { // package.json cache entries that differ from disk; written by package_json_write_back::flush. pub(crate) edited_package_jsons: Vec, + // pnpm migration: what it moved into the cached root package.json. The file is only written along with the + // migrated lockfile (package_json_write_back::record_migrated_root); loads that are never saved leave it alone. + pub(crate) migrated_package_json_moves: Vec<&'static str>, + // bun add: catalog references decided per target and the root entries they need; see add_catalog.rs pub(crate) catalog_add: add_catalog::State, @@ -2120,6 +2124,7 @@ pub fn init( wr!(filtered_link_targets, None); wr!(pending_filtered_write, None); wr!(edited_package_jsons, Vec::new()); + wr!(migrated_package_json_moves, Vec::new()); wr!(catalog_add, add_catalog::State::default()); wr!(patched_dependencies_to_remove, ArrayHashMap::default()); wr!(last_reported_slow_lifecycle_script_at, 0); @@ -2569,6 +2574,7 @@ fn init_with_runtime_once( wr!(filtered_link_targets, None); wr!(pending_filtered_write, None); wr!(edited_package_jsons, Vec::new()); + wr!(migrated_package_json_moves, Vec::new()); wr!(catalog_add, add_catalog::State::default()); wr!(patched_dependencies_to_remove, ArrayHashMap::default()); wr!(last_reported_slow_lifecycle_script_at, 0); diff --git a/src/install/PackageManager/install_with_manager.rs b/src/install/PackageManager/install_with_manager.rs index 3c134c0cc864..7e040561920f 100644 --- a/src/install/PackageManager/install_with_manager.rs +++ b/src/install/PackageManager/install_with_manager.rs @@ -858,6 +858,12 @@ pub fn install_with_manager( } } + if manager.options.enable.frozen_lockfile() && !manager.options.do_.save_lockfile() { + if let Some(source) = load_result.migrated().source_lockfile_name() { + note_migrated_lockfile_not_saved(manager, source); + } + } + // BACKREF: `manager.lockfile` is a `Box` whose allocation is // never replaced for the remainder of this function (only its fields // mutate). Wrap once as `ParentRef` so the two `save_lockfile` read sites @@ -966,7 +972,7 @@ pub fn install_with_manager( // It's unnecessary work to re-save the lockfile if there are no changes. // A loaded text lockfile is never re-saved just to bump its version: an // existing `bun.lock` keeps the version it was written with. - let should_save_lockfile = saves_migrated_lockfile(&load_result, save_format) + let should_save_lockfile = converts_binary_lockfile_to_text(&load_result, save_format) // check `save_lockfile` after checking if loaded from binary and save format is text // because `save_lockfile` is set to false for `--frozen-lockfile` || (manager.options.do_.save_lockfile() @@ -980,6 +986,7 @@ pub fn install_with_manager( || manager.options.enable.force_save_lockfile())); if should_save_lockfile { + super::package_json_write_back::record_migrated_root(manager); save_lockfile( manager, &load_result, @@ -1469,7 +1476,9 @@ fn overrides_field_name( } pub(crate) fn loaded_lockfile_name(load_result: &lockfile::LoadResult) -> &'static str { - if load_result.loaded_from_binary_lockfile() { + if let Some(source) = load_result.migrated().source_lockfile_name() { + source + } else if load_result.loaded_from_binary_lockfile() { "bun.lockb" } else { "bun.lock" @@ -2206,8 +2215,11 @@ fn run_security_scanner( } } -// bun.lockb / package-lock.json / yarn.lock / pnpm-lock.yaml -> bun.lock is written even under --frozen-lockfile. -fn saves_migrated_lockfile( +// The documented bun.lockb -> bun.lock recipe (`--save-text-lockfile --frozen-lockfile --lockfile-only`) is the one +// write that happens while `Do::SAVE_LOCKFILE` is off (--frozen-lockfile, --dry-run, --no-save). A migrated +// package-lock.json / yarn.lock / pnpm-lock.yaml is saved through `FORCE_SAVE_LOCKFILE` like any other change, so +// those flags keep it in memory. (A migrated load can report `Format::Binary` too, hence the `migrated` check.) +fn converts_binary_lockfile_to_text( load_result: &lockfile::LoadResult, save_format: lockfile::Format, ) -> bool { @@ -2215,10 +2227,30 @@ fn saves_migrated_lockfile( && matches!( load_result, lockfile::LoadResult::Ok(ok) - if ok.format == lockfile::Format::Binary || ok.migrated != lockfile::Migrated::None + if ok.format == lockfile::Format::Binary && ok.migrated == lockfile::Migrated::None ) } +#[cold] +#[inline(never)] +fn note_migrated_lockfile_not_saved(manager: &PackageManager, source: &str) { + if manager.options.log_level.is_silent() { + return; + } + Output::flush(); + let files = if manager.migrated_package_json_moves.is_empty() { + "bun.lock" + } else { + "bun.lock and package.json" + }; + bun_core::note!( + "the lockfile is frozen, so the migration from {} was not written to {}; run 'bun install' and commit the result", + source, + files, + ); + Output::flush(); +} + #[cold] #[inline(never)] #[allow(clippy::too_many_arguments)] @@ -2232,8 +2264,8 @@ fn save_lockfile_only( packages_len_before_install: usize, log_level: Options::LogLevel, ) -> crate::Result<()> { - if (manager.options.enable.frozen_lockfile() - && !saves_migrated_lockfile(load_result, save_format)) + if (!manager.options.do_.save_lockfile() + && !converts_binary_lockfile_to_text(load_result, save_format)) || (manager.subcommand == Subcommand::Dedupe && manager.dedupe_report.is_none()) { Output::flush(); @@ -2246,6 +2278,7 @@ fn save_lockfile_only( packages_len_before_install, )?; + super::package_json_write_back::record_migrated_root(manager); let saved = save_lockfile( manager, load_result, @@ -2255,6 +2288,7 @@ fn save_lockfile_only( packages_len_before_install, log_level, )?; + super::package_json_write_back::flush(manager)?; if manager.subcommand == Subcommand::Dedupe { if manager.options.do_.summary() { diff --git a/src/install/PackageManager/package_json_write_back.rs b/src/install/PackageManager/package_json_write_back.rs index becf4d246f84..81ed77ba674a 100644 --- a/src/install/PackageManager/package_json_write_back.rs +++ b/src/install/PackageManager/package_json_write_back.rs @@ -58,6 +58,37 @@ fn root_target() -> WorkspaceTarget { } } +/// The root package.json a pnpm migration edited in memory (`pnpm::update_package_json_after_migration`), announced +/// once; `None` when the load did not touch it. Only the places that save the migrated lockfile ask for it, so a load +/// that is never saved (`--frozen-lockfile`, `--dry-run`, `bun outdated`, ...) leaves the file alone. +fn take_migrated_root(manager: &mut PackageManager) -> Option { + if manager.migrated_package_json_moves.is_empty() { + return None; + } + let moved = core::mem::take(&mut manager.migrated_package_json_moves); + if !manager.options.log_level.is_silent() { + bun_core::pretty_errorln!("moved {} in package.json", moved.join(", ")); + } + Some(root_target()) +} + +/// `install_with_manager`: written by `flush` along with the command's other package.json edits. +pub(crate) fn record_migrated_root(manager: &mut PackageManager) { + if let Some(root) = take_migrated_root(manager) { + record(manager, root, false); + } +} + +/// `bun pm migrate` and `bun pm trust` save the lockfile without consulting `--dry-run` / `--no-save`, so the +/// package.json that belongs with it is written the same way instead of through `flush`, which those flags disable. +pub fn write_migrated_root(manager: &mut PackageManager) { + if let Some(root) = take_migrated_root(manager) { + if !write_target(manager, &root) { + Global::exit(1); + } + } +} + /// Phase 1 (before bun.lock is saved): write the resolved versions into the edited package.json entries and re-derive bun.lock's declared columns from them. #[inline] pub(crate) fn edit_after_resolve(manager: &mut PackageManager) -> crate::Result<()> { diff --git a/src/install/PackageManager/updatePackageJSONAndInstall.rs b/src/install/PackageManager/updatePackageJSONAndInstall.rs index 65ac909a28f0..9d95fe238385 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,34 @@ 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(), - ) - }; + 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 source printed above: a lockfile migration during the install edits the + // root entry too (`pnpm::update_package_json_after_migration`). + let entry = match manager + .workspace_package_json_cache + .get_with_path( + manager.log_mut(), + path.as_bytes(), + GetJSONOptions::default(), + ) + .unwrap() + { + Ok(e) => e, + 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/lockfile.rs b/src/install/lockfile.rs index 73d55df73e17..1429c1519038 100644 --- a/src/install/lockfile.rs +++ b/src/install/lockfile.rs @@ -332,6 +332,17 @@ pub enum Migrated { Pnpm, } +impl Migrated { + pub(crate) fn source_lockfile_name(self) -> Option<&'static str> { + match self { + Migrated::None => None, + Migrated::Npm => Some("package-lock.json"), + Migrated::Yarn => Some("yarn.lock"), + Migrated::Pnpm => Some("pnpm-lock.yaml"), + } + } +} + pub struct LoadResultErr { pub step: LoadStep, pub value: BunError, @@ -383,6 +394,13 @@ impl<'a> LoadResult<'a> { } } + pub(crate) fn migrated(&self) -> Migrated { + match self { + LoadResult::Ok(ok) => ok.migrated, + _ => Migrated::None, + } + } + pub(crate) fn save_format(&self, options: &PackageManagerOptions) -> LockfileFormat { match self { LoadResult::NotFound => { diff --git a/src/install/migration.rs b/src/install/migration.rs index dd5270fe8424..a28b81433c1d 100644 --- a/src/install/migration.rs +++ b/src/install/migration.rs @@ -100,7 +100,7 @@ pub fn detect_and_load_other_lockfile<'a>( let Ok(data) = File::read_from(dir, b"pnpm-lock.yaml") else { break 'pnpm; }; - let migrate_result = match pnpm::migrate_pnpm_lockfile(this, manager, log, &data, dir) { + let migrate_result = match pnpm::migrate_pnpm_lockfile(this, manager, log, &data) { Ok(r) => r, Err(MigratePnpmLockfileError::PnpmLockfileTooOld) => { report_unsupported_lockfile_version( diff --git a/src/install/pnpm.rs b/src/install/pnpm.rs index abf7916c98f8..c33fcbb4cb79 100644 --- a/src/install/pnpm.rs +++ b/src/install/pnpm.rs @@ -17,6 +17,8 @@ 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::add_remove_with_filter::root_package_json_path; +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}; @@ -481,7 +483,6 @@ pub(crate) fn migrate_pnpm_lockfile<'a>( manager: &mut PackageManager, log: &mut bun_ast::Log, data: &[u8], - dir: Fd, ) -> Result, MigratePnpmLockfileError> { lockfile.init_empty(); crate::initialize_store(); @@ -1629,7 +1630,7 @@ pub(crate) fn migrate_pnpm_lockfile<'a>( lockfile.fetch_necessary_package_metadata_after_yarn_or_pnpm_migration::(manager)?; - update_package_json_after_migration(manager, log, dir, &found_patches)?; + update_package_json_after_migration(manager, log, &found_patches)?; Ok(LoadResult::Ok(LoadResultOk { lockfile, @@ -2327,31 +2328,31 @@ 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. + // `join_buf` is reused by the next key; the tree is printed after this fn returns. 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 +/// Moves the workspace, catalog, override, and patch settings pnpm keeps in `pnpm-workspace.yaml` and the `pnpm` +/// key into the cached root package.json. Only the cache entry changes here: the rest of the load (frozen check, +/// differ) reads it from there, and `package_json_write_back::record_migrated_root` writes it when the migrated +/// lockfile is saved. 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 pkg_json_path = root_package_json_path(); 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(), + &pkg_json_path, crate::GetJsonOptions { guess_indentation: true, ..Default::default() @@ -2508,7 +2509,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`. + // `Expr` nodes it backs until the edited tree is printed below. let mut workspace_paths: Option> = None; let mut catalog_obj: Option = None; let mut catalogs_obj: Option = None; @@ -2519,8 +2520,7 @@ fn update_package_json_after_migration( 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 + // below) escape into `json`. 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`). @@ -2541,12 +2541,6 @@ fn update_package_json_after_migration( let mut paths: Vec<&'static [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)); } } @@ -2719,48 +2713,17 @@ 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); + 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()); + bun_core::Global::crash(); } - - root_pkg_json.source.contents = std::borrow::Cow::Owned( - package_json_writer - .ctx - .written_without_trailing_zero() - .to_vec(), - ); - - // 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(", ")); + // Commands that load the lockfile twice (`bun update -r`, `bun audit fix`) migrate the already + // edited entry the second time, so only the pnpm-workspace.yaml moves repeat. + for what in moved { + if !manager.migrated_package_json_moves.contains(&what) { + manager.migrated_package_json_moves.push(what); + } } } diff --git a/src/install/update_scope.rs b/src/install/update_scope.rs index 1dc64ac259bd..b379308abf34 100644 --- a/src/install/update_scope.rs +++ b/src/install/update_scope.rs @@ -15,6 +15,7 @@ use crate::package_manager::Options::LogLevel; use crate::package_manager::UpdateTargetWorkspace; use crate::package_manager::workspace_selection::{self, RootSelection}; use crate::package_manager_real::command_line_arguments::UpdateGroups; +use crate::package_manager_real::install_with_manager::loaded_lockfile_name; use crate::resolution::Tag as ResolutionTag; use crate::{DependencyID, PackageID, PackageManager, PackageNameHash, invalid_package_id}; @@ -295,15 +296,9 @@ fn exit_on_lockfile_load_failure(manager: &mut PackageManager, subject: &[u8]) - missing(silent, subject); } let load_result = manager.load_lockfile_from_cwd::(); - let from_binary = load_result.loaded_from_binary_lockfile(); + let lockfile_name = loaded_lockfile_name(&load_result); match load_result { - crate::lockfile::LoadResult::Ok(_) => { - if from_binary { - "bun.lockb" - } else { - "bun.lock" - } - } + crate::lockfile::LoadResult::Ok(_) => lockfile_name, crate::lockfile::LoadResult::NotFound => missing(silent, subject), crate::lockfile::LoadResult::Err(cause) => { if !silent && !crate::migration::reported_unsupported_lockfile_version(&cause) { diff --git a/src/runtime/cli/package_manager_command.rs b/src/runtime/cli/package_manager_command.rs index 62b21f210e1b..32c262dd8fb2 100644 --- a/src/runtime/cli/package_manager_command.rs +++ b/src/runtime/cli/package_manager_command.rs @@ -10,7 +10,7 @@ use bun_install::lockfile::{LoadResult, LoadStep, Lockfile, package::PackageColu use bun_install::npm as Npm; use bun_install::package_manager_real::{ CommandLineArguments, Subcommand, fetch_cache_directory_path, get_cache_directory, - package_manager_options::LogLevel, setup_global_dir, + package_json_write_back, package_manager_options::LogLevel, setup_global_dir, }; use bun_install::{DependencyID, PackageID, PackageManager, migration}; use bun_paths::{self as Path, PathBuffer}; @@ -737,6 +737,7 @@ Learn more about these at https://bun.com/docs/cli/pm.\n"; unsafe { (*lf).save_to_disk(&load_lockfile, &(*pm_raw).options); } + package_json_write_back::write_migrated_root(pm); Global::exit(0); } else if strings::eql_comptime(subcommand, b"version") { let positionals: &[&[u8]] = pm.options.positionals; diff --git a/src/runtime/cli/pm_trusted_command.rs b/src/runtime/cli/pm_trusted_command.rs index d03bd50c0d34..4898792f5c8a 100644 --- a/src/runtime/cli/pm_trusted_command.rs +++ b/src/runtime/cli/pm_trusted_command.rs @@ -12,7 +12,8 @@ use bun_install::lockfile::{ tree, }; use bun_install::package_manager_real::{ - PackageJSONEditor, ProgressStrings, ROOT_PACKAGE_JSON_PATH, update_lockfile_if_needed, + PackageJSONEditor, ProgressStrings, ROOT_PACKAGE_JSON_PATH, package_json_write_back, + update_lockfile_if_needed, }; use bun_install::{ self as install, DEFAULT_TRUSTED_DEPENDENCIES_LIST, DependencyID, LifecycleScriptSubprocess, @@ -533,6 +534,12 @@ impl TrustCommand { } } + // The `trustedDependencies` edit below re-reads package.json from disk, so the edits a pnpm + // migration made to it have to be written first (the migrated lockfile is saved below). + // SAFETY: `pm_raw` singleton; `load_lockfile` only borrows the boxed lockfile, which this + // does not touch. + unsafe { package_json_write_back::write_migrated_root(&mut *pm_raw) }; + // SAFETY: `pm_raw` singleton; this scope takes over the descriptor // (the original `pm.root_package_json_file` is replaced with INVALID so // its eventual drop is a no-op). diff --git a/test/cli/install/lockfile-only.test.ts b/test/cli/install/lockfile-only.test.ts index cd9a0e8586d3..2c16db65c677 100644 --- a/test/cli/install/lockfile-only.test.ts +++ b/test/cli/install/lockfile-only.test.ts @@ -89,7 +89,7 @@ it.each(["bun.lockb", "bun.lock"])("should not download tarballs with --lockfile await access(join(package_dir, lockfile)); }); -describe("--lockfile-only under --frozen-lockfile", () => { +describe("--lockfile-only and lockfile migration under --frozen-lockfile, --dry-run, and --no-save", () => { const project = { "foo/package.json": JSON.stringify({ name: "foo", version: "1.0.0" }), "package.json": JSON.stringify({ name: "mig", dependencies: { foo: "file:./foo" } }), @@ -138,7 +138,7 @@ describe("--lockfile-only under --frozen-lockfile", () => { } const lock = (dir: string) => join(dir, "bun.lock"); - it.concurrent.each(["--frozen-lockfile", "--production"])( + it.concurrent.each(["--frozen-lockfile", "--production", "--dry-run", "--no-save"])( "%s --lockfile-only leaves an up-to-date bun.lock byte-identical", async flag => { using tmp = tempDir("lockfile-only-frozen", project); @@ -157,7 +157,7 @@ describe("--lockfile-only under --frozen-lockfile", () => { }, ); - it.concurrent.each(["--frozen-lockfile", "--production"])( + it.concurrent.each(["--frozen-lockfile", "--production", "--dry-run", "--no-save"])( "%s --lockfile-only does not create a missing bun.lock", async flag => { using tmp = tempDir("lockfile-only-frozen-missing", project); @@ -199,32 +199,84 @@ describe("--lockfile-only under --frozen-lockfile", () => { expect(exitCode).toBe(0); }); + const frozenNote = (name: string) => + `note: the lockfile is frozen, so the migration from ${name} was not written to bun.lock; run 'bun install' and commit the result`; + it.concurrent.each(migrations)( - "--frozen-lockfile --lockfile-only still writes bun.lock when migrating from %s", + "--frozen-lockfile --lockfile-only migrates %s in memory and does not write bun.lock", async (name, contents) => { using tmp = tempDir("lockfile-only-frozen-migrate", { ...project, [name]: contents }); const dir = String(tmp); const { stdout, stderr, exitCode } = await run(dir, "--frozen-lockfile", "--lockfile-only"); expect(stderr).toContain(`migrated lockfile from ${name}`); - expect(stdout).toContain("Saved bun.lock (2 packages)"); - expect(readFileSync(lock(dir), "utf8")).toContain('"foo": ["foo@file:foo", {}]'); + expect(stderr).toContain(frozenNote(name)); + expect(stdout + stderr).not.toContain("Saved"); + expect(existsSync(lock(dir))).toBe(false); expect(existsSync(join(dir, "node_modules"))).toBe(false); expect(exitCode).toBe(0); }, ); - it.concurrent.each(migrations)("--frozen-lockfile writes bun.lock when migrating from %s", async (name, contents) => { - using tmp = tempDir("frozen-migrate", { ...project, [name]: contents }); + it.concurrent.each(migrations)( + "--frozen-lockfile installs from a migrated %s and does not write bun.lock", + async (name, contents) => { + using tmp = tempDir("frozen-migrate", { ...project, [name]: contents }); + const dir = String(tmp); + + const { stdout, stderr, exitCode } = await run(dir, "--frozen-lockfile"); + expect(stderr).toContain(`migrated lockfile from ${name}`); + expect(stderr).toContain(frozenNote(name)); + expect(stdout + stderr).not.toContain("Saved"); + expect(existsSync(lock(dir))).toBe(false); + expect(existsSync(join(dir, "node_modules", "foo", "package.json"))).toBe(true); + expect(exitCode).toBe(0); + }, + ); + + it.concurrent.each(migrations)("--dry-run does not write bun.lock when migrating from %s", async (name, contents) => { + using tmp = tempDir("dry-run-migrate", { ...project, [name]: contents }); const dir = String(tmp); - const { stderr, exitCode } = await run(dir, "--frozen-lockfile"); + const { stdout, stderr, exitCode } = await run(dir, "--dry-run"); expect(stderr).toContain(`migrated lockfile from ${name}`); - expect(existsSync(lock(dir))).toBe(true); - expect(readFileSync(lock(dir), "utf8")).toContain('"foo": ["foo@file:foo", {}]'); - expect(existsSync(join(dir, "node_modules", "foo", "package.json"))).toBe(true); + expect(stderr).not.toContain("note:"); + expect(stdout + stderr).not.toContain("Saved"); + expect(existsSync(lock(dir))).toBe(false); + expect(existsSync(join(dir, "node_modules"))).toBe(false); expect(exitCode).toBe(0); }); + + it.concurrent.each(migrations)( + "--no-save installs from a migrated %s and does not write bun.lock", + async (name, contents) => { + using tmp = tempDir("no-save-migrate", { ...project, [name]: contents }); + const dir = String(tmp); + + const { stdout, stderr, exitCode } = await run(dir, "--no-save"); + expect(stderr).toContain(`migrated lockfile from ${name}`); + expect(stderr).not.toContain("note:"); + expect(stdout + stderr).not.toContain("Saved"); + expect(existsSync(lock(dir))).toBe(false); + expect(existsSync(join(dir, "node_modules", "foo", "package.json"))).toBe(true); + expect(exitCode).toBe(0); + }, + ); + + it.concurrent.each(migrations)( + "a plain install still writes bun.lock when migrating from %s", + async (name, contents) => { + using tmp = tempDir("install-migrate", { ...project, [name]: contents }); + const dir = String(tmp); + + const { stderr, exitCode } = await run(dir); + expect(stderr).toContain(`migrated lockfile from ${name}`); + expect(stderr).toContain("Saved lockfile"); + expect(readFileSync(lock(dir), "utf8")).toContain('"foo": ["foo@file:foo", {}]'); + expect(existsSync(join(dir, "node_modules", "foo", "package.json"))).toBe(true); + expect(exitCode).toBe(0); + }, + ); }); describe("--lockfile-only with remove and update", () => { diff --git a/test/cli/install/migration/migrate.test.ts b/test/cli/install/migration/migrate.test.ts index 2429afe854aa..dc6675fa7fa9 100644 --- a/test/cli/install/migration/migrate.test.ts +++ b/test/cli/install/migration/migrate.test.ts @@ -1149,7 +1149,7 @@ describe("package-lock.json migration fixes", () => { await frozen(dir); using fresh = fixture("carbonium"); - const result = await run(fresh, "install", "--frozen-lockfile", "--lockfile-only"); + const result = await run(fresh, "install", "--lockfile-only"); expect(result.stderr).not.toContain("Ignoring lockfile"); expect(result.exitCode).toBe(0); expect((await readLock(fresh)).lock.packages).toStrictEqual(lock.packages); diff --git a/test/cli/install/migration/pnpm-lock-v9.test.ts b/test/cli/install/migration/pnpm-lock-v9.test.ts index 9fe4c0434e27..090262258200 100644 --- a/test/cli/install/migration/pnpm-lock-v9.test.ts +++ b/test/cli/install/migration/pnpm-lock-v9.test.ts @@ -2579,6 +2579,241 @@ importers: }); }); + // The settings moved out of `pnpm` / pnpm-workspace.yaml are edited into package.json in memory while the + // lockfile loads; the file is only written together with bun.lock. + describe("package.json edits", () => { + const packageJson = `{ + "name": "root", + "private": true, + "dependencies": { + "a": "workspace:*", + "foo": "file:vendor/foo" + }, + "pnpm": { + "overrides": { + "left-pad": "1.3.0" + } + } +} +`; + const migratedPackageJson = { + name: "root", + private: true, + dependencies: { a: "workspace:*", foo: "file:vendor/foo" }, + overrides: { "left-pad": "1.3.0" }, + workspaces: ["packages/*"], + }; + const files = { + "package.json": packageJson, + "pnpm-workspace.yaml": "packages:\n - 'packages/*'\n", + "packages/a/package.json": JSON.stringify({ name: "a", version: "1.0.0" }), + "vendor/foo/package.json": JSON.stringify({ + name: "foo", + version: "1.0.0", + scripts: { postinstall: "echo foo-postinstall" }, + }), + "pnpm-lock.yaml": `lockfileVersion: '9.0' + +overrides: + left-pad: 1.3.0 + +importers: + + .: + dependencies: + a: + specifier: workspace:* + version: link:packages/a + foo: + specifier: file:vendor/foo + version: file:vendor/foo + + packages/a: {} + +packages: + + foo@file:vendor/foo: + resolution: {directory: vendor/foo, type: directory} + version: 1.0.0 + +snapshots: + + foo@file:vendor/foo: {} +`, + }; + const movedLine = "moved pnpm.overrides to overrides, pnpm-workspace.yaml to workspaces in package.json"; + const frozenNote = + "note: the lockfile is frozen, so the migration from pnpm-lock.yaml was not written to bun.lock and package.json; run 'bun install' and commit the result"; + + async function expectUntouched(dir: string) { + expect(await Bun.file(join(dir, "package.json")).text()).toBe(packageJson); + expect(existsSync(join(dir, "bun.lock"))).toBe(false); + } + + test.concurrent.each([ + ["install --frozen-lockfile", ["install", "--frozen-lockfile"]], + ["ci", ["ci"]], + ["install --production", ["install", "--production"]], + ])("bun %s installs the migrated lockfile without writing package.json or bun.lock", async (_, args) => { + using dir = tempDir("pnpm-v9-package-json-frozen", files); + + const { stdout, stderr, exitCode } = await run(String(dir), ...args); + + expect(stderr).not.toContain("error:"); + expect(stderr).toContain("migrated lockfile from pnpm-lock.yaml"); + expect(stderr).toContain(frozenNote); + expect(stderr).not.toContain(movedLine); + expect(stdout + stderr).not.toContain("Saved"); + await expectUntouched(String(dir)); + expect(await installedPackageJson(String(dir), "", "foo")).toMatchObject({ name: "foo", version: "1.0.0" }); + expect(await installedPackageJson(String(dir), "", "a")).toStrictEqual({ name: "a", version: "1.0.0" }); + expect(exitCode).toBe(0); + }); + + test.concurrent("install --frozen-lockfile fails once the lockfile and package.json disagree", async () => { + using dir = tempDir("pnpm-v9-package-json-frozen-stale", { + ...files, + "pnpm-lock.yaml": files["pnpm-lock.yaml"].replace("left-pad: 1.3.0", "left-pad: 1.2.0"), + }); + + const { stderr, exitCode } = await run(String(dir), "install", "--frozen-lockfile"); + + expect(stderr).toContain("error: lockfile had changes, but lockfile is frozen"); + expect(stderr).toContain("note: overrides in package.json changed since pnpm-lock.yaml was saved"); + expect(exitCode).toBe(1); + expect(await Bun.file(join(String(dir), "package.json")).text()).toBe(packageJson); + expect(existsSync(join(String(dir), "bun.lock"))).toBe(false); + }); + + test.concurrent.each([ + ["install --dry-run", ["install", "--dry-run"]], + ["install --no-save", ["install", "--no-save"]], + ["outdated", ["outdated"]], + ["pm why a", ["pm", "why", "a"]], + ["pm untrusted", ["pm", "untrusted"]], + ])("bun %s leaves package.json and bun.lock alone", async (_, args) => { + using dir = tempDir("pnpm-v9-package-json-read-only", files); + + const { stdout, stderr, exitCode } = await run(String(dir), ...args); + + expect(stderr).not.toContain("error:"); + expect(stderr).toContain("migrated lockfile from pnpm-lock.yaml"); + expect(stderr).not.toContain("note:"); + expect(stdout + stderr).not.toContain("moved "); + expect(stdout + stderr).not.toContain("Saved"); + await expectUntouched(String(dir)); + expect(exitCode).toBe(0); + }); + + test.concurrent.each([ + ["install", ["install"]], + ["install --lockfile-only", ["install", "--lockfile-only"]], + ["pm migrate", ["pm", "migrate"]], + // pm migrate saves bun.lock whatever install flags are passed, so package.json has to follow it. + ["pm migrate --dry-run", ["pm", "migrate", "--dry-run"]], + ])("bun %s writes the edited package.json together with bun.lock", async (_, args) => { + using dir = tempDir("pnpm-v9-package-json-written", files); + + const { stderr, exitCode } = await run(String(dir), ...args); + + expect(stderr).not.toContain("error:"); + expect(stderr).toContain("migrated lockfile from pnpm-lock.yaml"); + expect(stderr.split(movedLine).length - 1).toBe(1); + expect(await Bun.file(join(String(dir), "package.json")).json()).toStrictEqual(migratedPackageJson); + expect(await bunLockOf(String(dir))).toContain(`"packages/a": {`); + expect(exitCode).toBe(0); + + const written = await Bun.file(join(String(dir), "package.json")).text(); + const frozen = await run(String(dir), "install", "--frozen-lockfile"); + + expect(frozen.stderr).not.toContain("error:"); + expect(frozen.stderr).not.toContain("migrated lockfile"); + expect(frozen.exitCode).toBe(0); + expect(await Bun.file(join(String(dir), "package.json")).text()).toBe(written); + }); + + test.concurrent("remove keeps the edits the migration made to package.json", async () => { + using dir = tempDir("pnpm-v9-package-json-remove", files); + + const { stderr, exitCode } = await run(String(dir), "remove", "foo"); + + expect(stderr).not.toContain("error:"); + expect(stderr.split(movedLine).length - 1).toBe(1); + expect(await Bun.file(join(String(dir), "package.json")).json()).toStrictEqual({ + ...migratedPackageJson, + dependencies: { a: "workspace:*" }, + }); + expect(await bunLockOf(String(dir))).not.toContain("foo"); + expect(exitCode).toBe(0); + + const frozen = await run(String(dir), "install", "--frozen-lockfile"); + + expect(frozen.stderr).not.toContain("error:"); + expect(frozen.exitCode).toBe(0); + }); + + test.concurrent("add writes its own edit and the migration's edits in one package.json", async () => { + using dir = tempDir("pnpm-v9-package-json-add", { + ...files, + "vendor/bar/package.json": JSON.stringify({ name: "bar", version: "1.0.0" }), + }); + + const { stderr, exitCode } = await run(String(dir), "add", "./vendor/bar"); + + expect(stderr).not.toContain("error:"); + expect(stderr.split(movedLine).length - 1).toBe(1); + const written = await Bun.file(join(String(dir), "package.json")).json(); + expect(written).toMatchObject({ + overrides: migratedPackageJson.overrides, + workspaces: migratedPackageJson.workspaces, + }); + expect(written).not.toHaveProperty("pnpm"); + expect(Object.keys(written.dependencies).sort()).toStrictEqual(["a", "bar", "foo"]); + expect(await bunLockOf(String(dir))).toContain(`"bar":`); + expect(exitCode).toBe(0); + + const frozen = await run(String(dir), "install", "--frozen-lockfile"); + + expect(frozen.stderr).not.toContain("error:"); + expect(frozen.exitCode).toBe(0); + }); + + test.concurrent("install --silent writes both files without printing", async () => { + using dir = tempDir("pnpm-v9-package-json-silent", files); + + const { stdout, stderr, exitCode } = await run(String(dir), "install", "--silent"); + + expect(stdout).toBe(""); + expect(stderr).toBe(""); + expect(await Bun.file(join(String(dir), "package.json")).json()).toStrictEqual(migratedPackageJson); + expect(existsSync(join(String(dir), "bun.lock"))).toBe(true); + expect(exitCode).toBe(0); + }); + + test.concurrent("pm trust writes the edited package.json along with the lockfile it saves", async () => { + using dir = tempDir("pnpm-v9-package-json-pm-trust", files); + + const install = await run(String(dir), "install", "--frozen-lockfile"); + expect(install.stderr).not.toContain("error:"); + expect(install.exitCode).toBe(0); + await expectUntouched(String(dir)); + + const { stdout, stderr, exitCode } = await run(String(dir), "pm", "trust", "foo"); + + expect(stderr).not.toContain("error:"); + expect(stderr).toContain(movedLine); + expect(stdout).toContain("1 script ran across 1 package"); + expect(await Bun.file(join(String(dir), "package.json")).json()).toStrictEqual({ + ...migratedPackageJson, + trustedDependencies: ["foo"], + }); + const bunLock = await bunLockOf(String(dir)); + expect(bunLock).toContain(`"packages/a": {`); + expect(bunLock).toContain(`"trustedDependencies": [`); + expect(exitCode).toBe(0); + }); + }); + test.concurrent("link: version with a semver specifier resolves to the workspace (pnpm/pnpm#7712)", async () => { // save-workspace-protocol=false / link-workspace-packages shape using dir = fixture("v9-link-semver-specifier");