From c60133d86cf0b89d3cca0653d5ab8685d4696bd2 Mon Sep 17 00:00:00 2001 From: Altay Date: Sat, 10 Oct 2026 20:16:11 +0300 Subject: [PATCH] perf(install): speed up frozen injected workspace installs (#16824) Avoid replacing an injected directory when its regular files are still hardlinked to the current source and its entries match. Keep the existing refresh path for mismatches, links, forced imports and build isolation. Preserve import reporting and add a frozen-repeat scenario to the existing integrated benchmark and CI comparison. Closes pnpm/tasks#121. --- .changeset/reuse-injected-hardlinks.md | 8 + .../scripts/compare-integrated-benchmarks.mjs | 3 +- .../compare-integrated-benchmarks.test.mjs | 4 +- .../pacquet-integrated-benchmark.yml | 41 +++- pnpm/CONTRIBUTING.md | 13 +- .../tests/suite/inject_workspace_packages.rs | 1 + .../hardlink_refresh.rs | 59 +++++ pnpm/crates/deps-restorer/Cargo.toml | 1 + .../src/create_virtual_dir_by_snapshot.rs | 1 + .../deps-restorer/src/import_indexed_dir.rs | 13 ++ .../src/import_indexed_dir/hardlinks.rs | 118 ++++++++++ .../src/import_indexed_dir/placement.rs | 43 ++-- .../import_indexed_dir/placement/symlinks.rs | 6 +- .../src/import_indexed_dir/tests.rs | 6 + .../src/import_indexed_dir/tests/hardlinks.rs | 209 ++++++++++++++++++ pnpm/crates/deps-restorer/src/link_file.rs | 24 +- .../src/link_file/tests/reporting.rs | 18 +- .../src/materialization_options.rs | 4 +- .../integrated-benchmark/src/cli_args.rs | 69 +++--- .../integrated-benchmark/src/work_env.rs | 4 + .../src/work_env/fixtures.rs | 10 +- .../src/work_env/injected_workspace.rs | 32 +++ .../src/work_env/tests.rs | 30 +++ .../src/workspace_manifest.rs | 2 + .../src/hardlinkedDirMatches.ts | 75 +++++++ pnpm11/fs/indexed-pkg-importer/src/index.ts | 7 +- .../src/repairIndexedDir.ts | 7 +- .../test/localHardlinks.test.ts | 123 +++++++++++ .../deps-restorer/src/linkAllPkgs.ts | 2 +- pnpm11/pnpm/test/injectedHardlinks.ts | 36 +++ 30 files changed, 875 insertions(+), 94 deletions(-) create mode 100644 .changeset/reuse-injected-hardlinks.md create mode 100644 pnpm/crates/cli/tests/suite/inject_workspace_packages/hardlink_refresh.rs create mode 100644 pnpm/crates/deps-restorer/src/import_indexed_dir/hardlinks.rs create mode 100644 pnpm/crates/deps-restorer/src/import_indexed_dir/tests/hardlinks.rs create mode 100644 pnpm/tasks/integrated-benchmark/src/work_env/injected_workspace.rs create mode 100644 pnpm11/fs/indexed-pkg-importer/src/hardlinkedDirMatches.ts create mode 100644 pnpm11/fs/indexed-pkg-importer/test/localHardlinks.test.ts create mode 100644 pnpm11/pnpm/test/injectedHardlinks.ts diff --git a/.changeset/reuse-injected-hardlinks.md b/.changeset/reuse-injected-hardlinks.md new file mode 100644 index 0000000000..33af343160 --- /dev/null +++ b/.changeset/reuse-injected-hardlinks.md @@ -0,0 +1,8 @@ +--- +"@pnpm/fs.indexed-pkg-importer": patch +"@pnpm/installing.deps-restorer": patch +"pnpm": patch +"pacquet": patch +--- + +Sped up frozen installs of unchanged injected workspace packages that use hardlinks. diff --git a/.github/scripts/compare-integrated-benchmarks.mjs b/.github/scripts/compare-integrated-benchmarks.mjs index 2d7658104d..85911bd5a4 100644 --- a/.github/scripts/compare-integrated-benchmarks.mjs +++ b/.github/scripts/compare-integrated-benchmarks.mjs @@ -13,6 +13,7 @@ export const scenarios = [ 'ISOLATED_FRESH_RESOLVE_HOT_CACHE_OFFLINE', 'ISOLATED_PEER_HEAVY_RESOLVE_HOT_CACHE_OFFLINE', 'ISOLATED_LINKED_WORKSPACE_RESOLVE_HOT_CACHE_OFFLINE', + 'ISOLATED_INJECTED_WORKSPACE_REPEAT_INSTALL_HOT_CACHE_HOT_STORE', 'ISOLATED_FRESH_RESTORE_COLD_CACHE_COLD_STORE_COLD_PNPR', 'GVS_FRESH_RESTORE_HOT_CACHE_HOT_STORE', ] @@ -76,7 +77,7 @@ export async function renderComparison (directory) { let failed = false for (const scenario of scenarios) { const report = JSON.parse(await readFile(join(directory, `BENCHMARK_REPORT_${scenario}.json`), 'utf8')) - const engines = scenario.includes('PEER_HEAVY') || scenario.includes('LINKED_WORKSPACE') ? ['pacquet'] : ['pacquet', 'pnpr'] + const engines = scenario.includes('PEER_HEAVY') || scenario.includes('LINKED_WORKSPACE') || scenario.includes('INJECTED_WORKSPACE') ? ['pacquet'] : ['pacquet', 'pnpr'] for (const engine of engines) { const result = compareConfirmed(report, engine) failed ||= result.status === 'Regression' diff --git a/.github/scripts/compare-integrated-benchmarks.test.mjs b/.github/scripts/compare-integrated-benchmarks.test.mjs index 0640f78310..d2f9adbf19 100644 --- a/.github/scripts/compare-integrated-benchmarks.test.mjs +++ b/.github/scripts/compare-integrated-benchmarks.test.mjs @@ -73,7 +73,7 @@ test('all scenarios and both engines are checked, and missing reports fail', asy try { for (const scenario of scenarios) { const input = report(stable, stable) - if (!scenario.includes('PEER_HEAVY') && !scenario.includes('LINKED_WORKSPACE')) { + if (!scenario.includes('PEER_HEAVY') && !scenario.includes('LINKED_WORKSPACE') && !scenario.includes('INJECTED_WORKSPACE')) { input.results.push(command('pnpr@HEAD', Array(10).fill(1.4)), command('pnpr@main', stable)) } input.confirmation = { results: input.results } @@ -82,7 +82,7 @@ test('all scenarios and both engines are checked, and missing reports fail', asy const result = await renderComparison(directory) assert.equal(result.failed, true) assert.equal(result.markdown.match(/\| Regression \|/g).length, 10) - assert.equal(result.markdown.match(/\| Within tolerance \|/g).length, 12) + assert.equal(result.markdown.match(/\| Within tolerance \|/g).length, 13) const cli = new URL('./compare-integrated-benchmarks.mjs', import.meta.url) const run = spawnSync(process.execPath, [fileURLToPath(cli), directory], { encoding: 'utf8' }) assert.equal(run.status, 1) diff --git a/.github/workflows/pacquet-integrated-benchmark.yml b/.github/workflows/pacquet-integrated-benchmark.yml index afe7dbe62e..95bcafb80f 100644 --- a/.github/workflows/pacquet-integrated-benchmark.yml +++ b/.github/workflows/pacquet-integrated-benchmark.yml @@ -272,6 +272,7 @@ jobs: - isolated-linker.fresh-resolve.hot-cache.offline - isolated-linker.peer-heavy-resolve.hot-cache.offline - isolated-linker.linked-workspace-resolve.hot-cache.offline + - isolated-linker.injected-workspace-repeat-install.hot-cache.hot-store - isolated-linker.fresh-restore.cold-cache.cold-store.cold-pnpr - gvs-linker.fresh-restore.hot-cache.hot-store name: ${{ matrix.scenario }} on ${{ matrix.os }} @@ -296,10 +297,9 @@ jobs: # command exercises client-side peer resolution rather than server or # tarball performance. PEER_HEAVY_BENCHMARK_TARGETS: ${{ github.ref_name == 'main' && 'pacquet@HEAD pnpm@HEAD' || 'pacquet@HEAD pacquet@main pnpm@HEAD' }} - # The linked-workspace case compares the PR against main only. Its - # fixture is a pure `workspace:*` graph, so there is nothing for a pnpr - # server to accelerate, and pnpm v11 never walks linked importers at all. - LINKED_WORKSPACE_BENCHMARK_TARGETS: ${{ github.ref_name == 'main' && 'pacquet@HEAD' || 'pacquet@HEAD pacquet@main' }} + # The workspace fixtures use only local packages, so they compare the + # PR against main without a pnpr server. + WORKSPACE_BENCHMARK_TARGETS: ${{ github.ref_name == 'main' && 'pacquet@HEAD' || 'pacquet@HEAD pacquet@main' }} BENCHMARK_BUILD_TARGETS: ${{ github.ref_name == 'main' && 'pacquet@HEAD pnpr@HEAD' || 'pacquet@HEAD pacquet@main pnpr@HEAD pnpr@main' }} # Network emulation so both install strategies cross the link they # would in production instead of free loopback (~GB/s, ~0 RTT), which @@ -338,11 +338,12 @@ jobs: - name: Cache pnpm-registry storage # The generated fixtures never populate the npm proxy cache: peer-heavy - # is seeded into hosted storage, linked-workspace names no registry - # package at all. + # is seeded into hosted storage, while the workspace fixtures name + # no registry package. if: >- matrix.scenario != 'isolated-linker.peer-heavy-resolve.hot-cache.offline' && - matrix.scenario != 'isolated-linker.linked-workspace-resolve.hot-cache.offline' + matrix.scenario != 'isolated-linker.linked-workspace-resolve.hot-cache.offline' && + matrix.scenario != 'isolated-linker.injected-workspace-repeat-install.hot-cache.hot-store' # Persists pnpm-registry's `--storage` dir across CI runs so # cold-cache benchmark scenarios don't refetch ~2.3k unscoped # packages from npmjs on every run. Replaces the older @@ -585,10 +586,19 @@ jobs: # (https://github.com/pnpm/pnpm/issues/14906). timeout-minutes: 20 run: | - node .github/scripts/run-integrated-benchmark.mjs --reuse-prebuilt-binaries --scenario=isolated-linker.linked-workspace-resolve.hot-cache.offline --registry=verdaccio --registry-latency-ms=${REGISTRY_LATENCY_MS} --registry-bandwidth-mbps=${REGISTRY_BANDWIDTH_MBPS} $LINKED_WORKSPACE_BENCHMARK_TARGETS + node .github/scripts/run-integrated-benchmark.mjs --reuse-prebuilt-binaries --scenario=isolated-linker.linked-workspace-resolve.hot-cache.offline --registry=verdaccio --registry-latency-ms=${REGISTRY_LATENCY_MS} --registry-bandwidth-mbps=${REGISTRY_BANDWIDTH_MBPS} $WORKSPACE_BENCHMARK_TARGETS cp bench-work-env/BENCHMARK_REPORT.md bench-work-env/BENCHMARK_REPORT_ISOLATED_LINKED_WORKSPACE_RESOLVE_HOT_CACHE_OFFLINE.md cp bench-work-env/BENCHMARK_REPORT.json bench-work-env/BENCHMARK_REPORT_ISOLATED_LINKED_WORKSPACE_RESOLVE_HOT_CACHE_OFFLINE.json + - name: 'Benchmark: Isolated linker: injected workspace repeat install, hot cache + hot store' + if: matrix.scenario == 'isolated-linker.injected-workspace-repeat-install.hot-cache.hot-store' + shell: bash + timeout-minutes: 20 + run: | + node .github/scripts/run-integrated-benchmark.mjs --reuse-prebuilt-binaries --scenario=isolated-linker.injected-workspace-repeat-install.hot-cache.hot-store --registry=npm $WORKSPACE_BENCHMARK_TARGETS + cp bench-work-env/BENCHMARK_REPORT.md bench-work-env/BENCHMARK_REPORT_ISOLATED_INJECTED_WORKSPACE_REPEAT_INSTALL_HOT_CACHE_HOT_STORE.md + cp bench-work-env/BENCHMARK_REPORT.json bench-work-env/BENCHMARK_REPORT_ISOLATED_INJECTED_WORKSPACE_REPEAT_INSTALL_HOT_CACHE_HOT_STORE.json + - name: 'Benchmark: Isolated linker: fresh restore, cold cache + cold store + cold pnpr' if: matrix.scenario == 'isolated-linker.fresh-restore.cold-cache.cold-store.cold-pnpr' shell: bash @@ -731,7 +741,7 @@ jobs: echo echo "**Commit:** [\`${SHA:0:12}\`](${{ github.server_url }}/${{ github.repository }}/commit/$SHA)" echo - echo 'Regular scenarios report direct and pnpr installs. The peer-heavy resolver scenario compares current Rust, main Rust, and TypeScript pnpm; the linked-workspace scenario compares current Rust against main Rust. Bencher consumes pacquet@HEAD and pnpr@HEAD.' + echo 'Regular scenarios report direct and pnpr installs. The peer-heavy resolver scenario compares current Rust, main Rust, and TypeScript pnpm; the linked and injected workspace scenarios compare current Rust against main Rust. Bencher consumes pacquet@HEAD and pnpr@HEAD.' echo echo 'The tables below show mean ± σ. Bencher tracks absolute minimum latency; PR regression checks compare HEAD and main samples from the same runner. Historical timing alerts do not gate PRs.' echo @@ -855,6 +865,18 @@ jobs: echo echo '' echo + echo '### Scenario: Isolated linker: injected workspace repeat install, hot cache + hot store' + echo + cat bench-work-env/BENCHMARK_REPORT_ISOLATED_INJECTED_WORKSPACE_REPEAT_INSTALL_HOT_CACHE_HOT_STORE.md + echo + echo '
BENCHMARK_REPORT.json' + echo + echo '```json' + jq 'del(.results[].exit_codes, .results[].times, .confirmation.results[]?.exit_codes, .confirmation.results[]?.times)' bench-work-env/BENCHMARK_REPORT_ISOLATED_INJECTED_WORKSPACE_REPEAT_INSTALL_HOT_CACHE_HOT_STORE.json + echo '```' + echo + echo '
' + echo echo '### Scenario: Isolated linker: fresh restore, cold cache + cold store + cold pnpr' echo cat bench-work-env/BENCHMARK_REPORT_ISOLATED_FRESH_RESTORE_COLD_CACHE_COLD_STORE_COLD_PNPR.md @@ -935,6 +957,7 @@ jobs: 'ISOLATED_FRESH_RESOLVE_HOT_CACHE_OFFLINE:isolated-linker.fresh-resolve.hot-cache.offline' \ 'ISOLATED_PEER_HEAVY_RESOLVE_HOT_CACHE_OFFLINE:isolated-linker.peer-heavy-resolve.hot-cache.offline' \ 'ISOLATED_LINKED_WORKSPACE_RESOLVE_HOT_CACHE_OFFLINE:isolated-linker.linked-workspace-resolve.hot-cache.offline' \ + 'ISOLATED_INJECTED_WORKSPACE_REPEAT_INSTALL_HOT_CACHE_HOT_STORE:isolated-linker.injected-workspace-repeat-install.hot-cache.hot-store' \ 'ISOLATED_FRESH_RESTORE_COLD_CACHE_COLD_STORE_COLD_PNPR:isolated-linker.fresh-restore.cold-cache.cold-store.cold-pnpr' \ 'GVS_FRESH_RESTORE_HOT_CACHE_HOT_STORE:gvs-linker.fresh-restore.hot-cache.hot-store' diff --git a/pnpm/CONTRIBUTING.md b/pnpm/CONTRIBUTING.md index 7c6a6c74eb..b647b1f753 100644 --- a/pnpm/CONTRIBUTING.md +++ b/pnpm/CONTRIBUTING.md @@ -206,6 +206,15 @@ just integrated-benchmark --scenario=isolated-linker.fresh-restore.cold-cache.co just integrated-benchmark --help ``` +To measure frozen repeat installs of an unchanged injected workspace package: + +```sh +just integrated-benchmark --scenario=isolated-linker.injected-workspace-repeat-install.hot-cache.hot-store pacquet@HEAD pacquet@main +``` + +This generated fixture uses hardlinked local files and needs no registry packages. +Its untimed install creates the lockfile and populates `node_modules` before measurement. + ### CI regression checks Each PR benchmark scenario measures HEAD and main on the same runner. The check @@ -216,8 +225,8 @@ A regression requires both the median slowdown and the gap between the fastest retained HEAD sample and slowest retained main sample to exceed that tolerance. The comparison trims `floor(sample count / 10)` samples from each tail and requires at least nine successful samples per target. Missing or invalid data -fails the check. Both pacquet and pnpr are checked, except in the two client-only -resolver scenarios. +fails the check. Both pacquet and pnpr are checked, except in client-only +workspace and resolver scenarios. Slowdowns above the tolerance without this sample separation are reported as inconclusive and do not fail CI. Suspected regressions are automatically rerun diff --git a/pnpm/crates/cli/tests/suite/inject_workspace_packages.rs b/pnpm/crates/cli/tests/suite/inject_workspace_packages.rs index 898bd67a0e..0af479ee49 100644 --- a/pnpm/crates/cli/tests/suite/inject_workspace_packages.rs +++ b/pnpm/crates/cli/tests/suite/inject_workspace_packages.rs @@ -10,6 +10,7 @@ //! flag flipped re-resolves. mod catalogs; +mod hardlink_refresh; use assert_cmd::prelude::*; use command_extra::CommandExtra; diff --git a/pnpm/crates/cli/tests/suite/inject_workspace_packages/hardlink_refresh.rs b/pnpm/crates/cli/tests/suite/inject_workspace_packages/hardlink_refresh.rs new file mode 100644 index 0000000000..8d1c503fda --- /dev/null +++ b/pnpm/crates/cli/tests/suite/inject_workspace_packages/hardlink_refresh.rs @@ -0,0 +1,59 @@ +use assert_cmd::prelude::*; +use pnpm_testing_utils::bin::CommandTempCwd; +use std::fs; + +#[test] +fn frozen_injected_installs_reuse_unchanged_hardlinks_and_refresh_replacements() { + for global_store in [false, true] { + let CommandTempCwd { + mut pacquet, root: _root, workspace, .. + } = CommandTempCwd::init(); + fs::write( + workspace.join("package.json"), + serde_json::json!({ + "name": "root", "private": true, + "dependencies": { "library": "workspace:*" }, + }) + .to_string(), + ) + .unwrap(); + fs::write(workspace.join("pnpm-workspace.yaml"), format!( + "packages:\n - packages/*\nstoreDir: ./store\ncacheDir: ./cache\ninjectWorkspacePackages: true\ndedupeInjectedDeps: false\nenableGlobalVirtualStore: {global_store}\n", + )).unwrap(); + let source = workspace.join("packages/library"); + fs::create_dir_all(&source).unwrap(); + fs::write(source.join("package.json"), r#"{"name":"library","version":"1.0.0"}"#).unwrap(); + fs::write(source.join("index.js"), "original").unwrap(); + pacquet + .args(["install", "--offline", "--ignore-scripts", "--no-frozen-lockfile"]) + .assert() + .success(); + let target = fs::canonicalize(workspace.join("node_modules/library")).unwrap(); + assert_ne!(target, fs::canonicalize(&source).unwrap()); + let original = same_file::Handle::from_path(&target).unwrap(); + let repeated = crate::_utils::pacquet_in(&workspace) + .args(["install", "--offline", "--frozen-lockfile", "--reporter=ndjson"]) + .assert() + .success(); + let stderr = String::from_utf8_lossy(&repeated.get_output().stderr); + let methods: Vec = stderr + .lines() + .map(|line| serde_json::from_str(line).unwrap()) + .filter(|event: &serde_json::Value| event["name"] == "pnpm:package-import-method") + .collect(); + assert_eq!(methods.len(), 1); + assert_eq!(methods[0]["method"], "hardlink"); + assert_eq!(same_file::Handle::from_path(&target).unwrap(), original); + + fs::write(source.join("replacement.js"), "modified").unwrap(); + fs::rename(source.join("replacement.js"), source.join("index.js")).unwrap(); + assert_eq!(fs::read_to_string(target.join("index.js")).unwrap(), "original"); + crate::_utils::pacquet_in(&workspace) + .args(["install", "--offline", "--frozen-lockfile"]) + .assert() + .success(); + assert_eq!(fs::read_to_string(target.join("index.js")).unwrap(), "modified"); + assert_ne!(same_file::Handle::from_path(&target).unwrap(), original); + assert!(same_file::is_same_file(source.join("index.js"), target.join("index.js")).unwrap()); + } +} diff --git a/pnpm/crates/deps-restorer/Cargo.toml b/pnpm/crates/deps-restorer/Cargo.toml index 47b2b4c8ab..92877f8af3 100644 --- a/pnpm/crates/deps-restorer/Cargo.toml +++ b/pnpm/crates/deps-restorer/Cargo.toml @@ -73,6 +73,7 @@ junction = { workspace = true } mockito = { workspace = true } p256 = { workspace = true } pretty_assertions = { workspace = true } +same-file = { workspace = true } serde-saphyr = { workspace = true } sha2 = { workspace = true } tar = { workspace = true } diff --git a/pnpm/crates/deps-restorer/src/create_virtual_dir_by_snapshot.rs b/pnpm/crates/deps-restorer/src/create_virtual_dir_by_snapshot.rs index 21b478e433..a225fb3149 100644 --- a/pnpm/crates/deps-restorer/src/create_virtual_dir_by_snapshot.rs +++ b/pnpm/crates/deps-restorer/src/create_virtual_dir_by_snapshot.rs @@ -405,6 +405,7 @@ fn slot_import_opts( if interrupted_build || source_is_mutable || force_import { return ImportIndexedDirOpts { force: true, + reuse_hardlinks: source_is_mutable && !force_import && !interrupted_build, keep_modules_dir: true, safe_to_skip, preserve_symlinks: source_is_mutable, diff --git a/pnpm/crates/deps-restorer/src/import_indexed_dir.rs b/pnpm/crates/deps-restorer/src/import_indexed_dir.rs index 24850396b9..71efe688bf 100644 --- a/pnpm/crates/deps-restorer/src/import_indexed_dir.rs +++ b/pnpm/crates/deps-restorer/src/import_indexed_dir.rs @@ -1,5 +1,6 @@ pub use placement::marker_present; +mod hardlinks; mod placement; use placement::{all_files_match, file_matches_store_entry, populate_dir}; @@ -37,6 +38,8 @@ pub struct ImportIndexedDirOpts { /// existing directory short-circuits this function (matches /// pnpm's pre-existence check in `importIndexedPackage`). pub force: bool, + /// Reuse an unchanged mutable directory when every imported file is still hardlinked. + pub reuse_hardlinks: bool, /// When `true` (only meaningful with `force`), preserve /// `dir_path/node_modules/` across the re-import so nested /// dependencies survive the rebuild. Required by the hoisted @@ -308,6 +311,16 @@ fn force_import_existing_dir( opts.preserve_symlinks, ); } + if opts.reuse_hardlinks + && import_method == PackageImportMethod::Hardlink + && hardlinks::directory_matches(dir_path, cas_paths, opts.keep_modules_dir) + { + crate::link_file::log_method_once::( + logged_methods, + pnpm_reporter::PackageImportMethod::Hardlink, + ); + return Ok(()); + } if opts.safe_to_skip { return import_into_shared_dir::( logged_methods, diff --git a/pnpm/crates/deps-restorer/src/import_indexed_dir/hardlinks.rs b/pnpm/crates/deps-restorer/src/import_indexed_dir/hardlinks.rs new file mode 100644 index 0000000000..19bdca41af --- /dev/null +++ b/pnpm/crates/deps-restorer/src/import_indexed_dir/hardlinks.rs @@ -0,0 +1,118 @@ +use super::placement::{ + file_identity_matches, + symlinks::{imported_paths, is_symlink}, +}; +use std::{ + collections::{HashMap, HashSet}, + fs, io, + path::{Component, Path, PathBuf}, +}; + +pub(super) fn directory_matches( + target: &Path, + files: &HashMap, + keep_modules_dir: bool, +) -> bool { + if files.is_empty() || files.contains_key(crate::NEEDS_BUILD_MARKER) { + return false; + } + if !pnpm_fs::symlink_metadata_with_retry(target) + .is_ok_and(|meta| meta.is_dir() && !is_symlink(target)) + { + return false; + } + let Some(expected) = ExpectedTree::new(files, keep_modules_dir) else { return false }; + expected.matches(target).unwrap_or(false) +} + +struct ExpectedTree<'a> { + files: HashMap, + directories: HashSet, + keep_modules_dir: bool, +} + +enum EntryKind { + File, + Directory, + Preserved, +} + +impl<'a> ExpectedTree<'a> { + fn new(files: &'a HashMap, keep_modules_dir: bool) -> Option { + let directories = imported_paths(files) + .into_iter() + .filter(|entry| !files.contains_key(*entry)) + .map(PathBuf::from) + .collect(); + let mut expected = ExpectedTree { + files: HashMap::with_capacity(files.len()), + directories, + keep_modules_dir, + }; + for (relative, source) in files { + let path = Path::new(relative); + if path.as_os_str().is_empty() + || !path + .components() + .all(|part| matches!(part, Component::Normal(_))) + || path.starts_with("node_modules") + { + return None; + } + if expected.files.insert(path.to_path_buf(), source).is_some() { + return None; + } + } + Some(expected) + } + + fn matches(&self, root: &Path) -> io::Result { + let mut pending = vec![PathBuf::new()]; + let mut matched = 0; + while let Some(relative) = pending.pop() { + let Some(count) = self.matching_files_in_directory(root, &relative, &mut pending)? + else { + return Ok(false); + }; + matched += count; + } + Ok(matched == self.files.len()) + } + + fn matching_files_in_directory( + &self, + root: &Path, + relative: &Path, + pending: &mut Vec, + ) -> io::Result> { + let mut matched = 0; + for entry in fs::read_dir(root.join(relative))? { + let entry = entry?; + let path = relative.join(entry.file_name()); + match self.entry_kind(&path, &entry.path())? { + Some(EntryKind::File) => matched += 1, + Some(EntryKind::Directory) => pending.push(path), + Some(EntryKind::Preserved) => {} + None => return Ok(None), + } + } + Ok(Some(matched)) + } + + fn entry_kind(&self, relative: &Path, target: &Path) -> io::Result> { + let metadata = fs::symlink_metadata(target)?; + if metadata.file_type().is_symlink() || (metadata.is_dir() && is_symlink(target)) { + return Ok(None); + } + if metadata.is_dir() { + if self.keep_modules_dir && relative == Path::new("node_modules") { + return Ok(Some(EntryKind::Preserved)); + } + return Ok(self.directories.contains(relative).then_some(EntryKind::Directory)); + } + let Some(source) = self.files.get(relative) else { return Ok(None) }; + let source_metadata = fs::symlink_metadata(source)?; + Ok(file_identity_matches((target, &metadata), (source, &source_metadata)) + .then_some(EntryKind::File)) + } +} diff --git a/pnpm/crates/deps-restorer/src/import_indexed_dir/placement.rs b/pnpm/crates/deps-restorer/src/import_indexed_dir/placement.rs index 5566d66f80..e2ee3654f8 100644 --- a/pnpm/crates/deps-restorer/src/import_indexed_dir/placement.rs +++ b/pnpm/crates/deps-restorer/src/import_indexed_dir/placement.rs @@ -1,3 +1,5 @@ +pub(super) mod symlinks; + use super::{ImportIndexedDirError, Placement, remove_non_dir_dirent, staging::import_atomic}; use crate::import_into_fresh_target; use pnpm_config::PackageImportMethod; @@ -370,19 +372,7 @@ pub(super) fn file_matches_store_entry(target: &Path, store_path: &Path) -> bool let Ok(store_meta) = fs::metadata(store_path) else { return false; }; - // Unix carries the file's identity in the stat results already. - // Windows keeps it behind an open handle, which `same-file` opens — - // worth two handles to spare a hardlinked package a full read on the - // platform where hardlinking is the default tier. - #[cfg(unix)] - { - use std::os::unix::fs::MetadataExt; - if target_meta.ino() == store_meta.ino() && target_meta.dev() == store_meta.dev() { - return true; - } - } - #[cfg(windows)] - if same_file::is_same_file(target, store_path).unwrap_or(false) { + if file_identity_matches((target, &target_meta), (store_path, &store_meta)) { return true; } target_meta.len() == store_meta.len() @@ -430,6 +420,31 @@ pub fn marker_present(dir_path: &Path, cas_paths: &HashMap) -> } } -mod symlinks; #[cfg(test)] mod tests; + +/// Compare regular files without accepting equal-byte copies or unknown file identities. +pub(super) fn file_identity_matches( + target: (&Path, &fs::Metadata), + source: (&Path, &fs::Metadata), +) -> bool { + if !target.1.is_file() || !source.1.is_file() { + return false; + } + #[cfg(unix)] + { + use std::os::unix::fs::MetadataExt; + target.1.ino() != 0 + && target.1.dev() != 0 + && target.1.ino() == source.1.ino() + && target.1.dev() == source.1.dev() + } + #[cfg(windows)] + { + same_file::is_same_file(target.0, source.0).unwrap_or(false) + } + #[cfg(not(any(unix, windows)))] + { + false + } +} diff --git a/pnpm/crates/deps-restorer/src/import_indexed_dir/placement/symlinks.rs b/pnpm/crates/deps-restorer/src/import_indexed_dir/placement/symlinks.rs index e33d03e3e7..c0db0a172b 100644 --- a/pnpm/crates/deps-restorer/src/import_indexed_dir/placement/symlinks.rs +++ b/pnpm/crates/deps-restorer/src/import_indexed_dir/placement/symlinks.rs @@ -24,7 +24,9 @@ pub(super) struct SymlinkRoots<'a> { /// Every key of `cas_paths` and each directory above it, the package root /// included as `""`. -pub(super) fn imported_paths(cas_paths: &HashMap) -> HashSet<&str> { +pub(in crate::import_indexed_dir) fn imported_paths( + cas_paths: &HashMap, +) -> HashSet<&str> { let mut imported = HashSet::from([""]); for entry in cas_paths.keys() { let mut path = entry.as_str(); @@ -37,7 +39,7 @@ pub(super) fn imported_paths(cas_paths: &HashMap) -> HashSet<&s /// Whether `path` is a symlink or, on Windows, a junction, which /// [`fs::FileType::is_symlink`] does not report. -pub(super) fn is_symlink(path: &Path) -> bool { +pub(in crate::import_indexed_dir) fn is_symlink(path: &Path) -> bool { let Ok(metadata) = fs::symlink_metadata(path) else { return false; }; diff --git a/pnpm/crates/deps-restorer/src/import_indexed_dir/tests.rs b/pnpm/crates/deps-restorer/src/import_indexed_dir/tests.rs index 33e817dba6..bd6af16ac5 100644 --- a/pnpm/crates/deps-restorer/src/import_indexed_dir/tests.rs +++ b/pnpm/crates/deps-restorer/src/import_indexed_dir/tests.rs @@ -2,6 +2,7 @@ mod reporting; mod store; +mod hardlinks; mod workspace; mod runtimes; @@ -37,18 +38,21 @@ fn cas_map(entries: &[(&str, PathBuf)]) -> HashMap { const FORCE_KEEP: ImportIndexedDirOpts = ImportIndexedDirOpts { force: true, + reuse_hardlinks: false, keep_modules_dir: true, safe_to_skip: false, preserve_symlinks: false, }; const FORCE_ONLY: ImportIndexedDirOpts = ImportIndexedDirOpts { force: true, + reuse_hardlinks: false, keep_modules_dir: false, safe_to_skip: false, preserve_symlinks: false, }; const FORCE_SHARED: ImportIndexedDirOpts = ImportIndexedDirOpts { force: true, + reuse_hardlinks: false, keep_modules_dir: false, safe_to_skip: true, preserve_symlinks: false, @@ -57,6 +61,7 @@ const FORCE_SHARED: ImportIndexedDirOpts = ImportIndexedDirOpts { // short-circuited by its marker before the import runs. const SHARED: ImportIndexedDirOpts = ImportIndexedDirOpts { force: false, + reuse_hardlinks: false, keep_modules_dir: false, safe_to_skip: true, preserve_symlinks: false, @@ -65,6 +70,7 @@ const SHARED: ImportIndexedDirOpts = ImportIndexedDirOpts { #[cfg(unix)] const FORCE_SHARED_KEEP: ImportIndexedDirOpts = ImportIndexedDirOpts { force: true, + reuse_hardlinks: false, keep_modules_dir: true, safe_to_skip: true, preserve_symlinks: false, diff --git a/pnpm/crates/deps-restorer/src/import_indexed_dir/tests/hardlinks.rs b/pnpm/crates/deps-restorer/src/import_indexed_dir/tests/hardlinks.rs new file mode 100644 index 0000000000..7fb526ef6c --- /dev/null +++ b/pnpm/crates/deps-restorer/src/import_indexed_dir/tests/hardlinks.rs @@ -0,0 +1,209 @@ +use super::{super::import_indexed_dir, ImportIndexedDirOpts, cas_map, write_source}; +use pnpm_config::PackageImportMethod; +use pnpm_reporter::SilentReporter; +use std::{collections::HashMap, fs, path::PathBuf, sync::atomic::AtomicU8}; +use tempfile::tempdir; + +const REUSE: ImportIndexedDirOpts = ImportIndexedDirOpts { + force: true, + reuse_hardlinks: true, + keep_modules_dir: true, + safe_to_skip: false, + preserve_symlinks: true, +}; + +#[test] +fn unchanged_injected_hardlinks_preserve_the_package_directory() { + let tmp = tempdir().unwrap(); + let source = tmp.path().join("source"); + let target = tmp.path().join("target"); + let files = cas_map(&[ + ("package.json", write_source(&source, "package.json", b"{}")), + ("lib/index.js", write_source(&source, "lib/index.js", b"original")), + ]); + let install = || { + import_indexed_dir::( + &AtomicU8::new(0), + PackageImportMethod::Hardlink, + &target, + &files, + REUSE, + ) + .unwrap(); + }; + install(); + fs::create_dir_all(target.join("node_modules/dependency")).unwrap(); + fs::write(target.join("node_modules/dependency/index.js"), b"dependency").unwrap(); + let identity = same_file::Handle::from_path(&target).unwrap(); + install(); + assert_eq!(same_file::Handle::from_path(&target).unwrap(), identity); + assert_eq!(fs::read(target.join("node_modules/dependency/index.js")).unwrap(), b"dependency"); + + let original = fs::metadata(source.join("lib/index.js")).unwrap(); + let replaced = write_source(&source, "replacement", b"modified"); + fs::File::options() + .write(true) + .open(&replaced) + .unwrap() + .set_modified(original.modified().unwrap()) + .unwrap(); + let replacement_metadata = fs::metadata(&replaced).unwrap(); + assert_eq!(replacement_metadata.modified().unwrap(), original.modified().unwrap()); + assert_eq!(replacement_metadata.len(), original.len()); + fs::rename(replaced, source.join("lib/index.js")).unwrap(); + assert_eq!(fs::read(target.join("lib/index.js")).unwrap(), b"original"); + install(); + assert_eq!(fs::read(target.join("lib/index.js")).unwrap(), b"modified"); + assert_ne!(same_file::Handle::from_path(&target).unwrap(), identity); +} + +#[test] +fn injected_refresh_removes_extra_entries_and_repairs_copied_or_missing_files() { + let tmp = tempdir().unwrap(); + let source = tmp.path().join("source"); + let target = tmp.path().join("target"); + let manifest = write_source(&source, "package.json", b"{}"); + let content = write_source(&source, "index.js", b"content"); + let files = cas_map(&[("package.json", manifest), ("index.js", content.clone())]); + let install = || { + import_indexed_dir::( + &AtomicU8::new(0), + PackageImportMethod::Hardlink, + &target, + &files, + REUSE, + ) + .unwrap(); + }; + install(); + fs::remove_file(target.join("index.js")).unwrap(); + fs::copy(&content, target.join("index.js")).unwrap(); + install(); + assert!(same_file::is_same_file(&content, target.join("index.js")).unwrap()); + fs::create_dir(target.join("stale-empty-dir")).unwrap(); + fs::write(target.join("removed.js"), b"removed").unwrap(); + install(); + assert!(!target.join("stale-empty-dir").exists()); + assert!(!target.join("removed.js").exists()); + fs::remove_file(target.join("package.json")).unwrap(); + fs::remove_file(target.join("index.js")).unwrap(); + install(); + assert_eq!(fs::read(target.join("package.json")).unwrap(), b"{}"); + assert_eq!(fs::read(target.join("index.js")).unwrap(), b"content"); +} + +#[test] +fn injected_hardlink_reuse_respects_copy_and_explicit_reimport() { + let tmp = tempdir().unwrap(); + let source = tmp.path().join("source"); + let target = tmp.path().join("target"); + let content = write_source(&source, "package.json", b"{}"); + let files = cas_map(&[("package.json", content.clone())]); + let import = |method, opts| { + import_indexed_dir::(&AtomicU8::new(0), method, &target, &files, opts) + .unwrap(); + }; + import(PackageImportMethod::Hardlink, REUSE); + let original = same_file::Handle::from_path(&target).unwrap(); + import(PackageImportMethod::Hardlink, ImportIndexedDirOpts { reuse_hardlinks: false, ..REUSE }); + assert_ne!(same_file::Handle::from_path(&target).unwrap(), original); + import(PackageImportMethod::Copy, REUSE); + assert!(!same_file::is_same_file(&content, target.join("package.json")).unwrap()); +} + +#[test] +fn injected_refresh_replaces_directory_links_without_mutating_their_targets() { + let tmp = tempdir().unwrap(); + let source = tmp.path().join("source"); + let target = tmp.path().join("target"); + let outside = tmp.path().join("outside"); + let content = write_source(&source, "lib/index.js", b"content"); + let manifest = write_source(&source, "package.json", b"{}"); + let files = cas_map(&[("package.json", manifest.clone()), ("lib/index.js", content.clone())]); + fs::create_dir_all(&target).unwrap(); + fs::create_dir_all(&outside).unwrap(); + fs::hard_link(&manifest, target.join("package.json")).unwrap(); + fs::hard_link(&content, outside.join("index.js")).unwrap(); + pnpm_fs::symlink_dir(&outside, &target.join("lib")).unwrap(); + import_indexed_dir::( + &AtomicU8::new(0), + PackageImportMethod::Hardlink, + &target, + &files, + REUSE, + ) + .unwrap(); + assert!(!pnpm_fs::is_symlink_or_junction(&target.join("lib")).unwrap()); + assert_eq!(fs::read(outside.join("index.js")).unwrap(), b"content"); +} + +#[test] +fn injected_refresh_replaces_root_directory_links() { + let tmp = tempdir().unwrap(); + let source = tmp.path().join("source"); + let target = tmp.path().join("target"); + let files = cas_map(&[("package.json", write_source(&source, "package.json", b"{}"))]); + pnpm_fs::symlink_dir(&source, &target).unwrap(); + import_indexed_dir::( + &AtomicU8::new(0), + PackageImportMethod::Hardlink, + &target, + &files, + REUSE, + ) + .unwrap(); + assert!(!pnpm_fs::is_symlink_or_junction(&target).unwrap()); + assert_eq!(fs::read(source.join("package.json")).unwrap(), b"{}"); +} + +#[test] +fn injected_refresh_adds_and_removes_source_entries() { + let tmp = tempdir().unwrap(); + let source = tmp.path().join("source"); + let target = tmp.path().join("target"); + let mut files = cas_map(&[("package.json", write_source(&source, "package.json", b"{}"))]); + let import = |files: &HashMap| { + import_indexed_dir::( + &AtomicU8::new(0), + PackageImportMethod::Hardlink, + &target, + files, + REUSE, + ) + .unwrap(); + }; + import(&files); + files.insert("lib/added.js".into(), write_source(&source, "lib/added.js", b"added")); + import(&files); + assert_eq!(fs::read(target.join("lib/added.js")).unwrap(), b"added"); + files.remove("lib/added.js"); + import(&files); + assert!(!target.join("lib").exists()); +} + +#[test] +fn build_markers_and_bundled_dependencies_always_reimport() { + for name in [crate::NEEDS_BUILD_MARKER, "node_modules/bundled/index.js"] { + let tmp = tempdir().unwrap(); + let source = tmp.path().join("source"); + let target = tmp.path().join("target"); + let files = cas_map(&[ + ("package.json", write_source(&source, "package.json", b"{}")), + (name, write_source(&source, name, b"content")), + ]); + let import = || { + import_indexed_dir::( + &AtomicU8::new(0), + PackageImportMethod::Hardlink, + &target, + &files, + REUSE, + ) + .unwrap(); + }; + import(); + let original = same_file::Handle::from_path(&target).unwrap(); + import(); + assert_ne!(same_file::Handle::from_path(&target).unwrap(), original, "{name}"); + } +} diff --git a/pnpm/crates/deps-restorer/src/link_file.rs b/pnpm/crates/deps-restorer/src/link_file.rs index 846a4a737d..49517c46f0 100644 --- a/pnpm/crates/deps-restorer/src/link_file.rs +++ b/pnpm/crates/deps-restorer/src/link_file.rs @@ -134,11 +134,15 @@ const LOG_FLAG_CLONE: u8 = 1 << 0; const LOG_FLAG_HARDLINK: u8 = 1 << 1; const LOG_FLAG_COPY: u8 = 1 << 2; -fn log_method_once( +pub(crate) fn log_method_once( logged: &AtomicU8, - flag: u8, method: WireImportMethod, ) { + let flag = match method { + WireImportMethod::Clone => LOG_FLAG_CLONE, + WireImportMethod::Hardlink => LOG_FLAG_HARDLINK, + WireImportMethod::Copy => LOG_FLAG_COPY, + }; if logged.fetch_or(flag, Ordering::Relaxed) & flag == 0 { let method_name = match method { WireImportMethod::Clone => "clone", @@ -347,7 +351,7 @@ impl ImportState { } PackageImportMethod::Clone => clone_file::(source_file, target_link) .inspect(|()| { - log_method_once::(logged, LOG_FLAG_CLONE, WireImportMethod::Clone); + log_method_once::(logged, WireImportMethod::Clone); }) .map(|()| WireImportMethod::Clone), PackageImportMethod::CloneOrCopy => clone_or_copy_link::( @@ -358,7 +362,7 @@ impl ImportState { ), PackageImportMethod::Copy => copy_file(source_file, target_link) .inspect(|()| { - log_method_once::(logged, LOG_FLAG_COPY, WireImportMethod::Copy); + log_method_once::(logged, WireImportMethod::Copy); }) .map(|()| WireImportMethod::Copy), } @@ -396,13 +400,13 @@ fn hardlink_file( if source_has_desired_mode(source_file)? { match Sys::hard_link(source_file, target_link) { Ok(()) => { - log_method_once::(logged, LOG_FLAG_HARDLINK, WireImportMethod::Hardlink); + log_method_once::(logged, WireImportMethod::Hardlink); Ok(WireImportMethod::Hardlink) } Err(error) if is_cross_device(&error) || is_too_many_links(&error) => { copy_file(source_file, target_link) .inspect(|()| { - log_method_once::(logged, LOG_FLAG_COPY, WireImportMethod::Copy); + log_method_once::(logged, WireImportMethod::Copy); }) .map(|()| WireImportMethod::Copy) } @@ -411,7 +415,7 @@ fn hardlink_file( } else { copy_file(source_file, target_link) .inspect(|()| { - log_method_once::(logged, LOG_FLAG_COPY, WireImportMethod::Copy); + log_method_once::(logged, WireImportMethod::Copy); }) .map(|()| WireImportMethod::Copy) } @@ -514,7 +518,7 @@ fn copy_and_log( target: &Path, ) -> io::Result { copy_file(source, target)?; - log_method_once::(logged, LOG_FLAG_COPY, WireImportMethod::Copy); + log_method_once::(logged, WireImportMethod::Copy); Ok(WireImportMethod::Copy) } @@ -569,7 +573,7 @@ fn clone_tier( match Sys::reflink(source, target) { Ok(()) => { align_target_mode(source, target)?; - log_method_once::(logged, LOG_FLAG_CLONE, WireImportMethod::Clone); + log_method_once::(logged, WireImportMethod::Clone); Ok(true) } Err(err) if is_call_error(&err) => Err(err), @@ -593,7 +597,7 @@ fn hardlink_tier( if source_has_desired_mode(source)? { match Sys::hard_link(source, target) { Ok(()) => { - log_method_once::(logged, LOG_FLAG_HARDLINK, WireImportMethod::Hardlink); + log_method_once::(logged, WireImportMethod::Hardlink); Ok(Some(WireImportMethod::Hardlink)) } Err(err) if is_too_many_links(&err) => { diff --git a/pnpm/crates/deps-restorer/src/link_file/tests/reporting.rs b/pnpm/crates/deps-restorer/src/link_file/tests/reporting.rs index bc44ea8846..c269cb4147 100644 --- a/pnpm/crates/deps-restorer/src/link_file/tests/reporting.rs +++ b/pnpm/crates/deps-restorer/src/link_file/tests/reporting.rs @@ -29,21 +29,9 @@ fn log_method_once_emits_first_call_per_method_only() { let logged = AtomicU8::new(0); - super::super::log_method_once::( - &logged, - super::super::LOG_FLAG_CLONE, - WireImportMethod::Clone, - ); - super::super::log_method_once::( - &logged, - super::super::LOG_FLAG_CLONE, - WireImportMethod::Clone, - ); - super::super::log_method_once::( - &logged, - super::super::LOG_FLAG_HARDLINK, - WireImportMethod::Hardlink, - ); + super::super::log_method_once::(&logged, WireImportMethod::Clone); + super::super::log_method_once::(&logged, WireImportMethod::Clone); + super::super::log_method_once::(&logged, WireImportMethod::Hardlink); let captured = EVENTS.lock().unwrap(); let kinds: Vec = captured diff --git a/pnpm/crates/deps-restorer/src/materialization_options.rs b/pnpm/crates/deps-restorer/src/materialization_options.rs index 374a7777f9..3feb5b7616 100644 --- a/pnpm/crates/deps-restorer/src/materialization_options.rs +++ b/pnpm/crates/deps-restorer/src/materialization_options.rs @@ -59,8 +59,8 @@ pub struct SlotImportSource<'a> { /// (a `file:` / [`pnpm_lockfile::LockfileResolution::Directory`] /// resolution) rather than immutable CAS entries. pnpm's `file:` is /// a copy taken at install time — unlike `link:`, which symlinks — - /// so the slot has to be rebuilt on every install: the source can - /// change without the lockfile changing, and the completion-marker + /// so each install must refresh or verify the slot's files: the source + /// can change without the lockfile changing, and the completion-marker /// short-circuit in [`fn@crate::import_indexed_dir`] would otherwise /// leave the previous install's copy in place forever. pub is_mutable: bool, diff --git a/pnpm/tasks/integrated-benchmark/src/cli_args.rs b/pnpm/tasks/integrated-benchmark/src/cli_args.rs index 72bd3dc9e6..4b214aae94 100644 --- a/pnpm/tasks/integrated-benchmark/src/cli_args.rs +++ b/pnpm/tasks/integrated-benchmark/src/cli_args.rs @@ -263,6 +263,9 @@ pub enum BenchmarkScenario { /// Frozen lockfile, hot cache + hot store, `enableGlobalVirtualStore: true` with a pre-warmed GVS. #[value(name = "gvs-linker.fresh-restore.hot-cache.hot-store")] GvsFreshRestoreHotCacheHotStore, + /// Frozen repeat install of a populated injected workspace package with many small files. + #[value(name = "isolated-linker.injected-workspace-repeat-install.hot-cache.hot-store")] + IsolatedInjectedWorkspaceRepeatInstallHotCacheHotStore, } /// Per-iteration cleanup applied by hyperfine's `--prepare`. @@ -295,7 +298,8 @@ impl BenchmarkScenario { BenchmarkScenario::IsolatedFreshRestoreColdCacheColdStore | BenchmarkScenario::IsolatedFreshRestoreColdCacheColdStoreColdPnpr | BenchmarkScenario::IsolatedFreshRestoreHotCacheHotStore - | BenchmarkScenario::GvsFreshRestoreHotCacheHotStore => { + | BenchmarkScenario::GvsFreshRestoreHotCacheHotStore + | BenchmarkScenario::IsolatedInjectedWorkspaceRepeatInstallHotCacheHotStore => { &["install", "--frozen-lockfile"] } BenchmarkScenario::IsolatedFreshAddDepHotCacheHotStore => &["add", "is-odd"], @@ -307,14 +311,14 @@ impl BenchmarkScenario { } } - /// Arguments of the online install that primes `cache-dir` / - /// `store-dir` before hyperfine runs, for scenarios whose measured - /// command can't do its own priming (an `--offline` run against the - /// freshly wiped mirror fails). `None` for every other scenario: - /// hyperfine's warmup run primes whatever their per-iteration - /// cleanup preserves. + /// Untimed install arguments that prepare state the measured command cannot + /// create, such as cached metadata for offline resolution or a generated + /// lockfile for a frozen install. `None` uses hyperfine's normal warmup. pub fn prewarm_install_args(self) -> Option<&'static [&'static str]> { match self { + BenchmarkScenario::IsolatedInjectedWorkspaceRepeatInstallHotCacheHotStore => { + Some(&["install", "--offline", "--no-frozen-lockfile"]) + } BenchmarkScenario::IsolatedFreshResolveHotCacheOffline => Some(&["install"]), // This resolution-only fixture has no fetchable tarballs, so its // metadata prewarm must not proceed to fetching or linking. @@ -347,7 +351,8 @@ impl BenchmarkScenario { | BenchmarkScenario::IsolatedLinkedWorkspaceResolveHotCacheOffline | BenchmarkScenario::GvsFreshRestoreHotCacheHotStore | BenchmarkScenario::IsolatedRepeatInstallHotCacheHotStore - | BenchmarkScenario::IsolatedRepeatInstallColdCacheHotStore => true, + | BenchmarkScenario::IsolatedRepeatInstallColdCacheHotStore + | BenchmarkScenario::IsolatedInjectedWorkspaceRepeatInstallHotCacheHotStore => true, } } @@ -357,7 +362,9 @@ impl BenchmarkScenario { /// the online pre-warm pass skip resolution, leaving the metadata /// mirror cold for the measured offline runs. pub fn seeds_lockfile(self) -> bool { - self.lockfile_enabled() && !self.is_offline_fresh_resolve() + self.lockfile_enabled() + && !self.is_offline_fresh_resolve() + && !self.uses_injected_workspace_fixture() } /// The `pnpm-lock.yaml` contents to seed during init, when @@ -375,7 +382,7 @@ impl BenchmarkScenario { /// restore) applied via hyperfine's `--prepare`. pub fn cleanup(self) -> Cleanup { match self { - BenchmarkScenario::IsolatedFreshInstallColdCacheColdStore => Cleanup { + Self::IsolatedFreshInstallColdCacheColdStore => Cleanup { // `cache-dir` (the packument-metadata mirror) is wiped // alongside `store-dir` so a direct fresh install pays the // full cold resolution — fetching every packument over the @@ -390,16 +397,15 @@ impl BenchmarkScenario { // refetches (and streams) every tarball from the warm origin each // iteration. (`cold-mock-storage` only exists under a `pnpr@` // bench dir; it's a harmless no-op for the other ids.) - BenchmarkScenario::IsolatedFreshRestoreColdCacheColdStoreColdPnpr => Cleanup { + Self::IsolatedFreshRestoreColdCacheColdStoreColdPnpr => Cleanup { remove: &["node_modules", "store-dir", "cache-dir", "cold-mock-storage"], restore: &[SAVED_LOCKFILE], }, - BenchmarkScenario::IsolatedFreshRestoreColdCacheColdStore => Cleanup { + Self::IsolatedFreshRestoreColdCacheColdStore => Cleanup { remove: &["node_modules", "store-dir", "cache-dir"], restore: &[SAVED_LOCKFILE], }, - BenchmarkScenario::IsolatedFreshRestoreHotCacheHotStore - | BenchmarkScenario::GvsFreshRestoreHotCacheHotStore => { + Self::IsolatedFreshRestoreHotCacheHotStore | Self::GvsFreshRestoreHotCacheHotStore => { Cleanup { remove: &["node_modules"], restore: &[SAVED_LOCKFILE] } } // A repeat install mutates nothing, so nothing is removed or @@ -407,22 +413,23 @@ impl BenchmarkScenario { // push every iteration off the pure-mtime fast path into the // heavier content re-check. The populated `node_modules` (and // workspace state) come from the pre-warm pass. - BenchmarkScenario::IsolatedRepeatInstallHotCacheHotStore => Cleanup::removing(&[]), - BenchmarkScenario::IsolatedRepeatInstallColdCacheHotStore => { - Cleanup::removing(&["cache-dir"]) + Self::IsolatedRepeatInstallHotCacheHotStore + | Self::IsolatedInjectedWorkspaceRepeatInstallHotCacheHotStore => { + Cleanup::removing(&[]) } - BenchmarkScenario::IsolatedFreshAddDepHotCacheHotStore => Cleanup { + Self::IsolatedRepeatInstallColdCacheHotStore => Cleanup::removing(&["cache-dir"]), + Self::IsolatedFreshAddDepHotCacheHotStore => Cleanup { remove: &["node_modules"], restore: &[SAVED_LOCKFILE, SAVED_PACKAGE_JSON], }, - BenchmarkScenario::IsolatedFreshInstallHotCacheHotStore => Cleanup { + Self::IsolatedFreshInstallHotCacheHotStore => Cleanup { remove: &["node_modules", "pnpm-lock.yaml"], restore: &[SAVED_PACKAGE_JSON], }, // Cold cache (wipe `cache-dir` → re-resolve from scratch) but // hot store (keep `store-dir` → no tarball download). Resolution // is the only variable cost, so it can't hide behind downloads. - BenchmarkScenario::IsolatedFreshInstallColdCacheHotStore => Cleanup { + Self::IsolatedFreshInstallColdCacheHotStore => Cleanup { remove: &["node_modules", "pnpm-lock.yaml", "cache-dir"], restore: &[SAVED_PACKAGE_JSON], }, @@ -433,9 +440,9 @@ impl BenchmarkScenario { // date"), and the timed runs would measure a no-op. The warm // `cache-dir` / `store-dir` the pre-warm populated are the // scenario's contract and survive. - BenchmarkScenario::IsolatedFreshResolveHotCacheOffline - | BenchmarkScenario::IsolatedPeerHeavyResolveHotCacheOffline - | BenchmarkScenario::IsolatedLinkedWorkspaceResolveHotCacheOffline => { + Self::IsolatedFreshResolveHotCacheOffline + | Self::IsolatedPeerHeavyResolveHotCacheOffline + | Self::IsolatedLinkedWorkspaceResolveHotCacheOffline => { Cleanup::removing(&["node_modules", "pnpm-lock.yaml"]) } } @@ -448,11 +455,9 @@ impl BenchmarkScenario { matches!(self, BenchmarkScenario::GvsFreshRestoreHotCacheHotStore) } - /// Whether the scenario's contract is a populated, up-to-date - /// `node_modules`. The pre-benchmark wipe empties it, so an untimed - /// install pass per target re-establishes it before hyperfine runs — - /// hyperfine's warmup run would too, but `--warmup 0` must not - /// silently turn the first timed run into a fresh install. + /// Whether to populate `node_modules` with the measured install command + /// before timing, including when `--warmup 0` is requested. Scenarios with + /// [`Self::prewarm_install_args`] use their separate script instead. pub fn prewarms_node_modules(self) -> bool { matches!( self, @@ -501,7 +506,13 @@ impl BenchmarkScenario { /// pass to populate the npm proxy cache, since it names none of the /// packages the static lockfile does. pub fn uses_generated_fixture(self) -> bool { - self.uses_peer_heavy_fixture() || self.uses_linked_workspace_fixture() + self.uses_peer_heavy_fixture() + || self.uses_linked_workspace_fixture() + || self.uses_injected_workspace_fixture() + } + + pub fn uses_injected_workspace_fixture(self) -> bool { + matches!(self, BenchmarkScenario::IsolatedInjectedWorkspaceRepeatInstallHotCacheHotStore) } /// Whether the measured command needs an online pre-warm followed by an diff --git a/pnpm/tasks/integrated-benchmark/src/work_env.rs b/pnpm/tasks/integrated-benchmark/src/work_env.rs index eb4cf8fbdb..1a0fe04c3a 100644 --- a/pnpm/tasks/integrated-benchmark/src/work_env.rs +++ b/pnpm/tasks/integrated-benchmark/src/work_env.rs @@ -10,6 +10,7 @@ mod fixtures; use fixtures::{create_npmrc, create_package_json, create_pnpm_workspace, save_pristine_copies}; +mod injected_workspace; mod linked_workspace; mod server_config; @@ -283,6 +284,9 @@ impl WorkEnv { let registry = self.registry_for(id, direct_registry, revision_mocks); fs::create_dir_all(&dir).expect("create directory for the revision"); create_package_json(&dir, self.options.selection.fixture_dir.as_deref(), scenario); + if scenario.uses_injected_workspace_fixture() { + injected_workspace::create_project(&dir); + } if scenario.uses_linked_workspace_fixture() { linked_workspace::create_projects(&dir); } diff --git a/pnpm/tasks/integrated-benchmark/src/work_env/fixtures.rs b/pnpm/tasks/integrated-benchmark/src/work_env/fixtures.rs index c403f165b0..9d0994d4a6 100644 --- a/pnpm/tasks/integrated-benchmark/src/work_env/fixtures.rs +++ b/pnpm/tasks/integrated-benchmark/src/work_env/fixtures.rs @@ -23,6 +23,11 @@ pub(super) fn create_package_json( fs::write(dst, peer_heavy_root_manifest()).expect("write peer-heavy package.json"); return; } + if scenario.uses_injected_workspace_fixture() { + fs::write(dst, super::injected_workspace::root_manifest()) + .expect("write injected workspace root manifest"); + return; + } if scenario.uses_linked_workspace_fixture() { fs::write(dst, super::linked_workspace::root_manifest()) .expect("write linked-workspace package.json"); @@ -188,7 +193,10 @@ pub(super) fn create_pnpm_workspace( let src_dir = if scenario.uses_generated_fixture() { None } else { src_dir }; let mut manifest = fixture_workspace_manifest(src_dir, &dst) .unwrap_or_else(MinimalWorkspaceManifest::default_for_benchmark); - if scenario.uses_linked_workspace_fixture() { + if scenario.uses_injected_workspace_fixture() { + manifest.dedupe_injected_deps = Some(false); + } + if scenario.uses_linked_workspace_fixture() || scenario.uses_injected_workspace_fixture() { manifest.packages = Some(vec![".".to_string(), "packages/*".to_string()]); } if manifest.store_dir.is_none() { diff --git a/pnpm/tasks/integrated-benchmark/src/work_env/injected_workspace.rs b/pnpm/tasks/integrated-benchmark/src/work_env/injected_workspace.rs new file mode 100644 index 0000000000..42d42f1aa9 --- /dev/null +++ b/pnpm/tasks/integrated-benchmark/src/work_env/injected_workspace.rs @@ -0,0 +1,32 @@ +use std::{fs, path::Path}; + +const PACKAGE: &str = "injected-benchmark-library"; +const FILE_COUNT: usize = 10_000; + +pub(super) fn root_manifest() -> String { + serde_json::json!({ + "name": "injected-benchmark-root", + "private": true, + "dependencies": { PACKAGE: "workspace:*" }, + "dependenciesMeta": { PACKAGE: { "injected": true } }, + }) + .to_string() +} + +pub(super) fn create_project(root: &Path) { + let project = root.join("packages/library"); + let data = project.join("data"); + fs::create_dir_all(&data).expect("create injected workspace package"); + fs::write( + project.join("package.json"), + serde_json::json!({ + "name": PACKAGE, "version": "1.0.0", + }) + .to_string(), + ) + .expect("write injected workspace manifest"); + for index in 0..FILE_COUNT { + fs::write(data.join(format!("file-{index}.txt")), format!("data-{index}\n")) + .expect("write injected workspace file"); + } +} diff --git a/pnpm/tasks/integrated-benchmark/src/work_env/tests.rs b/pnpm/tasks/integrated-benchmark/src/work_env/tests.rs index c2c2d9f709..048418acdf 100644 --- a/pnpm/tasks/integrated-benchmark/src/work_env/tests.rs +++ b/pnpm/tasks/integrated-benchmark/src/work_env/tests.rs @@ -571,3 +571,33 @@ fn client_binary_in_prefers_the_existing_binary() { let _ = fs::remove_dir_all(&root); } + +#[test] +fn injected_repeat_install_creates_its_lockfile_before_the_frozen_runs() { + let scenario = BenchmarkScenario::IsolatedInjectedWorkspaceRepeatInstallHotCacheHotStore; + let dir = std::env::temp_dir().join(format!("pnpm-benchmark-injected-{}", std::process::id())); + fs::create_dir_all(&dir).unwrap(); + create_package_json(&dir, None, scenario); + create_pnpm_workspace(&dir, None, "http://localhost:4873/", scenario); + create_install_script(&dir, scenario, "pnpm", BenchId::PnpmRevision("HEAD")); + let manifest: serde_json::Value = + serde_json::from_slice(&fs::read(dir.join("package.json")).unwrap()).unwrap(); + assert_eq!(manifest["dependencies"]["injected-benchmark-library"], "workspace:*"); + assert_eq!(manifest["dependenciesMeta"]["injected-benchmark-library"]["injected"], true); + let workspace = fs::read_to_string(dir.join("pnpm-workspace.yaml")).unwrap(); + assert!(workspace.contains("dedupeInjectedDeps: false")); + assert!(workspace.contains("packages/*")); + assert!( + fs::read_to_string(dir.join("prewarm.bash")) + .unwrap() + .ends_with("exec pnpm install --offline --no-frozen-lockfile\n"), + ); + assert!( + fs::read_to_string(dir.join("install.bash")).unwrap().contains("install --frozen-lockfile"), + ); + assert!(!scenario.seeds_lockfile()); + assert!(!scenario.prewarms_node_modules()); + assert!(scenario.cleanup().remove.is_empty()); + assert!(scenario.cleanup().restore.is_empty()); + fs::remove_dir_all(dir).unwrap(); +} diff --git a/pnpm/tasks/integrated-benchmark/src/workspace_manifest.rs b/pnpm/tasks/integrated-benchmark/src/workspace_manifest.rs index 1d92555390..cf21cd21a7 100644 --- a/pnpm/tasks/integrated-benchmark/src/workspace_manifest.rs +++ b/pnpm/tasks/integrated-benchmark/src/workspace_manifest.rs @@ -53,6 +53,8 @@ pub struct MinimalWorkspaceManifest { /// source tree into the install's project enumeration. #[serde(skip_serializing_if = "Option::is_none")] pub packages: Option>, + #[serde(skip_serializing_if = "Option::is_none")] + pub dedupe_injected_deps: Option, /// The `enableGlobalVirtualStore` setting. Effective default is /// `false` for non-`--global` installs in both pnpm v11 and pacquet /// (the `true` value is forced only by the `pnpm install --global` diff --git a/pnpm11/fs/indexed-pkg-importer/src/hardlinkedDirMatches.ts b/pnpm11/fs/indexed-pkg-importer/src/hardlinkedDirMatches.ts new file mode 100644 index 0000000000..5cd5de8b1d --- /dev/null +++ b/pnpm11/fs/indexed-pkg-importer/src/hardlinkedDirMatches.ts @@ -0,0 +1,75 @@ +import fs, { type BigIntStats } from 'node:fs' +import path from 'node:path' + +export function hardlinkedDirMatches (dir: string, files: Map, keepModulesDir: boolean): boolean { + if (files.size === 0) return false + const directories = expectedDirectories(files) + if (!directories) return false + try { + const rootStat = fs.lstatSync(dir) + if (!rootStat.isDirectory() || rootStat.isSymbolicLink()) return false + return treeMatches(dir, { files, directories, keepModulesDir }) + } catch { + return false + } +} + +function expectedDirectories (files: Map): Set | undefined { + const directories = new Set() + for (const relative of files.keys()) { + if (!isReusableFilePath(relative)) return undefined + for (let parent = path.posix.dirname(relative); parent !== '.'; parent = path.posix.dirname(parent)) directories.add(parent) + } + return directories +} + +function isReusableFilePath (relative: string): boolean { + if (relative === 'node_modules' || relative.startsWith('node_modules/')) return false + if (path.isAbsolute(relative) || relative.includes('\\')) return false + return relative.split('/').every(part => part !== '' && part !== '.' && part !== '..') +} + +interface ExpectedTree { + files: Map + directories: Set + keepModulesDir: boolean +} + +function treeMatches (root: string, expected: ExpectedTree): boolean { + const pending = [''] + let matched = 0 + for (let relative = pending.pop(); relative !== undefined; relative = pending.pop()) { + const count = matchingFilesInDirectory(root, relative, { expected, pending }) + if (count === undefined) return false + matched += count + } + return matched === expected.files.size +} + +function matchingFilesInDirectory (root: string, relative: string, { expected, pending }: { expected: ExpectedTree, pending: string[] }): number | undefined { + let matched = 0 + for (const entry of fs.readdirSync(path.join(root, relative), { withFileTypes: true })) { + const entryPath = relative ? `${relative}/${entry.name}` : entry.name + const kind = matchingEntryKind(path.join(root, entryPath), entryPath, expected) + if (!kind) return undefined + if (kind === 'directory') pending.push(entryPath) + if (kind === 'file') matched++ + } + return matched +} + +function matchingEntryKind (targetPath: string, relative: string, expected: ExpectedTree): 'file' | 'directory' | 'preserved' | undefined { + const target = fs.lstatSync(targetPath, { bigint: true }) + if (target.isSymbolicLink()) return undefined + if (target.isDirectory()) { + if (expected.keepModulesDir && relative === 'node_modules') return 'preserved' + return expected.directories.has(relative) ? 'directory' : undefined + } + const source = expected.files.get(relative) + return source && filesHaveSameIdentity(target, fs.lstatSync(source, { bigint: true })) ? 'file' : undefined +} + +export function filesHaveSameIdentity (target: BigIntStats, source: BigIntStats): boolean { + return target.isFile() && source.isFile() && target.ino !== 0n && target.dev !== 0n && + target.ino === source.ino && target.dev === source.dev +} diff --git a/pnpm11/fs/indexed-pkg-importer/src/index.ts b/pnpm11/fs/indexed-pkg-importer/src/index.ts index 2604f6ba1f..ae557a762a 100644 --- a/pnpm11/fs/indexed-pkg-importer/src/index.ts +++ b/pnpm11/fs/indexed-pkg-importer/src/index.ts @@ -10,6 +10,7 @@ import type { FilesMap, ImportIndexedPackage, ImportOptions } from '@pnpm/store. import { fastPathTemp as pathTemp } from 'path-temp' import { renameOverwriteSync } from 'rename-overwrite' +import { hardlinkedDirMatches } from './hardlinkedDirMatches.js' import { type Importer, type ImportFile, importIndexedDir } from './importIndexedDir.js' import { isNativeBinary, removeQuarantine } from './removeQuarantine.js' @@ -286,7 +287,11 @@ function hardlinkPkg ( // to bypass the missing-source preservation below. if (missingSourceHasSomethingToPreserve(to, opts)) return undefined if (opts.force || shouldRelinkPkg(to, opts)) { - importIndexedDir({ importFile, importFileAtomic: importFile }, to, opts.filesMap, opts) + const reuseHardlinks = !opts.force && opts.resolvedFrom === 'local-dir' && !opts.symlinks?.size && + hardlinkedDirMatches(to, opts.filesMap, opts.keepModulesDir ?? false) + if (!reuseHardlinks) { + importIndexedDir({ importFile, importFileAtomic: importFile }, to, opts.filesMap, opts) + } removeQuarantineFromNativeBinaries(to, opts) return 'hardlink' } diff --git a/pnpm11/fs/indexed-pkg-importer/src/repairIndexedDir.ts b/pnpm11/fs/indexed-pkg-importer/src/repairIndexedDir.ts index 3f62fc310e..a877ae8665 100644 --- a/pnpm11/fs/indexed-pkg-importer/src/repairIndexedDir.ts +++ b/pnpm11/fs/indexed-pkg-importer/src/repairIndexedDir.ts @@ -7,6 +7,7 @@ import { fastPathTemp as pathTemp } from 'path-temp' import { clearDirBlockingFile, clearDirentBlockingDir, makeFileMapDirs } from './dirUtils.js' import { filesHaveEqualContents } from './filesHaveEqualContents.js' +import { filesHaveSameIdentity } from './hardlinkedDirMatches.js' import type { ImportFile, IndexedDirImport } from './importIndexedDir.js' export function repairIndexedDir ({ importer, newDir, filenames, opts }: IndexedDirImport): void { @@ -109,11 +110,7 @@ export function mismatchReason (target: string, src: string): string | undefined const targetStat = fs.lstatSync(target, { bigint: true }) if (!targetStat.isFile()) return 'is not a regular file' const srcStat = gfs.statSync(src, { bigint: true }) - const hasSameFileIdentity = targetStat.ino !== 0n && - targetStat.dev !== 0n && - targetStat.ino === srcStat.ino && - targetStat.dev === srcStat.dev - if (hasSameFileIdentity) return undefined + if (filesHaveSameIdentity(targetStat, srcStat)) return undefined if (targetStat.size !== srcStat.size) return 'has a different size' if (!filesHaveEqualContents(target, src)) return 'has different content' return undefined diff --git a/pnpm11/fs/indexed-pkg-importer/test/localHardlinks.test.ts b/pnpm11/fs/indexed-pkg-importer/test/localHardlinks.test.ts new file mode 100644 index 0000000000..4de61b7dad --- /dev/null +++ b/pnpm11/fs/indexed-pkg-importer/test/localHardlinks.test.ts @@ -0,0 +1,123 @@ +import fs from 'node:fs' +import path from 'node:path' + +import { expect, test } from '@jest/globals' +import { createIndexedPkgImporter } from '@pnpm/fs.indexed-pkg-importer' +import { tempDir } from '@pnpm/prepare' + +function fixture () { + const root = tempDir() + const source = path.join(root, 'source') + const target = path.join(root, 'target') + fs.mkdirSync(path.join(source, 'lib'), { recursive: true }) + fs.writeFileSync(path.join(source, 'package.json'), '{}') + fs.writeFileSync(path.join(source, 'lib/index.js'), 'original') + const options = { + filesMap: new Map([ + ['package.json', path.join(source, 'package.json')], + ['lib/index.js', path.join(source, 'lib/index.js')], + ]), + force: false, + resolvedFrom: 'local-dir' as const, + keepModulesDir: true, + } + const importer = createIndexedPkgImporter('hardlink') + const install = () => importer(target, options) + install() + return { source, target, options, install } +} + +test('unchanged injected hardlinks preserve the package directory and dependencies', () => { + const { target, install } = fixture() + fs.mkdirSync(path.join(target, 'node_modules/dependency'), { recursive: true }) + fs.writeFileSync(path.join(target, 'node_modules/dependency/index.js'), 'dependency') + const before = fs.statSync(target, { bigint: true }) + expect(install()).toBe('hardlink') + expect(fs.statSync(target, { bigint: true }).ino).toBe(before.ino) + expect(fs.readFileSync(path.join(target, 'node_modules/dependency/index.js'), 'utf8')).toBe('dependency') +}) + +test('injected refresh detects atomic replacements with unchanged size and mtime', () => { + const { source, target, install } = fixture() + const sourceFile = path.join(source, 'lib/index.js') + fs.utimesSync(sourceFile, 1700000000, 1700000000) + const original = fs.statSync(sourceFile, { bigint: true }) + const replacement = path.join(source, 'replacement') + fs.writeFileSync(replacement, 'modified') + fs.utimesSync(replacement, 1700000000, 1700000000) + const replaced = fs.statSync(replacement, { bigint: true }) + expect(replaced.mtimeNs).toBe(original.mtimeNs) + expect(replaced.size).toBe(original.size) + expect(replaced.ino).not.toBe(original.ino) + fs.renameSync(replacement, path.join(source, 'lib/index.js')) + expect(fs.readFileSync(path.join(target, 'lib/index.js'), 'utf8')).toBe('original') + expect(install()).toBe('hardlink') + expect(fs.readFileSync(path.join(target, 'lib/index.js'), 'utf8')).toBe('modified') +}) + +test('injected refresh removes stale entries and restores missing files', () => { + const { source, target, install, options } = fixture() + fs.mkdirSync(path.join(target, 'stale-empty-dir')) + fs.writeFileSync(path.join(target, 'removed.js'), 'removed') + fs.rmSync(path.join(target, 'lib/index.js')) + expect(install()).toBe('hardlink') + expect(fs.existsSync(path.join(target, 'stale-empty-dir'))).toBe(false) + expect(fs.existsSync(path.join(target, 'removed.js'))).toBe(false) + expect(fs.readFileSync(path.join(target, 'lib/index.js'), 'utf8')).toBe('original') + fs.writeFileSync(path.join(source, 'added.js'), 'added') + options.filesMap.set('added.js', path.join(source, 'added.js')) + install() + expect(fs.readFileSync(path.join(target, 'added.js'), 'utf8')).toBe('added') + options.filesMap.delete('lib/index.js') + install() + expect(fs.existsSync(path.join(target, 'lib'))).toBe(false) +}) + +test('same-byte copies and explicit force still recreate injected hardlinks', () => { + const { source, target, install, options } = fixture() + fs.rmSync(path.join(target, 'lib/index.js')) + fs.copyFileSync(path.join(source, 'lib/index.js'), path.join(target, 'lib/index.js')) + expect(install()).toBe('hardlink') + expect(fs.statSync(path.join(target, 'lib/index.js'), { bigint: true }).ino) + .toBe(fs.statSync(path.join(source, 'lib/index.js'), { bigint: true }).ino) + const originalDirectory = fs.statSync(target, { bigint: true }).ino + options.force = true + expect(install()).toBe('hardlink') + expect(fs.statSync(target, { bigint: true }).ino).not.toBe(originalDirectory) + const copy = createIndexedPkgImporter('copy') + expect(copy(target, { ...options, force: false })).toBe('copy') + expect(fs.statSync(path.join(target, 'lib/index.js'), { bigint: true }).ino) + .not.toBe(fs.statSync(path.join(source, 'lib/index.js'), { bigint: true }).ino) +}) + +test('injected refresh replaces directory symlinks without modifying their targets', () => { + const { source, target, install } = fixture() + fs.rmSync(path.join(target, 'lib'), { recursive: true }) + fs.symlinkSync(path.join(source, 'lib'), path.join(target, 'lib'), 'junction') + expect(install()).toBe('hardlink') + expect(fs.lstatSync(path.join(target, 'lib')).isSymbolicLink()).toBe(false) + expect(fs.readFileSync(path.join(source, 'lib/index.js'), 'utf8')).toBe('original') +}) + +test('injected refresh replaces a symlink at the package root', () => { + const { source, target, install } = fixture() + fs.rmSync(target, { recursive: true }) + fs.symlinkSync(source, target, 'junction') + expect(install()).toBe('hardlink') + expect(fs.lstatSync(target).isSymbolicLink()).toBe(false) + expect(fs.readFileSync(path.join(source, 'package.json'), 'utf8')).toBe('{}') +}) + +test.each([false, true])('bundled dependencies prevent injected hardlink reuse with keepModulesDir=%s', (keepModulesDir) => { + const { source, target, install, options } = fixture() + options.keepModulesDir = keepModulesDir + const bundled = path.join(source, 'node_modules/bundled/index.js') + fs.mkdirSync(path.dirname(bundled), { recursive: true }) + fs.writeFileSync(bundled, 'bundled') + options.filesMap.set('node_modules/bundled/index.js', bundled) + expect(install()).toBe('hardlink') + const originalDirectory = fs.statSync(target, { bigint: true }).ino + expect(install()).toBe('hardlink') + expect(fs.statSync(target, { bigint: true }).ino).not.toBe(originalDirectory) + expect(fs.readFileSync(path.join(target, 'node_modules/bundled/index.js'), 'utf8')).toBe('bundled') +}) diff --git a/pnpm11/installing/deps-restorer/src/linkAllPkgs.ts b/pnpm11/installing/deps-restorer/src/linkAllPkgs.ts index 6e4e5433de..2004d6c8e0 100644 --- a/pnpm11/installing/deps-restorer/src/linkAllPkgs.ts +++ b/pnpm11/installing/deps-restorer/src/linkAllPkgs.ts @@ -206,7 +206,7 @@ async function importIntoSlot ( } return await storeController.importPackage(depNode.dir, { filesResponse, - force: depNode.forceImportPackage ?? opts.force, + force: depNode.forceImportPackage || opts.force, disableRelinkLocalDirDeps: opts.disableRelinkLocalDirDeps, requiresBuild: depNode.patch != null || depNode.requiresBuild, safeToSkip: opts.enableGlobalVirtualStore, diff --git a/pnpm11/pnpm/test/injectedHardlinks.ts b/pnpm11/pnpm/test/injectedHardlinks.ts new file mode 100644 index 0000000000..aabe057495 --- /dev/null +++ b/pnpm11/pnpm/test/injectedHardlinks.ts @@ -0,0 +1,36 @@ +import fs from 'node:fs' + +import { expect, test } from '@jest/globals' +import { preparePackages } from '@pnpm/prepare' +import { writeYamlFileSync } from 'write-yaml-file' + +import { execPnpm } from './utils/index.js' + +test('frozen injected installs reuse hardlinks unless force is requested', async () => { + preparePackages([ + { + location: '.', + package: { + name: 'root', + private: true, + dependencies: { library: 'workspace:*' }, + }, + }, + { name: 'library', version: '1.0.0' }, + ]) + writeYamlFileSync('pnpm-workspace.yaml', { + packages: ['library'], + injectWorkspacePackages: true, + dedupeInjectedDeps: false, + enableGlobalVirtualStore: false, + }) + fs.writeFileSync('library/index.js', 'source-content') + await execPnpm(['install', '--offline', '--no-frozen-lockfile']) + const target = fs.realpathSync('node_modules/library') + const original = fs.statSync(target, { bigint: true }).ino + await execPnpm(['install', '--offline', '--frozen-lockfile']) + expect(fs.statSync(target, { bigint: true }).ino).toBe(original) + await execPnpm(['install', '--offline', '--frozen-lockfile', '--force']) + expect(fs.statSync(target, { bigint: true }).ino).not.toBe(original) + expect(fs.readFileSync(`${target}/index.js`, 'utf8')).toBe('source-content') +})