Skip to content
This repository was archived by the owner on May 14, 2026. It is now read-only.

feat: foundation for the side-effects cache (#397 item #10, part 1)#422

Merged
zkochan merged 8 commits into
mainfrom
feat/side-effects-cache-read-397
May 12, 2026
Merged

feat: foundation for the side-effects cache (#397 item #10, part 1)#422
zkochan merged 8 commits into
mainfrom
feat/side-effects-cache-read-397

Conversation

@zkochan

@zkochan zkochan commented May 12, 2026

Copy link
Copy Markdown
Member

Summary

Foundation for porting pnpm's side-effects cache (item #10 of #397). This PR lands the three pieces that the actual cache read/write paths depend on:

  • New pacquet-graph-hasher crate porting pnpm's @pnpm/crypto.object-hasher (hashObject / hashObjectWithoutSorting) plus @pnpm/deps.graph-hasher (calcDepState / calcDepGraphHash) and ENGINE_NAME. Byte-for-byte parity with the JS object-hash@3.0.0 bytestream format is load-bearing — the cache key is persisted on disk and shared with pnpm. The headline parity test pins hashObject({b:1,a:2}) == "48AVoXIXcTKcnHt8qVKp5vNw4gyOB5VfztHwtYBRcAQ=" against the upstream fixture in crypto/object-hasher/test/index.ts:6.
  • VerifyResult.side_effects_maps in pacquet-store-dir. The verify path (check_pkg_files_integrity and build_file_maps_from_index) used to drop PackageFilesIndex.side_effects after extraction. It now applies the added/deleted overlay per cache key and surfaces a HashMap<cache_key, FilesMap> — the same shape pnpm uses for its PackageFilesResponse.sideEffectsMaps. Mirrors applySideEffectsDiffWithMaps.
  • Config.side_effects_cache config knob (default true, matching pnpm). Surfaced on the wire only — no consumer yet. Wires up cleanly once the build-phase gate lands in Wire is_built skip + side-effects cache WRITE path (#397 item #10 follow-up to PR #422) #421, with no config migration needed for downstream callers.

Scoped narrowly

This is only the foundation. There is no is_built field, no BuildModules skip gate, and no end-to-end test of the rebuild-skip behavior. Those land in a separate follow-up tracked in #421, which also covers the WRITE path (pacquet seeding the cache itself).

The natural slice for review: graph-hasher unblocks both READ and WRITE, the verify-path surfacing is the only intrusive cross-crate change, and the config knob is forward-looking.

Hashing scope, explicitly

The pacquet-graph-hasher port implements only the type arms pacquet actually feeds into hashObject for the cache-key path: strings, objects, numbers, booleans, null, and arrays in their ordered form. Set / Map / Date / Buffer / Array unordered-permutation arms from upstream object-hash are unimplemented — no caller in pacquet's tree feeds them in today, and the test fixtures upstream uses (hashObject({ b: new Set([…]), a: [...] })) hash deterministically across orderings only because they're testing the library, not exercising a real call path. Adding them can land alongside a future caller that needs them.

Test plan

  • just ready (494 tests + clippy clean)
  • taplo format --check (clean — just ready doesn't run this, CI does)
  • CI green on Linux / macOS / Windows

Next


Written by an agent (Claude Code, claude-opus-4-7).

Summary by CodeRabbit

  • New Features

    • Added a workspace crate providing deterministic object/graph hashing and engine-name formatting.
    • Added side_effects_cache configuration (default: enabled) and per-cache-key side-effects overlays in package integrity results.
  • Chores

    • Integrated stable hashing infrastructure to produce reproducible cache keys compatible with reference behavior.
  • Tests

    • Added unit tests covering hashing, engine-name formatting, and side-effects overlay semantics.

Review Change Stack

Copilot AI review requested due to automatic review settings May 12, 2026 11:34
@coderabbitai

coderabbitai Bot commented May 12, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: da99c9e5-b0ba-440c-a58b-dfc986c3467e

📥 Commits

Reviewing files that changed from the base of the PR and between e591f10 and b686120.

📒 Files selected for processing (5)
  • crates/config/src/lib.rs
  • crates/config/src/workspace_yaml.rs
  • crates/config/src/workspace_yaml/tests.rs
  • crates/store-dir/src/check_pkg_files_integrity.rs
  • crates/store-dir/src/check_pkg_files_integrity/tests.rs
🚧 Files skipped from review as they are similar to previous changes (1)
  • crates/store-dir/src/check_pkg_files_integrity.rs
📜 Recent review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: Run benchmark on ubuntu-latest
  • GitHub Check: Code Coverage
  • GitHub Check: Run benchmark on ubuntu-latest
  • GitHub Check: Lint and Test (windows-latest)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

**/*.rs: Log emissions are part of 'match pnpm'—when porting a function that fires pnpm:<channel> events through globalLogger, logger.debug(...), or streamParser.write(...), mirror the call site, payload, and ordering so @pnpm/cli.default-reporter parses pacquet's NDJSON the same way
Declare a newtype wrapper for branded string types instead of collapsing into plain String or &str to preserve the distinction between validating and non-validating brands
If upstream always validates before construction of a branded string type, implement validation via TryFrom<String> and/or FromStr without providing an infallible public constructor
If upstream never validates a branded string type, expose an infallible From<String> constructor to provide type-safety without runtime validation
When upstream occasionally constructs a branded string type without validation via bare as assertion, expose a from_str_unchecked constructor so callers can opt into the same unchecked path explicitly
Match upstream serde behavior for branded types—use #[serde(try_from = "String")] for deserialization to go through the validator and #[serde(into = "String")] for serialization
Use #[derive(derive_more::From)] and #[derive(derive_more::Into)] for mechanical conversion impls on branded types rather than handwriting impl blocks
Model string-literal unions (like 'auto' | 'always' | 'never') as Rust enums instead of newtype wrappers
Template literal types (like ${string}@${string}) should be treated as branded string types with the same validation discipline applied
Follow the code style guide in CODE_STYLE_GUIDE.md covering code-level conventions not enforced by tooling: imports, modules, naming, ownership and borrowing, parameter type selection, trait bounds, pattern matching, pipe-trait, error handling, test layout, and cloning of Arc and Rc
Warnings are errors in cargo clippy --locked -- --deny warnings, and should not be silenced with `#[allow(....

Files:

  • crates/config/src/workspace_yaml.rs
  • crates/config/src/workspace_yaml/tests.rs
  • crates/config/src/lib.rs
  • crates/store-dir/src/check_pkg_files_integrity/tests.rs
🧠 Learnings (2)
📚 Learning: 2026-05-07T23:19:08.272Z
Learnt from: KSXGitHub
Repo: pnpm/pacquet PR: 401
File: tasks/integrated-benchmark/src/work_env.rs:343-344
Timestamp: 2026-05-07T23:19:08.272Z
Learning: When reviewing Rust code in pnpm/pacquet for deprecated API usage, do not automatically treat `serde_saphyr::to_string` as deprecated. In `serde-saphyr` v0.0.25, `serde_saphyr::to_string` has no `#[deprecated]` attribute (the `#[deprecated]` later in `serde-saphyr-0.0.25/src/lib.rs` applies to a different function). Only flag `serde_saphyr::to_string` as deprecated if the resolved dependency version’s source shows `#[deprecated]` on that specific function.

Applied to files:

  • crates/config/src/workspace_yaml.rs
  • crates/config/src/workspace_yaml/tests.rs
  • crates/config/src/lib.rs
  • crates/store-dir/src/check_pkg_files_integrity/tests.rs
📚 Learning: 2026-05-01T10:01:33.766Z
Learnt from: zkochan
Repo: pnpm/pacquet PR: 349
File: crates/reporter/src/tests.rs:121-121
Timestamp: 2026-05-01T10:01:33.766Z
Learning: In Rust test code, follow the repo’s CODE_STYLE_GUIDE test-logging rule: add logging (e.g., `eprintln!`/`eprintln!(...)`) so that useful diagnostic values are printed when a test fails, unless the assertion is `assert_eq!` (where the differing values are already included). Concretely, if you use assertions like `assert!`, `assert_ne!`, etc., ensure the test logs the relevant actual/expected values (or context) before/around the assertion so failures can be diagnosed without rerunning.

Applied to files:

  • crates/config/src/workspace_yaml/tests.rs
  • crates/store-dir/src/check_pkg_files_integrity/tests.rs
🔇 Additional comments (5)
crates/config/src/workspace_yaml/tests.rs (1)

128-143: Good coverage for sideEffectsCache YAML wiring.

This test correctly validates both deserialization and apply_to override behavior for the new workspace setting.

crates/config/src/workspace_yaml.rs (1)

53-53: Field + apply wiring looks correct.

side_effects_cache is parsed as an optional camelCase YAML key and correctly applied onto Config when set.

Also applies to: 145-146

crates/config/src/lib.rs (1)

177-193: Nice addition of side_effects_cache default + intent docs.

The new config flag is introduced cleanly with the expected default and clear behavioral scope for follow-up implementation.

crates/store-dir/src/check_pkg_files_integrity/tests.rs (2)

326-454: Strong coverage for side-effects overlay semantics.

These tests lock down None vs populated maps, overlay merge precedence, and fail-closed dropping of malformed cache-key overlays—great regression protection for the read path.


456-496: Great isolation guard for per-cache-key overlays.

The k1/k2 assertions clearly verify no cross-key bleed while preserving shared base entries.


📝 Walkthrough

Walkthrough

This PR introduces a new pacquet-graph-hasher crate implementing pnpm-compatible object and dependency-graph hashing, integrates a side_effects_cache configuration flag into the Config struct, and extends package verification results to include per-cache-key side-effects overlays computed from PackageFilesIndex.

Changes

Graph Hasher Library

Layer / File(s) Summary
Crate setup and library root
Cargo.toml, crates/graph-hasher/Cargo.toml, crates/graph-hasher/src/lib.rs
Registers pacquet-graph-hasher as a workspace dependency and creates the crate manifest with workspace dependencies (base64, serde_json, sha2); library root provides documentation pinning pnpm reference implementations, declares internal modules, and exports public HashEncoding enum and hashing APIs.
Engine name formatting
crates/graph-hasher/src/engine_name.rs
engine_name(node_major, platform, arch) formats pnpm-style ENGINE_NAME strings as {platform};{arch};node{major}, with host-mapping helpers to translate Rust OS/ARCH naming to Node.js conventions and unit tests for explicit and defaulted cases.
SHA-256 object hashing
crates/graph-hasher/src/object_hasher.rs, crates/graph-hasher/src/tests.rs
hash_object, hash_object_without_sorting, and hash_object_with_encoding compute deterministic SHA-256 digests of JSON values via a recursive serializer that emits type-tagged bytes, optionally sorts object keys, and frames strings using UTF-16 code units; tests validate encoding equivalence, sorting semantics, and non-ASCII handling.
Dependency state and graph hashing
crates/graph-hasher/src/dep_state.rs
calc_dep_state builds stable cache keys by recursively hashing the dependency graph via calc_dep_graph_hash, memoizing results, and using ancestor tracking to short-circuit cycles; comprehensive tests validate determinism, diamond graphs, cycles, and segment ordering.

Side Effects Cache Configuration and Integration

Layer / File(s) Summary
Configuration and test setup
crates/config/src/lib.rs, crates/config/src/workspace_yaml.rs, crates/config/src/workspace_yaml/tests.rs, crates/package-manager/src/install_package_from_registry/tests.rs
Config gains a public boolean side_effects_cache field (default true) documented as read from pnpm-workspace.yaml; workspace YAML parsing and apply logic are updated and tests exercise YAML->Config application; test helpers enable the setting.
Verification result extension and overlay building
crates/store-dir/src/check_pkg_files_integrity.rs, crates/store-dir/src/check_pkg_files_integrity/tests.rs
VerifyResult gains optional side_effects_maps field; both build_file_maps_from_index and check_pkg_files_integrity extract side_effects from PackageFilesIndex and use build_side_effects_maps to construct per-cache-key overlaid FilesMap instances where added entries are merged (skipping malformed digests), deleted entries are removed, and added wins on filename collisions; tests validate overlay semantics and cache-key isolation.

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant CalcDepState
  participant CalcGraphHash
  participant HashObject
  participant Cache
  
  Caller->>CalcDepState: invoke with graph and options
  CalcDepState->>CalcGraphHash: compute dependency hash
  CalcGraphHash->>Cache: check memoized result
  alt cache hit
    Cache-->>CalcGraphHash: return stored hash
  else cache miss
    CalcGraphHash->>HashObject: hash node object
    HashObject-->>CalcGraphHash: SHA-256 digest
    CalcGraphHash->>Cache: store memoized result
  end
  CalcGraphHash-->>CalcDepState: return hash string
  CalcDepState-->>Caller: return formatted key
Loading
sequenceDiagram
  participant VerifyPath
  participant BuildOverlays
  participant SideEffects
  participant BaseMap
  participant Overlay
  
  VerifyPath->>BuildOverlays: pass side effects and base
  BuildOverlays->>SideEffects: iterate cache keys
  SideEffects->>BaseMap: clone for this key
  BaseMap->>Overlay: apply added entries
  Overlay->>Overlay: remove deleted filenames
  Overlay->>Overlay: added shadows base collisions
  Overlay-->>BuildOverlays: per-key FilesMap result
  BuildOverlays-->>VerifyPath: return overlay map or none
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~75 minutes

Possibly related issues

Possibly related PRs

  • pnpm/pacquet#369: Reorganized #[cfg(test)] modules into external tests.rs files across many crates that overlap with this PR's test extractions and additions.

Suggested reviewers

  • KSXGitHub
  • anthonyshew

Poem

🐇 I nibble bytes and weave a chart,

Hash each node and play my part.
Engine names in platform and arch,
Side-effects layered, keys march—
A rabbit signs the code with heart.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the main change as foundational work for the side-effects cache feature, with specific reference to the issue and part number.
Description check ✅ Passed The description is comprehensive and detailed, covering all required sections: summary of changes, linked issue (#397), upstream reference, and checklist items completed.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/side-effects-cache-read-397

Comment @coderabbitai help to get the list of available commands and usage tips.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Lays the groundwork for pnpm side-effects cache interop by adding a new hashing crate (pnpm-compatible object/dep-graph hashing), plumbing side-effects overlays through the store-dir verify path, and introducing an npmrc flag to gate cache usage.

Changes:

  • Added pacquet-graph-hasher crate implementing pnpm-compatible hashObject + dep-state/engine-name helpers with parity-oriented tests.
  • Extended store-dir verification results to surface per-cache-key overlaid side_effects_maps.
  • Added Npmrc.side_effects_cache configuration knob (default true) and updated package-manager test config construction.

Reviewed changes

Copilot reviewed 11 out of 12 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
crates/store-dir/src/check_pkg_files_integrity.rs Adds VerifyResult.side_effects_maps and builds per-cache-key overlay maps from PackageFilesIndex.side_effects.
crates/store-dir/src/check_pkg_files_integrity/tests.rs Adds unit tests validating side-effects overlay behavior and None vs Some(empty) semantics.
crates/package-manager/src/install_package_from_registry/tests.rs Updates Npmrc test config literal to include side_effects_cache.
crates/npmrc/src/lib.rs Introduces side_effects_cache config field (default true) with documentation.
crates/graph-hasher/Cargo.toml New crate manifest for pacquet-graph-hasher.
crates/graph-hasher/src/lib.rs New crate root exporting object hasher, engine name, and dep-state API.
crates/graph-hasher/src/object_hasher.rs Implements pnpm/object-hash compatible serialization + hashing.
crates/graph-hasher/src/dep_state.rs Implements calc_dep_state / dep-graph hashing with memoization.
crates/graph-hasher/src/engine_name.rs Implements pnpm ENGINE_NAME formatting and host OS/arch mapping.
crates/graph-hasher/src/tests.rs Adds parity and invariants tests for hashing behavior.
Cargo.toml Adds pacquet-graph-hasher to workspace dependencies.
Cargo.lock Records the new workspace crate entry.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread crates/store-dir/src/check_pkg_files_integrity.rs Outdated
Comment thread crates/graph-hasher/src/object_hasher.rs
Comment thread crates/graph-hasher/src/object_hasher.rs
Comment thread crates/graph-hasher/src/engine_name.rs Outdated
@github-actions

github-actions Bot commented May 12, 2026

Copy link
Copy Markdown

Micro-Benchmark Results

Linux

group                          main                                   pr
-----                          ----                                   --
tarball/download_dependency    1.02     16.3±0.44ms   265.7 KB/sec    1.00     16.1±0.41ms   270.0 KB/sec

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (1)
crates/store-dir/src/check_pkg_files_integrity/tests.rs (1)

331-338: ⚡ Quick win

Add failure context for this assert! check.

Please add a message (or eprintln!) with result.side_effects_maps so failures are diagnosable without reruns.

Based on learnings: "In Rust test code, follow the repo’s CODE_STYLE_GUIDE test-logging rule: add logging ... if you use assertions like assert!, assert_ne!, etc."

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/store-dir/src/check_pkg_files_integrity/tests.rs` around lines 331 -
338, The test no_side_effects_yields_none currently uses assert! without
context; modify the assertion to include diagnostic output by either using
assert!(result.side_effects_maps.is_none(), "side_effects_maps = {:?}",
result.side_effects_maps) or printing the value before the assertion (e.g.,
eprintln!("side_effects_maps = {:?}", result.side_effects_maps)) so failures
show the actual result; update the assertion surrounding
result.side_effects_maps in that test accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/graph-hasher/src/engine_name.rs`:
- Around line 50-57: The host_arch() function currently returns Rust's
std::env::consts::ARCH values directly for unknown architectures, which leaks
Rust-only identifiers (e.g., powerpc64le, loongarch64) into ENGINE_NAME and
breaks pnpm cache-key parity; update host_arch() (the match in function
host_arch) to explicitly map "powerpc64le" => "ppc64" and "loongarch64" =>
"loong64" alongside the existing arms so ENGINE_NAME uses Node.js-style arch
names while keeping the existing mappings for "x86_64", "aarch64", "x86", and
the fallback other => other.

In `@crates/npmrc/src/lib.rs`:
- Around line 196-211: The documentation for the side_effects_cache option
overstates behavior by claiming it skips lifecycle scripts and forces rebuilds,
but the current implementation only implements the read-path (consulting
PackageFilesIndex.sideEffects keyed by pacquet_graph_hasher::engine_name plus
dep-graph/patch hashes) and does not gate skipping builds or lifecycle scripts;
update the comment block that documents side_effects_cache (and any mentions of
sideEffectsCache) to explicitly describe only the implemented read-path
behavior: that a matching cache entry will be used as the import payload
(overlay applied to the base CAFS map) when present, and remove or reword any
language asserting that lifecycle scripts are skipped or rebuilds are prevented.

---

Nitpick comments:
In `@crates/store-dir/src/check_pkg_files_integrity/tests.rs`:
- Around line 331-338: The test no_side_effects_yields_none currently uses
assert! without context; modify the assertion to include diagnostic output by
either using assert!(result.side_effects_maps.is_none(), "side_effects_maps =
{:?}", result.side_effects_maps) or printing the value before the assertion
(e.g., eprintln!("side_effects_maps = {:?}", result.side_effects_maps)) so
failures show the actual result; update the assertion surrounding
result.side_effects_maps in that test accordingly.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 03ad0f96-a736-4432-83da-167e019292c3

📥 Commits

Reviewing files that changed from the base of the PR and between 98c9886 and 9c7fca7.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (11)
  • Cargo.toml
  • crates/graph-hasher/Cargo.toml
  • crates/graph-hasher/src/dep_state.rs
  • crates/graph-hasher/src/engine_name.rs
  • crates/graph-hasher/src/lib.rs
  • crates/graph-hasher/src/object_hasher.rs
  • crates/graph-hasher/src/tests.rs
  • crates/npmrc/src/lib.rs
  • crates/package-manager/src/install_package_from_registry/tests.rs
  • crates/store-dir/src/check_pkg_files_integrity.rs
  • crates/store-dir/src/check_pkg_files_integrity/tests.rs
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
  • GitHub Check: Agent
  • GitHub Check: Lint and Test (windows-latest)
  • GitHub Check: Lint and Test (macos-latest)
  • GitHub Check: Run benchmark on ubuntu-latest
  • GitHub Check: Run benchmark on ubuntu-latest
  • GitHub Check: Code Coverage
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

**/*.rs: Log emissions are part of 'match pnpm'—when porting a function that fires pnpm:<channel> events through globalLogger, logger.debug(...), or streamParser.write(...), mirror the call site, payload, and ordering so @pnpm/cli.default-reporter parses pacquet's NDJSON the same way
Declare a newtype wrapper for branded string types instead of collapsing into plain String or &str to preserve the distinction between validating and non-validating brands
If upstream always validates before construction of a branded string type, implement validation via TryFrom<String> and/or FromStr without providing an infallible public constructor
If upstream never validates a branded string type, expose an infallible From<String> constructor to provide type-safety without runtime validation
When upstream occasionally constructs a branded string type without validation via bare as assertion, expose a from_str_unchecked constructor so callers can opt into the same unchecked path explicitly
Match upstream serde behavior for branded types—use #[serde(try_from = "String")] for deserialization to go through the validator and #[serde(into = "String")] for serialization
Use #[derive(derive_more::From)] and #[derive(derive_more::Into)] for mechanical conversion impls on branded types rather than handwriting impl blocks
Model string-literal unions (like 'auto' | 'always' | 'never') as Rust enums instead of newtype wrappers
Template literal types (like ${string}@${string}) should be treated as branded string types with the same validation discipline applied
Follow the code style guide in CODE_STYLE_GUIDE.md covering code-level conventions not enforced by tooling: imports, modules, naming, ownership and borrowing, parameter type selection, trait bounds, pattern matching, pipe-trait, error handling, test layout, and cloning of Arc and Rc
Warnings are errors in cargo clippy --locked -- --deny warnings, and should not be silenced with `#[allow(....

Files:

  • crates/graph-hasher/src/engine_name.rs
  • crates/graph-hasher/src/lib.rs
  • crates/npmrc/src/lib.rs
  • crates/package-manager/src/install_package_from_registry/tests.rs
  • crates/store-dir/src/check_pkg_files_integrity/tests.rs
  • crates/graph-hasher/src/tests.rs
  • crates/graph-hasher/src/dep_state.rs
  • crates/store-dir/src/check_pkg_files_integrity.rs
  • crates/graph-hasher/src/object_hasher.rs
🧠 Learnings (3)
📚 Learning: 2026-05-07T23:19:08.272Z
Learnt from: KSXGitHub
Repo: pnpm/pacquet PR: 401
File: tasks/integrated-benchmark/src/work_env.rs:343-344
Timestamp: 2026-05-07T23:19:08.272Z
Learning: When reviewing Rust code in pnpm/pacquet for deprecated API usage, do not automatically treat `serde_saphyr::to_string` as deprecated. In `serde-saphyr` v0.0.25, `serde_saphyr::to_string` has no `#[deprecated]` attribute (the `#[deprecated]` later in `serde-saphyr-0.0.25/src/lib.rs` applies to a different function). Only flag `serde_saphyr::to_string` as deprecated if the resolved dependency version’s source shows `#[deprecated]` on that specific function.

Applied to files:

  • crates/graph-hasher/src/engine_name.rs
  • crates/graph-hasher/src/lib.rs
  • crates/npmrc/src/lib.rs
  • crates/package-manager/src/install_package_from_registry/tests.rs
  • crates/store-dir/src/check_pkg_files_integrity/tests.rs
  • crates/graph-hasher/src/tests.rs
  • crates/graph-hasher/src/dep_state.rs
  • crates/store-dir/src/check_pkg_files_integrity.rs
  • crates/graph-hasher/src/object_hasher.rs
📚 Learning: 2026-05-01T10:01:33.766Z
Learnt from: zkochan
Repo: pnpm/pacquet PR: 349
File: crates/reporter/src/tests.rs:121-121
Timestamp: 2026-05-01T10:01:33.766Z
Learning: In Rust test code, follow the repo’s CODE_STYLE_GUIDE test-logging rule: add logging (e.g., `eprintln!`/`eprintln!(...)`) so that useful diagnostic values are printed when a test fails, unless the assertion is `assert_eq!` (where the differing values are already included). Concretely, if you use assertions like `assert!`, `assert_ne!`, etc., ensure the test logs the relevant actual/expected values (or context) before/around the assertion so failures can be diagnosed without rerunning.

Applied to files:

  • crates/package-manager/src/install_package_from_registry/tests.rs
  • crates/store-dir/src/check_pkg_files_integrity/tests.rs
  • crates/graph-hasher/src/tests.rs
📚 Learning: 2026-05-01T10:01:33.766Z
Learnt from: zkochan
Repo: pnpm/pacquet PR: 349
File: crates/reporter/src/tests.rs:121-121
Timestamp: 2026-05-01T10:01:33.766Z
Learning: In this repo’s Rust tests (run via `cargo nextest`), stdout/stderr from passing tests is captured and is only shown when a test fails. As a result, `eprintln!` (and similar stderr logging) in these tests should generally not be flagged as CI “noise” on the happy path; CI output should only appear for failing tests.

Applied to files:

  • crates/graph-hasher/src/tests.rs
🔇 Additional comments (4)
crates/package-manager/src/install_package_from_registry/tests.rs (1)

39-39: Nice explicit fixture pin.

Setting side_effects_cache: true here keeps this test isolated from future default changes in Npmrc.

crates/store-dir/src/check_pkg_files_integrity.rs (1)

214-248: Overlay map construction looks solid.

The per-cache-key merge (added precedence + deleted filtering + base fallback) is implemented cleanly and wired consistently into both verify paths.

crates/store-dir/src/check_pkg_files_integrity/tests.rs (1)

340-456: Great coverage additions for side-effects overlays.

These tests pin the important semantics (add/delete merge, collision precedence, and per-key isolation) well.

crates/graph-hasher/src/object_hasher.rs (1)

107-111: ⚡ Quick win

Sort keys with UTF-16 ordering if needed for pnpm parity, but fix the proposed syntax.

The proposed diff has a compilation error: encode_utf16() returns an iterator without Ord, so it cannot be directly compared with .cmp(). If this change is necessary, use:

keys.sort_by(|a, b| {
    a.encode_utf16().collect::<Vec<u16>>()
        .cmp(&b.encode_utf16().collect::<Vec<u16>>())
})

However, the practical necessity is unclear: all object keys in tests are ASCII-only, and real-world package manager keys follow npm naming rules (ASCII). Confirm whether upstream's object-hash actually uses plain Array.prototype.sort for object keys and whether non-BMP ordering differences occur in practice before implementing.

Comment thread crates/graph-hasher/src/engine_name.rs
Comment thread crates/npmrc/src/lib.rs Outdated
zkochan added a commit that referenced this pull request May 12, 2026
Doc CI blocker + five Copilot/coderabbit threads:

- `npmrc` referenced `[pacquet_graph_hasher::engine_name]` from a
  crate that doesn't depend on graph-hasher, so rustdoc rejected
  the unresolvable link. Also flagged: a stale `[pacquet_platform](#)`
  placeholder link in `graph-hasher::engine_name`'s docstring, and a
  `[serialize]` link from a public doc to a private helper in
  `object_hasher`. Reworded all three to plain prose so docs build
  under `-D warnings`.
- Reworded the `Npmrc.side_effects_cache` doc to describe **only
  the foundation-on-the-wire** state this PR actually ships (no
  rebuild-skip gate, no WRITE path). The previous wording asserted
  behavior that lands in #421.
- `object_hasher` factored a `serialize_str` helper out of the
  string arm so `write_pair` no longer allocates a per-key
  `Value::String(key.to_string())`. Hot path on hashing large
  graphs; the extra `Number::to_string()` divergence question is
  noted in a clarifying doc comment on the `Value::Number` arm
  (pacquet only ever hashes integer values; non-integer floats
  would need a JS-shortest-round-trippable formatter — `ryu` or
  similar — before they can land safely on the cache-key path).
- `build_side_effects_maps` was O(|base| * |deleted|) — every
  base entry called `deleted_set.iter().any(...)`. Promote
  `deleted` to a `HashSet` once per cache key for linear time,
  matching what pnpm's TS side does with a `Set`.
- `engine_name::host_arch` was passing `powerpc64le` /
  `loongarch64` through verbatim, which would have leaked
  Rust-only identifiers into `ENGINE_NAME` and broken cache-key
  parity on those architectures. Added explicit mappings:
  `powerpc64le → ppc64` (Node uses the same string for BE and LE
  POWER; only `os.endianness()` distinguishes them) and
  `loongarch64 → loong64`.
zkochan added 5 commits May 12, 2026 13:49
New `pacquet-graph-hasher` crate porting:

- `@pnpm/crypto.object-hasher` (`hashObject` + `hashObjectWithoutSorting`)
  from <https://github.com/pnpm/pnpm/blob/b4f8f47ac2/crypto/object-hasher/src/index.ts>.
  Replicates the `object-hash@3.0.0` bytestream format
  (`object:<N>:<key>:<value>,...` with sorted keys when
  `unorderedObjects`, `string:<utf16_len>:<value>`, etc.) plus
  sha256 + base64/hex encoding. Byte-for-byte parity with pnpm is
  load-bearing — the cache key is persisted on disk and shared
  with pnpm. The known-base64 test pins
  `hashObject({b:1,a:2}) == "48AVoXIXcTKcnHt8qVKp5vNw4gyOB5VfztHwtYBRcAQ="`.
- `@pnpm/deps.graph-hasher`'s `calcDepState` / `calcDepGraphHash`
  from <https://github.com/pnpm/pnpm/blob/b4f8f47ac2/deps/graph-hasher/src/index.ts>.
  Recursive walk over a `DepsGraph` (alias → child key) memoized
  via `DepsStateCache`. Cycle-safe via a `parents` set; mirrors
  upstream's `if (!parents.has(node.fullPkgId))` short-circuit.
- `ENGINE_NAME` from `core/constants/src/index.ts:7` —
  `<platform>;<arch>;node<major>` with Node-naming mapping for
  the host (`darwin` / `win32` / `sunos`; `x64` / `arm64` /
  `ia32` / `ppc64`).

Tests: 8 hashObject parity cases (including the byte-exact pnpm
fixture, sorted vs unsorted, hex/base64, UTF-16 length, dep-state
shape) + 6 calcDepState cases (engine-only, patch-only, leaf,
diamond, cycle, ordering) + 2 engine_name cases (format + host
default shape).

Scoped narrowly to what pacquet's side-effects cache READ path
actually feeds in (strings, objects of strings, null, booleans,
numbers). Set / Map / Date / Buffer / Array unordered-permutation
arms from upstream are unimplemented — they have no caller in
pacquet's tree today. Adding them can land alongside a future
caller that exercises them.
Pacquet's `PackageFilesIndex.side_effects` field has been on the
wire round-trip since the schema landed but the verify path
dropped it — `check_pkg_files_integrity` and `build_file_maps_from_index`
discarded everything except `files` and `algo`, so callers couldn't
tell whether a stored package had a side-effects cache populated.

`VerifyResult` now carries `side_effects_maps: Option<HashMap<String, FilesMap>>`.
Each entry is the post-build files map for one cache key — built
by applying the entry's `SideEffectsDiff.added` overlay on top of
the base files_map and dropping any filenames in `deleted`.
Mirrors `applySideEffectsDiffWithMaps` at
<https://github.com/pnpm/pnpm/blob/b4f8f47ac2/store/create-cafs-store/src/index.ts#L103-L121>.

Three test cases cover the overlay's invariants: `added` wins on
filename collision, `deleted` drops base entries, and multiple
cache keys produce independent overlays. The fourth pins the
"no side-effects field on the row" path to return `None` so the
importer can distinguish "cache never written" from "cache
empty for the targeted key".

`None` for the `added` overlay's CAFS-path resolution drops that
single filename (with a debug log) instead of failing the whole
overlay — matches upstream's lenient `getFlatMap` which would
likewise surface the missing-CAS-blob error later at import
time, not during verify.
Pacquet `Npmrc` gains `side_effects_cache: bool` (default `true`,
matching pnpm). Mirrors pnpm's
[`side-effects-cache`](https://github.com/pnpm/pnpm/blob/b4f8f47ac2/config/config/src/index.ts)
config (camelCase `sideEffectsCache`).

When `true`, the build phase will consult `PackageFilesIndex.side_effects`
to decide whether a package is already built and skip its lifecycle
scripts. When `false`, the cache is ignored and lifecycle scripts
run unconditionally. The read-side cache lookup itself lands with
the `is_built` wiring in a follow-up commit; the write side
(populating the cache after a postinstall) is a separate PR.
… fixtures (#397)

`create_config` in install_package_from_registry's tests builds an
`Npmrc` literal that was missing the field added in the previous
commit. Default-on to match production.
Doc CI blocker + five Copilot/coderabbit threads:

- `npmrc` referenced `[pacquet_graph_hasher::engine_name]` from a
  crate that doesn't depend on graph-hasher, so rustdoc rejected
  the unresolvable link. Also flagged: a stale `[pacquet_platform](#)`
  placeholder link in `graph-hasher::engine_name`'s docstring, and a
  `[serialize]` link from a public doc to a private helper in
  `object_hasher`. Reworded all three to plain prose so docs build
  under `-D warnings`.
- Reworded the `Npmrc.side_effects_cache` doc to describe **only
  the foundation-on-the-wire** state this PR actually ships (no
  rebuild-skip gate, no WRITE path). The previous wording asserted
  behavior that lands in #421.
- `object_hasher` factored a `serialize_str` helper out of the
  string arm so `write_pair` no longer allocates a per-key
  `Value::String(key.to_string())`. Hot path on hashing large
  graphs; the extra `Number::to_string()` divergence question is
  noted in a clarifying doc comment on the `Value::Number` arm
  (pacquet only ever hashes integer values; non-integer floats
  would need a JS-shortest-round-trippable formatter — `ryu` or
  similar — before they can land safely on the cache-key path).
- `build_side_effects_maps` was O(|base| * |deleted|) — every
  base entry called `deleted_set.iter().any(...)`. Promote
  `deleted` to a `HashSet` once per cache key for linear time,
  matching what pnpm's TS side does with a `Set`.
- `engine_name::host_arch` was passing `powerpc64le` /
  `loongarch64` through verbatim, which would have leaked
  Rust-only identifiers into `ENGINE_NAME` and broken cache-key
  parity on those architectures. Added explicit mappings:
  `powerpc64le → ppc64` (Node uses the same string for BE and LE
  POWER; only `os.endianness()` distinguishes them) and
  `loongarch64 → loong64`.
@github-actions

github-actions Bot commented May 12, 2026

Copy link
Copy Markdown

Integrated-Benchmark Report (Linux)

Scenario: Frozen Lockfile

Command Mean [s] Min [s] Max [s] Relative
pacquet@HEAD 2.364 ± 0.091 2.242 2.504 1.02 ± 0.04
pacquet@main 2.308 ± 0.028 2.262 2.354 1.00
pnpm 5.730 ± 0.066 5.639 5.819 2.48 ± 0.04
BENCHMARK_REPORT.json
{
  "results": [
    {
      "command": "pacquet@HEAD",
      "mean": 2.36394728846,
      "stddev": 0.09138562609097754,
      "median": 2.3791960675599997,
      "user": 2.5093817599999997,
      "system": 3.26813546,
      "min": 2.24237128656,
      "max": 2.50397130356,
      "times": [
        2.36019679656,
        2.41070823656,
        2.50397130356,
        2.24237128656,
        2.42030176656,
        2.46511043256,
        2.32063816056,
        2.39819533856,
        2.25253542356,
        2.26544413956
      ]
    },
    {
      "command": "pacquet@main",
      "mean": 2.3078971159600004,
      "stddev": 0.027644467822877203,
      "median": 2.3122525940600003,
      "user": 2.53633296,
      "system": 3.250820859999999,
      "min": 2.26198936156,
      "max": 2.35358159356,
      "times": [
        2.2638349085600002,
        2.35358159356,
        2.32044115256,
        2.30212641056,
        2.32489312556,
        2.30517505856,
        2.32242436056,
        2.26198936156,
        2.31398014056,
        2.31052504756
      ]
    },
    {
      "command": "pnpm",
      "mean": 5.7301268834600005,
      "stddev": 0.06627787375035617,
      "median": 5.7329107835599995,
      "user": 8.35900466,
      "system": 4.23655966,
      "min": 5.63949244056,
      "max": 5.81878152756,
      "times": [
        5.77817834156,
        5.65262863156,
        5.69456559656,
        5.81878152756,
        5.80150765856,
        5.6672904195600005,
        5.63949244056,
        5.76473884256,
        5.70108272456,
        5.78300265156
      ]
    }
  ]
}

Scenario: Frozen Lockfile (Hot Cache)

Command Mean [ms] Min [ms] Max [ms] Relative
pacquet@HEAD 701.9 ± 145.9 635.8 1113.3 1.08 ± 0.23
pacquet@main 650.9 ± 35.1 625.2 723.8 1.00
pnpm 2432.0 ± 100.8 2313.8 2573.2 3.74 ± 0.25
BENCHMARK_REPORT.json
{
  "results": [
    {
      "command": "pacquet@HEAD",
      "mean": 0.70189347774,
      "stddev": 0.14586133810676244,
      "median": 0.66182034264,
      "user": 0.33230544,
      "system": 1.47272578,
      "min": 0.6358420756400001,
      "max": 1.1133423386399999,
      "times": [
        1.1133423386399999,
        0.6619665006400001,
        0.66167418464,
        0.63689118864,
        0.6632626196400001,
        0.63885713164,
        0.6360577016400001,
        0.68035607464,
        0.69068496164,
        0.6358420756400001
      ]
    },
    {
      "command": "pacquet@main",
      "mean": 0.6509423267400001,
      "stddev": 0.03514030011057033,
      "median": 0.6365340651400001,
      "user": 0.33397534,
      "system": 1.4705931799999996,
      "min": 0.6251678216400001,
      "max": 0.72377757064,
      "times": [
        0.72377757064,
        0.63089589264,
        0.64930018164,
        0.6333950166400001,
        0.6349134566400001,
        0.63906853064,
        0.7081813036400001,
        0.6381546736400001,
        0.6251678216400001,
        0.6265688196400001
      ]
    },
    {
      "command": "pnpm",
      "mean": 2.4319712743400004,
      "stddev": 0.10076581792743512,
      "median": 2.39829122264,
      "user": 2.8207817399999997,
      "system": 2.1747805799999997,
      "min": 2.31379282364,
      "max": 2.5731766246400003,
      "times": [
        2.5731766246400003,
        2.34874307864,
        2.4059903626400003,
        2.5422821136400002,
        2.3571400436400003,
        2.31379282364,
        2.39059208264,
        2.32757182964,
        2.54007594764,
        2.52034783664
      ]
    }
  ]
}

…base

The rebase onto main pulled in #420's rename of `pacquet-npmrc` →
`pacquet-config`, which dropped the ini-wiring attributes
(`#[serde(default = "bool_true", deserialize_with = "deserialize_bool")]`)
from every field. My original commit on the now-renamed file
carried those attributes along; once the helpers are gone they
fail to compile (`cannot find attribute serde in this scope`).

Strip them so the field matches the post-rename style — only the
`#[default = true]` from `smart-default` remains, which is what
the rest of the struct uses.
Copilot AI review requested due to automatic review settings May 12, 2026 11:52
@zkochan zkochan force-pushed the feat/side-effects-cache-read-397 branch from 238c9cd to 96a2104 Compare May 12, 2026 11:52
@codecov

codecov Bot commented May 12, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 93.47181% with 22 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.72%. Comparing base (0fe0b0c) to head (b686120).

Files with missing lines Patch % Lines
crates/graph-hasher/src/object_hasher.rs 81.25% 15 Missing ⚠️
crates/graph-hasher/src/engine_name.rs 84.84% 5 Missing ⚠️
crates/graph-hasher/src/dep_state.rs 99.47% 1 Missing ⚠️
crates/store-dir/src/check_pkg_files_integrity.rs 97.05% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #422      +/-   ##
==========================================
+ Coverage   85.23%   85.72%   +0.49%     
==========================================
  Files          79       82       +3     
  Lines        5372     5704     +332     
==========================================
+ Hits         4579     4890     +311     
- Misses        793      814      +21     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

`crate::engine_name` is ambiguous to rustdoc — the name resolves to
both a function (the public `engine_name()` re-export) and a module
(the private `mod engine_name`). Use the explicit function-call
form `[\`crate::engine_name()\`]` so rustdoc dispatches to the fn.
Caught by the workspace-wide `RUSTDOCFLAGS=-D warnings` CI check.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 12 changed files in this pull request and generated 2 comments.

Comment thread crates/config/src/lib.rs Outdated
Comment thread crates/store-dir/src/check_pkg_files_integrity.rs
Two Copilot threads:

- `side_effects_cache` was on the `Config` struct but unreachable
  from any config source — `WorkspaceSettings` had no
  `sideEffectsCache` field and `apply_to` didn't set it. So a
  user could only flip it via direct `Config { … }` construction.
  Added `WorkspaceSettings.side_effects_cache: Option<bool>` and
  threaded it through `apply_to`'s macro list. Now
  `sideEffectsCache: false` in `pnpm-workspace.yaml` takes effect.
  Tightened the field's docstring to describe what's actually
  wired (yaml-source plumbing on, build-phase gate still in
  #421). New test mirrors the existing
  `parses_verify_store_integrity_from_yaml_and_applies` shape:
  pin both the camelCase rename and the `apply_to` step.
- `build_side_effects_maps` previously continued building an
  overlay when a single `added` entry had a malformed digest —
  it logged + skipped just that file and kept going. Once the
  importer in #421 starts flipping `is_built = true`
  on overlay presence, that partial overlay becomes a silent
  corruption (build skipped, required artifact missing). Switch
  to `continue 'next_key` so the entire cache_key entry is
  dropped, sending the package back through the rebuild path.
  Other cache_key entries on the same package survive. New
  test pins the fail-closed behavior.
@zkochan zkochan merged commit 9c340a7 into main May 12, 2026
16 checks passed
@zkochan zkochan deleted the feat/side-effects-cache-read-397 branch May 12, 2026 12:18
zkochan added a commit that referenced this pull request May 12, 2026
…path (#421 slice A) (#423)

## Summary

Implements slice (A) of #421 — the READ-path gate that consumes the foundation merged in #422. With a matching side-effects-cache entry on disk (typically seeded by a prior `pnpm install`), `BuildModules` now skips lifecycle scripts for that snapshot, mirroring upstream's [`!node.isBuilt` filter](https://github.com/pnpm/pnpm/blob/b4f8f47ac2/building/during-install/src/index.ts#L73-L77).

User-visible behavior: a warm `pacquet install` after `pnpm install` no longer rebuilds packages whose post-build state is already in the store.

## What lands

- **`pacquet_graph_hasher::detect_node_major()`** — spawns `node --version` once at install start, parses the major version. The parse helper is factored out so the rule is unit-tested without a child process. Missing-node returns `None` and the gate falls through to "rebuild" (safe).
- **`DepsGraphNode` flipped to own its strings** — was borrowed `&'a str`, now `String`. Lets callers building the graph from a lockfile skip a separate-arena lifetime story. No external consumers besides in-crate tests yet, so this is internal-only.
- **`PrefetchResult.side_effects_maps`** surfaces the per-row `cache_key → FilesMap` overlay that the prefetch loop previously dropped (since #422 added it to `VerifyResult`). `Arc`-wrapped for the same cold-batch cheap-clone reason `cas_paths` is.
- **`CreateVirtualStore` returns a new `CreateVirtualStoreOutput`** struct: `package_manifests` (existing) plus `side_effects_maps_by_snapshot: HashMap<PackageKey, Arc<…>>` (new). Peer-variants of the same package share one `Arc<…>` because the store-index row is keyed peer-stripped.
- **New `pacquet_package_manager::deps_graph::build_deps_graph` adapter** — walks `snapshots` + `packages` and builds a `DepsGraph<PackageKey>` for the hasher. `full_pkg_id` derives from `<pkg_id>:<integrity>` for registry / tarball resolutions, or `<pkg_id>:<hashObject(resolution)>` for git / directory / integrity-less tarball, matching upstream's [`createFullPkgId`](https://github.com/pnpm/pnpm/blob/b4f8f47ac2/deps/graph-hasher/src/index.ts#L263-L292).
- **`BuildModules` is_built gate** — after the policy gate, before extraction, the build loop computes `calc_dep_state` per snapshot (with `include_dep_graph_hash: true` mirroring upstream's `hasSideEffects`), looks the cache key up in the threaded-in `side_effects_maps`, and `continue`s on hit. The `Config.side_effects_cache: false` flag short-circuits the lookup entirely.
- **`InstallFrozenLockfile`** detects the engine name once and threads it (plus the prefetch's side-effects map and the config flag) into `BuildModules`.

## Tests

- `using_side_effects_cache_skips_rebuild` ports the upstream `'using side effects cache'` case from [`installing/deps-installer/test/install/sideEffects.ts:79`](https://github.com/pnpm/pnpm/blob/b4f8f47ac2/installing/deps-installer/test/install/sideEffects.ts#L79-L131). Hand-builds the `side_effects_maps_by_snapshot` entry with the exact cache key `BuildModules` will compute, asserts no `pnpm:lifecycle` event fires for `@pnpm.e2e/failing-postinstall`'s postinstall (i.e. the build was skipped — if the gate were broken the script would have run and the install would have failed with `LifecycleScript(_)`).
- `side_effects_cache_disabled_bypasses_the_gate` (negative pair): same overlay shape with `side_effects_cache: false`, expects the build to run and the install to fail with the underlying `exit 1` from the postinstall.
- `deps_graph::tests`: registry-resolution uses integrity verbatim, dependencies + optionalDependencies fold into children, snapshot without metadata is skipped.

The upstream test runs the install twice — first to populate the cache via the WRITE path, then to consume it. Pacquet doesn't have a WRITE path yet (#421 slice B), so the port hand-crafts the same in-memory state directly. The behavior under test is identical.

## Out of scope

- **WRITE path** — pacquet seeding the cache itself (re-CAFS post-build, calc diff, mutate index row). Tracked as slice (B) in #421.
- **Patch-file hash in the cache key** — depends on `patchedDependencies` (#397 item 9).
KSXGitHub pushed a commit that referenced this pull request May 13, 2026
Pull in 28 commits from upstream main, including the
`pacquet-npmrc` → `pacquet-config` rename (#420) plus features:
- supportedArchitectures + --cpu/--os/--libc (#456)
- frozen-lockfile (#442, #443, #447, #450)
- git-fetcher (#436 / #446, #451, #454)
- side-effects cache (#421 / #422, #423, #424)
- real-hoist + global-virtual-store (#432 / #438 / #444, #449, #452)
- patchedDependencies + allow-builds (#425, #427, #428)
- engine/platform installability (#434 / #439)

Conflicts resolved:
- `crates/npmrc/` files migrated under the renamed
  `crates/config/` directory; `Npmrc` → `Config` everywhere
  except `NpmrcAuth` (which keeps the `.npmrc`-domain name).
- `Config::current` reads the env-var DI generic `Api: EnvVar`
  for ${VAR}-substitution in `.npmrc`. Production turbofish in
  `cli_args.rs` is `Config::current::<RealApi, _, _, _, _>(...)`.
- Two-phase `NpmrcAuth::apply_*` retained so default-registry
  creds key at the yaml-resolved registry URL.
- New `Config::auth_headers` field plumbed through
  `install_package_by_snapshot`'s `DownloadTarballToStore`.
- Tests under `crates/config/src/workspace_yaml/tests.rs`
  pick up the new ParseYaml unit test added on this branch.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants