Skip to content

Readme: remove name change notice#330

Closed
rstacruz wants to merge 1 commit into
masterfrom
rstacruz-patch-1
Closed

Readme: remove name change notice#330
rstacruz wants to merge 1 commit into
masterfrom
rstacruz-patch-1

Conversation

@rstacruz

Copy link
Copy Markdown
Member

That was forever ago. Probably not relevant anymore!

zkochan pushed a commit that referenced this pull request Aug 30, 2016
@zkochan

zkochan commented Aug 30, 2016

Copy link
Copy Markdown
Member

I merged it, just used a different commit message docs(README): remove name change notice

@zkochan zkochan closed this Aug 30, 2016
@rstacruz

Copy link
Copy Markdown
Member Author

Oops!

On Tue, Aug 30, 2016, 10:47 PM Zoltan Kochan notifications@github.com
wrote:

I merged it, just used a different commit message docs(README): remove
name change notice


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#330 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAEikSU3ke8tSJ8fSrxCVkVbnQ_cg7Ixks5qlEKDgaJpZM4Jv5zn
.

@zkochan zkochan deleted the rstacruz-patch-1 branch August 30, 2016 20:23
pull Bot referenced this pull request in dwongdev/pnpm May 14, 2026
Resolves #330.

Mirrors pnpm v11's `@pnpm/bins.resolver`, `@pnpm/bins.linker`, and `@zkochan/cmd-shim` (resolved at pnpm `4750fd370c` and cmd-shim `0d79ca9534`):

