Conversation
…ndexKey (#436 §A) Adds the foundation upstream pnpm uses to address git-hosted packages in the store. Three pieces, all scoped to lockfile types and store-index keying: - `TarballResolution.gitHosted: Option<bool>`. Lockfiles produced by recent pnpm carry this field on tarballs sourced from a git host. Without it, pacquet's strict `deny_unknown_fields` deserialization rejected real lockfiles before the install dispatcher ever ran. Mirrors upstream's `TarballResolution.gitHosted` at lockfile/types/src/index.ts:88-107. - Back-fill on read: when `gitHosted` is absent and the tarball URL prefix- matches a known git host (`codeload.github.com`, `gitlab.com`, `bitbucket.org`) with a `tar.gz` substring, set it to `true` during deserialize. Mirrors upstream's `enrichGitHostedFlag` at lockfile/fs/src/lockfileFormatConverters.ts:158-168 so every downstream reader can rely on the typed field instead of URL pattern-matching at each site. - `git_hosted_store_index_key` + `pick_store_index_key` helpers. Git-hosted cache content depends on whether build scripts ran during fetch, so the cache key carries a `built` / `not-built` dimension that the integrity- only key would collapse. Mirrors upstream's `gitHostedStoreIndexKey` and `pickStoreIndexKey` at store/index/src/index.ts:60-95. No call sites switch yet — the git fetcher and dispatcher wiring follow in later patches for #436. Foundation only: no behavior change for non-git-hosted installs. The `UnsupportedResolution { resolution_kind: "git" }` errors at `install_package_by_snapshot.rs:110-115` and `create_virtual_store.rs:513-520` remain in place until the fetcher lands.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📜 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). (7)
🧰 Additional context used📓 Path-based instructions (1)**/*.rs📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧠 Learnings (2)📚 Learning: 2026-05-01T10:01:33.766ZApplied to files:
📚 Learning: 2026-05-07T23:19:08.272ZApplied to files:
🔇 Additional comments (4)
📝 WalkthroughWalkthroughTarball resolutions now include an optional ChangesGit-hosted tarball support
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull request overview
Adds foundational support for git-hosted package handling by extending lockfile resolution parsing and introducing store-index key helpers that match pnpm’s semantics, without yet wiring any install dispatcher behavior.
Changes:
- Extend
TarballResolutionwith an optionalgit_hosted(gitHosted) field and back-fill it on deserialize for known git-hosted tarball URL patterns. - Add
git_hosted_store_index_keyandpick_store_index_keyhelpers to generate pnpm-compatible store index keys that include the built/not-built dimension for git-hosted content. - Add unit tests covering the new lockfile field behavior and store-index key selection logic.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| crates/lockfile/src/resolution.rs | Adds git_hosted field and deserialization back-fill via a pnpm-mirroring URL check. |
| crates/lockfile/src/resolution/tests.rs | Adds coverage for gitHosted round-trip and back-fill behavior across supported URL shapes. |
| crates/store-dir/src/store_index.rs | Introduces git-hosted store index key helpers and pnpm-mirroring key selection logic. |
| crates/store-dir/src/store_index/tests.rs | Adds tests for git-hosted key formatting and pick_store_index_key routing rules. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #440 +/- ##
==========================================
+ Coverage 86.98% 87.03% +0.04%
==========================================
Files 93 93
Lines 6647 6671 +24
==========================================
+ Hits 5782 5806 +24
Misses 865 865 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Micro-Benchmark ResultsLinux |
Integrated-Benchmark Report (Linux)Scenario: Frozen Lockfile
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 2.08415776044,
"stddev": 0.06855070444357962,
"median": 2.09442016674,
"user": 2.65809982,
"system": 2.0945317199999995,
"min": 1.99396865824,
"max": 2.19249465924,
"times": [
2.1278053992399997,
1.99396865824,
2.19249465924,
2.15763870224,
1.9944598682399999,
2.0947036032399997,
2.09413673024,
2.0833011142399998,
2.1001732342399997,
2.00289563524
]
},
{
"command": "pacquet@main",
"mean": 2.0190263005399998,
"stddev": 0.047851310677944986,
"median": 2.0061498152399997,
"user": 2.7083759199999995,
"system": 2.0612744199999997,
"min": 1.97424926924,
"max": 2.14195766924,
"times": [
1.99809636724,
2.02507656424,
2.14195766924,
1.97424926924,
2.03574634224,
1.99088899724,
1.99059205524,
1.98804742724,
2.0142032632399998,
2.03140505024
]
},
{
"command": "pnpm",
"mean": 5.372065662540001,
"stddev": 0.06264884974928055,
"median": 5.34679472574,
"user": 8.589173719999998,
"system": 2.7593678199999996,
"min": 5.29948588424,
"max": 5.46933855124,
"times": [
5.32042362424,
5.35149826124,
5.40737119324,
5.34000023124,
5.44990017824,
5.29948588424,
5.46933855124,
5.34209119024,
5.4345636192399995,
5.3059838922399996
]
}
]
}Scenario: Frozen Lockfile (Hot Cache)
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 0.4838535728,
"stddev": 0.018670941551688944,
"median": 0.4850899066,
"user": 0.34285511999999996,
"system": 0.8879916800000001,
"min": 0.4613099491,
"max": 0.5148187601,
"times": [
0.5148187601,
0.4992555281,
0.5045250831000001,
0.46964376210000003,
0.4613099491,
0.4651146081,
0.4872508191,
0.4637651511,
0.4899230731,
0.4829289941
]
},
{
"command": "pacquet@main",
"mean": 0.48659735810000004,
"stddev": 0.02324181775345013,
"median": 0.4820189166,
"user": 0.34563422000000005,
"system": 0.8786749799999999,
"min": 0.4631197221,
"max": 0.5426225741,
"times": [
0.5426225741,
0.4911660331,
0.4750851021,
0.5021981701,
0.4754490451,
0.4888125371,
0.4631197221,
0.4746948821,
0.4642367271,
0.4885887881
]
},
{
"command": "pnpm",
"mean": 2.2311556266,
"stddev": 0.11449564543804464,
"median": 2.2532653260999997,
"user": 2.78809022,
"system": 1.3165672799999997,
"min": 2.0581139901,
"max": 2.4234360171,
"times": [
2.4234360171,
2.3355007591,
2.2596787641,
2.1643120530999997,
2.2901073281,
2.0592001660999997,
2.0581139901,
2.2648877010999997,
2.2094675991,
2.2468518880999997
]
}
]
} |
…ePackage (#436 §B+D) Builds on §A (#440). Adds a new `pacquet-git-fetcher` crate that handles `LockfileResolution::Git` snapshots during frozen-lockfile installs, plus the supporting `preparePackage` port. The install dispatcher now routes git resolutions through it instead of returning `UnsupportedResolution { resolution_kind: "git" }`. New crate `pacquet-git-fetcher`: - `GitFetcher` shells out to the system `git` (clone, or `init` + `fetch --depth 1` on hosts in `git_shallow_hosts`), checks out the pinned commit, verifies it via `rev-parse HEAD`, runs `preparePackage`, removes `.git`, computes a packlist, and imports the resulting file set into the CAS. Surfaces a friendly `GitNotFound` when `git` isn't on `PATH` — pacquet does not bundle it. Mirrors `fetching/git-fetcher/src/index.ts`. - `prepare_package` ports `exec/prepare-package/src/index.ts`. Reads the manifest, decides via `package_should_be_built`, honors an `AllowBuildFn` (the dispatcher adapts pacquet's `AllowBuildPolicy` into this shape), runs `<pm>-install` plus any `prepublish` / `prepack` / `publish` scripts via the existing `pacquet_executor::run_lifecycle_hook`, then deletes `node_modules`. Emits the upstream error codes `GIT_DEP_PREPARE_NOT_ALLOWED`, `ERR_PNPM_PREPARE_PACKAGE`, and `INVALID_PATH`. - `detect_preferred_pm` sniffs the cloned tree for a lockfile to pick the install pm (`pnpm-lock.yaml` → pnpm, `yarn.lock` → yarn, etc.), defaulting to npm. Workspace-root walking is deferred — git-hosted snapshots almost always ship a lockfile at the repo root. - A minimal `packlist` honors the manifest's `files` field with a tiny `*` / `**` / `?` glob matcher, always-includes the README / LICENSE / package.json set, and always-excludes `.git`, `node_modules`, lockfiles for sibling pms, and common cruft. Full `.npmignore` / `.gitignore` semantics and `bundleDependencies` walking are deferred and tracked in module docs; `bundleDependencies` emits a `warn!` so the gap is visible. Wiring in `pacquet-package-manager`: - `InstallPackageBySnapshot` now matches `LockfileResolution::Git` and runs the fetcher in place of `DownloadTarballToStore`. The helper `tarball_url_and_integrity` factors out the per-resolution branch so each variant builds its own `cas_paths`. - Threads `&AllowBuildPolicy` from `InstallFrozenLockfile` → through `CreateVirtualStore` → into `InstallPackageBySnapshot`, computing the policy once per install instead of per snapshot. - `snapshot_cache_key` returns `Ok(None)` for `Git` resolutions so they go cold-batch in this PR. Warm prefetch for git-hosted slots is a follow-up alongside §C. Supporting changes: - `GitResolution.path: Option<String>` (§A back-fill) so a lockfile pinning a sub-directory of a git-hosted monorepo deserializes without tripping `deny_unknown_fields`. - `Config::git_shallow_hosts` with pnpm's default list (`github.com` / `gist.github.com` / `gitlab.com` / `bitbucket.com` / `bitbucket.org`) and `pnpm-workspace.yaml` override support. - `pacquet_executor::run_lifecycle_hook` is now `pub` so the preparePackage port can dispatch by arbitrary stage name without going through `run_postinstall_hooks`'s pre/install/post triple. Out of scope (follow-ups for #436): - §C — git-hosted *tarball* fetcher (the `gitHosted: true` shape). - §E — full integration-test matrix (`plans/TEST_PORTING.md` 563-572). This PR ships the seven crate-level tests against a local bare repo that prove the path works end-to-end. - Warm prefetch for git resolutions. The fetcher writes nothing to `index.db` today, so a second install re-clones — slow but correct. - Full `npm-packlist` semantics (`.npmignore`, gitignore layering, `bundleDependencies` walking).
…ePackage (#436 §B+D) (#446) Builds on §A (#440). Adds a new `pacquet-git-fetcher` crate that handles `LockfileResolution::Git` snapshots during frozen-lockfile installs, plus the supporting `preparePackage` port. The install dispatcher now routes git resolutions through it instead of returning `UnsupportedResolution { resolution_kind: "git" }`. ### New crate `pacquet-git-fetcher` - **`GitFetcher`** shells out to the system `git` (clone, or `init` + `fetch --depth 1` on hosts in `git_shallow_hosts`), checks out the pinned commit, verifies via `rev-parse HEAD`, runs `preparePackage`, removes `.git`, computes a packlist, and imports the resulting file set into the CAS. Surfaces a friendly `GitNotFound` when `git` isn't on `PATH` — pacquet does not bundle it. Mirrors [`fetching/git-fetcher/src/index.ts`](https://github.com/pnpm/pnpm/blob/94240bc046/fetching/git-fetcher/src/index.ts). - **`prepare_package`** ports [`exec/prepare-package/src/index.ts`](https://github.com/pnpm/pnpm/blob/94240bc046/exec/prepare-package/src/index.ts). Reads the manifest, decides via `package_should_be_built`, honors an `AllowBuildFn` (the dispatcher adapts pacquet's `AllowBuildPolicy` into this shape), runs `<pm>-install` plus any `prepublish` / `prepack` / `publish` scripts via the existing `pacquet_executor::run_lifecycle_hook`, then deletes `node_modules`. Emits the upstream error codes `GIT_DEP_PREPARE_NOT_ALLOWED`, `ERR_PNPM_PREPARE_PACKAGE`, `INVALID_PATH`. - **`detect_preferred_pm`** sniffs the cloned tree for a lockfile to pick the install pm. Workspace-root walking deferred. - **Minimal `packlist`** honors the manifest's `files` field with a tiny `*` / `**` / `?` glob matcher (`?` and single `*` reject `/`; `**` matches across directories), always-includes the README / LICENSE / package.json set, and always-excludes `.git`, `node_modules`, lockfiles for sibling pms, and common cruft. Full `.npmignore` / `.gitignore` semantics and `bundleDependencies` walking are deferred and tracked in module docs. ### Wiring in `pacquet-package-manager` - `InstallPackageBySnapshot` now matches `LockfileResolution::Git` and runs the fetcher in place of `DownloadTarballToStore`. - `&AllowBuildPolicy` is computed once per install in `InstallFrozenLockfile` and threaded through `CreateVirtualStore` → `InstallPackageBySnapshot`. - `snapshot_cache_key` returns `Ok(None)` for `Git` resolutions so they cold-batch. Warm prefetch for git-hosted slots is a follow-up alongside §C. ### Supporting changes - `GitResolution.path: Option<String>` so a lockfile pinning a sub-directory of a git-hosted monorepo deserializes without tripping `deny_unknown_fields`. - `Config::git_shallow_hosts` with pnpm's default list + `pnpm-workspace.yaml` override. - `pacquet_executor::run_lifecycle_hook` is now `pub` so the preparePackage port can dispatch by arbitrary stage name. ## Out of scope (follow-ups for #436) - §C — git-hosted *tarball* fetcher (the `gitHosted: true` shape). - §E — full integration-test matrix (`plans/TEST_PORTING.md` 563-572). This PR ships crate-level tests against a local bare repo that prove the path works end-to-end. - Warm prefetch for git resolutions (re-clones on every install today — slow but correct). - Full `npm-packlist` semantics (`.npmignore`, gitignore layering, `bundleDependencies` walking).
Summary
Foundation for git-hosted package install (#436 Section A). Three small,
self-contained changes:
TarballResolution.gitHosted: Option<bool>incrates/lockfile.Recent pnpm lockfiles ship this field on tarballs sourced from a git
host. Without it, pacquet's
deny_unknown_fieldsstrictness rejectedreal lockfiles before the install dispatcher ran — a silent correctness
gap independent of the rest of Install git-hosted packages from
pnpm-lock.yaml(frozen-lockfile) #436. Mirrors upstream'sTarballResolution.gitHosted.Back-fill on read. When
gitHostedis absent but the tarball URLprefix-matches a known git host (
codeload.github.com,gitlab.com,bitbucket.org) with atar.gzsubstring, set it totrueduringdeserialize. Mirrors upstream's
enrichGitHostedFlagso every downstream reader can rely on the typed field instead of
matching URL prefixes at each site.
git_hosted_store_index_key+pick_store_index_keyhelpers inpacquet-store-dir. Git-hosted cache content depends on whether thebuild ran during fetch, so the cache key carries a
built/not-builtdimension that the integrity-only key would collapse. Mirrors
upstream's
gitHostedStoreIndexKey/pickStoreIndexKey.No call sites switch yet — the dispatcher continues to error with
UnsupportedResolution { resolution_kind: "git" }until the git fetcherlands.
Out of scope (follow-ups for #436)
pacquet-git-fetchercrate,git_shallow_hostsconfig,dispatcher wiring at
install_package_by_snapshot.rs:110-115andcreate_virtual_store.rs:513-520.preparePackageport wiring into the existing lifecyclerunner.
Test plan
cargo nextest run -p pacquet-lockfile— gitHosted round-trip,back-fill from codeload/gitlab/bitbucket URLs, no back-fill from
registry URLs, no back-fill from a github URL without
tar.gz.cargo nextest run -p pacquet-store-dir—built/not-builtkey, route selection by
git_hostedflag, route selection bymissing integrity.
just readytaplo format --checkjust dylintWritten by an agent (Claude Code, claude-opus-4-7).
Summary by CodeRabbit
Bug Fixes
New Features