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) +}