- New `pacquet-cmd-shim` crate parses the `bin` field of a package manifest into commands. Both `bin: string | object` and the `directories.bin` glob fallback are supported, with the same path-traversal and URL-safe-name guards as upstream. Conflict resolution follows `pkgOwnsBin` + lexical compare.
- Shim generator writes all three flavors per bin: Unix `/bin/sh` (byte-compatible with pnpm's `generateShShim`), Windows `.cmd` (CRLF, `%~dp0` paths), and PowerShell `.ps1` (`$basedir`/`$exe` header with Windows/POSIX-pwsh detection). Mirrors `generateCmdShim` and `generatePwshShim` minus the unused `nodePath`/`prependToPath`/`progArgs` branches.
- All filesystem IO behind per-capability DI traits — `FsReadHead`, `FsReadFile`, `FsReadString`, `FsReadDir`, `FsWalkFiles`, `FsCreateDirAll`, `FsWrite`, `FsSetExecutable`, `FsEnsureExecutableBits` — following the pattern documented in [#332](pnpm/pacquet#332 (comment)). `FsWrite` is a thin shim over `std::fs::write` and is **not atomic**; a future atomic-write capability can build on it without renaming. Production callers turbofish the unit-struct provider `::<RealApi>`; tests inject unit-struct fakes to cover IO error paths real fs can't trigger portably. Generic param is named `Api` (not `Fs`) so the same provider can grow non-fs capabilities without a rename.
- New `LinkVirtualStoreBins` step walks each virtual-store slot and links its child packages' bins into the slot's own `node_modules/.bin`, matching `linkBinsOfDependencies` in pnpm's `building/during-install`. Slot iteration, per-slot child reads, and shim writes parallelised on rayon.
- `SymlinkDirectDependencies` and `InstallWithoutLockfile` call the bin linker after the symlink layout is in place. Direct-dep linking and the symlink loop are exposed as free `link_direct_dep_bins` / `symlink_direct_deps_into_node_modules` functions so they're unit-testable without the mock-registry scaffold.

Tests port the relevant `bins/resolver` and `bins/linker` cases from pnpm (`directories.bin` discovery, scoped bin names, dangerous locations, idempotent shim skip, conflict resolution) and add fakes-injected coverage for the IO error variants.

**Not in this PR**: hoisted-bin precedence and lifecycle-script-created bins. Both depend on subsystems pacquet hasn't built yet — hoisting (`hoistPattern` / `publicHoistPattern` resolution + the lift step) and lifecycle scripts (`exec/lifecycle/` runner + `allowBuilds` plumbing). The bin-linking side already does the right thing for both (shims are paths, so they pick up files generated post-extract; the precedence rule is a small addition once hoisting tags candidates as hoisted). Tracked separately in  with full porting coordinates.



## Performance

When the bin-linking step landed it added ~7% to the warm-cache Frozen Lockfile install ([benchmark](pnpm/pacquet#333 (comment))). Closing that gap took a sequence of changes that mirror how pnpm v11 avoids the same costs in its own `linkBinsOfDependencies` ([4750fd3](https://github.com/pnpm/pnpm/blob/4750fd370c/building/during-install/src/index.ts#L258-L309)):

- **Bundled manifests in `index.db`.** `extract_tarball_entries` captures the parsed `package.json`, picks the subset pnpm caches via a port of `normalizeBundledManifest` (`name`, `version`, `bin`, `directories`, `dependencies`, lifecycle `scripts`, …), and stuffs it into `PackageFilesIndex.manifest`. `prefetch_cas_paths` surfaces it back as a `HashMap<cache_key, Arc<Value>>` so the bin linker doesn't have to re-read `package.json` per child off disk.
- **msgpackr-records encoder learns `serde_json::Value`.** The encoder previously errored with `ManifestNotSupported` if anything ever set `manifest`; now it records-encodes nested JSON objects (slot-allocated per distinct key set) so `useRecords: true` readers see them as JS `Object`s rather than `Map`s — necessary for pnpm's `manifest.bin` / `manifest.directories?.bin` property accesses to resolve.
- **`Arc<Value>` for `PackageBinSource.manifest`.** The lockfile-driven bin-link path produces `slots × children` clones of the parsed manifest (~6k on the integrated-benchmark fixture). Sharing via `Arc` turns each push into a refcount bump.
- **`hasBin` filter via lockfile.** `LinkVirtualStoreBins` now takes the lockfile `packages:` section, pre-computes an `Option<HashSet<PackageKey>>` of `hasBin: true` keys at install start, and skips snapshot children that aren't in the set *before* any path-building or manifest lookup. ~95% of a real lockfile's packages don't declare a bin, so this short-circuits the bulk of the per-slot work. `None` is reserved for the pathological case where the lockfile lacks a `packages:` section (fall back to processing every child); `Some(empty)` is authoritative — lockfile says no package has a bin, every slot short-circuits. Mirrors pnpm's filter at `building/during-install/src/index.ts:283`.
- **Lockfile-driven slot iteration.** `run_lockfile_driven` walks `snapshots` directly and builds slot paths lexically. The previous `run_with_readdir` path enumerated every slot via `Api::read_dir(virtual_store_dir)` and probed each slot's own package directory with a wasted `read_dir` (~1267 wasted `open(O_DIRECTORY) + close` per warm install). The readdir-driven path survives as a fallback for `install_without_lockfile` and retains the existence probe there (small N, no virtual-store invariant from `create_virtual_dir_by_snapshot` to lean on).
- **Parallel prefetch decode.** `StoreIndex::get_many_raw` returns undecoded row bytes; the SQLite mutex releases before `prefetch_cas_paths` fans the msgpackr decode + integrity check across rayon.

Latest CI benchmark on this branch ([job 75462998251](https://github.com/pnpm/pacquet/actions/runs/25701679172/job/75462998251)):

| Scenario | pacquet@HEAD | pacquet@main | Relative |
|---|---|---|---|
| Frozen Lockfile | 2.074s ± 102ms | 2.026s ± 70ms | +2.4% (within noise) |
| Frozen Lockfile (Hot Cache) | **479.6ms ± 38ms** | **535.8ms ± 37ms** | **~10% faster than main** |

The follow-up pnpm#417 splits manifests into a dedicated `package_manifests` SQLite table so the unfiltered `package_index` prefetch payload stays at its original size and the manifest fetch becomes a targeted SELECT against just the `hasBin: true` subset.

---------

Co-authored-by: Zoltan Kochan <z@kochan.io>
github-actions Bot pushed a commit to Eyalm321/pnpm that referenced this pull request May 18, 2026
Resolves pnpm#330.

Mirrors pnpm v11's `@pnpm/bins.resolver`, `@pnpm/bins.linker`, and `@zkochan/cmd-shim` (resolved at pnpm `4750fd370c` and cmd-shim `0d79ca9534`):

- New `pacquet-cmd-shim` crate parses the `bin` field of a package manifest into commands. Both `bin: string | object` and the `directories.bin` glob fallback are supported, with the same path-traversal and URL-safe-name guards as upstream. Conflict resolution follows `pkgOwnsBin` + lexical compare.
- Shim generator writes all three flavors per bin: Unix `/bin/sh` (byte-compatible with pnpm's `generateShShim`), Windows `.cmd` (CRLF, `%~dp0` paths), and PowerShell `.ps1` (`$basedir`/`$exe` header with Windows/POSIX-pwsh detection). Mirrors `generateCmdShim` and `generatePwshShim` minus the unused `nodePath`/`prependToPath`/`progArgs` branches.
- All filesystem IO behind per-capability DI traits — `FsReadHead`, `FsReadFile`, `FsReadString`, `FsReadDir`, `FsWalkFiles`, `FsCreateDirAll`, `FsWrite`, `FsSetExecutable`, `FsEnsureExecutableBits` — following the pattern documented in [pnpm#332](pnpm/pacquet#332 (comment)). `FsWrite` is a thin shim over `std::fs::write` and is **not atomic**; a future atomic-write capability can build on it without renaming. Production callers turbofish the unit-struct provider `::<RealApi>`; tests inject unit-struct fakes to cover IO error paths real fs can't trigger portably. Generic param is named `Api` (not `Fs`) so the same provider can grow non-fs capabilities without a rename.
- New `LinkVirtualStoreBins` step walks each virtual-store slot and links its child packages' bins into the slot's own `node_modules/.bin`, matching `linkBinsOfDependencies` in pnpm's `building/during-install`. Slot iteration, per-slot child reads, and shim writes parallelised on rayon.
- `SymlinkDirectDependencies` and `InstallWithoutLockfile` call the bin linker after the symlink layout is in place. Direct-dep linking and the symlink loop are exposed as free `link_direct_dep_bins` / `symlink_direct_deps_into_node_modules` functions so they're unit-testable without the mock-registry scaffold.

Tests port the relevant `bins/resolver` and `bins/linker` cases from pnpm (`directories.bin` discovery, scoped bin names, dangerous locations, idempotent shim skip, conflict resolution) and add fakes-injected coverage for the IO error variants.

**Not in this PR**: hoisted-bin precedence and lifecycle-script-created bins. Both depend on subsystems pacquet hasn't built yet — hoisting (`hoistPattern` / `publicHoistPattern` resolution + the lift step) and lifecycle scripts (`exec/lifecycle/` runner + `allowBuilds` plumbing). The bin-linking side already does the right thing for both (shims are paths, so they pick up files generated post-extract; the precedence rule is a small addition once hoisting tags candidates as hoisted). Tracked separately in  with full porting coordinates.



## Performance

When the bin-linking step landed it added ~7% to the warm-cache Frozen Lockfile install ([benchmark](pnpm/pacquet#333 (comment))). Closing that gap took a sequence of changes that mirror how pnpm v11 avoids the same costs in its own `linkBinsOfDependencies` ([4750fd3](https://github.com/pnpm/pnpm/blob/4750fd370c/building/during-install/src/index.ts#L258-L309)):

- **Bundled manifests in `index.db`.** `extract_tarball_entries` captures the parsed `package.json`, picks the subset pnpm caches via a port of `normalizeBundledManifest` (`name`, `version`, `bin`, `directories`, `dependencies`, lifecycle `scripts`, …), and stuffs it into `PackageFilesIndex.manifest`. `prefetch_cas_paths` surfaces it back as a `HashMap<cache_key, Arc<Value>>` so the bin linker doesn't have to re-read `package.json` per child off disk.
- **msgpackr-records encoder learns `serde_json::Value`.** The encoder previously errored with `ManifestNotSupported` if anything ever set `manifest`; now it records-encodes nested JSON objects (slot-allocated per distinct key set) so `useRecords: true` readers see them as JS `Object`s rather than `Map`s — necessary for pnpm's `manifest.bin` / `manifest.directories?.bin` property accesses to resolve.
- **`Arc<Value>` for `PackageBinSource.manifest`.** The lockfile-driven bin-link path produces `slots × children` clones of the parsed manifest (~6k on the integrated-benchmark fixture). Sharing via `Arc` turns each push into a refcount bump.
- **`hasBin` filter via lockfile.** `LinkVirtualStoreBins` now takes the lockfile `packages:` section, pre-computes an `Option<HashSet<PackageKey>>` of `hasBin: true` keys at install start, and skips snapshot children that aren't in the set *before* any path-building or manifest lookup. ~95% of a real lockfile's packages don't declare a bin, so this short-circuits the bulk of the per-slot work. `None` is reserved for the pathological case where the lockfile lacks a `packages:` section (fall back to processing every child); `Some(empty)` is authoritative — lockfile says no package has a bin, every slot short-circuits. Mirrors pnpm's filter at `building/during-install/src/index.ts:283`.
- **Lockfile-driven slot iteration.** `run_lockfile_driven` walks `snapshots` directly and builds slot paths lexically. The previous `run_with_readdir` path enumerated every slot via `Api::read_dir(virtual_store_dir)` and probed each slot's own package directory with a wasted `read_dir` (~1267 wasted `open(O_DIRECTORY) + close` per warm install). The readdir-driven path survives as a fallback for `install_without_lockfile` and retains the existence probe there (small N, no virtual-store invariant from `create_virtual_dir_by_snapshot` to lean on).
- **Parallel prefetch decode.** `StoreIndex::get_many_raw` returns undecoded row bytes; the SQLite mutex releases before `prefetch_cas_paths` fans the msgpackr decode + integrity check across rayon.

Latest CI benchmark on this branch ([job 75462998251](https://github.com/pnpm/pacquet/actions/runs/25701679172/job/75462998251)):

| Scenario | pacquet@HEAD | pacquet@main | Relative |
|---|---|---|---|
| Frozen Lockfile | 2.074s ± 102ms | 2.026s ± 70ms | +2.4% (within noise) |
| Frozen Lockfile (Hot Cache) | **479.6ms ± 38ms** | **535.8ms ± 37ms** | **~10% faster than main** |

The follow-up pnpm#417 splits manifests into a dedicated `package_manifests` SQLite table so the unfiltered `package_index` prefetch payload stays at its original size and the manifest fetch becomes a targeted SELECT against just the `hasBin: true` subset.

---------

Co-authored-by: Zoltan Kochan <z@kochan.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants