From d515d4766ac148eddc8e22473ead20bbd2087ef2 Mon Sep 17 00:00:00 2001 From: Zoltan Kochan Date: Wed, 16 Sep 2026 10:38:22 +0200 Subject: [PATCH] fix(deploy): link bins exposed by workspace dependencies (#14942) Deploy-generated directory resolutions omitted `hasBin` from their lockfile metadata, so the precomputed has-bin set dropped every workspace dependency slot and the deployed project's `node_modules/.bin` directory came out empty. Record definitive bin metadata when generating workspace directory rows. Probe directory resolutions whose metadata is absent for compatibility with older lockfiles while continuing to skip resolutions explicitly marked `hasBin: false`. Closes pnpm/pnpm#14899 --- .changeset/cuddly-spoons-link.md | 5 ++ pnpm/crates/cli/src/cli_args/deploy.rs | 16 ++++- .../cli/src/cli_args/deploy/lockfile.rs | 4 +- pnpm/crates/cli/src/cli_args/deploy/tests.rs | 1 + pnpm/crates/cli/tests/suite/deploy.rs | 72 +++++++++++++++++++ pnpm/crates/deps-restorer/src/link_bins.rs | 10 ++- .../deps-restorer/src/link_bins/tests.rs | 19 ++--- .../src/dependencies_graph_to_lockfile.rs | 3 +- .../packages.rs | 2 +- 9 files changed, 110 insertions(+), 22 deletions(-) create mode 100644 .changeset/cuddly-spoons-link.md diff --git a/.changeset/cuddly-spoons-link.md b/.changeset/cuddly-spoons-link.md new file mode 100644 index 0000000000..ffc2b0f093 --- /dev/null +++ b/.changeset/cuddly-spoons-link.md @@ -0,0 +1,5 @@ +--- +"pacquet": patch +--- + +`pnpm deploy` now links commands exposed by workspace dependencies into the deployed project's `node_modules/.bin` directory [pnpm/pnpm#14899](https://github.com/pnpm/pnpm/issues/14899). diff --git a/pnpm/crates/cli/src/cli_args/deploy.rs b/pnpm/crates/cli/src/cli_args/deploy.rs index 8eaadb2b22..5908b840f0 100644 --- a/pnpm/crates/cli/src/cli_args/deploy.rs +++ b/pnpm/crates/cli/src/cli_args/deploy.rs @@ -26,7 +26,9 @@ use pnpm_lockfile::{ WantedLockfileSelection, }; use pnpm_lockfile_preferred_versions::get_preferred_versions_from_lockfile_and_manifests; -use pnpm_package_manager::{ImportIndexedDirOpts, apply_deploy_manifest_hook, import_indexed_dir}; +use pnpm_package_manager::{ + ImportIndexedDirOpts, apply_deploy_manifest_hook, import_indexed_dir, manifest_has_bin, +}; use pnpm_package_manifest::{DependencyGroup, PackageManifest}; use pnpm_reporter::{LogEvent, LogLevel, PnpmLog, Reporter}; use pnpm_resolving_resolver_base::PreferredVersions; @@ -125,6 +127,7 @@ enum DeployError { #[derive(Clone)] struct ProjectInfo { name: Option, + has_bin: bool, peer_dependencies: Vec, /// Names the project declares as prod or optional dependencies. A peer it /// depends on itself is already bound by that edge, whether or not the @@ -137,6 +140,16 @@ struct SelectedProject { projects_by_path: HashMap, } +impl SelectedProject { + fn project_info(&self, root: &Path) -> Option<&ProjectInfo> { + self.projects_by_path.get(&ProjectPathKey::new(root)) + } + + fn has_bin(&self, root: &Path) -> Option { + self.project_info(root).map(|project| project.has_bin) + } +} + struct DeployWorkspaceConfig { patched_dependencies: Option>, allow_builds: HashMap, @@ -347,6 +360,7 @@ fn index_projects(projects: &[Project]) -> HashMap .get("name") .and_then(Value::as_str) .map(str::to_string), + has_bin: manifest_has_bin(Some(project.manifest.value())).is_some(), peer_dependencies: manifest_dependency_names( &project.manifest, &["peerDependencies"], diff --git a/pnpm/crates/cli/src/cli_args/deploy/lockfile.rs b/pnpm/crates/cli/src/cli_args/deploy/lockfile.rs index 32ca619b20..396fdad397 100644 --- a/pnpm/crates/cli/src/cli_args/deploy/lockfile.rs +++ b/pnpm/crates/cli/src/cli_args/deploy/lockfile.rs @@ -219,7 +219,7 @@ fn convert_deploy_packages( os: None, libc: None, deprecated: None, - has_bin: None, + has_bin: selected.has_bin(&project_root), prepare: None, bundled_dependencies: None, peer_dependencies: None, @@ -258,7 +258,7 @@ fn convert_deploy_snapshots( validate_lockfile_local_path(&lockfile_dir.join(importer_path), lockfile_dir)?; let bases = ResolveBases { file_base: lockfile_dir, link_base: &project_root }; let package_key = create_file_url_key(&project_root, "", &selected.projects_by_path, None)?; - if let Some(project) = selected.projects_by_path.get(&ProjectPathKey::new(&project_root)) + if let Some(project) = selected.project_info(&project_root) && !project.peer_dependencies.is_empty() { linked_workspace_projects.insert(package_key.clone(), project.clone()); diff --git a/pnpm/crates/cli/src/cli_args/deploy/tests.rs b/pnpm/crates/cli/src/cli_args/deploy/tests.rs index 869f774bd9..a355b14efb 100644 --- a/pnpm/crates/cli/src/cli_args/deploy/tests.rs +++ b/pnpm/crates/cli/src/cli_args/deploy/tests.rs @@ -154,6 +154,7 @@ fn create_file_url_key_prefers_workspace_project_name() { ProjectPathKey::new(&project_root), ProjectInfo { name: Some("workspace-name".to_string()), + has_bin: false, peer_dependencies: Vec::new(), declared_dependencies: HashSet::default(), }, diff --git a/pnpm/crates/cli/tests/suite/deploy.rs b/pnpm/crates/cli/tests/suite/deploy.rs index 4c133cd5a5..a526a10aaf 100644 --- a/pnpm/crates/cli/tests/suite/deploy.rs +++ b/pnpm/crates/cli/tests/suite/deploy.rs @@ -113,6 +113,78 @@ fn deploy_from_shared_lockfile_installs_selected_project() { drop((root, mock_instance)); } +#[test] +fn deploy_links_workspace_dependency_bins() { + let CommandTempCwd { + pacquet, + root, + workspace, + npmrc_info, + .. + } = CommandTempCwd::init().add_mocked_registry(); + let AddMockedRegistry { mock_instance, .. } = npmrc_info; + write_workspace(&workspace, true); + write_project( + &workspace, + "lib", + &serde_json::json!({ + "name": "lib", + "version": "1.0.0", + "files": ["index.js", "bin.js"], + "bin": { "lib-cli": "bin.js" }, + "dependencies": { "binless": "workspace:*" }, + }), + ); + write_project( + &workspace, + "binless", + &serde_json::json!({ + "name": "binless", + "version": "1.0.0", + "files": ["index.js"], + }), + ); + fs::write(workspace.join("packages/lib/bin.js"), "#!/usr/bin/env node\n").unwrap(); + + pacquet + .with_arg("install") + .assert() + .success(); + for (deploy_dir, extra_args) in + [("deploy", [].as_slice()), ("legacy-deploy", ["--legacy"].as_slice())] + { + pacquet_cmd(&workspace) + .with_args(["--filter", "app", "deploy", "--prod"]) + .with_args(extra_args) + .with_arg(deploy_dir) + .assert() + .success(); + assert!( + workspace + .join(deploy_dir) + .join("node_modules/.bin/lib-cli") + .exists(), + "{deploy_dir} must link the workspace dependency's bin", + ); + if deploy_dir == "deploy" { + let lockfile = + Lockfile::load_wanted_from_dir(&workspace.join(deploy_dir)).unwrap().unwrap(); + assert_eq!(package_has_bin(&lockfile, "lib"), Some(Some(true))); + assert_eq!(package_has_bin(&lockfile, "binless"), Some(Some(false))); + } + } + + drop((root, mock_instance)); +} + +fn package_has_bin(lockfile: &Lockfile, name: &str) -> Option> { + lockfile.packages + .as_ref()? + .iter() + .find(|(key, _)| key.name.to_string() == name) + .map(|(_, metadata)| metadata.has_bin) +} + /// A pinned `lockfileDir` moves the shared lockfile `deploy` reads and /// the importer id naming the selected project in it. Reading either from /// the workspace root instead drops the deploy to its "shared lockfile not diff --git a/pnpm/crates/deps-restorer/src/link_bins.rs b/pnpm/crates/deps-restorer/src/link_bins.rs index 49da3dd789..bba4b31142 100644 --- a/pnpm/crates/deps-restorer/src/link_bins.rs +++ b/pnpm/crates/deps-restorer/src/link_bins.rs @@ -167,8 +167,8 @@ impl LinkVirtualStoreBins<'_> { } } -/// Pre-compute the set of package keys whose lockfile metadata sets -/// `hasBin: true`. Most packages don't declare a bin, so +/// Pre-compute the set of package keys whose lockfile metadata says they +/// may expose bins. Most packages don't declare a bin, so /// short-circuiting the per-child manifest lookup with this set is /// the cheapest win on warm-cache installs. /// @@ -180,7 +180,9 @@ impl LinkVirtualStoreBins<'_> { /// linker falls back to the conservative "process every child" /// path and lets the per-package bin resolver sort it out. /// - `Some(set)` — the section was present and we used it. The -/// `set` contains only entries with `hasBin == Some(true)`; an +/// `set` contains entries with `hasBin == Some(true)`, runtimes whose +/// bin metadata lives in their resolution, and local directories whose +/// generated metadata does not carry `hasBin`; an /// *empty* `Some(set)` is authoritative: the lockfile says no /// package has a bin, and every slot should short-circuit /// immediately. Conflating this case with `None` (the bug flagged @@ -199,6 +201,8 @@ fn build_has_bin_set( meta.resolution, LockfileResolution::Binary(_) | LockfileResolution::Variations(_), ) + || (meta.has_bin.is_none() + && matches!(meta.resolution, LockfileResolution::Directory(_))) }) .map(|(key, _)| key.clone()) .collect(), diff --git a/pnpm/crates/deps-restorer/src/link_bins/tests.rs b/pnpm/crates/deps-restorer/src/link_bins/tests.rs index f2d55fdc41..f686ad6f4a 100644 --- a/pnpm/crates/deps-restorer/src/link_bins/tests.rs +++ b/pnpm/crates/deps-restorer/src/link_bins/tests.rs @@ -624,17 +624,12 @@ fn dummy_binary_resolution() -> BinaryResolution { } } -/// `build_has_bin_set` must include runtime resolutions -/// (`Binary` / `Variations`) unconditionally, *regardless of* -/// `meta.has_bin`. Pnpm v11 doesn't emit `hasBin: true` for -/// runtime entries in `pnpm-lock.yaml` (the bin info lives on -/// `resolution.bin`, not at the metadata level), so the existing -/// `has_bin == Some(true)` filter would drop runtime slots from -/// the bin-link dispatch and the synthesized `package.json` -/// (from `install_package_by_snapshot::synthesize_runtime_manifest_bytes`) -/// would go unread. +/// Runtime resolutions must be included without `has_bin` because pnpm v11 +/// records their bin in the resolution instead of emitting `hasBin: true`. +/// Directory resolutions need the same fallback because deploy lockfiles can +/// omit `hasBin` for workspace packages. #[test] -fn build_has_bin_set_includes_runtime_resolutions_even_when_has_bin_is_absent() { +fn build_has_bin_set_includes_resolutions_with_implicit_bin_metadata() { let registry_with_bin: PackageKey = "react@18.0.0".parse().expect("parse react key"); let registry_no_bin: PackageKey = "lodash@4.17.0".parse().expect("parse lodash key"); let runtime_binary: PackageKey = "node@22.0.0".parse().expect("parse node key"); @@ -664,8 +659,6 @@ fn build_has_bin_set_includes_runtime_resolutions_even_when_has_bin_is_absent() ); packages.insert( runtime_binary.clone(), - // Runtime entry without `has_bin: true` (pnpm v11 does - // not emit it for runtimes). Must still land in the set. metadata_with_resolution(LockfileResolution::Binary(dummy_binary_resolution()), None), ); packages.insert( @@ -702,7 +695,7 @@ fn build_has_bin_set_includes_runtime_resolutions_even_when_has_bin_is_absent() set.contains(&runtime_variations), "Variations runtime must be in the set unconditionally", ); - assert!(!set.contains(&directory), "directory without has_bin must be filtered out"); + assert!(set.contains(&directory), "directory without has_bin must be probed for bins"); } #[test] diff --git a/pnpm/crates/package-manager/src/dependencies_graph_to_lockfile.rs b/pnpm/crates/package-manager/src/dependencies_graph_to_lockfile.rs index 79f30defcf..2ba3532d31 100644 --- a/pnpm/crates/package-manager/src/dependencies_graph_to_lockfile.rs +++ b/pnpm/crates/package-manager/src/dependencies_graph_to_lockfile.rs @@ -7,8 +7,7 @@ //! fanning it out on write. pub(crate) use importers::manifest_publish_config; -pub use packages::PackageMetadataSources; -pub(crate) use packages::manifest_has_bin; +pub use packages::{PackageMetadataSources, manifest_has_bin}; mod packages; diff --git a/pnpm/crates/package-manager/src/dependencies_graph_to_lockfile/packages.rs b/pnpm/crates/package-manager/src/dependencies_graph_to_lockfile/packages.rs index e8db8a53d4..3d80a9b177 100644 --- a/pnpm/crates/package-manager/src/dependencies_graph_to_lockfile/packages.rs +++ b/pnpm/crates/package-manager/src/dependencies_graph_to_lockfile/packages.rs @@ -279,7 +279,7 @@ pub(super) fn read_string_or_list( } /// `Some(true)` when the manifest declares executable files, recorded as /// the `hasBin: true` signal; the field is dropped entirely when absent. -pub(crate) fn manifest_has_bin(manifest: Option<&Value>) -> Option { +pub fn manifest_has_bin(manifest: Option<&Value>) -> Option { let manifest = manifest?; let has_bin = manifest .get("bin")