From 1ad6ffd15212c1062ece8eea6c39bec61bc32ccb Mon Sep 17 00:00:00 2001 From: Zoltan Kochan Date: Thu, 14 May 2026 10:36:20 +0200 Subject: [PATCH] feat(config,package-manager): hoistingLimits + externalDependencies knobs (#438 slice 10) (#522) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Plumbs the two programmatic-only hoister knobs from `pnpm-workspace.yaml` through to the slice 4 walker and the slice 3 hoister. Both fields already existed on `HoistOpts`; this slice wires them end-to-end. - `Config::hoisting_limits: BTreeMap>` — per-importer block-list, locator-keyed (`'.@'` for the root). Reads `hoistingLimits: { ".@": [foo, bar] }` from yaml. Mirrors upstream's https://github.com/pnpm/pnpm/blob/94240bc046/installing/linking/real-hoist/src/index.ts#L10 programmatic-only knob, exposed as yaml for parity since the ergonomics of the locator-keyed map don't translate to a CLI flag. - `Config::external_dependencies: BTreeSet` — name slots reserved at the root for an external linker (the Bit CLI is the only known consumer upstream). Reads `externalDependencies: [...]` from yaml. - `LockfileToHoistedDepGraphOptions` gains both fields and forwards them to `HoistOpts` in `build_dep_graph`. - `InstallFrozenLockfile::run` clones the two `Config` fields into the walker opts. Both knobs default to empty (no limits, no externals), matching upstream's default. Neither has any effect under `nodeLinker: isolated` — the isolated linker keeps per-importer subtrees by construction and doesn't consult the hoister. Tests: - `parses_hoisting_limits_from_yaml_and_applies` — yaml round-trip + apply_to. - `parses_external_dependencies_from_yaml_and_applies` — same. - `omitting_hoisting_limits_and_external_dependencies_keeps_defaults` — pins the apply_to skip-on-None branch so a yaml without these keys doesn't accidentally overwrite Config defaults. - `walker_forwards_external_dependencies_to_hoister` — end-to-end: the walker observes an empty graph for an externalised alias because the hoister stripped it. Pins the slice 10 plumbing. --- pacquet/crates/config/src/lib.rs | 39 ++++++++++- pacquet/crates/config/src/workspace_yaml.rs | 15 ++++- .../crates/config/src/workspace_yaml/tests.rs | 64 +++++++++++++++++++ .../package-manager/src/hoisted_dep_graph.rs | 63 +++++++++++++++++- .../src/install_frozen_lockfile.rs | 2 + .../install_package_from_registry/tests.rs | 2 + 6 files changed, 180 insertions(+), 5 deletions(-) diff --git a/pacquet/crates/config/src/lib.rs b/pacquet/crates/config/src/lib.rs index 785736e7e5..daef6ce02a 100644 --- a/pacquet/crates/config/src/lib.rs +++ b/pacquet/crates/config/src/lib.rs @@ -13,7 +13,11 @@ use pacquet_store_dir::StoreDir; use pipe_trait::Pipe; use serde::Deserialize; use smart_default::SmartDefault; -use std::{collections::HashMap, fs, path::PathBuf}; +use std::{ + collections::{BTreeMap, BTreeSet, HashMap}, + fs, + path::PathBuf, +}; pub use crate::defaults::{ available_parallelism, default_git_shallow_hosts, default_unsafe_perm, is_unsafe_perm_posix, @@ -363,6 +367,39 @@ pub struct Config { #[default = true] pub hoist_workspace_packages: bool, + /// Per-importer block-list of package aliases that may NOT be + /// hoisted past that importer's slot. Outer key is the + /// importer locator (e.g. `'.@'` for the root project, or the + /// percent-encoded importer id with the `@` slot suffix); + /// inner set is the alias names whose hoisting is bordered. + /// + /// Programmatic-only upstream — pnpm exposes it through the + /// embedded API and Bit CLI rather than `pnpm-workspace.yaml`, + /// because the ergonomics of the locator-keyed map don't + /// translate cleanly to a yaml setting. Pacquet exposes it + /// via `HoistOpts::hoisting_limits` (in `pacquet-real-hoist`) + /// and reads the same yaml shape (`hoistingLimits: { ".@": [...] }`) + /// for parity. + /// + /// Default empty (no aliases bordered). Mirrors upstream's + /// [`hoistingLimits`](https://github.com/pnpm/pnpm/blob/94240bc046/installing/linking/real-hoist/src/index.ts#L10). + /// No effect under `nodeLinker: isolated`. + pub hoisting_limits: BTreeMap>, + + /// Name slots reserved at the root for an external linker + /// (the Bit CLI is the only known consumer upstream). Any + /// dependency whose alias matches one of these names is + /// stripped from the hoist tree's top-level entries — the + /// external linker materializes those slots itself. + /// + /// Programmatic-only upstream; pacquet exposes the same yaml + /// shape (`externalDependencies: ["bit-bin"]`) for parity. + /// + /// Default empty. Mirrors upstream's + /// [`externalDependencies`](https://github.com/pnpm/pnpm/blob/94240bc046/installing/linking/real-hoist/src/index.ts#L18). + /// No effect under `nodeLinker: isolated`. + pub external_dependencies: BTreeSet, + /// When this setting is set to true, packages with peer dependencies will be deduplicated after peers resolution. #[default = true] pub dedupe_peer_dependents: bool, diff --git a/pacquet/crates/config/src/workspace_yaml.rs b/pacquet/crates/config/src/workspace_yaml.rs index 5444cd8376..681c1ed0cb 100644 --- a/pacquet/crates/config/src/workspace_yaml.rs +++ b/pacquet/crates/config/src/workspace_yaml.rs @@ -9,7 +9,7 @@ use pacquet_store_dir::StoreDir; use pipe_trait::Pipe; use serde::{Deserialize, Deserializer}; use std::{ - collections::HashMap, + collections::{BTreeMap, BTreeSet, HashMap}, fs, io::{self, ErrorKind}, path::{Path, PathBuf}, @@ -119,6 +119,18 @@ pub struct WorkspaceSettings { pub registry: Option, pub auto_install_peers: Option, pub hoist_workspace_packages: Option, + /// `hoistingLimits` from `pnpm-workspace.yaml`. Outer key is + /// the importer locator (e.g. `'.@'`); inner list is the + /// alias names whose hoisting is bordered. Mirrors upstream's + /// programmatic-only knob shape, exposed here as yaml for + /// parity. Empty / missing → no limits. + pub hoisting_limits: Option>>, + /// `externalDependencies` from `pnpm-workspace.yaml`. Names + /// whose top-level slot is reserved for an external linker + /// and stripped from the hoist tree. Mirrors upstream's + /// programmatic-only knob shape, exposed here as yaml for + /// parity. Empty / missing → no externals. + pub external_dependencies: Option>, pub dedupe_peer_dependents: Option, pub strict_peer_dependencies: Option, pub resolve_peers_from_workspace_root: Option, @@ -296,6 +308,7 @@ impl WorkspaceSettings { lockfile, prefer_frozen_lockfile, offline, prefer_offline, lockfile_include_tarball_url, auto_install_peers, hoist_workspace_packages, + hoisting_limits, external_dependencies, dedupe_peer_dependents, strict_peer_dependencies, resolve_peers_from_workspace_root, verify_store_integrity, side_effects_cache, side_effects_cache_readonly, diff --git a/pacquet/crates/config/src/workspace_yaml/tests.rs b/pacquet/crates/config/src/workspace_yaml/tests.rs index b86defb654..c0e73967d4 100644 --- a/pacquet/crates/config/src/workspace_yaml/tests.rs +++ b/pacquet/crates/config/src/workspace_yaml/tests.rs @@ -649,3 +649,67 @@ fn omitting_ignored_optional_dependencies_keeps_default() { settings.apply_to(&mut config, Path::new("/irrelevant")); assert!(config.ignored_optional_dependencies.is_none()); } + +/// `hoistingLimits` deserializes as a map keyed by importer +/// locator (e.g. `'.@'`); inner value is a list of alias names. +/// Mirrors upstream's [`HoistingLimits`](https://github.com/pnpm/pnpm/blob/94240bc046/installing/linking/real-hoist/src/index.ts#L10) +/// shape and threads straight into [`pacquet_real_hoist::HoistOpts`] +/// via the install pipeline. Yaml-empty / missing keeps the +/// `Config` field at its `BTreeMap::default()` empty value. +#[test] +fn parses_hoisting_limits_from_yaml_and_applies() { + let yaml = r#" +hoistingLimits: + ".@": + - foo + - bar +"#; + let settings: WorkspaceSettings = serde_saphyr::from_str(yaml).unwrap(); + let raw = settings.hoisting_limits.clone().expect("field present"); + let aliases = raw.get(".@").expect("locator present"); + assert!(aliases.contains("foo") && aliases.contains("bar")); + + let mut config = Config::new(); + assert!(config.hoisting_limits.is_empty(), "default is empty"); + settings.apply_to(&mut config, Path::new("/irrelevant")); + let aliases = config.hoisting_limits.get(".@").expect("locator present in config"); + assert!(aliases.contains("foo") && aliases.contains("bar")); +} + +/// `externalDependencies` deserializes as a flat list of names. +/// Yaml-empty / missing keeps the `Config` field at its +/// `BTreeSet::default()` empty value. +#[test] +fn parses_external_dependencies_from_yaml_and_applies() { + let yaml = r#" +externalDependencies: + - bit-bin + - some-other-external +"#; + let settings: WorkspaceSettings = serde_saphyr::from_str(yaml).unwrap(); + let raw = settings.external_dependencies.clone().expect("field present"); + assert!(raw.contains("bit-bin") && raw.contains("some-other-external")); + + let mut config = Config::new(); + assert!(config.external_dependencies.is_empty(), "default is empty"); + settings.apply_to(&mut config, Path::new("/irrelevant")); + assert!(config.external_dependencies.contains("bit-bin")); + assert!(config.external_dependencies.contains("some-other-external")); +} + +/// Both knobs absent → both `Config` fields stay at their empty +/// defaults. Pins the `apply_to` skip-on-None branch so future +/// edits don't accidentally overwrite with empty when the yaml +/// just doesn't mention these settings. +#[test] +fn omitting_hoisting_limits_and_external_dependencies_keeps_defaults() { + let yaml = ""; + let settings: WorkspaceSettings = serde_saphyr::from_str(yaml).unwrap(); + assert!(settings.hoisting_limits.is_none()); + assert!(settings.external_dependencies.is_none()); + + let mut config = Config::new(); + settings.apply_to(&mut config, Path::new("/irrelevant")); + assert!(config.hoisting_limits.is_empty()); + assert!(config.external_dependencies.is_empty()); +} diff --git a/pacquet/crates/package-manager/src/hoisted_dep_graph.rs b/pacquet/crates/package-manager/src/hoisted_dep_graph.rs index 2479a7dcc5..33620daa02 100644 --- a/pacquet/crates/package-manager/src/hoisted_dep_graph.rs +++ b/pacquet/crates/package-manager/src/hoisted_dep_graph.rs @@ -238,9 +238,24 @@ pub struct LockfileToHoistedDepGraphOptions { /// under `//node_modules`. When /// `false`, only the root importer's subtree is emitted (the /// hoister also skips adding the workspace children to its - /// shared tree). Pacquet's [`pacquet_config::Config::hoist_workspace_packages`] - /// drives this from the install pipeline. + /// shared tree). Pacquet's `Config::hoist_workspace_packages` + /// (in `pacquet-config`) drives this from the install pipeline. pub hoist_workspace_packages: bool, + + /// Per-importer block-list passed straight through to + /// [`pacquet_real_hoist::HoistOpts::hoisting_limits`]. See the + /// hoister's doc-comment for the locator-keyed shape and + /// `Config::hoisting_limits` in `pacquet-config` for how the + /// install pipeline derives this from `pnpm-workspace.yaml`. + pub hoisting_limits: pacquet_real_hoist::HoistingLimits, + + /// Reserved-name list passed straight through to + /// [`pacquet_real_hoist::HoistOpts::external_dependencies`]. + /// See the hoister's doc-comment for the strip semantics and + /// `Config::external_dependencies` in `pacquet-config` for how + /// the install pipeline derives this from + /// `pnpm-workspace.yaml`. + pub external_dependencies: BTreeSet, } impl Default for LockfileToHoistedDepGraphOptions { @@ -260,6 +275,8 @@ impl Default for LockfileToHoistedDepGraphOptions { // `..Default::default()`-style construction at the call // site doesn't silently disable workspace hoisting. hoist_workspace_packages: true, + hoisting_limits: pacquet_real_hoist::HoistingLimits::new(), + external_dependencies: BTreeSet::new(), } } } @@ -377,7 +394,8 @@ fn build_dep_graph( let hoist_opts = HoistOpts { auto_install_peers: opts.auto_install_peers, hoist_workspace_packages: opts.hoist_workspace_packages, - ..HoistOpts::default() + hoisting_limits: opts.hoisting_limits.clone(), + external_dependencies: opts.external_dependencies.clone(), }; let hoister_result = hoist(lockfile, &hoist_opts)?; @@ -1837,4 +1855,43 @@ mod tests { let foo_a = &result.direct_dependencies_by_importer_id["packages/foo"]["a"]; assert_ne!(root_a, foo_a, "conflict resolves to two distinct dirs"); } + + /// `external_dependencies` flows through to the hoister and + /// strips matching aliases from the post-hoist result. Pins + /// the slice 10 plumbing end-to-end: the walker observes an + /// empty graph for the `external` alias because the hoister + /// stripped it, even though the lockfile listed it as a + /// direct dep. + #[test] + fn walker_forwards_external_dependencies_to_hoister() { + let mut root_deps = ResolvedDependencyMap::new(); + root_deps.insert(pkg_name("a"), resolved_dep("1.0.0")); + + let mut packages = HashMap::new(); + packages.insert(dep_key("a", "1.0.0"), metadata_stub()); + + let mut snapshots = HashMap::new(); + snapshots.insert(dep_key("a", "1.0.0"), SnapshotEntry::default()); + + let lockfile = lockfile_with(root_deps, packages, snapshots); + let lockfile_dir = PathBuf::from("/repo"); + let mut externals = BTreeSet::new(); + externals.insert("a".to_string()); + let opts = LockfileToHoistedDepGraphOptions { + lockfile_dir: lockfile_dir.clone(), + external_dependencies: externals, + ..LockfileToHoistedDepGraphOptions::default() + }; + let result = + lockfile_to_hoisted_dep_graph(&lockfile, None, &opts).expect("walker succeeds"); + + // `a` was reserved as an external; hoister stripped it + // from the top-level result, so the walker emits an + // empty graph and an empty root direct-deps map. + assert!(result.graph.is_empty(), "external strips the alias from the hoist result"); + assert!( + result.direct_dependencies_by_importer_id[Lockfile::ROOT_IMPORTER_KEY].is_empty(), + "root direct deps drop the externalised alias", + ); + } } diff --git a/pacquet/crates/package-manager/src/install_frozen_lockfile.rs b/pacquet/crates/package-manager/src/install_frozen_lockfile.rs index 9289d58a6c..6ecb687162 100644 --- a/pacquet/crates/package-manager/src/install_frozen_lockfile.rs +++ b/pacquet/crates/package-manager/src/install_frozen_lockfile.rs @@ -693,6 +693,8 @@ where current_libc: pacquet_graph_hasher::host_libc().to_string(), supported_architectures: supported_architectures.cloned(), hoist_workspace_packages: config.hoist_workspace_packages, + hoisting_limits: config.hoisting_limits.clone(), + external_dependencies: config.external_dependencies.clone(), }; let walker_result = lockfile_to_hoisted_dep_graph(lockfile, current_lockfile, &walker_opts) diff --git a/pacquet/crates/package-manager/src/install_package_from_registry/tests.rs b/pacquet/crates/package-manager/src/install_package_from_registry/tests.rs index ffe3541044..dc5b9b32db 100644 --- a/pacquet/crates/package-manager/src/install_package_from_registry/tests.rs +++ b/pacquet/crates/package-manager/src/install_package_from_registry/tests.rs @@ -38,6 +38,8 @@ fn create_config(store_dir: &Path, modules_dir: &Path, virtual_store_dir: &Path) registry: "https://registry.npmjs.com/".to_string(), auto_install_peers: false, hoist_workspace_packages: true, + hoisting_limits: Default::default(), + external_dependencies: Default::default(), dedupe_peer_dependents: false, strict_peer_dependencies: false, resolve_peers_from_workspace_root: false,