diff --git a/.changeset/repair-pacquet-lockfiles.md b/.changeset/repair-pacquet-lockfiles.md new file mode 100644 index 0000000000..045ab83692 --- /dev/null +++ b/.changeset/repair-pacquet-lockfiles.md @@ -0,0 +1,6 @@ +--- +"pacquet": patch +"@pnpm/pnpr": patch +--- + +Recognize `pnpm install --fix-lockfile` and regenerate broken lockfile metadata while preserving compatible locked versions [pnpm/pnpm#14250](https://github.com/pnpm/pnpm/issues/14250). diff --git a/pnpm/crates/cli/README.md b/pnpm/crates/cli/README.md index dc1962679c..221bf5d65f 100644 --- a/pnpm/crates/cli/README.md +++ b/pnpm/crates/cli/README.md @@ -45,7 +45,7 @@ | ✅ | --dev | | | ✅ | --no-optional | | | | --lockfile-only | | -| | --fix-lockfile | | +| ✅ | --fix-lockfile | | | | --frozen-lockfile | | | | --reporter= | | | | --use-store-server | | diff --git a/pnpm/crates/cli/src/cli_args/import.rs b/pnpm/crates/cli/src/cli_args/import.rs index 3d572409d6..a104fd38a8 100644 --- a/pnpm/crates/cli/src/cli_args/import.rs +++ b/pnpm/crates/cli/src/cli_args/import.rs @@ -46,6 +46,7 @@ impl ImportArgs { frozen_lockfile: false, prefer_frozen_lockfile: false, update_patches: false, + fix_lockfile: false, lockfile_only: true, ignore_manifest_check: false, trust_lockfile: false, diff --git a/pnpm/crates/cli/src/cli_args/install.rs b/pnpm/crates/cli/src/cli_args/install.rs index bfc36da97c..38a2482d35 100644 --- a/pnpm/crates/cli/src/cli_args/install.rs +++ b/pnpm/crates/cli/src/cli_args/install.rs @@ -124,6 +124,11 @@ pub struct InstallArgs { #[clap(long = "lockfile-only")] pub lockfile_only: bool, + /// Repair broken lockfile entries by re-resolving their metadata while + /// preserving compatible locked versions. + #[clap(long = "fix-lockfile")] + pub fix_lockfile: bool, + #[clap(flatten)] pub lockfile_dir: LockfileDirArg, @@ -307,6 +312,7 @@ impl InstallArgs { frozen_lockfile: false, no_frozen_lockfile: true, lockfile_only: false, + fix_lockfile: false, lockfile_dir: LockfileDirArg::default(), merge_git_branch_lockfiles: false, merge_git_branch_lockfiles_branch_pattern: Vec::new(), @@ -379,6 +385,7 @@ impl InstallArgs { ) -> bool { if self.effective_frozen_lockfile(config) || self.lockfile_only + || self.fix_lockfile || self.force || self.refresh_artifact_pins || self.verify_deps_before_run_install @@ -510,7 +517,7 @@ impl InstallArgs { ) -> miette::Result<()> { state.http_client.set_warning_handler(pnpm_reporter::emit_global_warning::); let frozen_lockfile = match self.configured_frozen_lockfile(state.config) { - _ if self.refresh_artifact_pins => false, + _ if self.fix_lockfile || self.refresh_artifact_pins => false, Some(value) => value, None if state.config.ci && !self.lockfile_only @@ -531,6 +538,7 @@ impl InstallArgs { frozen_lockfile: _, no_frozen_lockfile: _, lockfile_only, + fix_lockfile, // Pinned onto `config` in the dispatch (`dispatch_install.rs`) // before the state is built, so the install reads it from // `config`, not from here. @@ -583,7 +591,7 @@ impl InstallArgs { // mutual `overrides_with` collapses both spellings to the // last-specified, so at most one is set and the precedence here // is straightforward. - let prefer_frozen_lockfile = if refresh_artifact_pins { + let prefer_frozen_lockfile = if fix_lockfile || refresh_artifact_pins { Some(false) } else if prefer_frozen_lockfile { Some(true) @@ -649,6 +657,7 @@ impl InstallArgs { prefer_frozen_lockfile: prefer_frozen_lockfile .unwrap_or(config.prefer_frozen_lockfile), update_patches: false, + fix_lockfile, lockfile_only, ignore_manifest_check, trust_lockfile, @@ -678,6 +687,8 @@ impl InstallArgs { } let install_lockfile = if refresh_artifact_pins { MaybeLazyLockfile::Loaded(refreshed_lockfile.as_ref()) + } else if fix_lockfile { + MaybeLazyLockfile::Repair(lockfile) } else { MaybeLazyLockfile::Lazy(lockfile) }; @@ -705,7 +716,11 @@ impl InstallArgs { lockfile_only, dry_run, persist_policy_excludes: true, - update_seed_policy: UpdateSeedPolicy::KeepAll, + update_seed_policy: if fix_lockfile { + UpdateSeedPolicy::FixLockfile + } else { + UpdateSeedPolicy::KeepAll + }, preferred_versions_override: None, auth_override: None, resolution_observer: None, @@ -764,6 +779,8 @@ pub(crate) struct PnprLink<'a> { /// version. This disables the exchange-free satisfied-lockfile path and /// is forwarded to `/-/pnpr/v0/resolve`. pub(crate) update_patches: bool, + /// Regenerate derived lockfile metadata while retaining compatible pins. + pub(crate) fix_lockfile: bool, /// `--lockfile-only`. Forwarded to `/-/pnpr/v0/resolve` so the server /// resolves only — returning the lockfile without fetching files — /// after which [`install_via_pnpr`] writes the lockfile and skips @@ -903,13 +920,19 @@ async fn install_via_pnpr_inner( // pay no deep copy. The server-exchange paths clone at the point a // request body actually needs one. let previous_wanted: Option<&Lockfile> = if link.use_state_lockfile { - state - .lockfile - .get() - .map_err(|err| miette::Report::new(err).wrap_err("load the lockfile"))? + let loaded = + if link.fix_lockfile { state.lockfile.get_for_fix() } else { state.lockfile.get() }; + loaded.map_err(|err| miette::Report::new(err).wrap_err("load the lockfile"))? } else { None }; + let merge_wanted = if link.fix_lockfile && link.use_state_lockfile { + state.lockfile.get().or_else(|_| state.lockfile.get_for_fix()).map_err(|err| { + miette::Report::new(err).wrap_err("load the lockfile for filtered merge") + })? + } else { + previous_wanted + }; // Importer ids name projects relative to the lockfile, which // `lockfileDir` can pin somewhere other than the workspace the // selection was resolved in. The server request, the merge below, and @@ -954,6 +977,7 @@ async fn install_via_pnpr_inner( // single-project one. let satisfied_without_server = !link.frozen_lockfile && !link.update_patches + && !link.fix_lockfile && !link.lockfile_only && link.prefer_frozen_lockfile && !partial_selection @@ -1215,6 +1239,7 @@ async fn install_via_pnpr_inner( frozen_lockfile: link.frozen_lockfile, prefer_frozen_lockfile: Some(link.prefer_frozen_lockfile), update_patches: link.update_patches, + fix_lockfile: link.fix_lockfile, ignore_manifest_check: link.ignore_manifest_check, trust_lockfile: link.trust_lockfile, resolution_mode: state.config.resolution_mode, @@ -1298,7 +1323,7 @@ async fn install_via_pnpr_inner( .map(|root| state.config.lockfile_dir_for(root)), ) { outcome.lockfile = merge_filtered_wanted_lockfile( - previous_wanted, + merge_wanted, outcome.lockfile, real_importer_ids, selected_importer_ids, diff --git a/pnpm/crates/cli/src/cli_args/install/tests.rs b/pnpm/crates/cli/src/cli_args/install/tests.rs index 1ee60c9370..215bca760d 100644 --- a/pnpm/crates/cli/src/cli_args/install/tests.rs +++ b/pnpm/crates/cli/src/cli_args/install/tests.rs @@ -181,6 +181,16 @@ fn dry_run_flag_parses() { assert!(parsed.args.dry_run, "flag present → true"); } +#[test] +fn fix_lockfile_flag_parses() { + let parsed = InstallArgsHarness::try_parse_from(["pacquet-test"]).expect("parses"); + assert!(!parsed.args.fix_lockfile, "flag absent → false"); + + let parsed = InstallArgsHarness::try_parse_from(["pacquet-test", "--fix-lockfile"]) + .expect("parses --fix-lockfile"); + assert!(parsed.args.fix_lockfile, "flag present → true"); +} + /// `--frozen-store` parses to `true`. Absent → `false`. The flag is /// folded into `config.frozen_store` at the dispatch in `cli_args.rs` /// (any `--frozen-store` upgrades a yaml `false` to `true`), so the diff --git a/pnpm/crates/cli/src/cli_args/update.rs b/pnpm/crates/cli/src/cli_args/update.rs index 886f84ede7..47c1cb61d4 100644 --- a/pnpm/crates/cli/src/cli_args/update.rs +++ b/pnpm/crates/cli/src/cli_args/update.rs @@ -592,6 +592,7 @@ impl UpdateArgs { frozen_lockfile: false, prefer_frozen_lockfile: false, update_patches: true, + fix_lockfile: false, lockfile_only: self.lockfile_only, ignore_manifest_check: false, trust_lockfile: state.config.trust_lockfile, diff --git a/pnpm/crates/cli/tests/suite/install.rs b/pnpm/crates/cli/tests/suite/install.rs index e96eec0989..95c677abab 100644 --- a/pnpm/crates/cli/tests/suite/install.rs +++ b/pnpm/crates/cli/tests/suite/install.rs @@ -95,6 +95,186 @@ fn store_files_outside_links(store_dir: &Path) -> Vec { get_all_files(store_dir).into_iter().filter(|path| !path.starts_with("v11/links/")).collect() } +#[test] +fn fix_lockfile_regenerates_broken_metadata_without_changing_locked_versions() { + let CommandTempCwd { pacquet, root, workspace, npmrc_info, .. } = + CommandTempCwd::init().add_mocked_registry(); + let AddMockedRegistry { mock_instance, .. } = npmrc_info; + fs::write( + workspace.join("package.json"), + serde_json::json!({ + "dependencies": { "@pnpm.e2e/pkg-with-1-dep": "100.0.0" }, + }) + .to_string(), + ) + .expect("write package.json"); + pacquet.with_args(["install", "--lockfile-only"]).assert().success(); + + let lockfile_path = workspace.join("pnpm-lock.yaml"); + let original = pnpm_lockfile::Lockfile::load_from_path(&lockfile_path) + .expect("load original lockfile") + .expect("original lockfile"); + let original_package_keys = original + .packages + .as_ref() + .expect("original packages") + .keys() + .cloned() + .collect::>(); + let original_snapshot_keys = original + .snapshots + .as_ref() + .expect("original snapshots") + .keys() + .cloned() + .collect::>(); + + let mut broken: serde_json::Value = + serde_saphyr::from_str(&fs::read_to_string(&lockfile_path).expect("read lockfile")) + .expect("parse lockfile as value"); + for metadata in broken["packages"].as_object_mut().expect("packages").values_mut() { + metadata.as_object_mut().expect("package metadata").remove("resolution"); + metadata["deprecated"] = serde_json::json!("stale metadata"); + } + for snapshot in broken["snapshots"].as_object_mut().expect("snapshots").values_mut() { + snapshot["transitivePeerDependencies"] = serde_json::json!("broken metadata"); + } + fs::write(&lockfile_path, serde_saphyr::to_string(&broken).expect("serialize broken lockfile")) + .expect("write broken lockfile"); + + let mut command = new_pacquet_command(&workspace); + command.env("CI", "true"); + command.with_args(["install", "--fix-lockfile", "--lockfile-only"]).assert().success(); + + let repaired = pnpm_lockfile::Lockfile::load_from_path(&lockfile_path) + .expect("load repaired lockfile") + .expect("repaired lockfile"); + assert_eq!( + repaired + .packages + .as_ref() + .expect("repaired packages") + .keys() + .cloned() + .collect::>(), + original_package_keys, + ); + assert_eq!( + repaired + .snapshots + .as_ref() + .expect("repaired snapshots") + .keys() + .cloned() + .collect::>(), + original_snapshot_keys, + ); + assert!( + repaired + .packages + .as_ref() + .expect("repaired packages") + .values() + .all(|metadata| metadata.deprecated.as_deref() != Some("stale metadata")), + ); + assert!( + repaired + .packages + .as_ref() + .expect("repaired packages") + .values() + .all(|metadata| metadata.resolution.checkable_integrity().is_some()), + ); + assert!( + repaired + .snapshots + .as_ref() + .expect("repaired snapshots") + .values() + .all(|snapshot| snapshot.transitive_peer_dependencies.is_none()), + ); + + drop((root, mock_instance)); +} + +#[test] +fn filtered_fix_lockfile_preserves_unselected_snapshot_metadata() { + let CommandTempCwd { pacquet, root, workspace, npmrc_info, .. } = + CommandTempCwd::init().add_mocked_registry(); + let AddMockedRegistry { mock_instance, .. } = npmrc_info; + fs::write(workspace.join("pnpm-workspace.yaml"), "packages:\n - packages/*\n") + .expect("write workspace manifest"); + fs::write( + workspace.join("package.json"), + serde_json::json!({ "name": "root", "private": true }).to_string(), + ) + .expect("write root manifest"); + let selected = workspace.join("packages/selected"); + let unselected = workspace.join("packages/unselected"); + fs::create_dir_all(&selected).expect("create selected project"); + fs::create_dir_all(&unselected).expect("create unselected project"); + fs::write( + selected.join("package.json"), + serde_json::json!({ + "name": "selected", + "version": "1.0.0", + "dependencies": { "is-positive": "1.0.0" }, + }) + .to_string(), + ) + .expect("write selected manifest"); + fs::write( + unselected.join("package.json"), + serde_json::json!({ + "name": "unselected", + "version": "1.0.0", + "optionalDependencies": { "@pnpm.e2e/pkg-with-1-dep": "100.0.0" }, + }) + .to_string(), + ) + .expect("write unselected manifest"); + pacquet.with_args(["install", "--lockfile-only"]).assert().success(); + + let lockfile_path = workspace.join("pnpm-lock.yaml"); + let original = pnpm_lockfile::Lockfile::load_from_path(&lockfile_path) + .expect("load original lockfile") + .expect("original lockfile"); + let optional_snapshot_keys: std::collections::HashSet<_> = original + .snapshots + .as_ref() + .expect("original snapshots") + .iter() + .filter(|(_, snapshot)| snapshot.optional) + .map(|(key, _)| key.clone()) + .collect(); + assert!(!optional_snapshot_keys.is_empty()); + + let mut broken: serde_json::Value = serde_saphyr::from_str( + &fs::read_to_string(&lockfile_path).expect("read original lockfile"), + ) + .expect("parse original lockfile as YAML value"); + broken["settings"] = serde_json::json!("invalid"); + fs::write(&lockfile_path, serde_saphyr::to_string(&broken).expect("serialize broken lockfile")) + .expect("write broken lockfile"); + + new_pacquet_command(&workspace) + .with_args(["--filter", "selected", "install", "--fix-lockfile", "--lockfile-only"]) + .assert() + .success(); + + let repaired = pnpm_lockfile::Lockfile::load_from_path(&lockfile_path) + .expect("load repaired lockfile") + .expect("repaired lockfile"); + let repaired_snapshots = repaired.snapshots.as_ref().expect("repaired snapshots"); + assert!( + optional_snapshot_keys + .iter() + .all(|key| { repaired_snapshots.get(key).is_some_and(|snapshot| snapshot.optional) }), + ); + + drop((root, mock_instance)); +} + #[test] fn no_optional_excludes_transitive_optional_dependencies() { let CommandTempCwd { pacquet, root, workspace, npmrc_info, .. } = diff --git a/pnpm/crates/lockfile/src/lazy_lockfile.rs b/pnpm/crates/lockfile/src/lazy_lockfile.rs index 7b3c0caa08..b3cd24f0bb 100644 --- a/pnpm/crates/lockfile/src/lazy_lockfile.rs +++ b/pnpm/crates/lockfile/src/lazy_lockfile.rs @@ -16,6 +16,8 @@ use std::{collections::HashMap, path::PathBuf, sync::OnceLock}; pub struct LazyLockfile { source: Option<(PathBuf, WantedLockfileSelection)>, cell: OnceLock, + fix_cell: OnceLock, + fix_merge_cell: OnceLock, } impl LazyLockfile { @@ -23,7 +25,12 @@ impl LazyLockfile { /// [`Lockfile::load_wanted`]) on first [`Self::get`]. #[must_use] pub fn deferred(dir: PathBuf, selection: WantedLockfileSelection) -> Self { - LazyLockfile { source: Some((dir, selection)), cell: OnceLock::new() } + LazyLockfile { + source: Some((dir, selection)), + cell: OnceLock::new(), + fix_cell: OnceLock::new(), + fix_merge_cell: OnceLock::new(), + } } /// A lockfile that is never loaded — [`Self::get`] yields `None` @@ -31,7 +38,12 @@ impl LazyLockfile { /// config. #[must_use] pub fn disabled() -> Self { - LazyLockfile { source: None, cell: OnceLock::new() } + LazyLockfile { + source: None, + cell: OnceLock::new(), + fix_cell: OnceLock::new(), + fix_merge_cell: OnceLock::new(), + } } /// A lockfile that is already in memory; [`Self::get`] returns it @@ -41,7 +53,12 @@ impl LazyLockfile { let cell = OnceLock::new(); cell.set(LoadedWantedLockfile { lockfile, pre_merge_importers: None }) .expect("a fresh OnceLock accepts the first set"); - LazyLockfile { source: None, cell } + LazyLockfile { + source: None, + cell, + fix_cell: OnceLock::new(), + fix_merge_cell: OnceLock::new(), + } } /// The parsed wanted lockfile, loading it on first call. `None` @@ -52,6 +69,15 @@ impl LazyLockfile { Ok(self.load()?.lockfile.as_ref()) } + /// Load after discarding fields that a repairing resolution regenerates. + pub fn get_for_fix(&self) -> Result, LoadLockfileError> { + Ok(self.load_for_fix()?.lockfile.as_ref()) + } + + fn get_for_fix_merge(&self) -> Result, LoadLockfileError> { + Ok(self.load_for_fix_merge()?.lockfile.as_ref()) + } + /// The importers the branch-lockfile fold started from, loading the /// lockfile on first call. `None` when no fold was attempted. See /// [`LoadedWantedLockfile`] for why the caller needs them. @@ -61,6 +87,12 @@ impl LazyLockfile { Ok(self.load()?.pre_merge_importers.as_ref()) } + fn pre_merge_importers_for_fix( + &self, + ) -> Result>, LoadLockfileError> { + Ok(self.load_for_fix()?.pre_merge_importers.as_ref()) + } + fn load(&self) -> Result<&LoadedWantedLockfile, LoadLockfileError> { if let Some(loaded) = self.cell.get() { return Ok(loaded); @@ -72,6 +104,34 @@ impl LazyLockfile { Ok(self.cell.get_or_init(|| loaded)) } + fn load_for_fix(&self) -> Result<&LoadedWantedLockfile, LoadLockfileError> { + if let Some(loaded) = self.fix_cell.get() { + return Ok(loaded); + } + let loaded = if let Some((dir, selection)) = self.source.as_ref() { + Lockfile::load_wanted_detailed_for_fix(dir, selection)? + } else { + let mut loaded = self.cell.get().cloned().unwrap_or_default(); + if let Some(lockfile) = loaded.lockfile.as_mut() { + lockfile.prepare_for_fix(); + } + loaded + }; + Ok(self.fix_cell.get_or_init(|| loaded)) + } + + fn load_for_fix_merge(&self) -> Result<&LoadedWantedLockfile, LoadLockfileError> { + if let Some(loaded) = self.fix_merge_cell.get() { + return Ok(loaded); + } + let loaded = if let Some((dir, selection)) = self.source.as_ref() { + Lockfile::load_wanted_detailed_for_fix_merge(dir, selection)? + } else { + self.cell.get().cloned().unwrap_or_default() + }; + Ok(self.fix_merge_cell.get_or_init(|| loaded)) + } + /// Whether a wanted lockfile is known to be available: the parsed /// document when already loaded, otherwise /// [`Lockfile::wanted_exists`]'s semantic-presence probe — @@ -80,7 +140,9 @@ impl LazyLockfile { /// the repeat-install fast path. #[must_use] pub fn is_loaded_or_on_disk(&self) -> bool { - if let Some(loaded) = self.cell.get() { + if let Some(loaded) = + self.cell.get().or_else(|| self.fix_cell.get()).or_else(|| self.fix_merge_cell.get()) + { return loaded.lockfile.is_some(); } self.source @@ -97,6 +159,7 @@ impl LazyLockfile { pub enum MaybeLazyLockfile<'a> { Loaded(Option<&'a Lockfile>), Lazy(&'a LazyLockfile), + Repair(&'a LazyLockfile), } impl<'a> MaybeLazyLockfile<'a> { @@ -106,6 +169,16 @@ impl<'a> MaybeLazyLockfile<'a> { match self { MaybeLazyLockfile::Loaded(lockfile) => Ok(lockfile), MaybeLazyLockfile::Lazy(lazy) => lazy.get(), + MaybeLazyLockfile::Repair(lazy) => lazy.get_for_fix(), + } + } + + /// The intact lockfile used to restore projects outside a filtered repair. + pub fn get_for_merge(self) -> Result, LoadLockfileError> { + match self { + MaybeLazyLockfile::Loaded(lockfile) => Ok(lockfile), + MaybeLazyLockfile::Lazy(lazy) => lazy.get(), + MaybeLazyLockfile::Repair(lazy) => lazy.get().or_else(|_| lazy.get_for_fix_merge()), } } @@ -115,7 +188,9 @@ impl<'a> MaybeLazyLockfile<'a> { pub fn is_loaded_or_on_disk(self) -> bool { match self { MaybeLazyLockfile::Loaded(lockfile) => lockfile.is_some(), - MaybeLazyLockfile::Lazy(lazy) => lazy.is_loaded_or_on_disk(), + MaybeLazyLockfile::Lazy(lazy) | MaybeLazyLockfile::Repair(lazy) => { + lazy.is_loaded_or_on_disk() + } } } @@ -128,6 +203,7 @@ impl<'a> MaybeLazyLockfile<'a> { match self { MaybeLazyLockfile::Loaded(_) => Ok(None), MaybeLazyLockfile::Lazy(lazy) => lazy.pre_merge_importers(), + MaybeLazyLockfile::Repair(lazy) => lazy.pre_merge_importers_for_fix(), } } } diff --git a/pnpm/crates/lockfile/src/lazy_lockfile/tests.rs b/pnpm/crates/lockfile/src/lazy_lockfile/tests.rs index 15210e2469..68dccc3b37 100644 --- a/pnpm/crates/lockfile/src/lazy_lockfile/tests.rs +++ b/pnpm/crates/lockfile/src/lazy_lockfile/tests.rs @@ -1,6 +1,7 @@ use super::{LazyLockfile, MaybeLazyLockfile}; use crate::{Lockfile, WantedLockfileSelection}; use std::fs; +use text_block_macros::text_block; fn minimal_lockfile() -> Lockfile { serde_saphyr::from_str("lockfileVersion: '9.0'\n").expect("parse a minimal lockfile") @@ -45,6 +46,122 @@ fn deferred_loads_from_the_given_dir_not_the_process_cwd() { assert!(lazy.get().expect("absent lockfile loads as None").is_none()); } +#[test] +fn normal_load_does_not_fill_the_repair_cache() { + let dir = tempfile::tempdir().expect("tempdir"); + fs::write( + dir.path().join(Lockfile::FILE_NAME), + text_block! { + "lockfileVersion: '9.0'" + "importers:" + " .: {}" + "packages:" + " pkg@1.0.0:" + " resolution: {integrity: sha512-TIE61hcgbI/SlJh/0c1sT1SZbBlpg7WiZcs65WPJhoIZQPhH1SCpcGA7LgrVXT15lwN3HV4GQM/MJ9aKEn3Qfg==}" + " deprecated: stale" + }, + ) + .expect("write pnpm-lock.yaml"); + + let lazy = LazyLockfile::deferred(dir.path().to_path_buf(), WantedLockfileSelection::default()); + let normal = lazy.get().expect("normal load succeeds").expect("normal lockfile"); + let package_key = "pkg@1.0.0".parse().expect("package key"); + assert_eq!( + normal + .packages + .as_ref() + .and_then(|packages| packages.get(&package_key)) + .and_then(|metadata| metadata.deprecated.as_deref()), + Some("stale"), + ); + + let repaired = lazy.get_for_fix().expect("repair load succeeds").expect("repair lockfile"); + assert!( + repaired + .packages + .as_ref() + .and_then(|packages| packages.get(&package_key)) + .is_some_and(|metadata| metadata.deprecated.is_none()), + ); + + let merge = MaybeLazyLockfile::Repair(&lazy) + .get_for_merge() + .expect("merge load succeeds") + .expect("merge lockfile"); + assert_eq!( + merge + .packages + .as_ref() + .and_then(|packages| packages.get(&package_key)) + .and_then(|metadata| metadata.deprecated.as_deref()), + Some("stale"), + ); +} + +#[test] +fn repair_merge_preserves_valid_metadata_when_strict_parsing_fails() { + let dir = tempfile::tempdir().expect("tempdir"); + fs::write( + dir.path().join(Lockfile::FILE_NAME), + text_block! { + "lockfileVersion: '9.0'" + "settings: invalid" + "importers:" + " .: {}" + "packages:" + " pkg@1.0.0:" + " resolution: {integrity: sha512-TIE61hcgbI/SlJh/0c1sT1SZbBlpg7WiZcs65WPJhoIZQPhH1SCpcGA7LgrVXT15lwN3HV4GQM/MJ9aKEn3Qfg==}" + " deprecated: stale" + "snapshots:" + " pkg@1.0.0:" + " transitivePeerDependencies: [peer]" + " optional: true" + }, + ) + .expect("write pnpm-lock.yaml"); + + let lazy = LazyLockfile::deferred(dir.path().to_path_buf(), WantedLockfileSelection::default()); + assert!(lazy.get().is_err(), "strict parsing must reject the malformed settings"); + + let package_key = "pkg@1.0.0".parse().expect("package key"); + let repaired = lazy.get_for_fix().expect("repair load succeeds").expect("repair lockfile"); + assert!(repaired.settings.is_none()); + assert!( + repaired + .packages + .as_ref() + .and_then(|packages| packages.get(&package_key)) + .is_some_and(|metadata| metadata.deprecated.is_none()), + ); + assert!( + repaired.snapshots.as_ref().and_then(|snapshots| snapshots.get(&package_key)).is_some_and( + |snapshot| { !snapshot.optional && snapshot.transitive_peer_dependencies.is_none() } + ), + ); + + let merge = MaybeLazyLockfile::Repair(&lazy) + .get_for_merge() + .expect("merge load succeeds") + .expect("merge lockfile"); + assert!(merge.settings.is_none()); + assert_eq!( + merge + .packages + .as_ref() + .and_then(|packages| packages.get(&package_key)) + .and_then(|metadata| metadata.deprecated.as_deref()), + Some("stale"), + ); + assert!( + merge.snapshots.as_ref().and_then(|snapshots| snapshots.get(&package_key)).is_some_and( + |snapshot| { + snapshot.optional + && snapshot.transitive_peer_dependencies.as_deref() == Some(&["peer".into()]) + } + ), + ); +} + #[test] fn empty_and_env_only_files_count_as_absent() { let dir = tempfile::tempdir().expect("tempdir"); diff --git a/pnpm/crates/lockfile/src/lib.rs b/pnpm/crates/lockfile/src/lib.rs index fc1c0bbf97..9fc33731ec 100644 --- a/pnpm/crates/lockfile/src/lib.rs +++ b/pnpm/crates/lockfile/src/lib.rs @@ -226,6 +226,33 @@ impl Lockfile { /// The key used to refer to the root project inside `importers`. pub const ROOT_IMPORTER_KEY: &str = "."; + /// Keep only the lockfile fields that seed a repairing resolution. + pub fn prepare_for_fix(&mut self) { + if let Some(packages) = self.packages.as_mut() { + for metadata in packages.values_mut() { + metadata.version = None; + metadata.engines = None; + metadata.cpu = None; + metadata.os = None; + metadata.libc = None; + metadata.deprecated = None; + metadata.has_bin = None; + metadata.prepare = None; + metadata.bundled_dependencies = None; + metadata.peer_dependencies = None; + metadata.peer_dependencies_meta = None; + } + } + if let Some(snapshots) = self.snapshots.as_mut() { + for snapshot in snapshots.values_mut() { + snapshot.id = None; + snapshot.transitive_peer_dependencies = None; + snapshot.patched = None; + snapshot.optional = false; + } + } + } + /// Convenience accessor for the root project's snapshot. #[must_use] pub fn root_project(&self) -> Option<&'_ ProjectSnapshot> { diff --git a/pnpm/crates/lockfile/src/load_lockfile.rs b/pnpm/crates/lockfile/src/load_lockfile.rs index b7e391a429..a98ebb97c9 100644 --- a/pnpm/crates/lockfile/src/load_lockfile.rs +++ b/pnpm/crates/lockfile/src/load_lockfile.rs @@ -1,4 +1,7 @@ -use crate::{Lockfile, ProjectSnapshot, extract_main_document, merge_lockfile_changes}; +use crate::{ + Lockfile, LockfileResolution, ProjectSnapshot, SnapshotEntry, extract_main_document, + merge_lockfile_changes, +}; use derive_more::{Display, Error}; use pipe_trait::Pipe; use pnpm_diagnostics::miette::{self, Diagnostic}; @@ -106,14 +109,42 @@ impl Lockfile { pub fn load_wanted_detailed( dir: &Path, selection: &WantedLockfileSelection, + ) -> Result { + Self::load_wanted_detailed_with(dir, selection, WantedLockfileLoadMode::Strict) + } + + pub(crate) fn load_wanted_detailed_for_fix( + dir: &Path, + selection: &WantedLockfileSelection, + ) -> Result { + Self::load_wanted_detailed_with(dir, selection, WantedLockfileLoadMode::RepairSeed) + } + + pub(crate) fn load_wanted_detailed_for_fix_merge( + dir: &Path, + selection: &WantedLockfileSelection, + ) -> Result { + Self::load_wanted_detailed_with(dir, selection, WantedLockfileLoadMode::RepairMerge) + } + + fn load_wanted_detailed_with( + dir: &Path, + selection: &WantedLockfileSelection, + mode: WantedLockfileLoadMode, ) -> Result { for file_name in selection.read_order() { - let Some(lockfile) = Self::load_from_path(&dir.join(file_name))? else { + let path = dir.join(file_name); + let loaded = match mode { + WantedLockfileLoadMode::Strict => Self::load_from_path(&path), + WantedLockfileLoadMode::RepairSeed => Self::load_from_path_for_fix(&path, true), + WantedLockfileLoadMode::RepairMerge => Self::load_from_path_for_fix(&path, false), + }?; + let Some(lockfile) = loaded else { continue; }; return if selection.merge_git_branch_lockfiles { let pre_merge_importers = lockfile.importers.clone(); - let merged = merge_git_branch_lockfiles(lockfile, dir)?; + let merged = merge_git_branch_lockfiles(lockfile, dir, mode)?; Ok(LoadedWantedLockfile { lockfile: Some(merged), pre_merge_importers: Some(pre_merge_importers), @@ -192,6 +223,43 @@ impl Lockfile { .map_err(|source| LoadLockfileError::parse_yaml(file_path, &source)) } + fn parse_for_fix( + content: &str, + file_path: &Path, + prepare: bool, + ) -> Result, LoadLockfileError> { + let main = extract_main_document(content); + if main.trim().is_empty() { + return Ok(None); + } + let mut value = serde_saphyr::from_str_with_options::( + &main, + serde_saphyr::options! { + budget: serde_saphyr::budget! { + max_events: main.len().max(DEFAULT_YAML_MAX_EVENTS), + max_nodes: main.len().max(DEFAULT_YAML_MAX_NODES), + max_total_scalar_bytes: main.len().max(DEFAULT_YAML_MAX_SCALAR_BYTES), + max_total_comment_bytes: main.len().max(DEFAULT_YAML_MAX_SCALAR_BYTES), + max_reader_input_bytes: Some(main.len().max(DEFAULT_YAML_MAX_READER_INPUT_BYTES)), + }, + }, + ) + .map_err(|source| LoadLockfileError::parse_yaml(file_path, &source))?; + prepare_value_for_fix(&mut value); + serde_json::from_value::(value) + .map(|mut lockfile| { + lockfile.reconstruct_missing_directory_resolutions(); + if prepare { + lockfile.prepare_for_fix(); + } + Some(lockfile) + }) + .map_err(|source| LoadLockfileError::ParseYaml { + path: file_path.to_path_buf(), + reason: source.to_string(), + }) + } + /// Load a lockfile from an explicit path. Returns `Ok(None)` when the /// file is absent or its main document is empty, the same absence /// rules the directory-addressed loaders use. @@ -203,6 +271,18 @@ impl Lockfile { }; Self::parse(&content, file_path) } + + fn load_from_path_for_fix( + file_path: &Path, + prepare: bool, + ) -> Result, LoadLockfileError> { + let content = match fs::read_to_string(file_path) { + Ok(content) => content, + Err(error) if error.kind() == ErrorKind::NotFound => return Ok(None), + Err(error) => return error.pipe(LoadLockfileError::ReadFile).pipe(Err), + }; + Self::parse_for_fix(&content, file_path, prepare) + } } #[cfg(test)] @@ -215,7 +295,7 @@ mod tests; /// carried needs that "before": the merged lockfile no longer holds it, /// and only the fold's own additions may be reconciled away against the /// manifests. `pre_merge_importers` is `None` when no fold was attempted. -#[derive(Debug, Default)] +#[derive(Debug, Default, Clone)] pub struct LoadedWantedLockfile { pub lockfile: Option, pub pre_merge_importers: Option>, @@ -252,14 +332,96 @@ impl WantedLockfileSelection { } } -fn merge_git_branch_lockfiles(base: Lockfile, dir: &Path) -> Result { +fn merge_git_branch_lockfiles( + base: Lockfile, + dir: &Path, + mode: WantedLockfileLoadMode, +) -> Result { let branch_lockfiles = Lockfile::git_branch_lockfiles(dir).map_err(LoadLockfileError::ReadFile)?; let mut merged = base; for path in branch_lockfiles { - if let Some(branch_lockfile) = Lockfile::load_from_path(&path)? { + let branch_lockfile = match mode { + WantedLockfileLoadMode::Strict => Lockfile::load_from_path(&path), + WantedLockfileLoadMode::RepairSeed => Lockfile::load_from_path_for_fix(&path, true), + WantedLockfileLoadMode::RepairMerge => Lockfile::load_from_path_for_fix(&path, false), + }?; + if let Some(branch_lockfile) = branch_lockfile { merged = merge_lockfile_changes(&merged, &branch_lockfile); } } Ok(merged) } + +#[derive(Clone, Copy)] +enum WantedLockfileLoadMode { + Strict, + RepairSeed, + RepairMerge, +} + +fn prepare_value_for_fix(value: &mut serde_json::Value) { + let Some(root) = value.as_object_mut() else { return }; + for key in [ + "settings", + "catalogs", + "overrides", + "packageExtensionsChecksum", + "pnpmfileChecksum", + "ignoredOptionalDependencies", + "patchedDependencies", + "time", + ] { + discard_invalid_generated_field(root, key); + } + if let Some(packages) = root.get_mut("packages").and_then(serde_json::Value::as_object_mut) { + packages.retain(|_, metadata| { + if serde_json::from_value::(metadata.clone()).is_ok() { + return true; + } + let Some(resolution) = metadata.get("resolution").cloned() else { return false }; + if serde_json::from_value::(resolution.clone()).is_err() { + return false; + } + *metadata = serde_json::json!({ "resolution": resolution }); + true + }); + } + if let Some(snapshots) = root.get_mut("snapshots").and_then(serde_json::Value::as_object_mut) { + for snapshot in snapshots.values_mut() { + if serde_json::from_value::(snapshot.clone()).is_ok() { + continue; + } + let dependencies = snapshot.get("dependencies").cloned(); + let optional_dependencies = snapshot.get("optionalDependencies").cloned(); + let mut retained = serde_json::Map::new(); + if let Some(dependencies) = dependencies { + retained.insert("dependencies".to_string(), dependencies); + } + if let Some(optional_dependencies) = optional_dependencies { + retained.insert("optionalDependencies".to_string(), optional_dependencies); + } + let candidate = serde_json::Value::Object(retained); + *snapshot = if serde_json::from_value::(candidate.clone()).is_ok() { + candidate + } else { + serde_json::json!({}) + }; + } + } +} + +fn discard_invalid_generated_field( + root: &mut serde_json::Map, + key: &str, +) { + let Some(value) = root.get(key).cloned() else { return }; + let mut candidate = serde_json::Map::from_iter([ + ("lockfileVersion".to_owned(), serde_json::json!("9.0")), + ("importers".to_owned(), serde_json::json!({})), + ]); + candidate.insert(key.to_owned(), value); + if serde_json::from_value::(serde_json::Value::Object(candidate)).is_err() { + root.remove(key); + } +} diff --git a/pnpm/crates/lockfile/src/load_lockfile/tests.rs b/pnpm/crates/lockfile/src/load_lockfile/tests.rs index 9e28a58d63..acebdb7904 100644 --- a/pnpm/crates/lockfile/src/load_lockfile/tests.rs +++ b/pnpm/crates/lockfile/src/load_lockfile/tests.rs @@ -1,6 +1,6 @@ use crate::{ - DirectoryResolution, ImporterDepVersion, Lockfile, LockfileResolution, PackageKey, PkgName, - SnapshotDepRef, + DirectoryResolution, ImporterDepVersion, LazyLockfile, Lockfile, LockfileResolution, + PackageKey, PkgName, SnapshotDepRef, WantedLockfileSelection, }; use pnpm_diagnostics::miette::Diagnostic; use pretty_assertions::assert_eq; @@ -87,6 +87,53 @@ fn parses_main_document_from_combined_yaml() { assert_eq!(combined_loaded, main_only_loaded); } +#[test] +fn fix_loader_discards_broken_and_derived_package_fields() { + let tmp = tempdir().expect("create tempdir"); + std::fs::write( + tmp.path().join(Lockfile::FILE_NAME), + text_block! { + "lockfileVersion: '9.0'" + "" + "settings: invalid" + "" + "importers:" + " .: {}" + "" + "packages:" + " broken@1.0.0:" + " engines: invalid" + " valid@1.0.0:" + " resolution: {integrity: sha512-TIE61hcgbI/SlJh/0c1sT1SZbBlpg7WiZcs65WPJhoIZQPhH1SCpcGA7LgrVXT15lwN3HV4GQM/MJ9aKEn3Qfg==}" + " engines: invalid" + " deprecated: stale" + "" + "snapshots:" + " broken@1.0.0: {}" + " valid@1.0.0:" + " dependencies:" + " child: 1.0.0" + " transitivePeerDependencies: invalid" + }, + ) + .expect("write wanted lockfile"); + + let lazy = LazyLockfile::deferred(tmp.path().to_path_buf(), WantedLockfileSelection::default()); + let lockfile = lazy.get_for_fix().expect("load for repair").expect("lockfile present"); + assert!(lockfile.settings.is_none()); + let packages = lockfile.packages.as_ref().expect("packages present"); + assert!(!packages.contains_key(&"broken@1.0.0".parse().expect("broken key"))); + let valid = packages.get(&"valid@1.0.0".parse().expect("valid key")).expect("valid entry"); + assert!(valid.engines.is_none()); + assert!(valid.deprecated.is_none()); + + let snapshots = lockfile.snapshots.as_ref().expect("snapshots present"); + let valid = + snapshots.get(&"valid@1.0.0".parse().expect("valid snapshot key")).expect("valid snapshot"); + assert!(valid.dependencies.as_ref().is_some_and(|deps| deps.len() == 1)); + assert!(valid.transitive_peer_dependencies.is_none()); +} + /// Regression test for : a /// combined lockfile checked out with CRLF line endings was handed to /// serde whole, failing as "multiple YAML documents detected" and diff --git a/pnpm/crates/package-manager/src/install/materialize.rs b/pnpm/crates/package-manager/src/install/materialize.rs index 2f93c8fd23..4cda1c3be3 100644 --- a/pnpm/crates/package-manager/src/install/materialize.rs +++ b/pnpm/crates/package-manager/src/install/materialize.rs @@ -17,6 +17,7 @@ pub(super) struct MaterializationInputs<'a, 'install> { pub(super) config: &'static Config, pub(super) manifest: &'a PackageManifest, pub(super) lockfile: Option<&'a Lockfile>, + pub(super) merge_wanted_lockfile: Option<&'a Lockfile>, pub(super) take_frozen_path: bool, pub(super) lockfile_verification_override: Option>, @@ -97,6 +98,7 @@ pub(super) async fn materialize( config, manifest, lockfile, + merge_wanted_lockfile, take_frozen_path, lockfile_verification_override, resolution_verifiers, @@ -405,6 +407,7 @@ pub(super) async fn materialize( // entries keep their pins on rewrite (the `update: false` // mode). State 4 (no lockfile) passes `None`. wanted_lockfile: lockfile, + merge_wanted_lockfile, node_version: effective_node_version, node_linker, supported_architectures, @@ -429,6 +432,7 @@ pub(super) async fn materialize( prior_hoisted_dependencies, prune_orphans, manifest_spec_bumps, + resolution_verifiers: &resolution_verifiers, lockfile_verification_gate, } .run::() diff --git a/pnpm/crates/package-manager/src/install/run.rs b/pnpm/crates/package-manager/src/install/run.rs index 90086ddcb8..7a4e0fe382 100644 --- a/pnpm/crates/package-manager/src/install/run.rs +++ b/pnpm/crates/package-manager/src/install/run.rs @@ -481,9 +481,10 @@ where // below: a load that failed leaves nothing cached, so asking later // would retry it and turn a lockfile this arm chose to ignore into // a fatal one. - let (lockfile, pre_merge_importers) = match lockfile_source.get() { + let (lockfile, merge_wanted_lockfile, pre_merge_importers) = match lockfile_source.get() { Ok(lockfile) => ( lockfile, + lockfile_source.get_for_merge().map_err(InstallError::LoadWantedLockfile)?, lockfile_source.pre_merge_importers().map_err(InstallError::LoadWantedLockfile)?, ), Err(error) if !frozen_lockfile => { @@ -495,7 +496,7 @@ where ), prefix: prefix.clone(), })); - (None, None) + (None, None, None) } Err(error) => return Err(InstallError::LoadWantedLockfile(error)), }; @@ -1093,6 +1094,7 @@ where config, manifest, lockfile, + merge_wanted_lockfile, take_frozen_path, lockfile_verification_override, resolution_verifiers, diff --git a/pnpm/crates/package-manager/src/install_with_fresh_lockfile.rs b/pnpm/crates/package-manager/src/install_with_fresh_lockfile.rs index 1ce186afb7..32879e1a1b 100644 --- a/pnpm/crates/package-manager/src/install_with_fresh_lockfile.rs +++ b/pnpm/crates/package-manager/src/install_with_fresh_lockfile.rs @@ -25,6 +25,7 @@ use pnpm_resolving_deps_resolver::{ ManifestHook, ResolveDependencyTreeError, UpdateDepth, UpdateTargets, }; use pnpm_resolving_npm_resolver::{InMemoryPackageMetaCache, MergeNamedRegistriesError}; +use pnpm_resolving_resolver_base::ResolutionVerifier; use pnpm_store_dir::SharedVerifiedFilesCache; use pnpm_tarball::{MemCache, SharedReportedProgressKeys}; use std::{ @@ -134,6 +135,9 @@ pub struct InstallWithFreshLockfile<'a, DependencyGroupList> { /// pins. `None` on the no-lockfile path. Corresponds to the /// `update: false` resolver mode. pub wanted_lockfile: Option<&'a Lockfile>, + /// Intact prior lockfile used to restore unselected projects after a + /// filtered repair resolves against a sanitized seed. + pub merge_wanted_lockfile: Option<&'a Lockfile>, /// Effective `nodeVersion`: an explicit config value, otherwise the /// minimum version declared by the root manifest's runtime engine. pub node_version: Option, @@ -238,6 +242,9 @@ pub struct InstallWithFreshLockfile<'a, DependencyGroupList> { /// versions it resolves, and the sink it reports them back through. /// `None` for every other install. pub manifest_spec_bumps: Option<&'a crate::ManifestSpecBumps>, + /// Resolution policies used to validate a filtered repair after the + /// sanitized merge view has been spliced into the freshly resolved graph. + pub resolution_verifiers: &'a [Arc], /// The pre-resolve verification of the existing lockfile, running in /// the background while this install resolves and materializes. The /// verdict is awaited before bin linking, dependency builds, and the @@ -272,6 +279,8 @@ pub enum UpdateSeedPolicy { /// `pacquet dedupe` uses this to preserve valid pins while rebuilding /// the graph around the fewest compatible versions. KeepAllResolveAll, + /// Preserve locked versions while regenerating all derived lockfile data. + FixLockfile, /// Re-resolve every registry edge at its locked version using fresh /// metadata. `pacquet update --patches` uses this to pick the registry's /// current revision without allowing semver movement. @@ -338,6 +347,7 @@ impl UpdateSeedPolicy { match self { UpdateSeedPolicy::KeepAll | UpdateSeedPolicy::KeepAllResolveAll + | UpdateSeedPolicy::FixLockfile | UpdateSeedPolicy::RefreshRevisions => UpdateDepth::UNLIMITED, UpdateSeedPolicy::DropAll { max_depth } | UpdateSeedPolicy::DropOnly { max_depth, .. } @@ -362,9 +372,9 @@ fn update_reuse_scopes( match policy { UpdateSeedPolicy::KeepAll => (UpdateReuseScope::All, BTreeMap::new()), - UpdateSeedPolicy::KeepAllResolveAll | UpdateSeedPolicy::RefreshRevisions => { - (UpdateReuseScope::None, BTreeMap::new()) - } + UpdateSeedPolicy::KeepAllResolveAll + | UpdateSeedPolicy::FixLockfile + | UpdateSeedPolicy::RefreshRevisions => (UpdateReuseScope::None, BTreeMap::new()), UpdateSeedPolicy::DropAll { .. } => (UpdateReuseScope::None, BTreeMap::new()), UpdateSeedPolicy::DropOnly { targets, .. } => { (UpdateReuseScope::Except(targets.clone()), BTreeMap::new()) @@ -750,6 +760,7 @@ impl InstallWithFreshLockfile<'_, DependencyGroupList> { workspace_packages, update_checksums, wanted_lockfile, + merge_wanted_lockfile, node_version, meta_cache, node_linker, @@ -775,6 +786,7 @@ impl InstallWithFreshLockfile<'_, DependencyGroupList> { prune_orphans, save_lockfile, manifest_spec_bumps, + resolution_verifiers, mut lockfile_verification_gate, } = self; @@ -795,6 +807,8 @@ impl InstallWithFreshLockfile<'_, DependencyGroupList> { let link_options = crate::shim_link_options(config, node_linker); let filtered_isolated = is_partial_workspace_selection(real_importer_ids, selected_importer_ids) && !is_hoisted; + let verify_filtered_repair = matches!(update_seed_policy, UpdateSeedPolicy::FixLockfile) + && is_partial_workspace_selection(real_importer_ids, selected_importer_ids); // Materialise the caller's iterator into a `Vec` so the same // group set can be replayed into both the resolver (consumes // the iterator) and `SymlinkDirectDependencies` (needs to walk @@ -935,6 +949,16 @@ impl InstallWithFreshLockfile<'_, DependencyGroupList> { .collect() }; + let fixed_wanted_lockfile = if matches!(update_seed_policy, UpdateSeedPolicy::FixLockfile) { + wanted_lockfile.cloned().map(|mut lockfile| { + lockfile.prepare_for_fix(); + lockfile + }) + } else { + None + }; + let wanted_lockfile = fixed_wanted_lockfile.as_ref().or(wanted_lockfile); + let (preferred_versions_seed, preferred_versions_seeds_by_importer) = resolve::preferred_versions_seeds( &update_seed_policy, @@ -1021,7 +1045,7 @@ impl InstallWithFreshLockfile<'_, DependencyGroupList> { // Withheld when `dedupe_injected_deps` is off, since the guard only // compensates for that pass not running on every re-resolution path. let guard_previous_importers: Option<&HashMap> = - wanted_lockfile + merge_wanted_lockfile .filter(|_| config.dedupe_injected_deps) .map(|lockfile| &lockfile.importers); let guard_update_reuse_scope = update_reuse_scope.clone(); @@ -1132,7 +1156,7 @@ impl InstallWithFreshLockfile<'_, DependencyGroupList> { None => true, Some(selected_importer_ids) => { !is_partial_workspace_selection(real_importer_ids, Some(selected_importer_ids)) - && wanted_lockfile.is_some_and(|wanted_lockfile| { + && merge_wanted_lockfile.is_some_and(|wanted_lockfile| { wanted_lockfile.importers.len() == selected_importer_ids.len() }) } @@ -1258,6 +1282,7 @@ impl InstallWithFreshLockfile<'_, DependencyGroupList> { update_reuse_scope: guard_update_reuse_scope.clone(), update_reuse_scopes_by_importer: guard_update_reuse_scopes_by_importer.clone(), wanted_lockfile, + merge_wanted_lockfile, real_importer_ids, selected_importer_ids, lockfile_dir, @@ -1265,6 +1290,9 @@ impl InstallWithFreshLockfile<'_, DependencyGroupList> { manifest_spec_bumps, versions_overrider: versions_overrider.as_deref(), })?; + if verify_filtered_repair { + verify_merged_repair::(&built_lockfile, resolution_verifiers).await?; + } return finish_lockfile_only::(LockfileOnlyOptions { built_lockfile, config, @@ -1301,6 +1329,7 @@ impl InstallWithFreshLockfile<'_, DependencyGroupList> { update_reuse_scope: guard_update_reuse_scope.clone(), update_reuse_scopes_by_importer: guard_update_reuse_scopes_by_importer.clone(), wanted_lockfile, + merge_wanted_lockfile, real_importer_ids, selected_importer_ids, lockfile_dir, @@ -1308,6 +1337,12 @@ impl InstallWithFreshLockfile<'_, DependencyGroupList> { manifest_spec_bumps, versions_overrider: versions_overrider.as_deref(), })?; + if verify_filtered_repair { + if let Some(gate) = lockfile_verification_gate.take() { + gate.wait().await.map_err(InstallWithFreshLockfileError::LockfileVerification)?; + } + verify_merged_repair::(&built_lockfile, resolution_verifiers).await?; + } tracing::info!( target: "pacquet::install::phase", phase = "build_fresh_lockfile", @@ -1913,6 +1948,19 @@ struct LockfileOnlyOptions<'a> { writer_task: tokio::task::JoinHandle>, } +async fn verify_merged_repair( + lockfile: &Lockfile, + resolution_verifiers: &[Arc], +) -> Result<(), InstallWithFreshLockfileError> { + pnpm_lockfile_verification::verify_lockfile_resolutions::( + lockfile, + resolution_verifiers, + &pnpm_lockfile_verification::VerifyLockfileResolutionsOptions::default(), + ) + .await + .map_err(InstallWithFreshLockfileError::LockfileVerification) +} + /// Tail of the `--lockfile-only` path: persist the freshly-built /// lockfile, close the store-index writer, and report the install done. /// @@ -2093,6 +2141,8 @@ struct FreshLockfileBuildOptions<'a> { /// The previous run's lockfile, spliced back over the importers a /// filtered install didn't resolve. `None` on a first install. wanted_lockfile: Option<&'a Lockfile>, + /// Intact prior lockfile used when splicing back unselected importers. + merge_wanted_lockfile: Option<&'a Lockfile>, /// Every importer the workspace declares, and the subset this run /// resolved. Both `Some` and unequal means the install is filtered, /// so the unselected importers keep their previous entries. @@ -2116,7 +2166,7 @@ struct FreshLockfileBuildOptions<'a> { fn build_lockfile( opts: FreshLockfileBuildOptions<'_>, ) -> Result { - let wanted_lockfile = opts.wanted_lockfile; + let wanted_lockfile = opts.merge_wanted_lockfile; let real_importer_ids = opts.real_importer_ids; let selected_importer_ids = opts.selected_importer_ids; let lockfile_dir = opts.lockfile_dir; @@ -2158,6 +2208,7 @@ fn build_fresh_lockfile( ) -> Result { let FreshLockfileBuildOptions { wanted_lockfile, + merge_wanted_lockfile: _, real_importer_ids: _, selected_importer_ids: _, lockfile_dir: _, diff --git a/pnpm/crates/package-manager/src/install_with_fresh_lockfile/resolve.rs b/pnpm/crates/package-manager/src/install_with_fresh_lockfile/resolve.rs index 7c56b9f07b..fe88884364 100644 --- a/pnpm/crates/package-manager/src/install_with_fresh_lockfile/resolve.rs +++ b/pnpm/crates/package-manager/src/install_with_fresh_lockfile/resolve.rs @@ -59,6 +59,7 @@ pub(super) fn preferred_versions_seeds( let mut workspace_seed = match update_seed_policy { UpdateSeedPolicy::KeepAll | UpdateSeedPolicy::KeepAllResolveAll + | UpdateSeedPolicy::FixLockfile | UpdateSeedPolicy::RefreshRevisions | UpdateSeedPolicy::ByImporter { .. } => from_lockfile(snapshots, manifests.as_slice()), UpdateSeedPolicy::DropAll { .. } => from_lockfile(None, manifests.as_slice()), diff --git a/pnpm/crates/package-manager/src/install_with_fresh_lockfile/tests.rs b/pnpm/crates/package-manager/src/install_with_fresh_lockfile/tests.rs index aca53d6df5..50c927983c 100644 --- a/pnpm/crates/package-manager/src/install_with_fresh_lockfile/tests.rs +++ b/pnpm/crates/package-manager/src/install_with_fresh_lockfile/tests.rs @@ -1,10 +1,12 @@ use super::{ ImporterUpdateSeedPolicy, UpdateSeedPolicy, compute_package_extensions_checksum, full_resolution_required, include_transitive_optional_dependencies, - is_partial_workspace_selection, update_reuse_scopes, + is_partial_workspace_selection, update_reuse_scopes, verify_merged_repair, }; use pnpm_config::{Config, PackageExtension}; +use pnpm_lockfile::Lockfile; use pnpm_package_manifest::DependencyGroup; +use pnpm_reporter::SilentReporter; use pretty_assertions::assert_eq; fn config_with_extensions(entries: &[(&str, &[(&str, &str)])]) -> Box { @@ -45,6 +47,24 @@ fn partial_installs_keep_transitive_optional_dependencies() { assert!(include_transitive_optional_dependencies(true, &with_optional)); } +#[tokio::test] +async fn filtered_repair_verifies_the_merged_lockfile() { + let lockfile: Lockfile = serde_saphyr::from_str( + "lockfileVersion: '9.0'\nimporters:\n unselected:\n dependencies:\n '../../../escape':\n specifier: 1.0.0\n version: 1.0.0\n", + ) + .expect("parse lockfile"); + + let error = verify_merged_repair::(&lockfile, &[]) + .await + .expect_err("the merged lockfile must pass structural verification"); + assert!(matches!( + error, + super::InstallWithFreshLockfileError::LockfileVerification( + pnpm_lockfile_verification::VerifyError::InvalidDependencyAlias { .. } + ) + )); +} + /// Ports `installing/.../packageExtensions.ts:103-153` /// `packageExtensionsChecksum does not change regardless of keys /// order` — two `Config::package_extensions` populated with the diff --git a/pnpm/crates/package-manager/src/update.rs b/pnpm/crates/package-manager/src/update.rs index cdaa63c599..c58c457966 100644 --- a/pnpm/crates/package-manager/src/update.rs +++ b/pnpm/crates/package-manager/src/update.rs @@ -1254,7 +1254,9 @@ async fn prepare_selected_manifests( } match prepared.seed_policy { UpdateSeedPolicy::KeepAll => {} - UpdateSeedPolicy::KeepAllResolveAll | UpdateSeedPolicy::RefreshRevisions => { + UpdateSeedPolicy::KeepAllResolveAll + | UpdateSeedPolicy::FixLockfile + | UpdateSeedPolicy::RefreshRevisions => { unreachable!("manifest preparation never uses a whole-graph seed policy") } UpdateSeedPolicy::DropAll { .. } => { diff --git a/pnpm/crates/pnpr-client/src/lib.rs b/pnpm/crates/pnpr-client/src/lib.rs index 2d206f522a..f904e7969c 100644 --- a/pnpm/crates/pnpr-client/src/lib.rs +++ b/pnpm/crates/pnpr-client/src/lib.rs @@ -175,6 +175,8 @@ pub struct ResolveProjectsOptions { pub frozen_lockfile: bool, pub prefer_frozen_lockfile: Option, pub update_patches: bool, + /// Regenerate derived lockfile metadata while retaining compatible pins. + pub fix_lockfile: bool, pub ignore_manifest_check: bool, pub trust_lockfile: bool, /// See [`ResolveOptions::resolution_mode`]. @@ -213,6 +215,7 @@ impl From for ResolveProjectsOptions { frozen_lockfile: opts.frozen_lockfile, prefer_frozen_lockfile: opts.prefer_frozen_lockfile, update_patches: opts.update_patches, + fix_lockfile: false, ignore_manifest_check: opts.ignore_manifest_check, trust_lockfile: opts.trust_lockfile, resolution_mode: opts.resolution_mode, @@ -391,6 +394,8 @@ struct HandshakeCapability { versions: Vec, #[serde(default)] artifacts: Vec, + #[serde(default, rename = "fixLockfile")] + fix_lockfile: Vec, } impl PnprClient { @@ -411,6 +416,21 @@ impl PnprClient { /// no protocol version with this client. pub async fn handshake(&self) -> Result<(), PnprClientError> { let capability = self.fetch_handshake(None).await?; + Self::require_resolver_protocol(&capability) + } + + async fn handshake_fix_lockfile(&self) -> Result<(), PnprClientError> { + let capability = self.fetch_handshake(None).await?; + Self::require_resolver_protocol(&capability)?; + if !capability.fix_lockfile.contains(&PROTOCOL_VERSION) { + return Err(PnprClientError::Server(format!( + "pnpr server does not advertise lockfile repair support for resolver protocol v{PROTOCOL_VERSION}", + ))); + } + Ok(()) + } + + fn require_resolver_protocol(capability: &HandshakeCapability) -> Result<(), PnprClientError> { if !capability.versions.contains(&PROTOCOL_VERSION) { return Err(PnprClientError::Server(format!( "pnpr server speaks protocol versions {:?}, but this client requires v{PROTOCOL_VERSION}", @@ -731,6 +751,9 @@ impl PnprClient { opts: ResolveProjectsOptions, mut on_package: impl FnMut(ResolvedPackage), ) -> Result { + if opts.fix_lockfile { + self.handshake_fix_lockfile().await?; + } // The server's response is untrusted, and the caller merges the // returned lockfile into `pnpm-lock.yaml`. Constrain it to the // importers this request is about — the requested projects plus @@ -765,6 +788,7 @@ impl PnprClient { "frozenLockfile": opts.frozen_lockfile, "preferFrozenLockfile": opts.prefer_frozen_lockfile, "updatePatches": opts.update_patches, + "fixLockfile": opts.fix_lockfile, "ignoreManifestCheck": opts.ignore_manifest_check, "trustLockfile": opts.trust_lockfile, "resolutionMode": opts.resolution_mode, diff --git a/pnpm/crates/pnpr-client/src/tests.rs b/pnpm/crates/pnpr-client/src/tests.rs index e12e462f82..7de3e395ce 100644 --- a/pnpm/crates/pnpr-client/src/tests.rs +++ b/pnpm/crates/pnpr-client/src/tests.rs @@ -30,6 +30,59 @@ async fn artifact_handshake_times_out() { server.abort(); } +#[tokio::test] +async fn lockfile_repair_rejects_a_server_without_the_capability() { + let mut options = resolve_projects_options(); + options.fix_lockfile = true; + options.update_patches = false; + let mut server = mockito::Server::new_async().await; + let handshake_mock = server + .mock("GET", "/-/pnpr") + .with_status(200) + .with_header("content-type", "application/json") + .with_body(r#"{"pnpr":{"versions":[0]}}"#) + .create_async() + .await; + + let Err(error) = PnprClient::new(server.url()).resolve_projects(options).await else { + panic!("an older server must not silently ignore repair mode"); + }; + + assert!(error.to_string().contains("does not advertise lockfile repair support")); + handshake_mock.assert_async().await; +} + +#[tokio::test] +async fn lockfile_repair_uses_an_advertised_capability() { + let mut options = resolve_projects_options(); + options.fix_lockfile = true; + options.update_patches = false; + let response_lockfile = matching_transform_lockfile(&options); + let mut server = mockito::Server::new_async().await; + let handshake_mock = server + .mock("GET", "/-/pnpr") + .with_status(200) + .with_header("content-type", "application/json") + .with_body(r#"{"pnpr":{"versions":[0],"fixLockfile":[0]}}"#) + .create_async() + .await; + let resolve_mock = server + .mock("POST", "/-/pnpr/v0/resolve") + .match_body(mockito::Matcher::PartialJson(json!({ "fixLockfile": true }))) + .with_header(PROJECT_TRANSFORMS_HEADER, PROJECT_TRANSFORMS_VERSION) + .with_body(format!("{}\n", json!({ "type": "done", "lockfile": response_lockfile }))) + .create_async() + .await; + + let _ = PnprClient::new(server.url()) + .resolve_projects(options) + .await + .expect("the advertised repair request succeeds"); + + handshake_mock.assert_async().await; + resolve_mock.assert_async().await; +} + /// The request body is the whole contract with the server: a field the /// client omits is not defaulted server-side but cleared, so the server /// resolves under a policy the user never configured and cannot see @@ -188,6 +241,7 @@ fn resolve_projects_options() -> ResolveProjectsOptions { frozen_lockfile: false, prefer_frozen_lockfile: None, update_patches: true, + fix_lockfile: false, ignore_manifest_check: false, trust_lockfile: true, resolution_mode: ResolutionMode::TimeBased, diff --git a/pnpm11/installing/deps-installer/test/install/fixLockfile.ts b/pnpm11/installing/deps-installer/test/install/fixLockfile.ts index d1a32d491e..76b9276bf9 100644 --- a/pnpm11/installing/deps-installer/test/install/fixLockfile.ts +++ b/pnpm11/installing/deps-installer/test/install/fixLockfile.ts @@ -240,6 +240,49 @@ test('--fix-lockfile should preserve all locked dependencies version', async () }) }) +test('--fix-lockfile preserves unselected optional snapshots in a filtered workspace install', async () => { + const selectedManifest = { + name: 'selected', + version: '1.0.0', + dependencies: { 'is-positive': '1.0.0' }, + } + const unselectedManifest = { + name: 'unselected', + version: '1.0.0', + optionalDependencies: { '@pnpm.e2e/pkg-with-1-dep': '100.0.0' }, + } + preparePackages([ + { location: 'selected', package: selectedManifest }, + { location: 'unselected', package: unselectedManifest }, + ]) + + const selectedRoot = path.resolve('selected') as ProjectRootDir + const unselectedRoot = path.resolve('unselected') as ProjectRootDir + const allProjects = [ + { buildIndex: 0, manifest: selectedManifest, rootDir: selectedRoot }, + { buildIndex: 0, manifest: unselectedManifest, rootDir: unselectedRoot }, + ] + await mutateModules([ + { mutation: 'install', rootDir: selectedRoot }, + { mutation: 'install', rootDir: unselectedRoot }, + ], testDefaults({ allProjects, lockfileOnly: true })) + + const original = readYamlFileSync(WANTED_LOCKFILE) + const optionalSnapshotKeys = Object.entries(original.snapshots ?? {}) + .filter(([, snapshot]) => snapshot.optional === true) + .map(([key]) => key) + expect(optionalSnapshotKeys.length).toBeGreaterThan(0) + + await mutateModules([ + { mutation: 'install', rootDir: selectedRoot }, + ], testDefaults({ allProjects, fixLockfile: true, lockfileOnly: true })) + + const repaired = readYamlFileSync(WANTED_LOCKFILE) + for (const key of optionalSnapshotKeys) { + expect(repaired.snapshots?.[key]?.optional).toBe(true) + } +}) + test( '--fix-lockfile should install successfully when package has no dependencies but has peer dependencies with version like 1.0.0_@pnpm+y@1.0.0', async () => { diff --git a/pnpr/crates/pnpr/src/resolver/cache.rs b/pnpr/crates/pnpr/src/resolver/cache.rs index 5cb193ebad..fe736e6c80 100644 --- a/pnpr/crates/pnpr/src/resolver/cache.rs +++ b/pnpr/crates/pnpr/src/resolver/cache.rs @@ -173,10 +173,9 @@ pub(super) fn resolution_cache_key( config: &PacquetConfig, request: &ResolveRequest, ) -> Option { - // A revision refresh is an explicit request for current upstream state. - // Serving or storing it in the whole-resolution cache would make the next - // refresh repeat an old registry selection until the cache TTL expires. - if request.update_patches { + // Explicit metadata refreshes must not repeat an old registry selection + // from the whole-resolution cache. + if request.update_patches || request.fix_lockfile { return None; } let projects: Vec = request diff --git a/pnpr/crates/pnpr/src/resolver/protocol.rs b/pnpr/crates/pnpr/src/resolver/protocol.rs index 6a4357d272..ea7f4dc1ae 100644 --- a/pnpr/crates/pnpr/src/resolver/protocol.rs +++ b/pnpr/crates/pnpr/src/resolver/protocol.rs @@ -128,6 +128,10 @@ pub struct ResolveRequest { /// version. Omitted by older clients and false for ordinary resolves. #[serde(default)] pub update_patches: bool, + /// Re-resolve every edge while preserving compatible locked versions and + /// regenerating derived lockfile fields. + #[serde(default)] + pub fix_lockfile: bool, /// `ignoreManifestCheck`: skip the manifest ↔ lockfile freshness /// comparison during the frozen resolve. #[serde(default)] diff --git a/pnpr/crates/pnpr/src/resolver/resolve.rs b/pnpr/crates/pnpr/src/resolver/resolve.rs index b719345ab9..2f590d1cc7 100644 --- a/pnpr/crates/pnpr/src/resolver/resolve.rs +++ b/pnpr/crates/pnpr/src/resolver/resolve.rs @@ -200,9 +200,9 @@ pub async fn resolve( DependencyGroup::Optional, ], frozen_lockfile, - // Default to reuse so unchanged entries keep their pins; a revision - // refresh always re-resolves the pinned registry versions. - prefer_frozen_lockfile: if request.update_patches { + // Default to reuse so unchanged entries keep their pins; an explicit + // metadata refresh always re-resolves the pinned registry versions. + prefer_frozen_lockfile: if request.update_patches || request.fix_lockfile { Some(false) } else { request.prefer_frozen_lockfile.or(Some(true)) @@ -223,6 +223,8 @@ pub async fn resolve( persist_policy_excludes: false, update_seed_policy: if request.update_patches { pnpm_package_manager::UpdateSeedPolicy::RefreshRevisions + } else if request.fix_lockfile { + pnpm_package_manager::UpdateSeedPolicy::FixLockfile } else { pnpm_package_manager::UpdateSeedPolicy::KeepAll }, @@ -261,6 +263,7 @@ pub async fn resolve( /// unchanged. pub fn fresh_frozen_input_lockfile(config: &Config, request: &ResolveRequest) -> Option { if request.update_patches + || request.fix_lockfile || !request.frozen_lockfile || request.prefer_frozen_lockfile == Some(false) { diff --git a/pnpr/crates/pnpr/src/resolver/tests.rs b/pnpr/crates/pnpr/src/resolver/tests.rs index 07be7d6d45..a76f17da15 100644 --- a/pnpr/crates/pnpr/src/resolver/tests.rs +++ b/pnpr/crates/pnpr/src/resolver/tests.rs @@ -325,7 +325,7 @@ fn resolution_cache_key_changes_with_project_transforms() { } #[test] -fn revision_refresh_bypasses_the_resolution_cache() { +fn metadata_refreshes_bypass_the_resolution_cache() { let ordinary = ResolveRequest { dependencies: Some(deps(&[("foo", "1.0.0")])), ..ResolveRequest::default() @@ -335,9 +335,15 @@ fn revision_refresh_bypasses_the_resolution_cache() { update_patches: true, ..ResolveRequest::default() }; + let repair = ResolveRequest { + dependencies: ordinary.dependencies.clone(), + fix_lockfile: true, + ..ResolveRequest::default() + }; assert!(resolution_cache_key(&config(), &ordinary).is_some()); assert!(resolution_cache_key(&config(), &refresh).is_none()); + assert!(resolution_cache_key(&config(), &repair).is_none()); } #[test] @@ -353,6 +359,19 @@ fn update_patches_defaults_to_false_for_older_clients() { assert!(refresh.update_patches); } +#[test] +fn fix_lockfile_defaults_to_false_for_older_clients() { + let request = serde_json::from_value::(serde_json::json!({})) + .expect("legacy request parses"); + let repair = serde_json::from_value::(serde_json::json!({ + "fixLockfile": true + })) + .expect("repair request parses"); + + assert!(!request.fix_lockfile); + assert!(repair.fix_lockfile); +} + #[test] fn resolution_cache_key_hashes_input_lockfile_stably() { let first_lockfile: Lockfile = serde_saphyr::from_str( diff --git a/pnpr/crates/pnpr/src/server.rs b/pnpr/crates/pnpr/src/server.rs index 25dd8883f9..8391bf4c5f 100644 --- a/pnpr/crates/pnpr/src/server.rs +++ b/pnpr/crates/pnpr/src/server.rs @@ -2708,9 +2708,11 @@ async fn serve_ping(State(_state): State) -> Response { /// `GET /-/pnpr` — capability handshake for the pnpr resolver /// protocol. A plain npm registry has no such route and 404s, so a /// client can fail fast against a misconfigured server. `versions` -/// lists the `/-/pnpr/vN/resolve` protocol versions this server speaks. +/// lists the `/-/pnpr/vN/resolve` protocol versions this server speaks; +/// `fixLockfile` narrows that list to versions that honor repair requests. async fn serve_pnpr_handshake(State(state): State) -> Response { let versions = state.inner.config.resolver.enabled.then_some(0).into_iter().collect::>(); + let fix_lockfile = versions.clone(); let artifacts = state.inner.config.artifacts.enabled.then_some(0).into_iter().collect::>(); ( @@ -2719,6 +2721,7 @@ async fn serve_pnpr_handshake(State(state): State) -> Response { "pnpr": { "versions": versions, "artifacts": artifacts, + "fixLockfile": fix_lockfile, } })), ) diff --git a/pnpr/crates/pnpr/tests/server.rs b/pnpr/crates/pnpr/tests/server.rs index dab6b8b575..bfead59764 100644 --- a/pnpr/crates/pnpr/tests/server.rs +++ b/pnpr/crates/pnpr/tests/server.rs @@ -2782,6 +2782,10 @@ async fn resolver_only_serves_resolver_endpoints_and_refuses_registry_routes() { let handshake = app.clone().oneshot(Request::get("/-/pnpr").body(Body::empty()).unwrap()).await.unwrap(); assert_eq!(handshake.status(), StatusCode::OK); + assert_eq!( + body_json(handshake.into_body()).await, + json!({ "pnpr": { "versions": [0], "artifacts": [], "fixLockfile": [0] } }), + ); let verify = app .clone() @@ -2895,7 +2899,7 @@ async fn artifacts_only_advertises_and_mounts_only_the_artifact_protocol() { assert_eq!(handshake.status(), StatusCode::OK); assert_eq!( body_json(handshake.into_body()).await, - json!({ "pnpr": { "versions": [], "artifacts": [0] } }), + json!({ "pnpr": { "versions": [], "artifacts": [0], "fixLockfile": [] } }), ); let artifact = app