fix(install): keep the repeat-install fast path for deduped siblings and large lockfiles (#15073)

Two gates of the repeat-install check refused every run in common Bit
workspaces, and one of them affects plain pnpm workspaces too.

modules_dirs_present treated a sibling project without node_modules as
never installed. Under dedupeDirectDeps a sibling whose every direct
dependency the root declares identically gets nothing linked, so the
linker never creates that directory; the sibling is installed all the
same. The gate now lets such a sibling through when the root's modules
directory exists and, for every alias the sibling declares in a group
the install materializes, the wanted lockfile records one target on
each side and the two agree, which is what the linker compares. link:
targets are resolved against each importer's directory; an alias a side
declares with differing targets across groups proves nothing, since the
linker picks one by group order and the check does not reproduce that;
a modules directory has to be a directory. The TypeScript checkDepsStatus
had the same gate and gets the same rule.

The merge-conflict scan of a changed lockfile refused files of 16 MiB or
more as unverifiable. Every changed lockfile that passes the scan is
parsed in full right after, so the size budget could only refuse what
the parse would read anyway; the scan now streams the whole file through
its fixed 8 KiB buffer. The symlink and non-regular-file refusals stay.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
Zoltan KochanandClaude Fable 5.1 authored and GitHub committed 2026-09-18 21:27:37 +02:00
1 parent c506e77fac
commit 4f37bb5429
10 files changed
+704 -79

No files matched your search

@@ -0,0 +1,8 @@
---
"pacquet": patch
"@pnpm/napi": patch
"@pnpm/deps.status": patch
"pnpm": patch
---
`pnpm install` now returns "Already up to date" in a workspace where `dedupeDirectDeps` left a project without a `node_modules` directory of its own. Such a project forced a full install on every run.
@@ -0,0 +1,6 @@
---
"pacquet": patch
"@pnpm/napi": patch
---
`pnpm install` no longer refuses the repeat-install fast path just because a changed `pnpm-lock.yaml` is 16 MiB or larger. Such a lockfile forced a full install on the run after every change.
@@ -434,7 +434,7 @@ fn settings_block_fast_path(
// overrides yet, so check the install-time `config.modules_dir`
// for the root + `<project_root>/node_modules` for siblings,
// matching the `isolated`-linker default.
if !modules_dirs_present(config, node_linker, project_manifests) {
if !modules_dirs_present(check) {
return Some("project has dependencies but no node_modules directory");
}
None
@@ -40,8 +40,6 @@ pub(crate) const CONFLICT_MARKER: &[u8] = b"<<<<<<<";
pub(crate) const LOCKFILE_CONFLICT_SCAN_BUFFER_SIZE: usize = 8 * 1024;
pub(crate) const MAX_LOCKFILE_CONFLICT_SCAN_BYTES: u64 = 16 * 1024 * 1024;
pub(crate) fn lockfile_conflict_check_failure(
path: &Path,
last_validated_timestamp: i64,
@@ -60,9 +58,6 @@ pub(crate) fn lockfile_conflict_check_failure(
if !lockfile_modified_since(mtime, last_validated_timestamp) {
return None;
}
if metadata.len() >= MAX_LOCKFILE_CONFLICT_SCAN_BYTES {
return Some(LockfileConflictCheckFailure::Unsafe);
}
modified_lockfile_conflict_check_failure(path)
}
@@ -72,22 +67,20 @@ pub(crate) fn modified_lockfile_conflict_check_failure(
let Some(mut file) = open_for_conflict_scan(path) else {
return Some(LockfileConflictCheckFailure::Unsafe);
};
// The scan streams the whole file through one buffer, whatever its
// size: every changed lockfile that passes it is parsed in full next,
// so a size budget here could only refuse what the parse would read
// anyway.
let mut buffer = [0; LOCKFILE_CONFLICT_SCAN_BUFFER_SIZE + CONFLICT_MARKER.len() - 1];
let mut carried = 0;
let mut scanned = 0_u64;
loop {
let remaining = MAX_LOCKFILE_CONFLICT_SCAN_BYTES.saturating_sub(scanned);
if remaining == 0 {
return Some(LockfileConflictCheckFailure::Unsafe);
}
let read_capacity = LOCKFILE_CONFLICT_SCAN_BUFFER_SIZE.min(remaining as usize);
let read = match file.read(&mut buffer[carried..carried + read_capacity]) {
Ok(0) => return None,
Ok(read) => read,
Err(error) if error.kind() == ErrorKind::Interrupted => continue,
Err(_) => return Some(LockfileConflictCheckFailure::Unsafe),
};
scanned += read as u64;
let read =
match file.read(&mut buffer[carried..carried + LOCKFILE_CONFLICT_SCAN_BUFFER_SIZE]) {
Ok(0) => return None,
Ok(read) => read,
Err(error) if error.kind() == ErrorKind::Interrupted => continue,
Err(_) => return Some(LockfileConflictCheckFailure::Unsafe),
};
let end = carried + read;
if chunk_contains_marker(&buffer[..end]) {
return Some(LockfileConflictCheckFailure::MergeConflict);
@@ -100,14 +93,14 @@ pub(crate) fn modified_lockfile_conflict_check_failure(
}
/// The lockfile, opened for the conflict scan. `None` for anything the scan
/// cannot read to a verdict: a missing or unreadable path, a non-file, or a
/// file too large to scan within the budget.
/// cannot read to a verdict: a missing or unreadable path, or a non-file.
fn open_for_conflict_scan(path: &Path) -> Option<fs::File> {
let file = fs::File::open(path).ok()?;
let metadata = file.metadata().ok()?;
(metadata.file_type().is_file() && metadata.len() < MAX_LOCKFILE_CONFLICT_SCAN_BYTES).then_some(
file,
)
metadata
.file_type()
.is_file()
.then_some(file)
}
fn chunk_contains_marker(bytes: &[u8]) -> bool {
@@ -135,7 +135,6 @@ fn first_workspace_drift(
config,
project_manifests,
is_workspace_install,
layout: crate::RepeatInstallLayout { node_linker, .. },
..
} = check;
if !project_structure_matches(state, project_manifests) {
@@ -144,7 +143,7 @@ fn first_workspace_drift(
// A filtered install legitimately leaves unselected projects
// without a modules directory.
if !state.filtered_install
&& let Some(id) = first_project_missing_modules_dir(config, node_linker, project_manifests)
&& let Some(id) = first_project_missing_modules_dir(check)
{
return Some(format!(
"Workspace package {id} has dependencies but does not have a modules directory",
@@ -4,9 +4,13 @@ use super::{
manifest_has_runtime_deps, manifest_string_field,
};
use pnpm_config::{Config, LinkWorkspacePackages, NodeLinker};
use pnpm_lockfile::Lockfile;
use pnpm_modules_yaml::Host;
use pnpm_package_manifest::PackageManifest;
use pnpm_fs::lexical_normalize;
use pnpm_lockfile::{
Lockfile, MaybeLazyLockfile, PkgName, ProjectSnapshot, ResolvedDependencySpec,
};
use pnpm_modules_yaml::{Host, IncludedDependencies};
use pnpm_package_manifest::{DependencyGroup, PackageManifest};
use pnpm_workspace::importer_id_from_root_dir;
use pnpm_workspace_state::{WorkspaceState, update_workspace_state};
use std::{
fs,
@@ -206,50 +210,174 @@ pub(super) fn project_structure_matches(
== manifest_string_field(manifest, "version").as_deref().unwrap_or("0.0.0")
})
}
pub(super) fn modules_dirs_present(
config: &Config,
node_linker: NodeLinker,
project_manifests: &[(PathBuf, &PackageManifest)],
) -> bool {
first_project_missing_modules_dir(config, node_linker, project_manifests).is_none()
pub(super) fn modules_dirs_present(check: &OptimisticRepeatInstallCheck<'_>) -> bool {
first_project_missing_modules_dir(check).is_none()
}
/// The id (`name` field, falling back to the root dir) of the first
/// project that declares dependencies but has no modules directory, or
/// `None` when every project with dependencies has one.
///
/// Under `dedupeDirectDeps` a sibling whose every direct dependency
/// resolves to the same target as the root's gets nothing linked, so the
/// linker never creates its modules directory; such a sibling is installed
/// all the same and does not count as missing one.
pub(super) fn first_project_missing_modules_dir(
config: &Config,
node_linker: NodeLinker,
project_manifests: &[(PathBuf, &PackageManifest)],
check: &OptimisticRepeatInstallCheck<'_>,
) -> Option<String> {
let root_modules_dir_exists = config.modules_dir.exists();
let &OptimisticRepeatInstallCheck {
workspace_root,
config,
project_manifests,
lockfile,
layout: crate::RepeatInstallLayout { node_linker, included, .. },
..
} = check;
let root_modules_dir_exists = config.modules_dir.is_dir();
project_manifests
.iter()
.find_map(|(root_dir, manifest)| {
if !manifest_has_runtime_deps(manifest) {
return None;
}
// The root importer uses `config.modules_dir`; siblings use
// their own `<root>/node_modules`. Matches the isolated-linker
// default — `config.modules_dir` is `<workspace_root>/node_modules`
// unless the user overrode it explicitly.
let modules_dir_exists = match node_linker {
NodeLinker::Hoisted => root_modules_dir_exists,
NodeLinker::Isolated | NodeLinker::Pnp => {
if *root_dir == workspace_dir_of(config, root_dir) {
root_modules_dir_exists
} else {
root_dir.join("node_modules").exists()
}
}
};
(!modules_dir_exists).then(|| {
let root_project_dir = workspace_dir_of(config, root_dir);
let is_root = *root_dir == root_project_dir;
let installed = !manifest_has_runtime_deps(manifest)
|| modules_dir_exists(node_linker, root_dir, is_root, root_modules_dir_exists)
|| (!is_root
&& root_modules_dir_exists
&& config.dedupe_direct_deps
&& dedupe_links_nothing(
lockfile,
&included_groups(included),
DedupeImporters {
lockfile_root: workspace_root,
root_dir: &root_project_dir,
sibling_dir: root_dir,
},
));
(!installed).then(|| {
manifest_string_field(manifest, "name")
.unwrap_or_else(|| root_dir.to_string_lossy().into_owned())
})
})
}
/// The root importer uses `config.modules_dir`; siblings use their own
/// `<root>/node_modules`. Matches the isolated-linker default —
/// `config.modules_dir` is `<workspace_root>/node_modules` unless the user
/// overrode it explicitly.
fn modules_dir_exists(
node_linker: NodeLinker,
root_dir: &Path,
is_root: bool,
root_modules_dir_exists: bool,
) -> bool {
match node_linker {
NodeLinker::Hoisted => root_modules_dir_exists,
NodeLinker::Isolated | NodeLinker::Pnp => {
if is_root {
root_modules_dir_exists
} else {
root_dir.join("node_modules").is_dir()
}
}
}
}
/// The dependency groups this install materializes, the only ones the
/// linker links and dedupes.
fn included_groups(included: IncludedDependencies) -> Vec<DependencyGroup> {
[
(included.dependencies, DependencyGroup::Prod),
(included.dev_dependencies, DependencyGroup::Dev),
(included.optional_dependencies, DependencyGroup::Optional),
]
.into_iter()
.filter_map(|(included, group)| included.then_some(group))
.collect()
}
/// The two importers a dedupe verdict compares, as directories.
#[derive(Clone, Copy)]
struct DedupeImporters<'a> {
/// The directory importer ids are relative to.
lockfile_root: &'a Path,
root_dir: &'a Path,
sibling_dir: &'a Path,
}
/// Whether `dedupeDirectDeps` links nothing into the sibling: for every
/// alias the sibling declares in a materialized group, the wanted lockfile
/// records one target on each side, and the two are the same, which is what
/// the linker compares. An alias declared with differing targets in several
/// groups has one effective target the linker picks by group order; that
/// choice is not reproduced here, so such an alias proves nothing. Neither
/// does a lockfile that cannot be loaded or lacks either importer.
fn dedupe_links_nothing(
lockfile: MaybeLazyLockfile<'_>,
groups: &[DependencyGroup],
importers: DedupeImporters<'_>,
) -> bool {
let DedupeImporters {
lockfile_root,
root_dir,
sibling_dir,
} = importers;
let Ok(Some(lockfile)) = lockfile.get() else { return false };
let importer =
|dir: &Path| lockfile.importers.get(&importer_id_from_root_dir(lockfile_root, dir));
let (Some(root), Some(sibling)) = (importer(root_dir), importer(sibling_dir)) else {
return false;
};
let mut seen = std::collections::HashSet::new();
sibling
.dependencies_by_groups(groups.iter().copied())
.all(|(alias, _)| {
if !seen.insert(alias) {
return true;
}
let (Some(dep), Some(root_dep)) =
(sole_target(sibling, groups, alias), sole_target(root, groups, alias))
else {
return false;
};
resolves_to_same_target(root_dir, root_dep, sibling_dir, dep)
})
}
/// The one target `importer` resolves `alias` to across `groups`, or `None`
/// when it declares the alias nowhere or with differing targets.
fn sole_target<'a>(
importer: &'a ProjectSnapshot,
groups: &[DependencyGroup],
alias: &PkgName,
) -> Option<&'a ResolvedDependencySpec> {
let mut declarations = importer
.dependencies_by_groups(groups.iter().copied())
.filter(|(declared, _)| *declared == alias)
.map(|(_, dep)| dep);
let first = declarations.next()?;
declarations
.all(|dep| dep.version == first.version)
.then_some(first)
}
/// Whether two importer dependencies resolve to one target: the same
/// snapshot, or `link:` paths that name the same directory once resolved
/// against their own importer directories.
fn resolves_to_same_target(
root_dir: &Path,
root_dep: &ResolvedDependencySpec,
sibling_dir: &Path,
dep: &ResolvedDependencySpec,
) -> bool {
match (root_dep.version.as_link_target(), dep.version.as_link_target()) {
(Some(root_target), Some(target)) => {
lexical_normalize(&root_dir.join(root_target))
== lexical_normalize(&sibling_dir.join(target))
}
(None, None) => root_dep.version == dep.version,
_ => false,
}
}
/// Recover the workspace root from `config.modules_dir`. The root
/// importer's `root_dir` equals `config.modules_dir.parent()` because
/// `config.modules_dir` is `<workspace_root>/node_modules`. Used by
@@ -1,7 +1,6 @@
use super::{
super::{
Decision, OptimisticRepeatInstallCheck, check_optimistic_repeat_install,
conflict_markers::MAX_LOCKFILE_CONFLICT_SCAN_BYTES,
deps_status::{RunDepsStatus, check_deps_status_before_run},
settings::current_settings,
timestamps::{FileMtime, lockfile_modified_since, modified_at_or_after},
@@ -286,16 +285,35 @@ fn returns_skipped_without_following_a_lockfile_symlink() {
Decision::Skipped { reason } if reason.contains("cannot be checked")
));
}
/// A changed lockfile is scanned whole, however large: every one that
/// passes the scan is parsed in full next, so no size budget can save
/// anything. Here the file is a valid lockfile padded past 16 MiB with a
/// comment.
#[test]
fn returns_skipped_without_scanning_an_oversized_changed_lockfile() {
fn scans_a_large_changed_lockfile_to_the_end() {
let (dir, config, manifest) =
setup_fresh_install(pnpm_config::NodeLinker::Isolated, "root", "1.0.0", "");
let lockfile = fs::OpenOptions::new()
.write(true)
.truncate(true)
.open(dir.path().join(Lockfile::FILE_NAME))
.expect("open lockfile");
lockfile.set_len(MAX_LOCKFILE_CONFLICT_SCAN_BYTES).expect("resize lockfile");
fs::write(dir.path().join(Lockfile::FILE_NAME), large_lockfile(None)).expect("write lockfile");
let lockfile = Lockfile::load_wanted_from_dir(dir.path())
.expect("parse the padded lockfile")
.expect("padded lockfile on disk");
let decision = check_with_lockfile(
dir.path(),
config,
pnpm_config::NodeLinker::Isolated,
&[(dir.path().to_path_buf(), &manifest)],
&lockfile,
);
assert_eq!(decision, Decision::UpToDate);
}
#[test]
fn finds_a_conflict_marker_deep_in_a_large_changed_lockfile() {
let (dir, config, manifest) =
setup_fresh_install(pnpm_config::NodeLinker::Isolated, "root", "1.0.0", "");
fs::write(dir.path().join(Lockfile::FILE_NAME), large_lockfile(Some("<<<<<<< HEAD\n")))
.expect("write lockfile");
let decision = check(
dir.path(),
@@ -304,11 +322,20 @@ fn returns_skipped_without_scanning_an_oversized_changed_lockfile() {
&[(dir.path().to_path_buf(), &manifest)],
);
dbg!(&decision);
assert!(matches!(
decision,
Decision::Skipped { reason } if reason.contains("cannot be checked")
));
assert!(matches!(decision, Decision::Skipped { reason } if reason.contains("conflict")));
}
/// An empty lockfile followed by 17 MiB of comment lines, then `tail`.
fn large_lockfile(tail: Option<&str>) -> String {
const COMMENT_LINE: &str =
"# padding padding padding padding padding padding padding padding\n";
let mut lockfile = String::from("lockfileVersion: '9.0'\n");
let target = 17 * 1024 * 1024;
lockfile.reserve(target + COMMENT_LINE.len());
while lockfile.len() < target {
lockfile.push_str(COMMENT_LINE);
}
lockfile.push_str(tail.unwrap_or_default());
lockfile
}
#[test]
fn returns_skipped_when_current_lockfile_missing_for_non_empty_wanted_lockfile() {
@@ -6,10 +6,12 @@ use super::{
FOO_MANIFEST, assert_content_check_converges_after_collision, backdate_validated_files, check,
check_with_lockfile, collide_mtimes_with_recorded_state, content_check_decision,
isolated_included, linked_sibling_decision_for_spec, setup_content_check_project,
setup_fresh_install, validate_existing_files, write_local_tarball_lockfile, write_state,
setup_fresh_install, setup_fresh_install_with_config, validate_existing_files,
write_local_tarball_lockfile, write_state,
};
use pnpm_config::Config;
use pnpm_lockfile::MaybeLazyLockfile;
use pnpm_lockfile::{Lockfile, MaybeLazyLockfile};
use pnpm_modules_yaml::IncludedDependencies;
use pnpm_package_manifest::PackageManifest;
use pnpm_testing_utils::fs::set_mtime;
use pnpm_workspace_state::{ProjectEntry, load_workspace_state, update_workspace_state};
@@ -517,3 +519,206 @@ fn returns_up_to_date_for_registry_resolution_when_workspace_linking_is_off() {
Decision::UpToDate,
);
}
/// Under `dedupeDirectDeps` a sibling whose direct dependencies resolve to
/// the root's targets gets nothing linked and no modules directory, so the
/// missing directory is not evidence of a missing install.
#[test]
fn returns_up_to_date_when_a_deduped_sibling_has_no_node_modules() {
assert_eq!(deduped_sibling_decision(true, "1.0.0", "1.0.0"), Decision::UpToDate);
}
#[test]
fn returns_skipped_when_a_sibling_without_dedupe_has_no_node_modules() {
let decision = deduped_sibling_decision(false, "1.0.0", "1.0.0");
assert!(matches!(decision, Decision::Skipped { reason } if reason.contains("node_modules")));
}
/// The same specifier can resolve to another peer set for the sibling; the
/// linker then links it into the sibling, so its missing `node_modules` is
/// real damage.
#[test]
fn returns_skipped_when_a_sibling_resolves_a_shared_specifier_to_another_peer_set() {
let decision = deduped_sibling_decision(true, "1.0.0", "1.0.0(bar@1.0.0)");
assert!(matches!(decision, Decision::Skipped { reason } if reason.contains("node_modules")));
}
/// `link:` targets are compared where they point, not as strings: the root's
/// `link:libs/lib` and the sibling's `link:../libs/lib` are one directory.
#[test]
fn returns_up_to_date_when_a_deduped_sibling_links_the_same_directory_by_another_path() {
assert_eq!(
deduped_sibling_decision(true, "link:libs/lib", "link:../libs/lib"),
Decision::UpToDate,
);
}
#[test]
fn returns_skipped_when_a_sibling_links_another_directory_under_the_same_specifier() {
let decision = deduped_sibling_decision(true, "link:libs/lib", "link:libs/lib");
assert!(matches!(decision, Decision::Skipped { reason } if reason.contains("node_modules")));
}
/// The root declares `foo` in two groups with one target: still one target,
/// so the sibling matches it.
#[test]
fn returns_up_to_date_when_the_root_declares_the_alias_in_two_groups_with_one_target() {
assert_eq!(
deduped_sibling_decision_with_root_dev("1.0.0", Some("1.0.0"), "1.0.0"),
Decision::UpToDate,
);
}
/// Two root declarations with differing targets have one effective target
/// the linker picks by group order; the check does not reproduce that
/// choice and falls through.
#[test]
fn returns_skipped_when_the_root_declares_the_alias_with_differing_targets() {
let decision = deduped_sibling_decision_with_root_dev("1.0.0", Some("2.0.0"), "2.0.0");
assert!(matches!(decision, Decision::Skipped { reason } if reason.contains("node_modules")));
}
/// A production-only install never links dev dependencies, so a sibling dev
/// dependency the root lacks does not make the sibling incomplete.
#[test]
fn returns_up_to_date_when_an_unmatched_dependency_is_in_an_excluded_group() {
let production_only = IncludedDependencies {
dependencies: true,
dev_dependencies: false,
optional_dependencies: false,
};
assert_eq!(
deduped_sibling_decision_in(DedupedSibling {
dedupe_direct_deps: true,
root_version: "1.0.0",
root_dev_version: None,
sibling_version: "1.0.0",
sibling_dev_bar_version: Some("2.0.0"),
included: production_only,
}),
Decision::UpToDate,
);
}
/// The same sibling dev dependency blocks the exemption once dev
/// dependencies are installed.
#[test]
fn returns_skipped_when_an_unmatched_dependency_is_in_an_included_group() {
let decision = deduped_sibling_decision_in(DedupedSibling {
dedupe_direct_deps: true,
root_version: "1.0.0",
root_dev_version: None,
sibling_version: "1.0.0",
sibling_dev_bar_version: Some("2.0.0"),
included: isolated_included(),
});
assert!(matches!(decision, Decision::Skipped { reason } if reason.contains("node_modules")));
}
fn deduped_sibling_decision(
dedupe_direct_deps: bool,
root_version: &str,
sibling_version: &str,
) -> Decision {
deduped_sibling_decision_in(DedupedSibling {
dedupe_direct_deps,
root_version,
root_dev_version: None,
sibling_version,
sibling_dev_bar_version: None,
included: isolated_included(),
})
}
fn deduped_sibling_decision_with_root_dev(
root_version: &str,
root_dev_version: Option<&str>,
sibling_version: &str,
) -> Decision {
deduped_sibling_decision_in(DedupedSibling {
dedupe_direct_deps: true,
root_version,
root_dev_version,
sibling_version,
sibling_dev_bar_version: None,
included: isolated_included(),
})
}
#[derive(Clone, Copy)]
struct DedupedSibling<'a> {
dedupe_direct_deps: bool,
/// The root's `dependencies.foo`.
root_version: &'a str,
/// The root's `devDependencies.foo`, when it declares one.
root_dev_version: Option<&'a str>,
/// The sibling's `devDependencies.foo`.
sibling_version: &'a str,
/// The sibling's `devDependencies.bar`, which the root never declares.
sibling_dev_bar_version: Option<&'a str>,
included: IncludedDependencies,
}
fn deduped_sibling_decision_in(sibling: DedupedSibling<'_>) -> Decision {
let DedupedSibling {
dedupe_direct_deps,
root_version,
root_dev_version,
sibling_version,
sibling_dev_bar_version,
included,
} = sibling;
let (dir, config, root_manifest) = setup_fresh_install_with_config(
pnpm_config::NodeLinker::Isolated,
"root",
"1.0.0",
r#""dependencies":{"foo":"1.0.0"}"#,
|config| config.dedupe_direct_deps = dedupe_direct_deps,
);
let sibling_dir = dir.path().join("pkg-a");
fs::create_dir_all(&sibling_dir).unwrap();
let sibling_manifest_path = sibling_dir.join("package.json");
let sibling_bar_manifest = sibling_dev_bar_version.map_or("", |_| r#","bar":"2.0.0""#);
fs::write(
&sibling_manifest_path,
format!(
r#"{{"name":"pkg-a","version":"1.0.0","devDependencies":{{"foo":"1.0.0"{sibling_bar_manifest}}}}}"#,
),
)
.unwrap();
let sibling_manifest = PackageManifest::from_path(sibling_manifest_path).unwrap();
let root_dev_block = root_dev_version.map_or_else(String::new, |version| {
format!(" devDependencies:\n foo:\n specifier: 1.0.0\n version: {version}\n")
});
let sibling_bar_block = sibling_dev_bar_version.map_or_else(String::new, |version| {
format!(" bar:\n specifier: 2.0.0\n version: {version}\n")
});
fs::write(
dir.path().join(Lockfile::FILE_NAME),
format!(
"lockfileVersion: '9.0'\n\nimporters:\n\n .:\n dependencies:\n foo:\n specifier: 1.0.0\n version: {root_version}\n{root_dev_block}\n pkg-a:\n devDependencies:\n foo:\n specifier: 1.0.0\n version: {sibling_version}\n{sibling_bar_block}",
),
)
.unwrap();
let lockfile = Lockfile::load_wanted_from_dir(dir.path())
.expect("parse the two-importer lockfile")
.expect("lockfile on disk");
let settings = current_settings(config, pnpm_config::NodeLinker::Isolated, included, None);
let mut projects = BTreeMap::new();
projects.insert(
dir.path()
.to_string_lossy()
.into_owned(),
ProjectEntry { name: Some("root".into()), version: Some("1.0.0".into()) },
);
projects.insert(
sibling_dir.to_string_lossy().into_owned(),
ProjectEntry { name: Some("pkg-a".into()), version: Some("1.0.0".into()) },
);
write_state(dir.path(), backdate_validated_files(dir.path()), settings, projects);
check_optimistic_repeat_install(&OptimisticRepeatInstallCheck {
workspace_root: dir.path(),
config,
project_manifests: &[
(dir.path().to_path_buf(), &root_manifest),
(sibling_dir, &sibling_manifest),
],
is_workspace_install: true,
lockfile: MaybeLazyLockfile::Loaded(Some(&lockfile)),
catalogs: &BTreeMap::default(),
layout: crate::RepeatInstallLayout {
node_linker: pnpm_config::NodeLinker::Isolated,
included,
supported_architectures: None,
},
manifest_freshness: crate::ManifestFreshness::Mtime,
})
}
+78 -4
View File
@@ -15,8 +15,10 @@ import {
getLockfileImporterId,
getWantedLockfileName,
type LockfileObject,
type ProjectSnapshot,
readCurrentLockfile,
readWantedLockfile,
type ResolvedDependencies,
wantedLockfileHasMergeConflictsSync,
} from '@pnpm/lockfile.fs'
import {
@@ -52,6 +54,7 @@ import { statManifestFile } from './statManifestFile.js'
export type CheckDepsStatusOptions = Pick<Config,
| 'autoInstallPeers'
| 'catalogs'
| 'dedupeDirectDeps'
| 'excludeLinksFromLockfile'
| 'injectWorkspacePackages'
| 'linkWorkspacePackages'
@@ -328,12 +331,32 @@ async function _checkDepsStatus (opts: CheckDepsStatusOptions, workspaceState: W
}))
if (!workspaceState.filteredInstall) {
for (const { modulesDirStats, project } of allManifestStats) {
if (modulesDirStats) continue
if (isEmpty({
const withoutModulesDir = allManifestStats.filter(({ modulesDirStats, project }) =>
modulesDirStats?.isDirectory() !== true && !isEmpty({
...project.manifest.dependencies,
...project.manifest.devDependencies,
})) continue
}))
// Under dedupeDirectDeps a project whose direct dependencies resolve to
// the root's targets gets nothing linked, so the linker never creates
// its modules directory; it is installed all the same.
const rootModulesDirExists = allManifestStats.some(({ modulesDirStats, project }) =>
modulesDirStats?.isDirectory() === true && project.rootDir === rootProjectManifestDir)
const dedupeLockfileDir = opts.lockfileDir ?? workspaceDir ?? rootProjectManifestDir
const mayBeDeduped = (project: Project): boolean =>
opts.dedupeDirectDeps === true && rootModulesDirExists && project.rootDir !== rootProjectManifestDir
const wantedLockfileForDedupe = withoutModulesDir.some(({ project }) => mayBeDeduped(project))
? await readWantedLockfile(dedupeLockfileDir, {
ignoreIncompatible: false,
useGitBranchLockfile: opts.useGitBranchLockfile,
mergeGitBranchLockfiles: opts.mergeGitBranchLockfiles,
})
: null
for (const { project } of withoutModulesDir) {
if (
wantedLockfileForDedupe != null &&
mayBeDeduped(project) &&
dedupeLinksNothing(wantedLockfileForDedupe, dedupeLockfileDir, rootProjectManifestDir, project.rootDir, opts.include)
) continue
const id = project.manifest.name ?? project.rootDir
return {
upToDate: false,
@@ -980,3 +1003,54 @@ function modifiedAtOrAfter (stats: fs.Stats, referenceMs: number): boolean {
const mtimeMs = stats.mtime.valueOf()
return wholeSecond ? mtimeMs + 1000 > referenceMs : mtimeMs > referenceMs
}
/**
* Whether `dedupeDirectDeps` links nothing into the project at `projectDir`:
* for every alias the project declares in a materialized group, the wanted
* lockfile records one target on each side, and the two are the same, which
* is what the linker compares. An alias declared with differing targets in
* several groups has one effective target the linker picks by group order;
* that choice is not reproduced here, so such an alias proves nothing.
* Neither does a lockfile that lacks either importer.
*/
function dedupeLinksNothing (
lockfile: LockfileObject,
lockfileDir: string,
rootDir: string,
projectDir: string,
include?: IncludedDependencies
): boolean {
const root = lockfile.importers[getLockfileImporterId(lockfileDir, rootDir)]
const project = lockfile.importers[getLockfileImporterId(lockfileDir, projectDir)]
if (root == null || project == null) return false
const materializedGroups = (importer: ProjectSnapshot): Array<ResolvedDependencies | undefined> => [
include?.dependencies === false ? undefined : importer.dependencies,
include?.devDependencies === false ? undefined : importer.devDependencies,
include?.optionalDependencies === false ? undefined : importer.optionalDependencies,
]
const soleTarget = (importer: ProjectSnapshot, alias: string): string | undefined => {
const versions = materializedGroups(importer)
.map((deps) => deps?.[alias])
.filter((version): version is string => version != null)
return versions.length > 0 && versions.every((version) => version === versions[0]) ? versions[0] : undefined
}
const aliases = new Set(materializedGroups(project).flatMap((deps) => Object.keys(deps ?? {})))
return [...aliases].every((alias) => {
const version = soleTarget(project, alias)
const rootVersion = soleTarget(root, alias)
return version != null && rootVersion != null && resolvesToSameTarget(rootDir, rootVersion, projectDir, version)
})
}
/**
* Whether two importer dependency versions resolve to one target: the same
* snapshot, or `link:` paths that name the same directory once resolved
* against their own importer directories.
*/
function resolvesToSameTarget (rootDir: string, rootVersion: string, projectDir: string, version: string): boolean {
const rootLink = rootVersion.startsWith('link:')
const projectLink = version.startsWith('link:')
if (rootLink !== projectLink) return false
if (!rootLink) return rootVersion === version
return path.resolve(rootDir, rootVersion.slice('link:'.length)) === path.resolve(projectDir, version.slice('link:'.length))
}
+187 -2
View File
@@ -6,7 +6,7 @@ import path from 'node:path'
import { beforeEach, describe, expect, it, jest } from '@jest/globals'
import type { CheckDepsStatusOptions } from '@pnpm/deps.status'
import type { LockfileObject } from '@pnpm/lockfile.fs'
import type { ProjectId, ProjectRootDir, ProjectRootDirRealPath } from '@pnpm/types'
import type { IncludedDependencies, ProjectId, ProjectRootDir, ProjectRootDirRealPath } from '@pnpm/types'
import type { WorkspaceState } from '@pnpm/workspace.state'
{
@@ -485,7 +485,8 @@ describe('checkDepsStatus - pnpmfile modification', () => {
return {
mtime: new Date(beforeLastValidation),
mtimeMs: beforeLastValidation,
} as Stats
isDirectory: () => true,
} as unknown as Stats
})
jest.mocked(statManifestFileUtils.statManifestFile).mockImplementation(async () => ({
mtime: new Date(beforeLastValidation),
@@ -1777,3 +1778,187 @@ describe('checkDepsStatus - workspace discovery', () => {
}
})
})
describe('checkDepsStatus - deduped sibling without a modules directory', () => {
beforeEach(() => {
jest.resetModules()
jest.clearAllMocks()
})
interface DedupedSibling {
dedupeDirectDeps: boolean
/** The root's `dependencies.foo` as the lockfile resolved it. */
rootVersion?: string
/** The root's `devDependencies.foo`, when it declares one. */
rootDevVersion?: string
/** The sibling's `devDependencies.foo` as the lockfile resolved it. */
siblingVersion?: string
/** The sibling's `devDependencies.bar`, which the root never declares. */
siblingDevBarVersion?: string
include?: IncludedDependencies
}
// A root and a sibling that both declare foo; only the root has a
// node_modules directory, which is what dedupeDirectDeps leaves behind.
async function checkWithDedupe ({
dedupeDirectDeps,
rootVersion = '1.0.0',
rootDevVersion,
siblingVersion = '1.0.0',
siblingDevBarVersion,
include,
}: DedupedSibling) {
const workspaceDir = await fs.mkdtemp(path.join(os.tmpdir(), 'pnpm-check-deps-dedupe-'))
try {
const lastValidatedTimestamp = Date.now() - 10_000
const beforeLastValidation = lastValidatedTimestamp - 10_000
const rootDir = workspaceDir as ProjectRootDir
const rootDirRealPath = await fs.realpath(workspaceDir) as ProjectRootDirRealPath
const siblingDir = path.join(workspaceDir, 'pkg-a') as ProjectRootDir
const rootManifest = { name: 'root', version: '1.0.0', dependencies: { foo: '1.0.0' } }
const siblingManifest = {
name: 'pkg-a',
version: '1.0.0',
devDependencies: { foo: '1.0.0', ...(siblingDevBarVersion == null ? {} : { bar: '2.0.0' }) },
}
const mockWorkspaceState: WorkspaceState = {
lastValidatedTimestamp,
pnpmfiles: [],
settings: {
dedupeDirectDeps,
excludeLinksFromLockfile: false,
linkWorkspacePackages: true,
preferWorkspacePackages: true,
},
projects: {
[rootDir]: { name: 'root', version: '1.0.0' },
[siblingDir]: { name: 'pkg-a', version: '1.0.0' },
},
filteredInstall: false,
}
const lockfilePath = path.join(workspaceDir, 'pnpm-lock.yaml')
await fs.writeFile(lockfilePath, "lockfileVersion: '9.0'\n")
await fs.utimes(lockfilePath, beforeLastValidation / 1000, beforeLastValidation / 1000)
const beforeValidation = {
mtime: new Date(beforeLastValidation),
mtimeMs: beforeLastValidation,
isDirectory: () => true,
} as unknown as Stats
jest.mocked(loadWorkspaceState).mockReturnValue(mockWorkspaceState)
jest.mocked(fsUtils.safeStatSync).mockImplementation((filePath: string) =>
filePath.endsWith('pnpm-lock.yaml') ? beforeValidation : undefined)
jest.mocked(fsUtils.safeStat).mockImplementation(async (filePath: string) => {
if (filePath === path.join(workspaceDir, 'node_modules')) return beforeValidation
if (filePath.endsWith('pnpm-lock.yaml')) return beforeValidation
return undefined
})
jest.mocked(statManifestFileUtils.statManifestFile).mockResolvedValue(beforeValidation)
const lockfile: LockfileObject = {
lockfileVersion: '9.0',
importers: {
['.' as ProjectId]: {
specifiers: { foo: '1.0.0' },
dependencies: { foo: rootVersion },
...(rootDevVersion == null ? {} : { devDependencies: { foo: rootDevVersion } }),
},
['pkg-a' as ProjectId]: {
specifiers: { foo: '1.0.0' },
devDependencies: {
foo: siblingVersion,
...(siblingDevBarVersion == null ? {} : { bar: siblingDevBarVersion }),
},
},
},
}
jest.mocked(lockfileFs.readWantedLockfile).mockResolvedValue(lockfile)
jest.mocked(lockfileFs.readCurrentLockfile).mockResolvedValue(lockfile)
const opts: CheckDepsStatusOptions = {
allProjects: [
{ rootDir, rootDirRealPath, manifest: rootManifest, writeProjectManifest: async () => {} },
{
rootDir: siblingDir,
rootDirRealPath: siblingDir as unknown as ProjectRootDirRealPath,
manifest: siblingManifest,
writeProjectManifest: async () => {},
},
],
workspaceDir,
rootProjectManifest: rootManifest,
rootProjectManifestDir: workspaceDir,
pnpmfile: [],
include,
...mockWorkspaceState.settings,
}
return await checkDepsStatus(opts)
} finally {
await fs.rm(workspaceDir, { force: true, recursive: true })
}
}
const MISSING_MODULES_DIR = 'Workspace package pkg-a has dependencies but does not have a modules directory'
it('is up to date when dedupeDirectDeps left the sibling nothing to link', async () => {
const result = await checkWithDedupe({ dedupeDirectDeps: true })
expect(result.issue).toBeUndefined()
expect(result.upToDate).toBe(true)
})
it('is outdated when the sibling was not deduped', async () => {
const result = await checkWithDedupe({ dedupeDirectDeps: false })
expect(result.upToDate).toBe(false)
expect(result.issue).toBe(MISSING_MODULES_DIR)
})
// The same specifier resolved to another peer set for the sibling: the
// linker links it into the sibling, so the missing directory is real damage.
it('is outdated when the shared specifier resolves to another peer set for the sibling', async () => {
const result = await checkWithDedupe({ dedupeDirectDeps: true, siblingVersion: '1.0.0(bar@1.0.0)' })
expect(result.upToDate).toBe(false)
expect(result.issue).toBe(MISSING_MODULES_DIR)
})
// link: targets are compared where they point, not as strings.
it('is up to date when the sibling links the same directory by another relative path', async () => {
const result = await checkWithDedupe({ dedupeDirectDeps: true, rootVersion: 'link:libs/lib', siblingVersion: 'link:../libs/lib' })
expect(result.issue).toBeUndefined()
expect(result.upToDate).toBe(true)
})
it('is outdated when equal link strings point at different directories', async () => {
const result = await checkWithDedupe({ dedupeDirectDeps: true, rootVersion: 'link:libs/lib', siblingVersion: 'link:libs/lib' })
expect(result.upToDate).toBe(false)
expect(result.issue).toBe(MISSING_MODULES_DIR)
})
it('is up to date when the root declares the alias in two groups with one target', async () => {
const result = await checkWithDedupe({ dedupeDirectDeps: true, rootDevVersion: '1.0.0' })
expect(result.issue).toBeUndefined()
expect(result.upToDate).toBe(true)
})
// Two root declarations with differing targets have one effective target
// the linker picks by group order; the check does not reproduce that choice.
it('is outdated when the root declares the alias with differing targets', async () => {
const result = await checkWithDedupe({ dedupeDirectDeps: true, rootDevVersion: '2.0.0', siblingVersion: '2.0.0' })
expect(result.upToDate).toBe(false)
expect(result.issue).toBe(MISSING_MODULES_DIR)
})
it('ignores a dependency in a group the install does not materialize', async () => {
const result = await checkWithDedupe({
dedupeDirectDeps: true,
siblingDevBarVersion: '2.0.0',
include: { dependencies: true, devDependencies: false, optionalDependencies: false },
})
expect(result.issue).toBeUndefined()
expect(result.upToDate).toBe(true)
})
it('is outdated when a dependency in a materialized group has no root counterpart', async () => {
const result = await checkWithDedupe({ dedupeDirectDeps: true, siblingDevBarVersion: '2.0.0' })
expect(result.upToDate).toBe(false)
expect(result.issue).toBe(MISSING_MODULES_DIR)
})
})