From cdb69caaf1b11d6cf70d7547ab948da8bf008d5d Mon Sep 17 00:00:00 2001 From: Zoltan Kochan Date: Tue, 27 Oct 2026 09:09:29 +0100 Subject: [PATCH] fix(install): relink renamed workspace bins with dedicated lockfiles (#17407) With sharedWorkspaceLockfile: false each project installs from its own lockfile and records its own workspace state, and the bins digest that proves an importer installed covered only the projects of that lockfile. A sibling's bin field was in none of them, so the repeat-install gate, the frozen-tree check and the run gate all reported a dependent up to date after the sibling renamed its command, and the dependent's .bin kept the old name. The digest now covers every workspace project in that case. The install takes them from the workspace the dedicated loop already discovered, and the run gate loads them, as it does for a shared lockfile. A state that an older version recorded has no such digest, so the first install after the upgrade is a full one in a workspace whose projects declare commands. The repeat-install oracle's renamed-bin scenario now runs with dedicated lockfiles too. Related to pnpm/tasks#224. --- ...dicated-lockfiles-relink-workspace-bins.md | 5 ++ .../cli/tests/suite/repeat_install_oracle.rs | 5 -- pnpm/crates/cli/tests/suite/workspace_bins.rs | 33 ++++++++++ pnpm/crates/package-manager/src/install.rs | 6 +- .../src/install/apply_materialization.rs | 5 +- .../src/install/prepare_modules_state.rs | 7 ++- .../install/prepare_modules_state/tests.rs | 8 ++- .../prepare_modules_state/up_to_date.rs | 7 ++- .../src/install/run/dispatch.rs | 1 + .../src/install/run/execution.rs | 1 + .../install/run/time_machine_capture/tests.rs | 1 + .../src/install/run/workspace.rs | 9 +++ .../src/install/state_options.rs | 7 +++ .../src/install/tests/repeat_install.rs | 15 ++++- .../src/install/workspace_state.rs | 2 +- .../src/install/workspace_state/discovery.rs | 60 +++++++++++++++---- .../src/install/workspace_state/write.rs | 15 ++++- .../src/optimistic_repeat_install.rs | 8 +++ .../optimistic_repeat_install/freshness.rs | 5 +- .../freshness/drift.rs | 6 +- .../manifest_source.rs | 10 +++- .../src/optimistic_repeat_install/tests.rs | 7 ++- .../tests/validation.rs | 2 + .../optimistic_repeat_install/validation.rs | 10 +++- .../validation/bins.rs | 41 +++++++++---- 25 files changed, 224 insertions(+), 52 deletions(-) create mode 100644 .changeset/dedicated-lockfiles-relink-workspace-bins.md diff --git a/.changeset/dedicated-lockfiles-relink-workspace-bins.md b/.changeset/dedicated-lockfiles-relink-workspace-bins.md new file mode 100644 index 0000000000..a66bd1ba02 --- /dev/null +++ b/.changeset/dedicated-lockfiles-relink-workspace-bins.md @@ -0,0 +1,5 @@ +--- +"pacquet": patch +--- + +`pnpm install` now relinks a command that a workspace project renames into the projects that depend on it when `sharedWorkspaceLockfile` is `false` [pnpm/tasks#224](https://github.com/pnpm/tasks/issues/224). diff --git a/pnpm/crates/cli/tests/suite/repeat_install_oracle.rs b/pnpm/crates/cli/tests/suite/repeat_install_oracle.rs index 80b1ac8516..7bd3e9afee 100644 --- a/pnpm/crates/cli/tests/suite/repeat_install_oracle.rs +++ b/pnpm/crates/cli/tests/suite/repeat_install_oracle.rs @@ -1006,11 +1006,6 @@ fn dedupe_peers_toggled(layout: Layout) { /// A workspace project's command renamed, which its dependent `c` links. fn a_workspace_bin_renamed(layout: Layout) { - // TODO: pnpm/tasks#224, an install with dedicated lockfiles does not - // relink a renamed workspace command. - if !layout.shared_lockfile { - return; - } let mut scenario = Scenario::new(layout); fs::create_dir_all(scenario.workspace().join("packages/a")).expect("create project a"); fs::write(scenario.workspace().join("packages/a/tool.js"), "#!/usr/bin/env node\n") diff --git a/pnpm/crates/cli/tests/suite/workspace_bins.rs b/pnpm/crates/cli/tests/suite/workspace_bins.rs index 1903b4d745..9d566ba54c 100644 --- a/pnpm/crates/cli/tests/suite/workspace_bins.rs +++ b/pnpm/crates/cli/tests/suite/workspace_bins.rs @@ -69,6 +69,39 @@ fn assert_relinked_bins(linker: &str, optimistic: &str) { assert_installed_bins(&root.join("node_modules/.bin"), &[]); } +/// Each project installs from its own lockfile, which records nothing about +/// a sibling's commands, so a command the sibling renames is relinked from +/// the sibling's manifest. +#[test] +fn dedicated_lockfiles_relink_a_renamed_workspace_bin() { + for linker in ["isolated", "hoisted"] { + for optimistic in ["true", "false"] { + assert_dedicated_lockfiles_relink(linker, optimistic); + } + } +} + +fn assert_dedicated_lockfiles_relink(linker: &str, optimistic: &str) { + let fixture = workspace(linker, optimistic); + let root = &fixture.workspace; + fs::OpenOptions::new() + .append(true) + .open(root.join("pnpm-workspace.yaml")) + .and_then(|mut yaml| io::Write::write_all(&mut yaml, b"sharedWorkspaceLockfile: false\n")) + .unwrap(); + for command in ["a-one", "a-renamed"] { + write_bins(root, &json!({ command: "one.js" })); + pnpm(root) + .args(["install", "--offline", "--ignore-scripts"]) + .assert() + .success(); + for pkg in ["packages/b", "packages/c"] { + assert_installed_bins(&root.join(pkg).join("node_modules/.bin"), &[command]); + } + assert_bin(root, "c", command, "one"); + } +} + #[test] fn hoisted_workspace_package_prunes_removed_bins_from_root() { for linker in ["isolated", "hoisted"] { diff --git a/pnpm/crates/package-manager/src/install.rs b/pnpm/crates/package-manager/src/install.rs index 6881223f64..09429db630 100644 --- a/pnpm/crates/package-manager/src/install.rs +++ b/pnpm/crates/package-manager/src/install.rs @@ -19,9 +19,9 @@ pub(crate) use modules_state::{ }; pub use run::{InstallExecution, InstallLockfilePolicy, ResolutionInputs}; pub(crate) use workspace_state::{ - CarriedSettings, StateWrite, configured_or_discovered_workspace_dir, lockfile_root_dir, - recorded_workspace_state, registry_keys_current, update_workspace_state_or_warn, - workspace_packages_for_freshness, + CarriedSettings, StateProjects, StateWrite, configured_or_discovered_workspace_dir, + lockfile_root_dir, recorded_workspace_state, registry_keys_current, + update_workspace_state_or_warn, workspace_packages_for_freshness, }; pub use workspace_state::{ UpToDateFastPathCheck, UpToDateWorkspace, build_workspace_packages_map, diff --git a/pnpm/crates/package-manager/src/install/apply_materialization.rs b/pnpm/crates/package-manager/src/install/apply_materialization.rs index 98e45aa626..cd299b1f92 100644 --- a/pnpm/crates/package-manager/src/install/apply_materialization.rs +++ b/pnpm/crates/package-manager/src/install/apply_materialization.rs @@ -381,7 +381,10 @@ fn write_applied_workspace_state( supported_architectures: inputs.projects.supported_architectures.as_ref(), }, catalogs: &inputs.completion.catalogs, - project_manifests: inputs.projects.importers.manifests, + projects: crate::install::StateProjects { + manifests: inputs.projects.importers.manifests, + workspace_projects: inputs.projects.importers.workspace_projects, + }, filesystem_now_ms: filesystem_now_ms_in_modules(&modules_dir), carried: applied_carried_settings(inputs), vouched: &vouched, diff --git a/pnpm/crates/package-manager/src/install/prepare_modules_state.rs b/pnpm/crates/package-manager/src/install/prepare_modules_state.rs index b5beae86a6..c2e426f06d 100644 --- a/pnpm/crates/package-manager/src/install/prepare_modules_state.rs +++ b/pnpm/crates/package-manager/src/install/prepare_modules_state.rs @@ -116,6 +116,7 @@ fn recorded_workspace<'a>( moved: tree_may_move(config, node_linker) && state.map_or(existing_tree, |state| recorded_elsewhere(state, projects)), projects, + bin_projects: projects, } } @@ -202,6 +203,7 @@ async fn report_prepared_up_to_date( projects: crate::install::state_options::InstallProjectMetadata { catalogs: inputs.projects.catalogs, manifests: inputs.projects.manifests, + workspace_projects: inputs.projects.workspace_projects, prefix: inputs.projects.prefix, workspace_packages: inputs.projects.workspace_packages, }, @@ -249,7 +251,10 @@ fn frozen_tree_inputs<'a>( lockfile: inputs.lockfiles.wanted, current_lockfile: inputs.lockfiles.current, modules_manifest, - recorded, + recorded: RecordedWorkspace { + bin_projects: inputs.projects.workspace_projects.unwrap_or(recorded.projects), + ..recorded + }, } } diff --git a/pnpm/crates/package-manager/src/install/prepare_modules_state/tests.rs b/pnpm/crates/package-manager/src/install/prepare_modules_state/tests.rs index 03f941a13f..f09637c1dc 100644 --- a/pnpm/crates/package-manager/src/install/prepare_modules_state/tests.rs +++ b/pnpm/crates/package-manager/src/install/prepare_modules_state/tests.rs @@ -186,7 +186,12 @@ fn short_circuits_over_a_bin( short_circuits_over( (&project_root, config, node_linker), &recorded_modules(config, node_linker), - RecordedWorkspace { state, moved: tree_moved, projects: &projects }, + RecordedWorkspace { + state, + moved: tree_moved, + projects: &projects, + bin_projects: &projects, + }, ) } @@ -228,6 +233,7 @@ fn short_circuits_over_allow_builds( state: Some(&WorkspaceState::default()), moved: false, projects: &projects, + bin_projects: &projects, }, ) } diff --git a/pnpm/crates/package-manager/src/install/prepare_modules_state/up_to_date.rs b/pnpm/crates/package-manager/src/install/prepare_modules_state/up_to_date.rs index 6176ed57d3..62c89cdbbc 100644 --- a/pnpm/crates/package-manager/src/install/prepare_modules_state/up_to_date.rs +++ b/pnpm/crates/package-manager/src/install/prepare_modules_state/up_to_date.rs @@ -99,7 +99,7 @@ pub(super) fn frozen_tree_up_to_date<'a>( fn workspace_bins_unchanged(context: &FrozenTreeUpToDate<'_>) -> bool { let Some(state) = context.recorded.state else { return false }; - let bins = workspace_bins_digest(context.tree.workspace_root, context.recorded.projects); + let bins = workspace_bins_digest(context.tree.workspace_root, context.recorded.bin_projects); context.recorded.projects .iter() .all(|(dir, manifest)| { @@ -283,7 +283,10 @@ pub(super) fn refresh_up_to_date_workspace( supported_architectures: context.supported_architectures, }, catalogs: context.projects.catalogs, - project_manifests: context.projects.manifests, + projects: crate::install::StateProjects { + manifests: context.projects.manifests, + workspace_projects: context.projects.workspace_projects, + }, filesystem_now_ms: filesystem_now_ms_in_modules( &context.tree.config.workspace_state_modules_dir(context.tree.workspace_root), ), diff --git a/pnpm/crates/package-manager/src/install/run/dispatch.rs b/pnpm/crates/package-manager/src/install/run/dispatch.rs index c3a4ca1053..381e9880be 100644 --- a/pnpm/crates/package-manager/src/install/run/dispatch.rs +++ b/pnpm/crates/package-manager/src/install/run/dispatch.rs @@ -174,6 +174,7 @@ pub(super) async fn prepare_dispatched_modules<'install, Reporter: self::Reporte projects: crate::install::state_options::InstallProjectMetadata { catalogs: &workspace.catalogs, manifests: project_manifests, + workspace_projects: scope.workspace_projects.as_deref(), prefix: &workspace.prefix, workspace_packages: workspace.workspace_packages.as_deref(), }, diff --git a/pnpm/crates/package-manager/src/install/run/execution.rs b/pnpm/crates/package-manager/src/install/run/execution.rs index 36ea28d8e5..2ea9a18adf 100644 --- a/pnpm/crates/package-manager/src/install/run/execution.rs +++ b/pnpm/crates/package-manager/src/install/run/execution.rs @@ -326,6 +326,7 @@ impl<'a> RunExecution<'a> { real_ids: &scope.importers.real_importer_ids, manifests: project_manifests, declared_manifests: &scope.project_manifests, + workspace_projects: scope.workspace_projects.as_deref(), ignore_manifest_check: self.install.lockfile_policy.ignore_manifest_check, } } diff --git a/pnpm/crates/package-manager/src/install/run/time_machine_capture/tests.rs b/pnpm/crates/package-manager/src/install/run/time_machine_capture/tests.rs index 95c8c51e91..21ea8807e4 100644 --- a/pnpm/crates/package-manager/src/install/run/time_machine_capture/tests.rs +++ b/pnpm/crates/package-manager/src/install/run/time_machine_capture/tests.rs @@ -40,6 +40,7 @@ fn captures_unselected_workspace_projects_reached_by_workspace_dependencies() { prune_stale_importers: false, project_scripts_current: false, dependency_manifests: None, + workspace_projects: None, }; assert_eq!( diff --git a/pnpm/crates/package-manager/src/install/run/workspace.rs b/pnpm/crates/package-manager/src/install/run/workspace.rs index c85c49cb16..032eb07dd7 100644 --- a/pnpm/crates/package-manager/src/install/run/workspace.rs +++ b/pnpm/crates/package-manager/src/install/run/workspace.rs @@ -86,6 +86,8 @@ pub(super) struct InstallScope<'w> { /// See [`crate::install::dependency_manifests_by_dir`]. The freshness /// checks compare an injected project's lockfile snapshot with these. pub(super) dependency_manifests: Option>, + /// See [`crate::RepeatInstallManifests::workspace_projects`]. + pub(super) workspace_projects: Option>, } impl<'a> InstallWorkspace<'a> { /// Consumes the catalogs and workspace-projects overrides off `owned`. @@ -227,12 +229,18 @@ impl<'w> InstallScope<'w> { workspace_projects_are_overridden, config: install.context.config, }); + let bin_projects = crate::optimistic_repeat_install::validation::dedicated_bin_projects( + install.context.config, + &project_manifests, + workspace_projects, + ); Self { project_manifests, importers, prune_stale_importers, project_scripts_current: false, dependency_manifests: dependency_manifests_by_dir(workspace_projects), + workspace_projects: bin_projects, } } @@ -282,6 +290,7 @@ impl<'w> InstallScope<'w> { }, manifests: crate::RepeatInstallManifests { dependency_manifests: self.dependency_manifests.as_ref(), + workspace_projects: self.workspace_projects.as_deref(), }, }, mutation: install.execution.mutation, diff --git a/pnpm/crates/package-manager/src/install/state_options.rs b/pnpm/crates/package-manager/src/install/state_options.rs index a052e8be22..37673f95b3 100644 --- a/pnpm/crates/package-manager/src/install/state_options.rs +++ b/pnpm/crates/package-manager/src/install/state_options.rs @@ -147,6 +147,8 @@ pub(crate) struct SelectedImporters<'a> { pub(crate) manifests: &'a [(PathBuf, &'a PackageManifest)], /// [`Self::manifests`] before `packageExtensions` and `readPackage`. pub(crate) declared_manifests: &'a [(PathBuf, &'a PackageManifest)], + /// See [`crate::RepeatInstallManifests::workspace_projects`]. + pub(crate) workspace_projects: Option<&'a [(PathBuf, &'a PackageManifest)]>, pub(crate) ignore_manifest_check: bool, } @@ -160,6 +162,8 @@ pub(crate) struct LockfileVerificationInputs<'a, 'install> { pub(crate) struct InstallProjectMetadata<'a> { pub(crate) catalogs: &'a Catalogs, pub(crate) manifests: &'a [(PathBuf, &'a PackageManifest)], + /// See [`crate::RepeatInstallManifests::workspace_projects`]. + pub(crate) workspace_projects: Option<&'a [(PathBuf, &'a PackageManifest)]>, pub(crate) prefix: &'a str, pub(crate) workspace_packages: Option<&'a pnpm_resolving_resolver_base::WorkspacePackages>, } @@ -200,6 +204,9 @@ pub(crate) struct RecordedWorkspace<'a> { /// Its bins may still name where it was. pub(crate) moved: bool, pub(crate) projects: &'a [(PathBuf, &'a PackageManifest)], + /// The projects whose commands the importers link, per + /// [`crate::RepeatInstallManifests::workspace_projects`]. + pub(crate) bin_projects: &'a [(PathBuf, &'a PackageManifest)], } /// Why a run materializes no `node_modules`. diff --git a/pnpm/crates/package-manager/src/install/tests/repeat_install.rs b/pnpm/crates/package-manager/src/install/tests/repeat_install.rs index f3c67c5705..4d1a3baf6f 100644 --- a/pnpm/crates/package-manager/src/install/tests/repeat_install.rs +++ b/pnpm/crates/package-manager/src/install/tests/repeat_install.rs @@ -113,7 +113,10 @@ async fn optimistic_repeat_install_skips_entire_pipeline_when_state_is_fresh() { supported_architectures: None, }, catalogs: &std::collections::BTreeMap::default(), - project_manifests: &[(dirs.project_root.clone(), &manifest)], + projects: crate::install::StateProjects { + manifests: &[(dirs.project_root.clone(), &manifest)], + workspace_projects: None, + }, filesystem_now_ms: Some(pnpm_testing_utils::fs::backdate_existing_files( &dirs.project_root, )), @@ -254,7 +257,10 @@ fn sync_fast_path_matches_optimistic_short_circuit() { supported_architectures: None, }, catalogs: &std::collections::BTreeMap::default(), - project_manifests: &[(project_root.clone(), &manifest)], + projects: crate::install::StateProjects { + manifests: &[(project_root.clone(), &manifest)], + workspace_projects: None, + }, filesystem_now_ms: Some(last_validated_timestamp), carried: crate::install::CarriedSettings { established_auto_dedupe: None, @@ -402,7 +408,10 @@ async fn partial_install_disables_optimistic_short_circuit() { supported_architectures: None, }, catalogs: &std::collections::BTreeMap::default(), - project_manifests: &[(dirs.project_root.clone(), &manifest)], + projects: crate::install::StateProjects { + manifests: &[(dirs.project_root.clone(), &manifest)], + workspace_projects: None, + }, filesystem_now_ms: Some(pnpm_testing_utils::fs::backdate_existing_files( &dirs.project_root, )), diff --git a/pnpm/crates/package-manager/src/install/workspace_state.rs b/pnpm/crates/package-manager/src/install/workspace_state.rs index a90a811f3f..c5fbdeaf98 100644 --- a/pnpm/crates/package-manager/src/install/workspace_state.rs +++ b/pnpm/crates/package-manager/src/install/workspace_state.rs @@ -4,7 +4,7 @@ pub(super) use projects::{ build_selected_project_manifests_list, mutated_project_dirs, projects_running_own_scripts, selected_manifest_freshness_inputs, }; -pub(crate) use write::{CarriedSettings, StateWrite, recorded_workspace_state}; +pub(crate) use write::{CarriedSettings, StateProjects, StateWrite, recorded_workspace_state}; mod projects; diff --git a/pnpm/crates/package-manager/src/install/workspace_state/discovery.rs b/pnpm/crates/package-manager/src/install/workspace_state/discovery.rs index 618886ad4b..2afd7e2a7e 100644 --- a/pnpm/crates/package-manager/src/install/workspace_state/discovery.rs +++ b/pnpm/crates/package-manager/src/install/workspace_state/discovery.rs @@ -1,7 +1,8 @@ use super::{ super::{ Catalogs, Config, IncludedDependencies, MaybeLazyLockfile, OptimisticRepeatInstallCheck, - PackageManifest, Path, get_catalogs_from_workspace_manifest, load_workspace_projects, + PackageManifest, Path, PathBuf, get_catalogs_from_workspace_manifest, + load_workspace_projects, }, build_project_manifests_list, configured_or_discovered_workspace_dir, lazy_wanted_lockfile, lockfile_root_for, @@ -181,19 +182,13 @@ pub(super) fn check_discovered_deps( let Some(catalogs) = configured_catalogs(config, workspace_manifest) else { return cannot_check_deps(); }; - // The sibling projects only belong in the comparison when one - // lockfile and one state file cover them all; a dedicated-lockfile - // install records this project alone. - let ignored_directories = config.managed_directories(); - let Ok(workspace_projects) = config - .shares_one_lockfile() - .then(|| load_workspace_projects(workspace_root, workspace_manifest, &ignored_directories)) - .transpose() + let Ok(workspace_projects) = + load_gated_workspace(config, manifest, workspace_root, workspace_manifest) else { return cannot_check_deps(); }; - let workspace_projects = workspace_projects.flatten(); - let project_manifests = build_project_manifests_list(manifest, workspace_projects.as_deref()); + let (project_manifests, bin_projects) = + gated_projects(config, manifest, workspace_projects.as_deref()); Some(crate::check_deps_status_before_run( &OptimisticRepeatInstallCheck { workspace_root: lockfile_root, @@ -214,12 +209,53 @@ pub(super) fn check_discovered_deps( optional_dependencies: true, }, }, - manifests: crate::RepeatInstallManifests::ON_DISK, + manifests: crate::RepeatInstallManifests { + workspace_projects: bin_projects.as_deref(), + ..crate::RepeatInstallManifests::ON_DISK + }, }, workspace_state, selected_project_dirs, )) } +/// The workspace projects the gate reads: every one when one lockfile and one +/// state file cover them all, and for a dedicated-lockfile install the ones +/// whose commands the project links, when it links any. +fn load_gated_workspace( + config: &Config, + manifest: &PackageManifest, + workspace_root: &Path, + workspace_manifest: Option<&pnpm_workspace::WorkspaceManifest>, +) -> Result>, pnpm_workspace::FindWorkspaceProjectsError> { + let needed = config.shares_one_lockfile() + || crate::optimistic_repeat_install::validation::links_workspace_commands(manifest); + if !needed { + return Ok(None); + } + load_workspace_projects(workspace_root, workspace_manifest, &config.managed_directories()) +} + +/// The projects the gate compares, and the ones whose commands they link per +/// [`crate::RepeatInstallManifests::workspace_projects`]. +#[expect( + clippy::type_complexity, + reason = "the two lists are the ones `OptimisticRepeatInstallCheck` takes" +)] +fn gated_projects<'a>( + config: &Config, + manifest: &'a PackageManifest, + workspace_projects: Option<&'a [pnpm_workspace::Project]>, +) -> (Vec<(PathBuf, &'a PackageManifest)>, Option>) { + let shared = workspace_projects.filter(|_| config.shares_one_lockfile()); + let project_manifests = build_project_manifests_list(manifest, shared); + let bin_projects = crate::optimistic_repeat_install::validation::dedicated_bin_projects( + config, + &project_manifests, + workspace_projects, + ); + (project_manifests, bin_projects) +} + /// The manifest the verify-deps gate compares against. pub(super) enum GateManifest { Found(Box), diff --git a/pnpm/crates/package-manager/src/install/workspace_state/write.rs b/pnpm/crates/package-manager/src/install/workspace_state/write.rs index 11994a8653..3dfac9de31 100644 --- a/pnpm/crates/package-manager/src/install/workspace_state/write.rs +++ b/pnpm/crates/package-manager/src/install/workspace_state/write.rs @@ -14,7 +14,7 @@ pub(crate) struct StateWrite<'a> { pub(crate) config: &'a Config, pub(crate) layout: crate::RepeatInstallLayout<'a>, pub(crate) catalogs: &'a Catalogs, - pub(crate) project_manifests: &'a [(PathBuf, &'a PackageManifest)], + pub(crate) projects: StateProjects<'a>, /// The filesystem time, probed before the writer read what it validated, /// per [`build_workspace_state`]. pub(crate) filesystem_now_ms: Option, @@ -22,6 +22,14 @@ pub(crate) struct StateWrite<'a> { pub(crate) vouched: &'a crate::optimistic_repeat_install::validation::Vouched<'a>, } +/// The projects a write records. +#[derive(Clone, Copy)] +pub(crate) struct StateProjects<'a> { + pub(crate) manifests: &'a [(PathBuf, &'a PackageManifest)], + /// See [`crate::RepeatInstallManifests::workspace_projects`]. + pub(crate) workspace_projects: Option<&'a [(PathBuf, &'a PackageManifest)]>, +} + /// The settings a write takes from what earlier runs established rather than /// from this run. pub(crate) struct CarriedSettings<'a> { @@ -51,7 +59,7 @@ pub(crate) fn recorded_workspace_state(write: &StateWrite<'_>) -> WorkspaceState write.layout.included, write.layout.supported_architectures, write.catalogs, - write.project_manifests, + write.projects.manifests, write.filesystem_now_ms, ); state.settings.auto_dedupe = @@ -73,7 +81,8 @@ pub(crate) fn recorded_workspace_state(write: &StateWrite<'_>) -> WorkspaceState &crate::optimistic_repeat_install::validation::RecordValidation { workspace_root: write.workspace_root, config: write.config, - project_manifests: write.project_manifests, + project_manifests: write.projects.manifests, + workspace_projects: write.projects.workspace_projects, vouched: write.vouched, }, ); diff --git a/pnpm/crates/package-manager/src/optimistic_repeat_install.rs b/pnpm/crates/package-manager/src/optimistic_repeat_install.rs index f1c9c6d4e4..4b35403b0a 100644 --- a/pnpm/crates/package-manager/src/optimistic_repeat_install.rs +++ b/pnpm/crates/package-manager/src/optimistic_repeat_install.rs @@ -139,6 +139,14 @@ pub struct OptimisticRepeatInstallCheck<'a> { pub manifests: RepeatInstallManifests<'a>, } +impl<'a> OptimisticRepeatInstallCheck<'a> { + /// The projects whose commands the importers link, per + /// [`RepeatInstallManifests::workspace_projects`]. + pub(crate) fn bin_projects(&self) -> &'a [(PathBuf, &'a PackageManifest)] { + self.manifests.workspace_projects.unwrap_or(self.project_manifests) + } +} + #[derive(Clone, Copy)] pub struct RepeatInstallLayout<'a> { pub node_linker: NodeLinker, diff --git a/pnpm/crates/package-manager/src/optimistic_repeat_install/freshness.rs b/pnpm/crates/package-manager/src/optimistic_repeat_install/freshness.rs index 2f38ea1f37..d7f11228c9 100644 --- a/pnpm/crates/package-manager/src/optimistic_repeat_install/freshness.rs +++ b/pnpm/crates/package-manager/src/optimistic_repeat_install/freshness.rs @@ -493,7 +493,10 @@ fn refreshed_state( config: check.config, layout: check.layout, catalogs: check.catalogs, - project_manifests: check.project_manifests, + projects: crate::install::StateProjects { + manifests: check.project_manifests, + workspace_projects: check.manifests.workspace_projects, + }, filesystem_now_ms: filesystem_now, carried: crate::install::CarriedSettings { established_auto_dedupe: state.settings.auto_dedupe, diff --git a/pnpm/crates/package-manager/src/optimistic_repeat_install/freshness/drift.rs b/pnpm/crates/package-manager/src/optimistic_repeat_install/freshness/drift.rs index dfac7bd171..d5e81671f0 100644 --- a/pnpm/crates/package-manager/src/optimistic_repeat_install/freshness/drift.rs +++ b/pnpm/crates/package-manager/src/optimistic_repeat_install/freshness/drift.rs @@ -193,10 +193,8 @@ fn project_stats<'a>( recorded: Option<&Validation>, lockfile_digest: Option<&str>, ) -> Vec> { - let bins = super::super::validation::workspace_bins_digest( - check.workspace_root, - check.project_manifests, - ); + let bins = + super::super::validation::workspace_bins_digest(check.workspace_root, check.bin_projects()); let mut stats = super::super::manifest_agreement::manifest_stats( check.project_manifests, |root_dir, manifest, digest| { diff --git a/pnpm/crates/package-manager/src/optimistic_repeat_install/manifest_source.rs b/pnpm/crates/package-manager/src/optimistic_repeat_install/manifest_source.rs index 8f57974d95..e58511f77d 100644 --- a/pnpm/crates/package-manager/src/optimistic_repeat_install/manifest_source.rs +++ b/pnpm/crates/package-manager/src/optimistic_repeat_install/manifest_source.rs @@ -1,6 +1,8 @@ //! How the repeat-install check reads the project manifests it is handed. use crate::install::ProjectManifestsByDir; +use pnpm_package_manifest::PackageManifest; +use std::path::PathBuf; /// How the check reads the project manifests it is handed. #[derive(Clone, Copy)] @@ -12,9 +14,15 @@ pub struct RepeatInstallManifests<'a> { /// project's lockfile snapshot with this manifest. `None` when no /// project has one. pub dependency_manifests: Option<&'a ProjectManifestsByDir<'a>>, + /// Every workspace project, when the projects the check is handed are + /// fewer because each project has its own lockfile. An importer links + /// the commands of the workspace projects it depends on, so their `bin` + /// fields are part of what proves it installed. `None` when the projects + /// the check is handed are the whole workspace. + pub workspace_projects: Option<&'a [(PathBuf, &'a PackageManifest)]>, } impl RepeatInstallManifests<'_> { /// The `package.json` files on disk, which carry no dependency manifests. - pub const ON_DISK: Self = Self { dependency_manifests: None }; + pub const ON_DISK: Self = Self { dependency_manifests: None, workspace_projects: None }; } diff --git a/pnpm/crates/package-manager/src/optimistic_repeat_install/tests.rs b/pnpm/crates/package-manager/src/optimistic_repeat_install/tests.rs index 205bc09acd..ed3a156276 100644 --- a/pnpm/crates/package-manager/src/optimistic_repeat_install/tests.rs +++ b/pnpm/crates/package-manager/src/optimistic_repeat_install/tests.rs @@ -261,6 +261,7 @@ fn record_installed( workspace_root, config, project_manifests, + workspace_projects: None, vouched: &super::validation::Vouched::All, }, ); @@ -292,6 +293,7 @@ fn record_validation_on_disk(workspace_root: &std::path::Path, state: &mut Works workspace_root, config: &config, project_manifests: &project_manifests, + workspace_projects: None, vouched: &super::validation::Vouched::All, }, ); @@ -402,7 +404,10 @@ fn record_test_workspace_state( supported_architectures: None, }, catalogs: &BTreeMap::default(), - project_manifests, + projects: crate::install::StateProjects { + manifests: project_manifests, + workspace_projects: None, + }, filesystem_now_ms: Some(backdate_validated_files(workspace_root)), carried: crate::install::CarriedSettings { established_auto_dedupe: config.auto_dedupe.then_some(true), diff --git a/pnpm/crates/package-manager/src/optimistic_repeat_install/tests/validation.rs b/pnpm/crates/package-manager/src/optimistic_repeat_install/tests/validation.rs index dcfa95101b..e10869ce45 100644 --- a/pnpm/crates/package-manager/src/optimistic_repeat_install/tests/validation.rs +++ b/pnpm/crates/package-manager/src/optimistic_repeat_install/tests/validation.rs @@ -122,6 +122,7 @@ fn a_filtered_write_carries_the_other_records_while_the_shared_inputs_stand() { workspace_root: root, config: &config, project_manifests: &project_manifests, + workspace_projects: None, vouched, }, ); @@ -181,6 +182,7 @@ fn source_date_epoch_records_deterministic_pnpmfile_digests_without_trusting_mti workspace_root: root, config: &config, project_manifests: &project_manifests, + workspace_projects: None, vouched: &Vouched::All, }, ); diff --git a/pnpm/crates/package-manager/src/optimistic_repeat_install/validation.rs b/pnpm/crates/package-manager/src/optimistic_repeat_install/validation.rs index 9cb6c527af..4a8a11c7ca 100644 --- a/pnpm/crates/package-manager/src/optimistic_repeat_install/validation.rs +++ b/pnpm/crates/package-manager/src/optimistic_repeat_install/validation.rs @@ -9,7 +9,8 @@ //! others survive it unchanged. pub(crate) use bins::{ - held_workspace_bins_match, project_bins_digest, project_bins_match, workspace_bins_digest, + dedicated_bin_projects, held_workspace_bins_match, links_workspace_commands, + project_bins_digest, project_bins_match, workspace_bins_digest, }; use super::{Config, Lockfile, PackageManifest, Path, PathBuf}; @@ -309,6 +310,8 @@ pub(crate) struct RecordValidation<'a> { pub(crate) workspace_root: &'a Path, pub(crate) config: &'a Config, pub(crate) project_manifests: &'a [(PathBuf, &'a PackageManifest)], + /// See [`crate::RepeatInstallManifests::workspace_projects`]. + pub(crate) workspace_projects: Option<&'a [(PathBuf, &'a PackageManifest)]>, pub(crate) vouched: &'a Vouched<'a>, } @@ -397,7 +400,10 @@ fn record_projects( carried: Option<&Validation>, lockfile: Option<&str>, ) -> usize { - let bins = workspace_bins_digest(inputs.workspace_root, inputs.project_manifests); + let bins = workspace_bins_digest( + inputs.workspace_root, + inputs.workspace_projects.unwrap_or(inputs.project_manifests), + ); let records = inputs.project_manifests .iter() .filter_map(|(root_dir, manifest)| { diff --git a/pnpm/crates/package-manager/src/optimistic_repeat_install/validation/bins.rs b/pnpm/crates/package-manager/src/optimistic_repeat_install/validation/bins.rs index 50ecf16f08..1a04f00f30 100644 --- a/pnpm/crates/package-manager/src/optimistic_repeat_install/validation/bins.rs +++ b/pnpm/crates/package-manager/src/optimistic_repeat_install/validation/bins.rs @@ -1,5 +1,5 @@ use super::{ - PackageManifest, Path, PathBuf, Validation, WorkspaceState, importer_id_from_root_dir, + Config, PackageManifest, Path, PathBuf, Validation, WorkspaceState, importer_id_from_root_dir, value_digest, }; use crate::optimistic_repeat_install::{ManifestStat, OptimisticRepeatInstallCheck}; @@ -36,25 +36,25 @@ pub(crate) fn workspace_bins_digest( } /// The bins digest `importer_id` records: `bins` when the project links -/// workspace bins, which the root and any project with dependencies do, and -/// `None` otherwise. +/// workspace bins, which the root and any project that +/// [`links_workspace_commands`] do, and `None` otherwise. pub(crate) fn project_bins_digest<'a>( importer_id: &str, manifest: &PackageManifest, bins: Option<&'a str>, ) -> Option<&'a str> { - if importer_id == "." - || crate::optimistic_repeat_install::manifest_has_runtime_deps(manifest) + if importer_id == "." || links_workspace_commands(manifest) { bins } else { None } +} + +/// Whether the project declares a dependency or a peer, through which a +/// workspace project's commands reach its `node_modules/.bin`. +pub(crate) fn links_workspace_commands(manifest: &PackageManifest) -> bool { + crate::optimistic_repeat_install::manifest_has_runtime_deps(manifest) || manifest .value() .get("peerDependencies") .and_then(serde_json::Value::as_object) .is_some_and(|peers| !peers.is_empty()) - { - bins - } else { - None - } } /// Whether `recorded` proves `importer_id` installed with the bins digest @@ -78,7 +78,7 @@ pub(crate) fn held_workspace_bins_match( if held.is_empty() || state.validation.is_none() { return true; } - let bins = workspace_bins_digest(check.workspace_root, check.project_manifests); + let bins = workspace_bins_digest(check.workspace_root, check.bin_projects()); held.iter() .all(|stat| { let id = importer_id_from_root_dir(check.workspace_root, stat.root_dir); @@ -89,3 +89,22 @@ pub(crate) fn held_workspace_bins_match( ) }) } + +/// [`crate::RepeatInstallManifests::workspace_projects`] for an install with +/// dedicated lockfiles whose projects link workspace commands. `None` with a +/// shared lockfile, whose projects already are the whole workspace. +pub(crate) fn dedicated_bin_projects<'w>( + config: &Config, + project_manifests: &[(PathBuf, &PackageManifest)], + workspace_projects: Option<&'w [pnpm_workspace::Project]>, +) -> Option> { + let links = project_manifests.iter().any(|(_, manifest)| links_workspace_commands(manifest)); + if config.shares_one_lockfile() || !links { + return None; + } + let projects = workspace_projects? + .iter() + .map(|project| (project.root_dir.clone(), &project.manifest)) + .collect(); + Some(projects) +}