Skip to content

Less code in the repo#442

Merged
zkochan merged 12 commits into
masterfrom
less-code
Oct 29, 2016
Merged

Less code in the repo#442
zkochan merged 12 commits into
masterfrom
less-code

Conversation

@zkochan

@zkochan zkochan commented Oct 21, 2016

Copy link
Copy Markdown
Member

Ref #441

@zkochan zkochan changed the title refactor: use a package for shimming commands Less code in the repo Oct 22, 2016
@zkochan zkochan force-pushed the less-code branch 3 times, most recently from bd9fb59 to 3e3fe5d Compare October 22, 2016 20:06
@zkochan zkochan force-pushed the less-code branch 2 times, most recently from b96fe46 to fc5139f Compare October 23, 2016 15:33
@iamstarkov

Copy link
Copy Markdown
Contributor

not ready for CR?

@zkochan

zkochan commented Oct 24, 2016

Copy link
Copy Markdown
Member Author

I think ready for now.

I will further optimize it though. I want to make the code in installMultiple.ts and install/index.ts more straightforward. I don't know yet how. Maybe rxjs would help but I don't have experience with it.

Also I wonder whether it will be more contributor friendly with reactive programming. People are probably more used to promises... so maybe we have to use some of Sindre Sorhus's new promise libraries

@zkochan

zkochan commented Oct 24, 2016

Copy link
Copy Markdown
Member Author

The most complex part to solve is in installMultiple currently. It has different conditions when to install subdeps of a package and when not to, and everything is async.

@zkochan zkochan force-pushed the less-code branch 2 times, most recently from fb6bdca to 0edeb2b Compare October 25, 2016 21:13
@zkochan zkochan merged commit 720a8d5 into master Oct 29, 2016
@zkochan zkochan deleted the less-code branch October 30, 2016 18:28
pull Bot pushed a commit to dwongdev/pnpm that referenced this pull request May 14, 2026
## Summary

Closes pnpm#433. Implements partial install with `--frozen-lockfile`:

- **Read** `<virtual_store_dir>/lock.yaml` at install start (`Lockfile::load_current_from_virtual_store_dir`), mirroring upstream pnpm v11's `readCurrentLockfile`.
- **Skip** any snapshot whose `dependencies`, `optionalDependencies`, and `resolution.integrity` match the current lockfile **and** whose virtual-store slot exists on disk. No fetch, no extract, no relink for skipped snapshots.
- **Write** `<virtual_store_dir>/lock.yaml` at end-of-install (atomically; deletes the file when the lockfile is empty), so the next install sees a fresh diff target.
- **Emit** `pnpm:_broken_node_modules` (`BrokenModulesLog`) when the slot is gone despite the cache key matching, then fall through to a full install for that snapshot.
- `pnpm:context.currentLockfileExists` now reflects reality (was hard-coded `false`); `pnpm:stats.added` reports the post-skip delta instead of the total snapshot count, matching upstream.
- **Side-effects-cache prefetch still covers skipped snapshots** so `BuildModules`'s `is_built` gate fires on warm reinstalls — without this, packages with `allowBuilds: true` re-execute their lifecycle scripts every install (added in 1603f94f after benchmarking caught the regression — see [benchmark comment](pnpm/pacquet#442 (comment))).

Tracks pnpm v11 `94240bc046` for `readCurrentLockfile` / `writeCurrentLockfile` / `lockfileToDepGraph`'s per-`depPath` skip and `isIntegrityEqual` helper.

## Performance

Two scenarios benchmarked against a 1352-package fixture, hyperfine `--warmup 2 --runs 10`.

**Pure warm reinstall** (every snapshot intact, full `lock.yaml` on disk):

| | mean | sys time |
|---|---:|---:|
| baseline (`main` HEAD~2) | 969 ms | 985 ms |
| **PR** | **896 ms** | **349 ms** |

PR is 1.08× faster, with ~65% less kernel work — the skip filter bypasses the warm-batch's per-snapshot syscalls (1352 → 0). [Full breakdown + the regression we caught + fixed.](pnpm/pacquet#442 (comment))

