diff --git a/.changeset/repeat-install-verify-deps-unification.md b/.changeset/repeat-install-verify-deps-unification.md index 0b3fdcc2cf..c9eadc891b 100644 --- a/.changeset/repeat-install-verify-deps-unification.md +++ b/.changeset/repeat-install-verify-deps-unification.md @@ -3,9 +3,10 @@ "@pnpm/exec.commands": patch "@pnpm/installing.commands": patch "@pnpm/installing.deps-restorer": patch +"@pnpm/workspace.projects-graph": patch "@pnpm/workspace.state": patch "pnpm": patch "pacquet": patch --- -`pnpm run`, `pnpm exec`, and `pnpm restart` now pass the install flags they were given, such as `--lockfile-dir`, `--modules-dir`, `--store-dir`, `--offline`, and `--ignore-scripts`, to the `pnpm install` that `verifyDepsBeforeRun: install` starts. The check that runs before a script now holds the workspace projects the script's project depends on. It agrees with the repeat-install check on which projects a filtered install left uninstalled, including with per-project lockfiles. A filtered install no longer forgets the ignored build scripts of the projects it did not select. +`pnpm run`, `pnpm exec`, and `pnpm restart` now pass the install flags they were given, such as `--lockfile-dir`, `--modules-dir`, `--store-dir`, `--offline`, and `--ignore-scripts`, to the `pnpm install` that `verifyDepsBeforeRun: install` starts. The check that runs before a script now holds the workspace projects the script's project depends on. It agrees with the repeat-install check on which projects a filtered install left uninstalled, including with per-project lockfiles. Workspace dependencies declared with `link:` are now included in the workspace project graph, so `--filter=...` selects them and, with `sharedWorkspaceLockfile: false` and `injectWorkspacePackages: true`, the check before a script still requires them to be installed. A filtered install no longer forgets the ignored build scripts of the projects it did not select. diff --git a/pnpm/crates/cli/src/cli_args/recursive.rs b/pnpm/crates/cli/src/cli_args/recursive.rs index 702539520b..537f3e27df 100644 --- a/pnpm/crates/cli/src/cli_args/recursive.rs +++ b/pnpm/crates/cli/src/cli_args/recursive.rs @@ -7,9 +7,7 @@ //! command-specific error codes) live in `run/recursive.rs` and //! `exec/recursive.rs`. -pub use dependencies::{ - filtered_projects_dependencies, projects_with_workspace_dependencies, sequence_graph, -}; +pub use dependencies::{filtered_projects_dependencies, projects_to_verify, sequence_graph}; pub use execution_args::RecursiveExecutionArgs; pub use importer_selection::{selected_workspace_importer_ids, selectors_narrow_the_run}; pub use summary::{ExecutionStatus, Status, count_failures, write_recursive_summary}; @@ -20,10 +18,7 @@ mod execution_args; mod importer_selection; mod unmatched; -use crate::cli_args::{ - catalogs::{configured_catalogs, workspace_catalogs}, - verify_deps::with_dependencies, -}; +use crate::cli_args::catalogs::{configured_catalogs, workspace_catalogs}; use derive_more::{Display, Error}; use indexmap::IndexMap; use miette::{Context, Diagnostic, IntoDiagnostic}; @@ -150,84 +145,6 @@ impl<'a> RecursiveSelection<'a> { } } -/// The projects a gated recursive command holds to the modules-directory -/// requirement: `project_dirs` themselves, plus every workspace dependency -/// they reach that the installs the gate spawns cover. -/// -/// A workspace dependency that is not installed leaves the selection out of -/// date too. With one shared lockfile, the install the gate spawns selects the -/// dependencies of the selected projects, and a negated selector reaches it -/// unchanged, so a dependency it excludes is not required; `project_dirs` -/// themselves always are. With a lockfile per project the gate checks and -/// installs each project on its own, and a workspace dependency the project -/// injects is part of that project's own install. `prefix` is where the -/// command's path selectors resolve. -pub fn projects_to_verify<'dirs>( - project_dirs: impl IntoIterator, - selection: &RecursiveSelection<'_>, - config: &Config, - prefix: &Path, -) -> miette::Result> { - if !config.verify_deps_before_run.is_enabled() { - return Ok(Vec::new()); - } - let project_dirs: Vec<&Path> = project_dirs.into_iter().collect(); - // An injected workspace dependency is a copy inside the project that - // depends on it, which that project's own install covers. - if !config.shares_one_lockfile() && config.inject_workspace_packages { - return Ok(project_dirs - .into_iter() - .map(Path::to_path_buf) - .collect()); - } - let mut projects = projects_with_workspace_dependencies( - project_dirs.iter().copied(), - selection.full_graph(), - selection.prod_all.as_ref(), - &selection.prod_only_selected, - ); - if config.shares_one_lockfile() && has_negated_selector(config) { - let mut kept = install_selected_projects(selection, config, prefix)?; - kept.extend(project_dirs.iter().map(|dir| dir.to_path_buf())); - projects.retain(|project_dir| kept.contains(project_dir)); - } - Ok(projects) -} - -fn has_negated_selector(config: &Config) -> bool { - config.filter - .iter() - .chain(&config.filter_prod) - .any(|selector| selector.starts_with('!')) -} - -/// The projects the install the gate spawns selects, evaluated against the -/// graphs the command's own selection was drawn from. -fn install_selected_projects( - selection: &RecursiveSelection<'_>, - config: &Config, - prefix: &Path, -) -> miette::Result> { - let walk_opts = recursive_filter_options(config, prefix); - let filter = with_dependencies_selectors(&config.filter); - let mut installed: HashSet = - filter_against(selection.full_graph(), &filter, None, false, prefix, &walk_opts)? - .into_iter() - .collect(); - if let Some(prod_all) = &selection.prod_all { - let filter_prod = with_dependencies_selectors(&config.filter_prod); - installed.extend(filter_against(prod_all, &filter_prod, None, true, prefix, &walk_opts)?); - } - Ok(installed) -} - -fn with_dependencies_selectors(selectors: &[String]) -> Vec { - selectors - .iter() - .map(|selector| with_dependencies(selector)) - .collect() -} - /// Build the `--filter`-selected workspace projects the recursive command /// runs over, together with the graphs [`filtered_projects_dependencies`] resolves /// order through. `prefix` is where path selectors resolve; `auto_exclude_root` diff --git a/pnpm/crates/cli/src/cli_args/recursive/dependencies.rs b/pnpm/crates/cli/src/cli_args/recursive/dependencies.rs index 88fb1df15a..28571032e8 100644 --- a/pnpm/crates/cli/src/cli_args/recursive/dependencies.rs +++ b/pnpm/crates/cli/src/cli_args/recursive/dependencies.rs @@ -2,9 +2,20 @@ //! dependency edges among the `--filter`-selected projects, their topological //! order, and the workspace dependencies a selection reaches. +use super::{RecursiveSelection, filter_against, recursive_filter_options}; +use crate::cli_args::{ + catalogs::{configured_catalogs, workspace_catalogs}, + verify_deps::with_dependencies, +}; use indexmap::IndexMap; +use pnpm_catalogs_resolver::{ + CatalogAnchor, CatalogResolutionResult, WantedDependency, resolve_from_catalog, +}; +use pnpm_config::Config; +use pnpm_fs::lexical_normalize; use pnpm_package_manager::{GraphSequencerResult, graph_sequencer}; -use pnpm_workspace_projects_graph::ProjectGraph; +use pnpm_package_manifest::{DependencyGroup, PackageManifest}; +use pnpm_workspace_projects_graph::{ProjectGraph, WorkspaceCatalogs}; use rayon::prelude::*; use std::{ collections::{HashMap, HashSet}, @@ -123,8 +134,158 @@ fn sorted_dependencies( dependencies } +/// The projects a gated recursive command holds to the modules-directory +/// requirement: `project_dirs` themselves, plus every workspace dependency +/// they reach that the installs the gate spawns cover. +/// +/// A workspace dependency that is not installed leaves the selection out of +/// date too. With one shared lockfile, the install the gate spawns selects the +/// dependencies of the selected projects, and a negated selector reaches it +/// unchanged, so a dependency it excludes is not required; `project_dirs` +/// themselves always are. With a lockfile per project the gate checks and +/// installs each project on its own, and a workspace dependency the project +/// injects is part of that project's own install. `prefix` is where the +/// command's path selectors resolve. +pub fn projects_to_verify<'dirs>( + project_dirs: impl IntoIterator, + selection: &RecursiveSelection<'_>, + config: &Config, + prefix: &Path, +) -> miette::Result> { + if !config.verify_deps_before_run.is_enabled() { + return Ok(Vec::new()); + } + let project_dirs: Vec<&Path> = project_dirs.into_iter().collect(); + // An injected workspace dependency is a copy inside the project that + // depends on it, which that project's own install covers; explicit `link:` + // dependencies remain symlinks and still need their target project installed. + let only_linked = !config.shares_one_lockfile() && config.inject_workspace_packages; + let catalogs = if only_linked { Some(configured_catalogs(config)?) } else { None }; + let ws_catalogs = catalogs.as_ref().and_then(|catalogs| workspace_catalogs(config, catalogs)); + let mut projects = projects_with_workspace_dependencies_filtered( + project_dirs.iter().copied(), + selection.full_graph(), + selection.prod_all.as_ref(), + &selection.prod_only_selected, + |parent_dir, pkg, deps| { + retained_dependency_dirs( + parent_dir, + &pkg.project.manifest, + deps, + only_linked, + ws_catalogs, + ) + }, + ); + if config.shares_one_lockfile() && has_negated_selector(config) { + let mut kept = install_selected_projects(selection, config, prefix)?; + kept.extend(project_dirs.iter().map(|dir| dir.to_path_buf())); + projects.retain(|project_dir| kept.contains(project_dir)); + } + Ok(projects) +} + +fn retained_dependency_dirs<'graph>( + parent_dir: &Path, + manifest: &PackageManifest, + deps: &'graph [PathBuf], + only_linked: bool, + catalogs: Option>, +) -> Vec<&'graph Path> { + if !only_linked || deps.is_empty() { + return deps + .iter() + .map(PathBuf::as_path) + .collect(); + } + let linked = linked_dependency_dirs(parent_dir, manifest, catalogs); + deps.iter() + .map(PathBuf::as_path) + .filter(|dep_dir| linked.contains(&lexical_normalize(dep_dir))) + .collect() +} + +fn linked_dependency_dirs( + parent_dir: &Path, + manifest: &PackageManifest, + catalogs: Option>, +) -> rustc_hash::FxHashSet { + manifest + .dependencies([ + DependencyGroup::Prod, + DependencyGroup::Dev, + DependencyGroup::Optional, + DependencyGroup::Peer, + ]) + .filter_map(|(dep_name, raw_spec)| { + let resolved = resolve_catalog_spec(parent_dir, dep_name, raw_spec, catalogs); + let spec = resolved.as_deref().unwrap_or(raw_spec); + let rel = spec.strip_prefix("link:")?; + Some(lexical_normalize(&parent_dir.join(rel.replace('\\', "/")))) + }) + .collect() +} + +fn resolve_catalog_spec( + consumer_dir: &Path, + dep_name: &str, + raw_spec: &str, + catalogs: Option>, +) -> Option { + let catalogs = catalogs?; + if !raw_spec.starts_with("catalog:") { + return None; + } + let wanted = + WantedDependency { alias: dep_name.to_string(), bare_specifier: raw_spec.to_string() }; + let anchor = CatalogAnchor::Reanchor { + workspace_dir: catalogs.workspace_dir, + consumer_dir: Some(consumer_dir), + }; + match resolve_from_catalog(catalogs.catalogs, &wanted, anchor) { + CatalogResolutionResult::Found(found) => Some(found.resolution.specifier), + CatalogResolutionResult::Misconfiguration(_) | CatalogResolutionResult::Unused => None, + } +} + +fn has_negated_selector(config: &Config) -> bool { + config.filter + .iter() + .chain(&config.filter_prod) + .any(|selector| selector.starts_with('!')) +} + +/// The projects the install the gate spawns selects, evaluated against the +/// graphs the command's own selection was drawn from. +fn install_selected_projects( + selection: &RecursiveSelection<'_>, + config: &Config, + prefix: &Path, +) -> miette::Result> { + let walk_opts = recursive_filter_options(config, prefix); + let filter = with_dependencies_selectors(&config.filter); + let mut installed: HashSet = + filter_against(selection.full_graph(), &filter, None, false, prefix, &walk_opts)? + .into_iter() + .collect(); + if let Some(prod_all) = &selection.prod_all { + let filter_prod = with_dependencies_selectors(&config.filter_prod); + installed.extend(filter_against(prod_all, &filter_prod, None, true, prefix, &walk_opts)?); + } + Ok(installed) +} + +fn with_dependencies_selectors(selectors: &[String]) -> Vec { + selectors + .iter() + .map(|selector| with_dependencies(selector)) + .collect() +} + /// `project_dirs` together with every workspace project they depend on, -/// directly or transitively. +/// directly or transitively, along the dependency edges that `filter_edges` +/// retains for each parent node `(parent_dir, &parent_node.package, +/// &parent_node.dependencies)`. /// /// The install a gated command spawns selects the dependencies of the projects /// the command selected, so a project whose workspace dependency has no modules @@ -135,14 +296,18 @@ fn sorted_dependencies( /// /// Each graph keeps one visited set across all roots, so a dependency shared /// by several selected projects is walked once. -pub fn projects_with_workspace_dependencies<'graph, 'dirs, Pkg>( +pub(super) fn projects_with_workspace_dependencies_filtered<'graph, 'dirs, Pkg, Filter>( project_dirs: impl IntoIterator, all: &'graph ProjectGraph, prod_all: Option<&'graph ProjectGraph>, prod_only_selected: &HashSet, -) -> Vec { - let mut all_walk = DependencyWalk::new(all); - let mut prod_walk = prod_all.map(DependencyWalk::new); + filter_edges: Filter, +) -> Vec +where + Filter: Fn(&Path, &Pkg, &'graph [PathBuf]) -> Vec<&'graph Path>, +{ + let mut all_walk = DependencyWalk::new(all, &filter_edges); + let mut prod_walk = prod_all.map(|graph| DependencyWalk::new(graph, &filter_edges)); let mut outside_graph: Vec<&Path> = Vec::new(); for project_dir in project_dirs { let walk = match &mut prod_walk { @@ -164,16 +329,25 @@ pub fn projects_with_workspace_dependencies<'graph, 'dirs, Pkg>( } /// A walk along the dependency edges of one graph, from any number of roots. -struct DependencyWalk<'graph, Pkg> { +struct DependencyWalk<'graph, 'filter, Pkg, Filter> { graph: &'graph ProjectGraph, + filter_edges: &'filter Filter, seen: rustc_hash::FxHashSet<&'graph Path>, /// The projects reached, in the order they were first reached. visited: Vec<&'graph Path>, } -impl<'graph, Pkg> DependencyWalk<'graph, Pkg> { - fn new(graph: &'graph ProjectGraph) -> Self { - DependencyWalk { graph, seen: rustc_hash::FxHashSet::default(), visited: Vec::new() } +impl<'graph, 'filter, Pkg, Filter> DependencyWalk<'graph, 'filter, Pkg, Filter> +where + Filter: Fn(&Path, &Pkg, &'graph [PathBuf]) -> Vec<&'graph Path>, +{ + fn new(graph: &'graph ProjectGraph, filter_edges: &'filter Filter) -> Self { + DependencyWalk { + graph, + filter_edges, + seen: rustc_hash::FxHashSet::default(), + visited: Vec::new(), + } } /// Visit `root` and every project reachable from it that no earlier root @@ -187,17 +361,15 @@ impl<'graph, Pkg> DependencyWalk<'graph, Pkg> { return true; } self.visited.push(root.as_path()); - let mut stack: Vec<&'graph Path> = node.dependencies - .iter() - .map(PathBuf::as_path) - .collect(); + let mut stack: Vec<&'graph Path> = + (self.filter_edges)(root, &node.package, &node.dependencies); while let Some(project_dir) = stack.pop() { if !self.seen.insert(project_dir) { continue; } self.visited.push(project_dir); if let Some(node) = self.graph.get(project_dir) { - stack.extend(node.dependencies.iter().map(PathBuf::as_path)); + stack.extend((self.filter_edges)(project_dir, &node.package, &node.dependencies)); } } true diff --git a/pnpm/crates/cli/src/cli_args/recursive/tests.rs b/pnpm/crates/cli/src/cli_args/recursive/tests.rs index d4a688f331..ca09d9c4b5 100644 --- a/pnpm/crates/cli/src/cli_args/recursive/tests.rs +++ b/pnpm/crates/cli/src/cli_args/recursive/tests.rs @@ -6,15 +6,41 @@ //! selection resolves through the prod-pruned graph so the dev edges it //! dropped are not pulled back in. -use super::{filtered_projects_dependencies, projects_with_workspace_dependencies, sequence_graph}; +use super::{ + RecursiveSelection, dependencies::projects_with_workspace_dependencies_filtered, + filtered_projects_dependencies, projects_to_verify, sequence_graph, +}; +use pnpm_config::{Config, VerifyDepsBeforeRun}; use pnpm_package_manager::graph_sequencer; +use pnpm_package_manifest::PackageManifest; +use pnpm_workspace::{GraphPkg, Project}; use pnpm_workspace_projects_graph::{ProjectGraph, ProjectGraphNode}; use pretty_assertions::assert_eq; use std::{ collections::{HashMap, HashSet}, + fs, path::{Path, PathBuf}, }; +fn projects_with_workspace_dependencies<'graph, 'dirs, Pkg>( + project_dirs: impl IntoIterator, + all: &'graph ProjectGraph, + prod_all: Option<&'graph ProjectGraph>, + prod_only_selected: &HashSet, +) -> Vec { + projects_with_workspace_dependencies_filtered( + project_dirs, + all, + prod_all, + prod_only_selected, + |_, _, deps| { + deps.iter() + .map(PathBuf::as_path) + .collect() + }, + ) +} + fn make_graph(adjacency: &[(&str, &[&str])]) -> ProjectGraph<()> { adjacency .iter() @@ -253,3 +279,80 @@ fn projects_with_workspace_dependencies_lists_a_shared_dependency_once() { dirs(&["a", "c", "b"]), ); } + +#[test] +fn projects_to_verify_under_dedicated_lockfiles_with_injection_follows_only_link_edges() { + let tmp = tempfile::tempdir().unwrap(); + fs::write( + tmp.path().join("pnpm-workspace.yaml"), + "packages:\n - '*'\ncatalog:\n d: link:./d\n", + ) + .unwrap(); + let write_project = |name: &str, json: &str| { + let dir = tmp.path().join(name); + fs::create_dir_all(&dir).unwrap(); + let manifest_path = dir.join("package.json"); + fs::write(&manifest_path, json).unwrap(); + Project { + root_dir: dir, + manifest: PackageManifest::from_path(manifest_path).unwrap(), + dependency_manifest: None, + } + }; + let pkg_a = write_project( + "a", + r#"{"name":"a","version":"1.0.0","dependencies":{"b":"workspace:*","c":"link:../c","d":"catalog:"}}"#, + ); + let pkg_b = write_project("b", r#"{"name":"b","version":"1.0.0"}"#); + let pkg_c = write_project("c", r#"{"name":"c","version":"1.0.0"}"#); + let pkg_d = write_project("d", r#"{"name":"d","version":"1.0.0"}"#); + let all: ProjectGraph> = [ + ( + pkg_a.root_dir.clone(), + ProjectGraphNode { + package: GraphPkg { project: &pkg_a }, + dependencies: vec![ + pkg_b.root_dir.clone(), + pkg_c.root_dir.clone(), + pkg_d.root_dir.clone(), + ], + }, + ), + ( + pkg_b.root_dir.clone(), + ProjectGraphNode { package: GraphPkg { project: &pkg_b }, dependencies: Vec::new() }, + ), + ( + pkg_c.root_dir.clone(), + ProjectGraphNode { package: GraphPkg { project: &pkg_c }, dependencies: Vec::new() }, + ), + ( + pkg_d.root_dir.clone(), + ProjectGraphNode { package: GraphPkg { project: &pkg_d }, dependencies: Vec::new() }, + ), + ] + .into_iter() + .collect(); + let selected = + std::iter::once((pkg_a.root_dir.clone(), all[&pkg_a.root_dir].clone())).collect(); + let selection = RecursiveSelection { + selected, + all: Some(all), + prod_all: None, + prod_only_selected: HashSet::new(), + }; + let config = Config { + verify_deps_before_run: VerifyDepsBeforeRun::Error, + shared_workspace_lockfile: false, + inject_workspace_packages: true, + workspace_dir: Some(tmp.path().to_path_buf()), + ..Config::default() + }; + + let mut verified = + projects_to_verify([pkg_a.root_dir.as_path()], &selection, &config, tmp.path()).unwrap(); + verified.sort(); + let mut expected = vec![pkg_a.root_dir, pkg_c.root_dir, pkg_d.root_dir]; + expected.sort(); + assert_eq!(verified, expected); +} diff --git a/pnpm/crates/cli/tests/suite/verify_deps_before_run.rs b/pnpm/crates/cli/tests/suite/verify_deps_before_run.rs index 506811d31a..0cbed0aa98 100644 --- a/pnpm/crates/cli/tests/suite/verify_deps_before_run.rs +++ b/pnpm/crates/cli/tests/suite/verify_deps_before_run.rs @@ -1683,6 +1683,141 @@ fn filtered_exec_installs_a_workspace_dependency_a_filtered_install_left_alone() drop((root, mock_instance)); } +/// Under dedicated lockfiles with `injectWorkspacePackages`, an injected +/// `workspace:*` dependency is a copy inside the dependent project that its +/// own install covers, whereas an explicit `link:` dependency remains a +/// symlink whose target project must still be installed +/// ([pnpm/tasks#179](https://github.com/pnpm/tasks/issues/179)). +#[test] +fn filtered_exec_holds_a_link_dependency_under_dedicated_lockfiles_with_injection() { + let CommandTempCwd { root, workspace, npmrc_info, .. } = + CommandTempCwd::init().add_mocked_registry(); + let AddMockedRegistry { mock_instance, .. } = npmrc_info; + fs::write( + workspace.join("package.json"), + json!({ "name": "workspace-root", "version": "0.0.0" }).to_string(), + ) + .expect("write root package.json"); + fs::write( + workspace.join("pnpm-workspace.yaml"), + "sharedWorkspaceLockfile: false\ninjectWorkspacePackages: true\npackages:\n - packages/*\n", + ) + .expect("write pnpm-workspace.yaml"); + let projects = [ + ("foo", json!({ "dependencies": { "bar": "workspace:*" } })), + ("bar", json!({ "dependencies": { "@pnpm.e2e/foo": "100.0.0" } })), + ("baz", json!({ "dependencies": { "@pnpm.e2e/foo": "100.0.0" } })), + ]; + for (name, groups) in projects { + let project = workspace.join("packages").join(name); + fs::create_dir_all(&project).expect("create workspace project"); + write_named_manifest_with_dependency_groups( + &project, + name, + &project.join("marker.txt"), + groups, + ); + } + + pacquet_in(&workspace) + .with_args(["--filter", "foo", "install"]) + .assert() + .success(); + assert!(!workspace.join("packages/bar/node_modules").exists()); + + // An injected `workspace:*` dependency is covered by `foo`'s own install, + // so an uninstalled `bar` does not block a filtered exec on `foo`. + let output = pacquet_in(&workspace) + .with_args([ + "--config.verify-deps-before-run=error", + "--filter", + "foo", + "exec", + "node", + "-e", + r#"process.stdout.write("injected-ok")"#, + ]) + .output() + .expect("spawn pacquet exec"); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + output.status.success(), + "an uninstalled injected workspace dependency must not block the filtered exec:\n{stderr}", + ); + assert_eq!(String::from_utf8_lossy(&output.stdout), "injected-ok"); + + // Adding a `link:` dependency to `baz` and installing `foo` alone leaves + // `baz` without a modules directory; `link:` remains a symlink even with + // injection on, so the gate holds `baz` to the modules-directory check. + write_named_manifest_with_dependency_groups( + &workspace.join("packages/foo"), + "foo", + &workspace.join("packages/foo/marker.txt"), + json!({ "dependencies": { "bar": "workspace:*", "baz": "link:../baz" } }), + ); + pacquet_in(&workspace) + .with_args(["--filter", "foo", "install"]) + .assert() + .success(); + assert!(!workspace.join("packages/baz/node_modules").exists()); + + let output = pacquet_in(&workspace) + .with_args([ + "--config.verify-deps-before-run=error", + "--filter", + "foo", + "exec", + "node", + "-e", + "0", + ]) + .output() + .expect("spawn pacquet exec"); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + !output.status.success() && stderr.contains("ERR_PNPM_VERIFY_DEPS_BEFORE_RUN"), + "a never-installed link: dependency must fail the verify-deps check:\n{stderr}", + ); + + let output = pacquet_in(&workspace) + .with_args([ + "--config.verify-deps-before-run=install", + "--filter", + "foo", + "exec", + "node", + "-e", + r#"process.stdout.write("linked-ok")"#, + ]) + .output() + .expect("spawn pacquet exec"); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + output.status.success(), + "the filtered exec must install the link: dependency:\n{stderr}", + ); + assert_eq!(String::from_utf8_lossy(&output.stdout), "linked-ok"); + assert!( + workspace.join("packages/baz/node_modules/@pnpm.e2e/foo").exists(), + "the gate must install the link: dependency:\n{stderr}", + ); + + pacquet_in(&workspace) + .with_args([ + "--config.verify-deps-before-run=error", + "--filter", + "foo", + "exec", + "node", + "-e", + "0", + ]) + .assert() + .success(); + + drop((root, mock_instance)); +} + /// A negated selector reaches the install the gate spawns unchanged, so that /// install never materializes the project it excludes. The status check must /// not hold that project to the modules-directory requirement through the 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 d0416a4eb1..618886ad4b 100644 --- a/pnpm/crates/package-manager/src/install/workspace_state/discovery.rs +++ b/pnpm/crates/package-manager/src/install/workspace_state/discovery.rs @@ -60,7 +60,7 @@ pub fn check_deps_status_before_run_at( return cannot_check_deps(); }; let workspace_manifest = workspace_manifest.flatten(); - let config = gate_config(config, manifest_dir, &manifest); + let config = gate_config(config, workspace_dir_opt.as_deref(), manifest_dir, &manifest); // A pinned `lockfileDir` is where the install left the state and the // lockfile; otherwise it follows the manifest read above, just as it // does during install. @@ -142,20 +142,26 @@ pub fn deps_install_root(dir: &Path, config: &Config) -> std::path::PathBuf { } fn gate_config<'a>( config: &'a Config, + workspace_dir: Option<&Path>, manifest_dir: &Path, manifest: &PackageManifest, ) -> Cow<'a, Config> { - if config.shares_one_lockfile() { - Cow::Borrowed(config) - } else { - let mut project_config = config.clone(); + let needs_workspace_dir = config.workspace_dir.is_none() && workspace_dir.is_some(); + if config.shares_one_lockfile() && !needs_workspace_dir { + return Cow::Borrowed(config); + } + let mut project_config = config.clone(); + if needs_workspace_dir { + project_config.workspace_dir = workspace_dir.map(Path::to_path_buf); + } + if !config.shares_one_lockfile() { let project_name = manifest .value() .get("name") .and_then(serde_json::Value::as_str); project_config.anchor_dedicated_project(manifest_dir, project_name); - Cow::Owned(project_config) } + Cow::Owned(project_config) } pub(super) fn cannot_check_deps() -> Option { Some(crate::RunDepsStatus::Outdated { diff --git a/pnpm/crates/package-manager/src/optimistic_repeat_install/modules_dirs.rs b/pnpm/crates/package-manager/src/optimistic_repeat_install/modules_dirs.rs index d7d5f9d343..d2cc24eec7 100644 --- a/pnpm/crates/package-manager/src/optimistic_repeat_install/modules_dirs.rs +++ b/pnpm/crates/package-manager/src/optimistic_repeat_install/modules_dirs.rs @@ -1,23 +1,23 @@ +mod workspace_deps; + use super::{ - CatalogAnchor, CatalogResolutionResult, Catalogs, OptimisticRepeatInstallCheck, - WantedDependency, manifest_has_required_peers, manifest_has_runtime_deps, - manifest_string_field, resolve_from_catalog, + OptimisticRepeatInstallCheck, manifest_has_required_peers, manifest_has_runtime_deps, + manifest_string_field, }; use pnpm_config::{Config, NodeLinker}; use pnpm_fs::lexical_normalize; use pnpm_lockfile::{MaybeLazyLockfile, PkgName, ProjectSnapshot, ResolvedDependencySpec}; use pnpm_modules_yaml::IncludedDependencies; use pnpm_package_manifest::{DependencyGroup, PackageManifest}; -use pnpm_semver_include_prerelease::IncludePrereleaseRange; use pnpm_workspace::importer_id_from_root_dir; use pnpm_workspace_state::WorkspaceState; use rayon::prelude::*; use std::{ - borrow::Cow, - collections::{HashMap, HashSet}, + collections::HashSet, fs, path::{Path, PathBuf}, }; +use workspace_deps::reached_workspace_project_dirs; /// The id (`name` field, falling back to the root dir) of the first /// project that declares dependencies but has no modules directory, or @@ -49,151 +49,6 @@ pub(super) fn first_selected_project_missing_modules_dir( selected.contains(&lexical_normalize(root_dir)) }) } - -struct WorkspaceProjectIndex<'a> { - by_dir: HashMap, - by_name: HashMap)>>, - link_workspace_packages: bool, - catalogs: &'a Catalogs, -} - -impl<'a> WorkspaceProjectIndex<'a> { - fn from_check(check: &OptimisticRepeatInstallCheck<'a>) -> Self { - let mut by_dir = HashMap::new(); - let mut by_name: HashMap)>> = HashMap::new(); - for (dir, manifest) in check.project_manifests { - let norm = lexical_normalize(dir); - by_dir.insert(norm.clone(), *manifest); - if let Some(name) = manifest_string_field(manifest, "name") { - let version = manifest_string_field(manifest, "version"); - by_name - .entry(name) - .or_default() - .push((norm, version)); - } - } - Self { - by_dir, - by_name, - link_workspace_packages: check.config.link_workspace_packages.enabled_at_depth(0), - catalogs: check.catalogs, - } - } - - fn dereference_catalog<'b>(&self, alias: &str, spec: &'b str) -> Option> { - if !spec.starts_with("catalog:") { - return Some(Cow::Borrowed(spec)); - } - let CatalogResolutionResult::Found(found) = resolve_from_catalog( - self.catalogs, - &WantedDependency { alias: alias.to_string(), bare_specifier: spec.to_string() }, - CatalogAnchor::AsWritten, - ) else { - return None; - }; - Some(Cow::Owned(found.resolution.specifier)) - } - - fn dep_targets(&self, dir: &Path, alias: &str, spec: &str) -> Vec { - let Some(spec) = self.dereference_catalog(alias, spec) else { - return Vec::new(); - }; - let spec = spec.as_ref(); - if let Some(rel) = spec - .strip_prefix("link:") - .or_else(|| spec.strip_prefix("file:")) - { - return self.resolve_dir_target(dir, rel); - } - let is_workspace = spec.starts_with("workspace:"); - if !is_workspace && !self.link_workspace_packages { - return Vec::new(); - } - if let Some(ws_spec) = spec.strip_prefix("workspace:") - && is_relative_workspace_path(ws_spec) - { - return self.resolve_dir_target(dir, ws_spec); - } - let (target_name, range) = crate::install::spec_target_and_range(spec); - let name = target_name.unwrap_or(alias); - self.by_name - .get(name) - .into_iter() - .flatten() - .filter(|(_, version)| { - workspace_version_matches_range(version.as_deref(), range, is_workspace) - }) - .map(|(dir, _)| dir.clone()) - .collect() - } - - fn resolve_dir_target(&self, dir: &Path, rel: &str) -> Vec { - let normalized_rel = rel.replace('\\', "/"); - let target = lexical_normalize(&dir.join(normalized_rel)); - self.by_dir - .contains_key(&target) - .then_some(target) - .into_iter() - .collect() - } -} - -fn is_relative_workspace_path(ws_spec: &str) -> bool { - matches!(ws_spec, "." | "..") - || ws_spec.starts_with("./") - || ws_spec.starts_with("../") - || ws_spec.starts_with(r".\") - || ws_spec.starts_with(r"..\") -} - -fn reached_workspace_project_dirs( - check: &OptimisticRepeatInstallCheck<'_>, - state: &WorkspaceState, - roots: &[&Path], -) -> HashSet { - let index = WorkspaceProjectIndex::from_check(check); - let groups = [ - (state.settings.production != Some(false)).then_some(DependencyGroup::Prod), - (state.settings.dev != Some(false)).then_some(DependencyGroup::Dev), - (state.settings.optional != Some(false)).then_some(DependencyGroup::Optional), - (check.config.auto_install_peers && state.settings.auto_install_peers != Some(false)) - .then_some(DependencyGroup::Peer), - ]; - let mut reached = HashSet::new(); - let mut stack: Vec = roots - .iter() - .map(|dir| lexical_normalize(dir)) - .collect(); - while let Some(dir) = stack.pop() { - let Some(&manifest) = index.by_dir - .get(&dir) - .filter(|_| reached.insert(dir.clone())) - else { - continue; - }; - for (alias, spec) in manifest.dependencies(groups.into_iter().flatten()) { - stack.extend(index.dep_targets(&dir, alias, spec)); - } - } - reached -} - -fn workspace_version_matches_range(version: Option<&str>, range: &str, is_workspace: bool) -> bool { - if matches!(range, "" | "*" | "^" | "~") { - return true; - } - if range.parse::().is_err() { - return is_workspace; - } - let Some(version) = version else { - return is_workspace; - }; - let parsed_range = IncludePrereleaseRange::parse(range); - version == range - || version - .parse::() - .is_ok_and(|parsed| parsed_range.satisfies(&parsed)) -} /// The first project that needs a modules directory and lacks one, over the /// projects `is_selected` accepts. fn first_missing_modules_dir( diff --git a/pnpm/crates/package-manager/src/optimistic_repeat_install/modules_dirs/workspace_deps.rs b/pnpm/crates/package-manager/src/optimistic_repeat_install/modules_dirs/workspace_deps.rs new file mode 100644 index 0000000000..6cb7ed99f2 --- /dev/null +++ b/pnpm/crates/package-manager/src/optimistic_repeat_install/modules_dirs/workspace_deps.rs @@ -0,0 +1,183 @@ +use crate::optimistic_repeat_install::{ + CatalogAnchor, CatalogResolutionResult, Catalogs, OptimisticRepeatInstallCheck, + WantedDependency, manifest_string_field, resolve_from_catalog, +}; +use pnpm_fs::lexical_normalize; +use pnpm_package_manifest::{DependencyGroup, PackageManifest}; +use pnpm_semver_include_prerelease::IncludePrereleaseRange; +use pnpm_workspace_state::WorkspaceState; +use std::{ + borrow::Cow, + collections::{HashMap, HashSet}, + path::{Path, PathBuf}, +}; + +struct WorkspaceProjectIndex<'a> { + workspace_dir: PathBuf, + by_dir: HashMap, + by_name: HashMap)>>, + link_workspace_packages: bool, + only_link_deps: bool, + catalogs: &'a Catalogs, +} + +impl<'a> WorkspaceProjectIndex<'a> { + fn from_check(check: &OptimisticRepeatInstallCheck<'a>) -> Self { + let mut by_dir = HashMap::new(); + let mut by_name: HashMap)>> = HashMap::new(); + for (dir, manifest) in check.project_manifests { + let norm = lexical_normalize(dir); + by_dir.insert(norm.clone(), *manifest); + if let Some(name) = manifest_string_field(manifest, "name") { + let version = manifest_string_field(manifest, "version"); + by_name + .entry(name) + .or_default() + .push((norm, version)); + } + } + // Where `pnpm-workspace.yaml` sits, so a local-path catalog entry is + // re-anchored from the workspace directory even when `lockfile_dir` + // moves `check.workspace_root` elsewhere. + let workspace_dir = check.config.workspace_dir + .as_deref() + .or_else(|| { + check.config.lockfile_dir + .is_some() + .then(|| check.project_manifests.first().map(|(dir, _)| dir.as_path())) + .flatten() + }) + .unwrap_or(check.workspace_root); + Self { + workspace_dir: lexical_normalize(workspace_dir), + by_dir, + by_name, + link_workspace_packages: check.config.link_workspace_packages.enabled_at_depth(0), + only_link_deps: !check.config.shares_one_lockfile() + && check.config.inject_workspace_packages, + catalogs: check.catalogs, + } + } + + fn dereference_catalog<'b>( + &self, + dir: &Path, + alias: &str, + spec: &'b str, + ) -> Option> { + if !spec.starts_with("catalog:") { + return Some(Cow::Borrowed(spec)); + } + let CatalogResolutionResult::Found(found) = resolve_from_catalog( + self.catalogs, + &WantedDependency { alias: alias.to_string(), bare_specifier: spec.to_string() }, + CatalogAnchor::Reanchor { workspace_dir: &self.workspace_dir, consumer_dir: Some(dir) }, + ) else { + return None; + }; + Some(Cow::Owned(found.resolution.specifier)) + } + + fn dep_targets(&self, dir: &Path, alias: &str, spec: &str) -> Vec { + let Some(spec) = self.dereference_catalog(dir, alias, spec) else { + return Vec::new(); + }; + let spec = spec.as_ref(); + if let Some(rel) = spec.strip_prefix("link:") { + return self.resolve_dir_target(dir, rel); + } + if self.only_link_deps { + return Vec::new(); + } + if let Some(rel) = spec.strip_prefix("file:") { + return self.resolve_dir_target(dir, rel); + } + let is_workspace = spec.starts_with("workspace:"); + if !is_workspace && !self.link_workspace_packages { + return Vec::new(); + } + if let Some(ws_spec) = spec.strip_prefix("workspace:") + && is_relative_workspace_path(ws_spec) + { + return self.resolve_dir_target(dir, ws_spec); + } + let (target_name, range) = crate::install::spec_target_and_range(spec); + let name = target_name.unwrap_or(alias); + self.by_name + .get(name) + .into_iter() + .flatten() + .filter(|(_, version)| { + workspace_version_matches_range(version.as_deref(), range, is_workspace) + }) + .map(|(dir, _)| dir.clone()) + .collect() + } + + fn resolve_dir_target(&self, dir: &Path, rel: &str) -> Vec { + let normalized_rel = rel.replace('\\', "/"); + let target = lexical_normalize(&dir.join(normalized_rel)); + self.by_dir + .contains_key(&target) + .then_some(target) + .into_iter() + .collect() + } +} + +fn is_relative_workspace_path(ws_spec: &str) -> bool { + matches!(ws_spec, "." | "..") + || ws_spec.starts_with("./") + || ws_spec.starts_with("../") + || ws_spec.starts_with(r".\") + || ws_spec.starts_with(r"..\") +} + +pub(super) fn reached_workspace_project_dirs( + check: &OptimisticRepeatInstallCheck<'_>, + state: &WorkspaceState, + roots: &[&Path], +) -> HashSet { + let index = WorkspaceProjectIndex::from_check(check); + let groups = [ + (state.settings.production != Some(false)).then_some(DependencyGroup::Prod), + (state.settings.dev != Some(false)).then_some(DependencyGroup::Dev), + (state.settings.optional != Some(false)).then_some(DependencyGroup::Optional), + (check.config.auto_install_peers && state.settings.auto_install_peers != Some(false)) + .then_some(DependencyGroup::Peer), + ]; + let mut reached = HashSet::new(); + let mut stack: Vec = roots + .iter() + .map(|dir| lexical_normalize(dir)) + .collect(); + while let Some(dir) = stack.pop() { + let Some(&manifest) = index.by_dir + .get(&dir) + .filter(|_| reached.insert(dir.clone())) + else { + continue; + }; + for (alias, spec) in manifest.dependencies(groups.into_iter().flatten()) { + stack.extend(index.dep_targets(&dir, alias, spec)); + } + } + reached +} + +fn workspace_version_matches_range(version: Option<&str>, range: &str, is_workspace: bool) -> bool { + if matches!(range, "" | "*" | "^" | "~") { + return true; + } + if range.parse::().is_err() { + return is_workspace; + } + let Some(version) = version else { + return is_workspace; + }; + let parsed_range = IncludePrereleaseRange::parse(range); + version == range + || version + .parse::() + .is_ok_and(|parsed| parsed_range.satisfies(&parsed)) +} diff --git a/pnpm/crates/package-manager/src/optimistic_repeat_install/tests/catalogs.rs b/pnpm/crates/package-manager/src/optimistic_repeat_install/tests/catalogs.rs index 5b8b1f9ec1..b8ca7e0eff 100644 --- a/pnpm/crates/package-manager/src/optimistic_repeat_install/tests/catalogs.rs +++ b/pnpm/crates/package-manager/src/optimistic_repeat_install/tests/catalogs.rs @@ -251,3 +251,67 @@ fn returns_outdated_when_single_project_catalog_cache_changes() { ); assert_eq!(decision, Decision::Skipped { reason: "catalogs cache outdated" }); } + +#[test] +fn expanded_selected_modules_dir_check_reanchors_catalog_links_from_workspace_dir_not_lockfile_dir() +{ + let linker = pnpm_config::NodeLinker::Isolated; + let external_lockfile_dir = tempdir().unwrap(); + let (dir, config, root_manifest) = setup_fresh_install_with_config( + linker, + "root", + "1.0.0", + r#""dependencies":{"linked":"catalog:"}"#, + |config| { + config.lockfile_dir = Some(external_lockfile_dir.path().to_path_buf()); + }, + ); + let linked_dir = dir.path().join("packages/linked"); + fs::create_dir_all(&linked_dir).unwrap(); + let linked_manifest_path = linked_dir.join("package.json"); + fs::write( + &linked_manifest_path, + r#"{"name":"linked","version":"1.0.0","dependencies":{"foo":"1.0.0"}}"#, + ) + .unwrap(); + let linked_manifest = PackageManifest::from_path(linked_manifest_path).unwrap(); + + let project_manifests = + [(dir.path().to_path_buf(), &root_manifest), (linked_dir.clone(), &linked_manifest)]; + let catalogs = BTreeMap::from([( + "default".to_string(), + BTreeMap::from([("linked".to_string(), "link:./packages/linked".to_string())]), + )]); + let check = OptimisticRepeatInstallCheck { + workspace_root: external_lockfile_dir.path(), + config, + project_manifests: &project_manifests, + is_workspace_install: true, + lockfile: MaybeLazyLockfile::Loaded(None), + catalogs: &catalogs, + layout: crate::RepeatInstallLayout { + node_linker: linker, + included: isolated_included(), + supported_architectures: None, + }, + manifests: crate::RepeatInstallManifests::ON_DISK, + }; + let state = pnpm_workspace_state::WorkspaceState::default(); + let selected = [dir.path()]; + + assert_eq!( + super::super::modules_dirs::first_selected_project_missing_modules_dir( + &check, &state, &selected, true, + ) + .as_deref(), + Some("linked"), + ); + + fs::create_dir_all(linked_dir.join("node_modules")).unwrap(); + assert_eq!( + super::super::modules_dirs::first_selected_project_missing_modules_dir( + &check, &state, &selected, true, + ), + None, + ); +} diff --git a/pnpm/crates/package-manager/src/optimistic_repeat_install/tests/modules_dir.rs b/pnpm/crates/package-manager/src/optimistic_repeat_install/tests/modules_dir.rs index f1d559195d..405e4d2229 100644 --- a/pnpm/crates/package-manager/src/optimistic_repeat_install/tests/modules_dir.rs +++ b/pnpm/crates/package-manager/src/optimistic_repeat_install/tests/modules_dir.rs @@ -408,3 +408,79 @@ fn expanded_selected_modules_dir_check_follows_aliases_paths_and_matching_versio Some("pkg-f"), ); } + +#[test] +fn expanded_selected_modules_dir_check_under_dedicated_lockfiles_with_injection_follows_only_link_deps() + { + let linker = pnpm_config::NodeLinker::Isolated; + let (dir, config, root_manifest) = + setup_fresh_install_with_config(linker, "root", "1.0.0", "", |config| { + config.shared_workspace_lockfile = false; + config.inject_workspace_packages = true; + }); + let write_pkg = |rel: &str, json: &str, create_modules_dir: bool| { + let pkg_dir = dir.path().join(rel); + fs::create_dir_all(&pkg_dir).unwrap(); + if create_modules_dir { + fs::create_dir_all(pkg_dir.join("node_modules")).unwrap(); + } + let manifest_path = pkg_dir.join("package.json"); + fs::write(&manifest_path, json).unwrap(); + (pkg_dir, PackageManifest::from_path(manifest_path).unwrap()) + }; + let (app_dir, app_manifest) = write_pkg( + "app", + r#"{"name":"app","version":"1.0.0","dependencies":{"injected":"workspace:*","linked":"link:../linked"}}"#, + true, + ); + let (injected_dir, injected_manifest) = write_pkg( + "injected", + r#"{"name":"injected","version":"1.0.0","dependencies":{"foo":"1.0.0"}}"#, + false, + ); + let (linked_dir, linked_manifest) = write_pkg( + "linked", + r#"{"name":"linked","version":"1.0.0","dependencies":{"foo":"1.0.0"}}"#, + false, + ); + + let project_manifests = [ + (dir.path().to_path_buf(), &root_manifest), + (app_dir.clone(), &app_manifest), + (injected_dir, &injected_manifest), + (linked_dir.clone(), &linked_manifest), + ]; + let catalogs = BTreeMap::default(); + let check = OptimisticRepeatInstallCheck { + workspace_root: dir.path(), + config, + project_manifests: &project_manifests, + is_workspace_install: true, + lockfile: MaybeLazyLockfile::Loaded(None), + catalogs: &catalogs, + layout: crate::RepeatInstallLayout { + node_linker: linker, + included: isolated_included(), + supported_architectures: None, + }, + manifests: crate::RepeatInstallManifests::ON_DISK, + }; + let state = pnpm_workspace_state::WorkspaceState::default(); + let selected = [app_dir.as_path()]; + + assert_eq!( + super::super::modules_dirs::first_selected_project_missing_modules_dir( + &check, &state, &selected, true, + ) + .as_deref(), + Some("linked"), + ); + + fs::create_dir_all(linked_dir.join("node_modules")).unwrap(); + assert_eq!( + super::super::modules_dirs::first_selected_project_missing_modules_dir( + &check, &state, &selected, true, + ), + None, + ); +} diff --git a/pnpm11/__fixtures__/pnpm-workspace.yaml b/pnpm11/__fixtures__/pnpm-workspace.yaml index fea4dd5fb4..c1668526b4 100644 --- a/pnpm11/__fixtures__/pnpm-workspace.yaml +++ b/pnpm11/__fixtures__/pnpm-workspace.yaml @@ -14,6 +14,7 @@ packages: - '!has-yarn2-lock' - '!workspace-with-lockfile-dupes' - '!workspace-with-lockfile-subdep-dupes' + - '!workspace-with-nested-workspace-deps' - '!workspace-has-shared-yarn-lock' - '!workspace-has-shared-package-lock-json' - '!workspace-has-shared-npm-shrinkwrap-json' diff --git a/pnpm11/deps/status/test/recordedValidation.test.ts b/pnpm11/deps/status/test/recordedValidation.test.ts index 3374889f93..d4322d9e28 100644 --- a/pnpm11/deps/status/test/recordedValidation.test.ts +++ b/pnpm11/deps/status/test/recordedValidation.test.ts @@ -486,6 +486,45 @@ describe('checkDepsStatus with recorded validations', () => { await fs.mkdir(bLockfilePath) await fs.utimes(bLockfilePath, bLockfileMtime, bLockfileMtime) expect(await check(workspace, ['a'], { sharedWorkspaceLockfile: false })).toMatchObject({ upToDate: true }) + + // Under dedicated lockfiles with injection, a non-recursive check in `a` + // skips injected `workspace:*` dependencies while still holding `link:` dependencies. + await fs.rm(bLockfilePath, { recursive: true, force: true }) + await fs.rm(path.join(workspace.projectDir('b'), 'node_modules'), { recursive: true, force: true }) + const recordVouchedA = async (bSpec: string): Promise => { + workspace.manifestOf('a').dependencies = { foo: '1.0.0', b: bSpec } + await updateWorkspaceState({ + allProjects: workspace.allProjects, + workspaceDir: workspace.workspaceDir, + pnpmfiles: [], + settings: { ...SETTINGS, injectWorkspacePackages: true }, + configDependencies: CONFIG_DEPENDENCIES, + filteredInstall: true, + validation: { + lockfileDir: workspace.workspaceDir, + wantedLockfileName: 'pnpm-lock.yaml', + sharedInputs: [], + sharedWorkspaceLockfile: false, + vouchedProjects: [workspace.projectDir('a')], + }, + }) + } + await recordVouchedA('workspace:*') + expect(await check(workspace, undefined, { + dir: workspace.projectDir('a'), + injectWorkspacePackages: true, + sharedWorkspaceLockfile: false, + })).toMatchObject({ upToDate: true }) + + await recordVouchedA('link:../b') + expect(await check(workspace, undefined, { + dir: workspace.projectDir('a'), + injectWorkspacePackages: true, + sharedWorkspaceLockfile: false, + })).toMatchObject({ + upToDate: false, + issue: 'Workspace package b has dependencies but does not have a modules directory', + }) }) }) diff --git a/pnpm11/exec/commands/src/selectProjectsToVerify.ts b/pnpm11/exec/commands/src/selectProjectsToVerify.ts index 809d52fb17..daccd128a4 100644 --- a/pnpm11/exec/commands/src/selectProjectsToVerify.ts +++ b/pnpm11/exec/commands/src/selectProjectsToVerify.ts @@ -66,29 +66,27 @@ function walkDependencies (projectsGraph: ProjectsGraph, roots: ProjectRootDir[] const node = projectsGraph[dir] if (reached[dir] != null || node == null) continue reached[dir] = node - const dependencies = onlyLinked - ? node.dependencies.filter((depDir) => hasLinkDependencyTo(dir, node.package.manifest, depDir)) - : node.dependencies + let dependencies = node.dependencies + if (onlyLinked && dependencies.length > 0) { + const linkedTargets = linkedDependencyDirs(dir, node.package.manifest) + dependencies = dependencies.filter((depDir) => linkedTargets.has(path.resolve(depDir))) + } stack.push(...dependencies) } return reached } -function hasLinkDependencyTo (parentDir: ProjectRootDir, manifest: ProjectManifest | undefined, targetDir: ProjectRootDir): boolean { - if (manifest == null) return false - const resolvedTarget = path.resolve(targetDir) +function linkedDependencyDirs (parentDir: ProjectRootDir, manifest: ProjectManifest | undefined): Set { + const targets = new Set() + if (manifest == null) return targets for (const field of ['dependencies', 'devDependencies', 'optionalDependencies', 'peerDependencies'] as const) { for (const spec of Object.values(manifest[field] ?? {})) { - if ( - typeof spec === 'string' && - spec.startsWith('link:') && - path.resolve(parentDir, spec.slice('link:'.length).replace(/\\/g, '/')) === resolvedTarget - ) { - return true + if (typeof spec === 'string' && spec.startsWith('link:')) { + targets.add(path.resolve(parentDir, spec.slice('link:'.length).replace(/\\/g, '/'))) } } } - return false + return targets } function hasNegatedSelector (opts: ProjectsToVerifyOptions): boolean { diff --git a/pnpm11/pnpm/test/verifyDepsBeforeRun/multiProjectWorkspace.ts b/pnpm11/pnpm/test/verifyDepsBeforeRun/multiProjectWorkspace.ts index b386cc34f4..8d9505be20 100644 --- a/pnpm11/pnpm/test/verifyDepsBeforeRun/multiProjectWorkspace.ts +++ b/pnpm11/pnpm/test/verifyDepsBeforeRun/multiProjectWorkspace.ts @@ -1146,6 +1146,80 @@ test('exec installs a workspace dependency of the selected project after a filte expect(fs.existsSync(path.resolve('bar/node_modules/@pnpm.e2e/foo'))).toBeTruthy() }) +// Under dedicated lockfiles with `injectWorkspacePackages`, an injected +// `workspace:*` dependency is a copy inside the dependent project that its own +// install covers, whereas an explicit `link:` dependency remains a symlink +// whose target project must still be installed +// (https://github.com/pnpm/tasks/issues/179). +test('exec holds a link: dependency under dedicated lockfiles with injectWorkspacePackages', async () => { + const projects = preparePackages([ + { + location: '.', + package: { name: 'root', private: true }, + }, + { + name: 'foo', + private: true, + dependencies: { bar: 'workspace:*' }, + }, + { + name: 'bar', + private: true, + dependencies: { '@pnpm.e2e/foo': '=100.0.0' }, + }, + { + name: 'baz', + private: true, + dependencies: { '@pnpm.e2e/foo': '=100.0.0' }, + }, + ]) + writeYamlFileSync('pnpm-workspace.yaml', { + packages: ['**', '!store/**'], + sharedWorkspaceLockfile: false, + injectWorkspacePackages: true, + }) + + execPnpmSync(['--filter=foo', 'install'], { expectSuccess: true }) + expect(fs.existsSync(path.resolve('bar/node_modules'))).toBeFalsy() + + const injectedResult = execPnpmSync([ + ...CONFIG, + '--filter=foo', + 'exec', + 'node', + '-e', + "console.log('injected-ok')", + ], { expectSuccess: true }) + expect(injectedResult.stdout.toString()).toContain('injected-ok') + + projects.foo.writePackageJson({ + name: 'foo', + private: true, + dependencies: { + bar: 'workspace:*', + baz: 'link:../baz', + }, + }) + execPnpmSync(['--filter=foo', 'install'], { expectSuccess: true }) + expect(fs.existsSync(path.resolve('baz/node_modules'))).toBeFalsy() + + const { status, stdout } = execPnpmSync([...CONFIG, '--filter=foo', 'exec', 'node', '-e', '0']) + expect(status).not.toBe(0) + expect(stdout.toString()).toContain('ERR_PNPM_VERIFY_DEPS_BEFORE_RUN') + + const linkedResult = execPnpmSync([ + '--config.verify-deps-before-run=install', + '--filter=foo', + 'exec', + 'node', + '-e', + "console.log('linked-ok')", + ], { expectSuccess: true }) + expect(linkedResult.stdout.toString()).toContain('linked-ok') + expect(fs.existsSync(path.resolve('baz/node_modules/@pnpm.e2e/foo'))).toBeTruthy() + execPnpmSync([...CONFIG, '--filter=foo', 'exec', 'node', '-e', '0'], { expectSuccess: true }) +}) + // A negated selector reaches the install the gate spawns unchanged, so the // project it excludes is not required to have a modules directory. test('exec does not require a workspace dependency that a negated selector excludes', async () => { diff --git a/pnpm11/workspace/projects-graph/src/index.ts b/pnpm11/workspace/projects-graph/src/index.ts index 6028924b35..4a61603022 100644 --- a/pnpm11/workspace/projects-graph/src/index.ts +++ b/pnpm11/workspace/projects-graph/src/index.ts @@ -104,6 +104,7 @@ function parseDependencySpec ( isWorkspaceSpec: boolean ): { depName: string, rawSpec: string, spec: { fetchSpec: string, type: string } } | undefined { try { + const isLinkSpec = !isWorkspaceSpec && rawSpec.startsWith('link:') if (isWorkspaceSpec) { const npmSpec = workspacePrefToNpm(rawSpec) if (isRelativePathSpec(npmSpec)) { @@ -113,8 +114,13 @@ function parseDependencySpec ( } } else if (rawSpec.startsWith('npm:')) { ({ depName, rawSpec } = parseRegistrySpec(depName, rawSpec)) + } else if (isLinkSpec) { + rawSpec = `file:${rawSpec.slice('link:'.length)}` } const spec = npa.resolve(depName, rawSpec, projectRootDir) + if (isLinkSpec && spec.type === 'file') { + spec.type = 'directory' + } return { depName, rawSpec, spec } } catch { return undefined diff --git a/pnpm11/workspace/projects-graph/test/index.ts b/pnpm11/workspace/projects-graph/test/index.ts index f85be3a2ee..b4ea99f3c5 100644 --- a/pnpm11/workspace/projects-graph/test/index.ts +++ b/pnpm11/workspace/projects-graph/test/index.ts @@ -166,6 +166,7 @@ test('create package graph using peer dependencies', () => { }) test('create package graph for local directory dependencies', () => { + const TAR_DIR_PATH = pathResolve('/zkochan/src/bar.tgz') const result = createProjectsGraph([ { rootDir: BAR1_PATH, @@ -199,9 +200,18 @@ test('create package graph for local directory dependencies', () => { dependencies: { foo: 'file:../foo@2', + bar1: 'link:../bar', + tarDir: 'link:../bar.tgz', }, }, }, + { + rootDir: TAR_DIR_PATH, + manifest: { + name: 'tar-dir', + version: '1.0.0', + }, + }, { rootDir: FOO2_PATH, manifest: { @@ -243,7 +253,7 @@ test('create package graph for local directory dependencies', () => { }, }, [BAR2_PATH]: { - dependencies: [FOO2_PATH], + dependencies: [FOO2_PATH, BAR1_PATH, TAR_DIR_PATH], package: { rootDir: BAR2_PATH, manifest: { @@ -252,10 +262,22 @@ test('create package graph for local directory dependencies', () => { dependencies: { foo: 'file:../foo@2', + bar1: 'link:../bar', + tarDir: 'link:../bar.tgz', }, }, }, }, + [TAR_DIR_PATH]: { + dependencies: [], + package: { + rootDir: TAR_DIR_PATH, + manifest: { + name: 'tar-dir', + version: '1.0.0', + }, + }, + }, [FOO2_PATH]: { dependencies: [], package: {