**Non-up-to-date `node_modules`** (N virtual-store slots deleted per iteration):

| Damaged slots | PR | Baseline | PR/Baseline |
|---:|---:|---:|---:|
| 0 | 581 ms | 661 ms | **1.14×** |
| 1 | 615 ms | 707 ms | **1.15×** |
| 10 | 660 ms | 707 ms | 1.07× |
| 100 | 2.25 s | 2.25 s | 1.00× |
| 500 | 6.13 s | 7.93 s | **1.29×** |

PR is consistently a small win and never a loss across the damage spectrum; `_broken_node_modules` debug events fire correctly for each missing slot. [Full sweep + caveats.](pnpm/pacquet#442 (comment))
pull Bot pushed a commit to dwongdev/pnpm that referenced this pull request May 14, 2026
…incompatibles (pnpm#434 slice 1) (pnpm#439)

Slice 1 of pnpm#434 — foundational installability gate. Ports
[`@pnpm/config.package-is-installable`](https://github.com/pnpm/pnpm/tree/94240bc046/config/package-is-installable)
and threads the resulting skip-set through every phase of the
frozen-lockfile install pipeline.

Closes #266 once shipped (covers the "install respects every snapshot
— no os/cpu/libc filter" gap). Does **not** close pnpm#434 — that umbrella
has six more slices to follow.

Upstream reference: pnpm/pnpm@94240bc046.

## What landed

### New crate: `pacquet-package-is-installable`

Ports the upstream `config/package-is-installable` package's three
helpers:

- `check_platform` (`Option<&[String]>` for each `os`/`cpu`/`libc`
  axis, plus a `SupportedArchitectures` override) — returns
  `Option<UnsupportedPlatformError>` matching upstream's
  `ERR_PNPM_UNSUPPORTED_PLATFORM` code, message shape, and JSON
  payload. Handles negation entries (`!foo`), the `any` sentinel,
  the `current` placeholder, and the `currentLibc !== 'unknown'`
  skip from `checkPlatform.ts:38`.
- `check_engine` — evaluates `engines.node` / `engines.pnpm` via
  `node-semver`. Approximates npm-semver's `includePrerelease: true`
  via a strip-prerelease fallback; one over-acceptance edge case
  (`>=X.Y.Z` against `X.Y.Z-rc1`) is pinned in the known-failures
  integration test for follow-up.
- `package_is_installable` — composes the two, returns the
  tri-state verdict matching upstream's `boolean | null` (Installable
  / SkipOptional / ProceedWithWarning), plus an `Err` arm for
  `engine_strict` aborts and `ERR_PNPM_INVALID_NODE_VERSION`.

`InstallabilityOptions<'a>` borrows its host strings so a caller
running through many snapshots in a row can build the host part
once and only toggle `optional` per snapshot. `WantedPlatformRef<'a>`
plays the same role for the manifest axes so `check_platform` runs
the happy path without any allocation.

### New module: `pacquet_package_manager::installability`

`compute_skipped_snapshots` is the per-install entry point. For each
snapshot:

1. Look up the matching `PackageMetadata`.
2. Run `check_package` (cached per peer-stripped `metadata_key` so
   peer-resolved variants of the same package share one verdict).
3. Dispatch on `(verdict, snapshot.optional, host.engine_strict)`:
   - `Installable`: nothing to do.
   - `SkipOptional` + `optional`: add to `SkippedSnapshots`, emit
     `pnpm:skipped-optional-dependency` (deduped per metadata key,
     matching upstream's emit-per-pkgId).
   - `Incompatible` + non-optional + `engine_strict`: abort.
   - `Incompatible` + non-optional + non-strict: `tracing::warn!`
     and proceed. (Upstream's `pnpm:install-check` channel isn't
     wired into pacquet's reporter yet — slice 1 follow-up.)

`any_installability_constraint(packages)` is the caller-side fast
path: if no metadata row declares an `engines.{node,pnpm}` or a
non-empty / non-`["any"]` `cpu`/`os`/`libc`, the entire installability
pass is skipped. The probe runs synchronously in
`install_frozen_lockfile` *before* the `tokio::task::spawn_blocking`
that would invoke `node --version` — so the common no-constraints
lockfile pays nothing for the new pipeline, restoring main's overlap
of node-binary startup with extraction.

### Install-pipeline plumbing

The `SkippedSnapshots` set is threaded into every downstream phase
of `InstallFrozenLockfile::run`:

- `CreateVirtualStore`: installability-skipped snapshots are
  dropped from both `survivors` (no virtual-store slot extracted)
  and `skipped_entries` (no warm-cache row). Layered ahead of
  main's pnpm#442 already-installed-and-on-disk skip filter.
- `SymlinkDirectDependencies`: a direct dep whose resolved
  snapshot is in the skip set is omitted from `node_modules/<name>`
  (no symlink, no `pnpm:root added` event, no bin link).
- `LinkVirtualStoreBins`: per-slot bin link skips slots whose
  snapshot is installability-skipped (their virtual-store
  directories don't exist).
- `BuildModules` via `build_sequence`: `get_subgraph_to_build`
  consults `skipped` *before* recursion, so a skipped snapshot's
  subtree doesn't contribute to the build graph via that edge.
  Descendants reachable from a non-skipped root still build
  normally.

### Performance

CI integrated-benchmark on the 1352-package fixture, latest run:

| Scenario | `pacquet@HEAD` | `pacquet@main` | Relative |
|---|---|---|---|
| Frozen Lockfile (cold) | 2.476 ± 0.083 s | 2.442 ± 0.071 s | 1.01 ± 0.05 |
| Frozen Lockfile (Hot Cache) | 685.8 ± 59.3 ms | 700.2 ± 47.4 ms | 1.00 |

Earlier iterations of this PR showed a ~5% cold-install regression
from the `node --version` spawn landing on the extraction critical
path. Closed by hoisting the no-constraints fast-path probe to the
caller (commit `cf47ce51`) so the spawn is gated on actual
constraint presence.

Other perf passes folded in:

- `compute_skipped_snapshots` caches the per-metadata-row check
  verdict so peer-resolved variants share one `check_package` call.
- `check_platform` borrows its three wanted axes through
  `WantedPlatformRef<'a>`; the owned `WantedPlatform` only
  materialises in the error path.

## Tests

| Suite | Count | What it covers |
|---|---|---|
| `pacquet-package-is-installable::tests::check_platform` | 16 | Port of upstream `checkPlatform.ts` — `any`/`current` sentinels, negation, `supportedArchitectures` override, libc unknown-skip |
| `pacquet-package-is-installable::tests::check_engine` | 7 | Port of upstream `checkEngine.ts` — node/pnpm range checks, prerelease cases, `ERR_PNPM_INVALID_NODE_VERSION` |
| `pacquet-package-is-installable::tests::package_is_installable` | 6 | Tri-state verdict + optional/engine-strict dispatch |
| `pacquet-package-is-installable::tests::known_failures` | 1 | The `>=X.Y.Z` vs `X.Y.Z-rc1` over-acceptance, picked up by `just known-failures` |
| `pacquet_package_manager::installability::tests` | 11 | Per-install skip-set computation: skip on bad OS, skip on bad node engine, dedup events across peer variants, fast-path triggers, constraint predicate's edge cases (`engines.npm` only, `["any"]` sentinel, empty lists) |
| `pacquet_package_manager::build_sequence::tests` | 3 (new) | Skipped+patched doesn't enter build queue; skipped parent doesn't drag descendants in; descendant with non-skipped parent still builds |

All ported tests verified to catch regressions by temporarily
breaking the subject under test, observing the failure, then
reverting. The "test the tests" workflow from CLAUDE.md.

## Deferred to follow-up slices

- `.modules.yaml.skipped` write/read + headless re-check
  (slice 3).
- `supportedArchitectures` config + `--cpu` / `--os` / `--libc`
  CLI flags (slice 2).
- `pnpm:install-check` warn channel on the reporter side
  (currently `tracing::warn!`).
- Real libc detection — `host_libc()` returns `"unknown"` today;
  matches non-Linux host behavior, but on Alpine/musl this
  over-installs glibc-only optional packages. Slice 2.
- `engine_strict` config wiring — defaults to false today, so
  the error path is unreachable from production. Wired through
  end-to-end so the slice that flips the config doesn't churn
  the error enum.
pull Bot pushed a commit to dwongdev/pnpm that referenced this pull request May 14, 2026
## Summary

Threads `VirtualStoreLayout` (from pnpm#444) through the frozen-lockfile install pipeline so packages installed with `enableGlobalVirtualStore: true` (pacquet's default since pnpm#444) land at the upstream-shaped `<store_dir>/links/<scope>/<name>/<version>/<hash>/node_modules/<pkg>` path. Closes the rest of pnpm#432 (Section B + C activation + D entry tests).

## Pipeline changes (frozen-lockfile path)

- `InstallFrozenLockfile::run` constructs the install-scoped `VirtualStoreLayout` from the lockfile + engine name + config, then threads `&VirtualStoreLayout` through `CreateVirtualStore`, `CreateVirtualDirBySnapshot`, `create_symlink_layout`, `SymlinkDirectDependencies`, `InstallPackageBySnapshot`, `BuildModules`, and `LinkVirtualStoreBins`. Every site that used to compute `virtual_store_dir.join(key.to_virtual_store_name())` now goes through one `layout.slot_dir(key)` lookup.
- `Install::run` calls `pacquet_store_dir::register_project` **once per importer** when `enable_global_virtual_store` is on, writing `<store_dir>/projects/<short-hash>` per workspace package so a future `pacquet store prune` can find every reachable consumer of `<store_dir>/links/...`. Best-effort: registry write failures degrade to `tracing::warn!` and the install continues. The walk runs *after* `InstallFrozenLockfile::run` succeeds so importer keys have already been validated.
- `Config::current` now applies `apply_global_virtual_store_derivation` after yaml — pacquet's variant of upstream's [`extendInstallOptions.ts:343-355`](https://github.com/pnpm/pnpm/blob/94240bc046/installing/deps-installer/src/install/extendInstallOptions.ts#L343-L355). An explicit `globalVirtualStoreDir:` in yaml wins over the derivation; otherwise the field falls back to the user's pinned `virtualStoreDir` (under GVS-on) or to `<store_dir>/links`.

## Field-split deviation from upstream

Upstream pnpm *mutates* `virtualStoreDir` in place under GVS, so every consumer that reads `virtualStoreDir` sees `<storeDir>/links`. Pacquet keeps `virtual_store_dir` at its project-local value and writes the GVS path into the separate `global_virtual_store_dir` field. The frozen-lockfile path consumes `global_virtual_store_dir` through `VirtualStoreLayout`; the legacy non-frozen `InstallWithoutLockfile` keeps reading `virtual_store_dir` (now via `VirtualStoreLayout::legacy`). The split is a pacquet-only concern: upstream has no without-lockfile install path and so doesn't need the second field. See the doc on `Config::apply_global_virtual_store_derivation` for the reasoning.

## Benchmarks

The integrated-benchmark fixture pins `enableGlobalVirtualStore: false` so existing scenarios continue to compare apples-to-apples against the project-local `<modules>/.pnpm/<flat-name>` shape. Without the pin, every revision newer than this PR would silently switch to the GVS layout, while `pacquet@main` and the pnpm side of the comparison would still use the isolated layout — different shape, unfair comparison. GVS-specific benchmark runs are available by passing `--fixture-dir <dir>` at a copy of the fixture with the flag flipped.

## Tests

- `install::tests::frozen_lockfile_under_gvs_registers_project_and_runs_clean` pins the registry write under default GVS-on (single-project case).
- `install::tests::frozen_lockfile_under_gvs_registers_each_workspace_importer` pins per-importer registration: workspace root + `packages/web` each end up with their own symlink in `<store_dir>/projects/`.
- `install::tests::frozen_lockfile_with_gvs_off_skips_project_registry` pins that GVS-off opts out of the registry write.
- `config::tests::yaml_global_virtual_store_dir_wins_over_derivation` pins the yaml-override precedence.
- All **829 workspace tests** pass under the new layout-threaded path.
- Existing per-snapshot legacy tests construct `VirtualStoreLayout::legacy(...)` explicitly so they keep asserting the flat-name layout regardless of any global default. Partial-install tests from pnpm#442 pin `enable_global_virtual_store = false` so their seeded legacy-shape slots match the probe path.

End-to-end port of upstream's [`installing/deps-installer/test/install/globalVirtualStore.ts`](https://github.com/pnpm/pnpm/blob/94240bc046/installing/deps-installer/test/install/globalVirtualStore.ts) is tracked as a follow-up — those tests need a small frozen-lockfile fixture against the registry-mock that doesn't exist yet.
github-actions Bot pushed a commit to Eyalm321/pnpm that referenced this pull request May 18, 2026
## Summary

Closes pnpm#433. Implements partial install with `--frozen-lockfile`:

- **Read** `<virtual_store_dir>/lock.yaml` at install start (`Lockfile::load_current_from_virtual_store_dir`), mirroring upstream pnpm v11's `readCurrentLockfile`.
- **Skip** any snapshot whose `dependencies`, `optionalDependencies`, and `resolution.integrity` match the current lockfile **and** whose virtual-store slot exists on disk. No fetch, no extract, no relink for skipped snapshots.
- **Write** `<virtual_store_dir>/lock.yaml` at end-of-install (atomically; deletes the file when the lockfile is empty), so the next install sees a fresh diff target.
- **Emit** `pnpm:_broken_node_modules` (`BrokenModulesLog`) when the slot is gone despite the cache key matching, then fall through to a full install for that snapshot.
- `pnpm:context.currentLockfileExists` now reflects reality (was hard-coded `false`); `pnpm:stats.added` reports the post-skip delta instead of the total snapshot count, matching upstream.
- **Side-effects-cache prefetch still covers skipped snapshots** so `BuildModules`'s `is_built` gate fires on warm reinstalls — without this, packages with `allowBuilds: true` re-execute their lifecycle scripts every install (added in 1603f94f after benchmarking caught the regression — see [benchmark comment](pnpm/pacquet#442 (comment))).

Tracks pnpm v11 `94240bc046` for `readCurrentLockfile` / `writeCurrentLockfile` / `lockfileToDepGraph`'s per-`depPath` skip and `isIntegrityEqual` helper.

## Performance

Two scenarios benchmarked against a 1352-package fixture, hyperfine `--warmup 2 --runs 10`.

**Pure warm reinstall** (every snapshot intact, full `lock.yaml` on disk):

| | mean | sys time |
|---|---:|---:|
| baseline (`main` HEAD~2) | 969 ms | 985 ms |
| **PR** | **896 ms** | **349 ms** |

PR is 1.08× faster, with ~65% less kernel work — the skip filter bypasses the warm-batch's per-snapshot syscalls (1352 → 0). [Full breakdown + the regression we caught + fixed.](pnpm/pacquet#442 (comment))

**Non-up-to-date `node_modules`** (N virtual-store slots deleted per iteration):

| Damaged slots | PR | Baseline | PR/Baseline |
|---:|---:|---:|---:|
| 0 | 581 ms | 661 ms | **1.14×** |
| 1 | 615 ms | 707 ms | **1.15×** |
| 10 | 660 ms | 707 ms | 1.07× |
| 100 | 2.25 s | 2.25 s | 1.00× |
| 500 | 6.13 s | 7.93 s | **1.29×** |

PR is consistently a small win and never a loss across the damage spectrum; `_broken_node_modules` debug events fire correctly for each missing slot. [Full sweep + caveats.](pnpm/pacquet#442 (comment))
github-actions Bot pushed a commit to Eyalm321/pnpm that referenced this pull request May 18, 2026
…incompatibles (pnpm#434 slice 1) (pnpm#439)

Slice 1 of pnpm#434 — foundational installability gate. Ports
[`@pnpm/config.package-is-installable`](https://github.com/pnpm/pnpm/tree/94240bc046/config/package-is-installable)
and threads the resulting skip-set through every phase of the
frozen-lockfile install pipeline.

Closes pnpm#266 once shipped (covers the "install respects every snapshot
— no os/cpu/libc filter" gap). Does **not** close pnpm#434 — that umbrella
has six more slices to follow.

Upstream reference: pnpm/pnpm@94240bc046.

## What landed

### New crate: `pacquet-package-is-installable`

Ports the upstream `config/package-is-installable` package's three
helpers:

- `check_platform` (`Option<&[String]>` for each `os`/`cpu`/`libc`
  axis, plus a `SupportedArchitectures` override) — returns
  `Option<UnsupportedPlatformError>` matching upstream's
  `ERR_PNPM_UNSUPPORTED_PLATFORM` code, message shape, and JSON
  payload. Handles negation entries (`!foo`), the `any` sentinel,
  the `current` placeholder, and the `currentLibc !== 'unknown'`
  skip from `checkPlatform.ts:38`.
- `check_engine` — evaluates `engines.node` / `engines.pnpm` via
  `node-semver`. Approximates npm-semver's `includePrerelease: true`
  via a strip-prerelease fallback; one over-acceptance edge case
  (`>=X.Y.Z` against `X.Y.Z-rc1`) is pinned in the known-failures
  integration test for follow-up.
- `package_is_installable` — composes the two, returns the
  tri-state verdict matching upstream's `boolean | null` (Installable
  / SkipOptional / ProceedWithWarning), plus an `Err` arm for
  `engine_strict` aborts and `ERR_PNPM_INVALID_NODE_VERSION`.

`InstallabilityOptions<'a>` borrows its host strings so a caller
running through many snapshots in a row can build the host part
once and only toggle `optional` per snapshot. `WantedPlatformRef<'a>`
plays the same role for the manifest axes so `check_platform` runs
the happy path without any allocation.

### New module: `pacquet_package_manager::installability`

`compute_skipped_snapshots` is the per-install entry point. For each
snapshot:

1. Look up the matching `PackageMetadata`.
2. Run `check_package` (cached per peer-stripped `metadata_key` so
   peer-resolved variants of the same package share one verdict).
3. Dispatch on `(verdict, snapshot.optional, host.engine_strict)`:
   - `Installable`: nothing to do.
   - `SkipOptional` + `optional`: add to `SkippedSnapshots`, emit
     `pnpm:skipped-optional-dependency` (deduped per metadata key,
     matching upstream's emit-per-pkgId).
   - `Incompatible` + non-optional + `engine_strict`: abort.
   - `Incompatible` + non-optional + non-strict: `tracing::warn!`
     and proceed. (Upstream's `pnpm:install-check` channel isn't
     wired into pacquet's reporter yet — slice 1 follow-up.)

`any_installability_constraint(packages)` is the caller-side fast
path: if no metadata row declares an `engines.{node,pnpm}` or a
non-empty / non-`["any"]` `cpu`/`os`/`libc`, the entire installability
pass is skipped. The probe runs synchronously in
`install_frozen_lockfile` *before* the `tokio::task::spawn_blocking`
that would invoke `node --version` — so the common no-constraints
lockfile pays nothing for the new pipeline, restoring main's overlap
of node-binary startup with extraction.

### Install-pipeline plumbing

The `SkippedSnapshots` set is threaded into every downstream phase
of `InstallFrozenLockfile::run`:

- `CreateVirtualStore`: installability-skipped snapshots are
  dropped from both `survivors` (no virtual-store slot extracted)
  and `skipped_entries` (no warm-cache row). Layered ahead of
  main's pnpm#442 already-installed-and-on-disk skip filter.
- `SymlinkDirectDependencies`: a direct dep whose resolved
  snapshot is in the skip set is omitted from `node_modules/<name>`
  (no symlink, no `pnpm:root added` event, no bin link).
- `LinkVirtualStoreBins`: per-slot bin link skips slots whose
  snapshot is installability-skipped (their virtual-store
  directories don't exist).
- `BuildModules` via `build_sequence`: `get_subgraph_to_build`
  consults `skipped` *before* recursion, so a skipped snapshot's
  subtree doesn't contribute to the build graph via that edge.
  Descendants reachable from a non-skipped root still build
  normally.

### Performance

CI integrated-benchmark on the 1352-package fixture, latest run:

| Scenario | `pacquet@HEAD` | `pacquet@main` | Relative |
|---|---|---|---|
| Frozen Lockfile (cold) | 2.476 ± 0.083 s | 2.442 ± 0.071 s | 1.01 ± 0.05 |
| Frozen Lockfile (Hot Cache) | 685.8 ± 59.3 ms | 700.2 ± 47.4 ms | 1.00 |

Earlier iterations of this PR showed a ~5% cold-install regression
from the `node --version` spawn landing on the extraction critical
path. Closed by hoisting the no-constraints fast-path probe to the
caller (commit `cf47ce51`) so the spawn is gated on actual
constraint presence.

Other perf passes folded in:

- `compute_skipped_snapshots` caches the per-metadata-row check
  verdict so peer-resolved variants share one `check_package` call.
- `check_platform` borrows its three wanted axes through
  `WantedPlatformRef<'a>`; the owned `WantedPlatform` only
  materialises in the error path.

## Tests

| Suite | Count | What it covers |
|---|---|---|
| `pacquet-package-is-installable::tests::check_platform` | 16 | Port of upstream `checkPlatform.ts` — `any`/`current` sentinels, negation, `supportedArchitectures` override, libc unknown-skip |
| `pacquet-package-is-installable::tests::check_engine` | 7 | Port of upstream `checkEngine.ts` — node/pnpm range checks, prerelease cases, `ERR_PNPM_INVALID_NODE_VERSION` |
| `pacquet-package-is-installable::tests::package_is_installable` | 6 | Tri-state verdict + optional/engine-strict dispatch |
| `pacquet-package-is-installable::tests::known_failures` | 1 | The `>=X.Y.Z` vs `X.Y.Z-rc1` over-acceptance, picked up by `just known-failures` |
| `pacquet_package_manager::installability::tests` | 11 | Per-install skip-set computation: skip on bad OS, skip on bad node engine, dedup events across peer variants, fast-path triggers, constraint predicate's edge cases (`engines.npm` only, `["any"]` sentinel, empty lists) |
| `pacquet_package_manager::build_sequence::tests` | 3 (new) | Skipped+patched doesn't enter build queue; skipped parent doesn't drag descendants in; descendant with non-skipped parent still builds |

All ported tests verified to catch regressions by temporarily
breaking the subject under test, observing the failure, then
reverting. The "test the tests" workflow from CLAUDE.md.

## Deferred to follow-up slices

- `.modules.yaml.skipped` write/read + headless re-check
  (slice 3).
- `supportedArchitectures` config + `--cpu` / `--os` / `--libc`
  CLI flags (slice 2).
- `pnpm:install-check` warn channel on the reporter side
  (currently `tracing::warn!`).
- Real libc detection — `host_libc()` returns `"unknown"` today;
  matches non-Linux host behavior, but on Alpine/musl this
  over-installs glibc-only optional packages. Slice 2.
- `engine_strict` config wiring — defaults to false today, so
  the error path is unreachable from production. Wired through
  end-to-end so the slice that flips the config doesn't churn
  the error enum.
github-actions Bot pushed a commit to Eyalm321/pnpm that referenced this pull request May 18, 2026
## Summary

Threads `VirtualStoreLayout` (from pnpm#444) through the frozen-lockfile install pipeline so packages installed with `enableGlobalVirtualStore: true` (pacquet's default since pnpm#444) land at the upstream-shaped `<store_dir>/links/<scope>/<name>/<version>/<hash>/node_modules/<pkg>` path. Closes the rest of pnpm#432 (Section B + C activation + D entry tests).

## Pipeline changes (frozen-lockfile path)

- `InstallFrozenLockfile::run` constructs the install-scoped `VirtualStoreLayout` from the lockfile + engine name + config, then threads `&VirtualStoreLayout` through `CreateVirtualStore`, `CreateVirtualDirBySnapshot`, `create_symlink_layout`, `SymlinkDirectDependencies`, `InstallPackageBySnapshot`, `BuildModules`, and `LinkVirtualStoreBins`. Every site that used to compute `virtual_store_dir.join(key.to_virtual_store_name())` now goes through one `layout.slot_dir(key)` lookup.
- `Install::run` calls `pacquet_store_dir::register_project` **once per importer** when `enable_global_virtual_store` is on, writing `<store_dir>/projects/<short-hash>` per workspace package so a future `pacquet store prune` can find every reachable consumer of `<store_dir>/links/...`. Best-effort: registry write failures degrade to `tracing::warn!` and the install continues. The walk runs *after* `InstallFrozenLockfile::run` succeeds so importer keys have already been validated.
- `Config::current` now applies `apply_global_virtual_store_derivation` after yaml — pacquet's variant of upstream's [`extendInstallOptions.ts:343-355`](https://github.com/pnpm/pnpm/blob/94240bc046/installing/deps-installer/src/install/extendInstallOptions.ts#L343-L355). An explicit `globalVirtualStoreDir:` in yaml wins over the derivation; otherwise the field falls back to the user's pinned `virtualStoreDir` (under GVS-on) or to `<store_dir>/links`.

## Field-split deviation from upstream

Upstream pnpm *mutates* `virtualStoreDir` in place under GVS, so every consumer that reads `virtualStoreDir` sees `<storeDir>/links`. Pacquet keeps `virtual_store_dir` at its project-local value and writes the GVS path into the separate `global_virtual_store_dir` field. The frozen-lockfile path consumes `global_virtual_store_dir` through `VirtualStoreLayout`; the legacy non-frozen `InstallWithoutLockfile` keeps reading `virtual_store_dir` (now via `VirtualStoreLayout::legacy`). The split is a pacquet-only concern: upstream has no without-lockfile install path and so doesn't need the second field. See the doc on `Config::apply_global_virtual_store_derivation` for the reasoning.

## Benchmarks

The integrated-benchmark fixture pins `enableGlobalVirtualStore: false` so existing scenarios continue to compare apples-to-apples against the project-local `<modules>/.pnpm/<flat-name>` shape. Without the pin, every revision newer than this PR would silently switch to the GVS layout, while `pacquet@main` and the pnpm side of the comparison would still use the isolated layout — different shape, unfair comparison. GVS-specific benchmark runs are available by passing `--fixture-dir <dir>` at a copy of the fixture with the flag flipped.

## Tests

- `install::tests::frozen_lockfile_under_gvs_registers_project_and_runs_clean` pins the registry write under default GVS-on (single-project case).
- `install::tests::frozen_lockfile_under_gvs_registers_each_workspace_importer` pins per-importer registration: workspace root + `packages/web` each end up with their own symlink in `<store_dir>/projects/`.
- `install::tests::frozen_lockfile_with_gvs_off_skips_project_registry` pins that GVS-off opts out of the registry write.
- `config::tests::yaml_global_virtual_store_dir_wins_over_derivation` pins the yaml-override precedence.
- All **829 workspace tests** pass under the new layout-threaded path.
- Existing per-snapshot legacy tests construct `VirtualStoreLayout::legacy(...)` explicitly so they keep asserting the flat-name layout regardless of any global default. Partial-install tests from pnpm#442 pin `enable_global_virtual_store = false` so their seeded legacy-shape slots match the probe path.

End-to-end port of upstream's [`installing/deps-installer/test/install/globalVirtualStore.ts`](https://github.com/pnpm/pnpm/blob/94240bc046/installing/deps-installer/test/install/globalVirtualStore.ts) is tracked as a follow-up — those tests need a small frozen-lockfile fixture against the registry-mock that doesn't exist yet.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants