Conversation
|
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 (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📜 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). (8)
🧰 Additional context used📓 Path-based instructions (2)pacquet/**/*.rs📄 CodeRabbit inference engine (pacquet/AGENTS.md)
Files:
pacquet/**/tests/**/*.rs📄 CodeRabbit inference engine (pacquet/AGENTS.md)
Files:
🧠 Learnings (3)📚 Learning: 2026-05-20T19:40:55.051ZApplied to files:
📚 Learning: 2026-05-22T00:08:44.646ZApplied to files:
📚 Learning: 2026-05-20T23:07:58.444ZApplied to files:
🔇 Additional comments (1)
📝 WalkthroughWalkthroughThis PR implements resolution-time tarball fetching and integrity computation for HTTP/HTTPS tarball dependencies. The tarball fetch/extract pipeline supports verifying known integrity at install time or computing SHA-512 during resolution. A new TarballFetchContext wires store/index/cache/auth/retry into the resolver so resolution can fetch/extract tarballs and capture bundled manifests. Fresh-lockfile install reorders initialization to share store-index handles; name/version fallbacks derive missing metadata from fetched manifests. ChangesResolution-time tarball fetching and integrity preservation
Sequence Diagram(s)sequenceDiagram
participant Installer
participant Resolver
participant TarballResolver
participant FetchForResolution as FetchTarballForResolution
participant TarballLib as tarball::fetch_and_extract_with_retry
participant StoreIndex
participant MemCache
Installer->>Resolver: build resolver chain (with TarballFetchContext)
Resolver->>TarballResolver: resolve(url)
TarballResolver->>FetchForResolution: run(expected_integrity=None, auth, store handles)
FetchForResolution->>TarballLib: fetch_and_extract_with_retry
TarballLib-->>FetchForResolution: (Integrity, CAS paths, manifest)
FetchForResolution->>StoreIndex: queue entry (name@version)
FetchForResolution->>MemCache: warm cache (url -> cas paths)
FetchForResolution-->>TarballResolver: ResolvedTarball(integrity, manifest)
TarballResolver-->>Resolver: ResolveResult(integrity, manifest)
Resolver-->>Installer: resolved dependency with integrity/manifest
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related issues
Possibly related PRs
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)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Micro-Benchmark ResultsLinux |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #12076 +/- ##
==========================================
+ Coverage 87.12% 87.19% +0.06%
==========================================
Files 243 243
Lines 26680 26798 +118
==========================================
+ Hits 23246 23366 +120
+ Misses 3434 3432 -2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Integrated-Benchmark Report (Linux)Scenario: Isolated linker: fresh restore, cold cache + cold store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 2.0762402314400004,
"stddev": 0.12073024496585608,
"median": 2.04945675714,
"user": 2.6042301799999996,
"system": 3.3302193599999996,
"min": 1.99416201114,
"max": 2.4105446721400003,
"times": [
2.0423394261400003,
2.05848998114,
2.0186868061400003,
1.99906101314,
1.99416201114,
2.05657408814,
2.4105446721400003,
2.07034330514,
2.0750705161400003,
2.03713049514
]
},
{
"command": "pacquet@main",
"mean": 2.01541480234,
"stddev": 0.020070360409999873,
"median": 2.0196317821400003,
"user": 2.66518768,
"system": 3.32857486,
"min": 1.9819707611400001,
"max": 2.04343035214,
"times": [
2.0010366211400004,
2.0227502371400004,
2.01651332714,
1.9819707611400001,
2.04343035214,
2.02950844314,
2.00434080514,
2.0270574321400003,
2.0361086251400002,
1.99143141914
]
}
]
}Scenario: Isolated linker: fresh restore, hot cache + hot store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 0.6586396517199999,
"stddev": 0.03427436674910925,
"median": 0.6463502214200001,
"user": 0.3566351,
"system": 1.32091662,
"min": 0.63205356942,
"max": 0.74879009342,
"times": [
0.74879009342,
0.63960582742,
0.64894263042,
0.6437578124200001,
0.64031168242,
0.6566287364200001,
0.6419367274200001,
0.6793481464200001,
0.63205356942,
0.6550212914200001
]
},
{
"command": "pacquet@main",
"mean": 0.6676749488200001,
"stddev": 0.02079440118679335,
"median": 0.66305271692,
"user": 0.3644737,
"system": 1.33240012,
"min": 0.6442812214200001,
"max": 0.7165735054200001,
"times": [
0.7165735054200001,
0.65939691542,
0.6495215724200001,
0.6755957854200001,
0.68261240842,
0.65747339542,
0.65499733542,
0.6442812214200001,
0.6695888304200001,
0.66670851842
]
}
]
}Scenario: Isolated linker: fresh install, cold cache + cold store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 2.34954648394,
"stddev": 0.03430545871879044,
"median": 2.3473935702400004,
"user": 3.8219358799999994,
"system": 3.13737552,
"min": 2.29900723824,
"max": 2.41067265524,
"times": [
2.3436725222400003,
2.35623044724,
2.3464191482400003,
2.41067265524,
2.34836799224,
2.3670729922400002,
2.30081704324,
2.29900723824,
2.38648594224,
2.3367188582400003
]
},
{
"command": "pacquet@main",
"mean": 2.37786590494,
"stddev": 0.034877463866717376,
"median": 2.38954264474,
"user": 3.8093407800000003,
"system": 3.16118672,
"min": 2.2992680232400002,
"max": 2.41449695924,
"times": [
2.34334780924,
2.2992680232400002,
2.41449695924,
2.41022902524,
2.36404311324,
2.40109647924,
2.3759144772400003,
2.39115005124,
2.3879352382400003,
2.39117787324
]
}
]
}Scenario: Isolated linker: fresh install, hot cache + hot store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 1.53686065292,
"stddev": 0.03003544845807274,
"median": 1.53083083252,
"user": 1.7323798,
"system": 1.8583711400000003,
"min": 1.51133117002,
"max": 1.6014741940200001,
"times": [
1.57768380102,
1.5350995630200002,
1.5130792880200001,
1.51853832702,
1.53214674102,
1.6014741940200001,
1.5295149240199999,
1.51133117002,
1.51149335302,
1.53824516802
]
},
{
"command": "pacquet@main",
"mean": 1.5045975943200003,
"stddev": 0.01944251852850025,
"median": 1.5020156190200002,
"user": 1.7128933999999998,
"system": 1.83995754,
"min": 1.47566352402,
"max": 1.5345241730199999,
"times": [
1.50257432702,
1.49071280102,
1.5345241730199999,
1.53178631102,
1.47566352402,
1.49118310902,
1.5022976250200002,
1.49162731302,
1.50173361302,
1.52387314702
]
}
]
} |
|
Bencher Report| Branch | pr/12076 |
| Testbed | pacquet |
Click to view all benchmark results
| Benchmark | Latency | Benchmark Result milliseconds (ms) (Result Δ%) | Upper Boundary milliseconds (ms) (Limit %) |
|---|---|---|---|
| isolated-linker.fresh-install.cold-cache.cold-store | 📈 view plot 🚷 view threshold | 2,349.55 ms(-1.49%)Baseline: 2,385.11 ms | 2,862.14 ms (82.09%) |
| isolated-linker.fresh-install.hot-cache.hot-store | 📈 view plot 🚷 view threshold | 1,536.86 ms(+1.34%)Baseline: 1,516.56 ms | 1,819.87 ms (84.45%) |
| isolated-linker.fresh-restore.cold-cache.cold-store | 📈 view plot 🚷 view threshold | 2,076.24 ms(-1.95%)Baseline: 2,117.57 ms | 2,541.08 ms (81.71%) |
| isolated-linker.fresh-restore.hot-cache.hot-store | 📈 view plot 🚷 view threshold | 658.64 ms(-2.76%)Baseline: 677.33 ms | 812.79 ms (81.03%) |
A direct dependency whose specifier is a remote (non-registry) http(s) tarball URL (e.g. "foo": "https://cdn.example.com/foo-1.0.0.tgz") panicked while building the lockfile: TarballResolver returned no name/version/integrity (those live in the tarball's package.json), so the dep path was just the URL and the importer-dep-path parse hit MissingSuffix. pnpm learns name/version/integrity in the package-requester after the fetch; pacquet builds the lockfile before the install pass, so the TarballResolver now fetches the tarball during resolution: it downloads, computes the sha512 integrity, extracts to the store, and reads the bundled manifest, warming the shared mem cache (keyed by URL) so the install pass reuses the extraction without a second download. name_ver stays None so the depPath is name@<url>, matching pnpm; the name_ver-dependent call sites fall back to the manifest. Flips the gated regression test (its premise was inverted: the test registry binds to 127.0.0.1, so the tarball host must be localhost to miss the registry prefix and reach TarballResolver). Refs #12053. Co-authored-by: Claude <noreply@anthropic.com>
Review Summary by QodoSupport remote https-tarball direct dependencies with resolve-time fetch
WalkthroughsDescription• Implement end-to-end support for remote https-tarball direct dependencies • TarballResolver now fetches tarball during resolution to compute integrity • Extract name/version/integrity from tarball's package.json for lockfile • Warm shared mem cache so install pass reuses extraction without re-downloading • Fix regression test by correcting tarball host URL routing logic Diagramflowchart LR
A["Remote tarball URL"] -->|TarballResolver| B["Fetch during resolution"]
B -->|Download + extract| C["Compute sha512 integrity"]
C -->|Read manifest| D["Extract name/version"]
D -->|Warm mem cache| E["Lockfile builder"]
E -->|Install pass| F["Reuse extraction"]
File Changes1. pacquet/crates/cli/tests/tarball_url_dependency.rs
|
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pacquet/crates/resolving-tarball-resolver/src/tarball_resolver.rs (1)
89-94:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAttach auth to the HEAD preflight.
When
fetch_contextis present, the new resolve-time GET is authenticated, but this HEAD probe still isn't. Private tarball URLs will fail here beforeFetchTarballForResolutionever gets a chance to run, so the new direct-tarball flow remains broken for authenticated hosts.Suggested fix
- let client = self.http_client.acquire_for_url(&normalized_bare_specifier).await; - let response = client - .head(&normalized_bare_specifier) - .send() + let client = self.http_client.acquire_for_url(&normalized_bare_specifier).await; + let mut request = client.head(&normalized_bare_specifier); + if let Some(value) = self + .fetch_context + .as_ref() + .and_then(|ctx| ctx.auth_headers.for_url(&normalized_bare_specifier)) + { + request = request.header("authorization", value); + } + let response = request + .send() .await .map_err(|err| Box::new(err) as ResolveError)?;🤖 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 `@pacquet/crates/resolving-tarball-resolver/src/tarball_resolver.rs` around lines 89 - 94, The HEAD preflight created with self.http_client.acquire_for_url(&normalized_bare_specifier) is not using the resolution fetch_context auth, so private tarballs fail before FetchTarballForResolution; modify the HEAD probe in tarball_resolver.rs to attach the same authentication used for the later GET when fetch_context is present (e.g., pull auth headers or credentials from fetch_context and add them to the client/request before calling client.head(&normalized_bare_specifier).send()). Ensure you still map errors to ResolveError as before.
🤖 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 `@pacquet/crates/cli/tests/tarball_url_dependency.rs`:
- Around line 41-49: The helper currently scans until "snapshots:" which can
cross into the next package section; update the stopping condition so the
iterator stops at the next sibling "packages:" entry as well. Specifically, in
the chain that starts with lockfile.lines().skip_while(|line|
!is_header(line)).take_while(...).find_map(...), change the take_while predicate
to also break when line.trim_start().starts_with("packages:") (e.g.
.take_while(|line| { let t = line.trim_start(); !t.starts_with("snapshots:") &&
!t.starts_with("packages:") })). This ensures the search for "integrity:" for
the package identified by is_header ends at the next packages: sibling.
In `@pacquet/crates/package-manager/src/install_with_fresh_lockfile.rs`:
- Around line 509-527: The store-index writer join result from TarballResolver's
TarballFetchContext (store_index_writer) can yield Ok(Err(_)) which is currently
ignored on the --lockfile-only path; update the cleanup that awaits the
store_index_writer join (the code that currently only checks join failure) to
also match and handle the inner Err variant: when the join returns Ok(Err(e))
log or propagate that error (same warning/handling used by the materializing
path) so store-index flush failures are not silently dropped; locate
TarballResolver / TarballFetchContext and the cleanup awaiting
store_index_writer to implement this additional match arm.
---
Outside diff comments:
In `@pacquet/crates/resolving-tarball-resolver/src/tarball_resolver.rs`:
- Around line 89-94: The HEAD preflight created with
self.http_client.acquire_for_url(&normalized_bare_specifier) is not using the
resolution fetch_context auth, so private tarballs fail before
FetchTarballForResolution; modify the HEAD probe in tarball_resolver.rs to
attach the same authentication used for the later GET when fetch_context is
present (e.g., pull auth headers or credentials from fetch_context and add them
to the client/request before calling
client.head(&normalized_bare_specifier).send()). Ensure you still map errors to
ResolveError as before.
🪄 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: 36e263d6-7cab-4feb-b2c1-d26900fd35e9
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (10)
pacquet/crates/cli/tests/tarball_url_dependency.rspacquet/crates/package-manager/src/dependencies_graph_to_lockfile.rspacquet/crates/package-manager/src/install_package_from_registry.rspacquet/crates/package-manager/src/install_with_fresh_lockfile.rspacquet/crates/resolving-tarball-resolver/Cargo.tomlpacquet/crates/resolving-tarball-resolver/src/lib.rspacquet/crates/resolving-tarball-resolver/src/tarball_resolver.rspacquet/crates/resolving-tarball-resolver/src/tarball_resolver/tests.rspacquet/crates/tarball/src/lib.rspacquet/crates/tarball/src/tests.rs
📜 Review details
🧰 Additional context used
📓 Path-based instructions (3)
pacquet/**/*.rs
📄 CodeRabbit inference engine (pacquet/AGENTS.md)
pacquet/**/*.rs: Log emissions are part of matching pnpm — when porting a function that firespnpm:<channel>events throughglobalLogger,logger.debug(...), orstreamParser.write(...), mirror the call site, payload, and ordering so@pnpm/cli.default-reporterparses pacquet's NDJSON the same way
Declare a newtype wrapper for branded string types instead of collapsing the brand into a plainStringor&strin Rust
If upstream TypeScript always validates before construction of a branded string, validate in the Rust wrapper too viaTryFrom<String>and/orFromStrand do not provide an infallible public constructor
If upstream TypeScript never validates a branded string, just brand for type-safety in Rust by exposing an infallibleFrom<String>constructor
If upstream TypeScript occasionally constructs a branded string without validation, exposefrom_str_uncheckedin Rust as an escape hatch alongside the validating constructor
Match upstream serde behavior for branded strings crossing JSON, YAML, or INI boundaries by using#[serde(try_from = "String")]for deserialization and#[serde(into = "String")]for serialization
Derive simple conversions for branded strings using#[derive(derive_more::From)]and#[derive(derive_more::Into)]instead of handwritingimplblocks; use manualimplonly when conversion needs custom logic
Model TypeScript string literal unions (like'auto' | 'always' | 'never') as Rustenums instead of newtype wrappers, since the set of valid values is closed
Treat TypeScript string template literal types (like`${string}@${string}`) the same as branded string types in Rust, using a newtype wrapper with validation
Follow the code style guide inCODE_STYLE_GUIDE.md— imports, modules, naming, ownership and borrowing, parameter type selection, trait bounds, pattern matching,pipe-trait, error handling, test layout, and cloning ofArcandRc
Choose owned vs. borrowed parameters to minimize copies; widen to t...
Files:
pacquet/crates/resolving-tarball-resolver/src/tarball_resolver/tests.rspacquet/crates/package-manager/src/install_package_from_registry.rspacquet/crates/resolving-tarball-resolver/src/lib.rspacquet/crates/package-manager/src/dependencies_graph_to_lockfile.rspacquet/crates/resolving-tarball-resolver/src/tarball_resolver.rspacquet/crates/tarball/src/tests.rspacquet/crates/package-manager/src/install_with_fresh_lockfile.rspacquet/crates/cli/tests/tarball_url_dependency.rspacquet/crates/tarball/src/lib.rs
pacquet/**/Cargo.toml
📄 CodeRabbit inference engine (pacquet/AGENTS.md)
pacquet/**/Cargo.toml: Check whether the workspace already depends on something suitable in[workspace.dependencies]in the rootCargo.tomlbefore adding a new dependency
Keep dependencies at the right level — add a new dependency to the specific crate that needs it, not to the workspace root or shared crate unless multiple crates depend on it
Files:
pacquet/crates/resolving-tarball-resolver/Cargo.toml
pacquet/**/tests/**/*.rs
📄 CodeRabbit inference engine (pacquet/AGENTS.md)
pacquet/**/tests/**/*.rs: Prefer real fixtures over dependency-injection seams — usetempfile::TempDir, the mocked registry, or integration tests spawning the actual binary for happy paths and most error paths; use the DI seam only for filesystem error kinds, deterministic time, shared process-global state, or external-service happy paths
Follow test-logging guidance — log before non-assert_eq!assertions,dbg!complex structures, skip logging for simple scalarassert_eq!
Tests must not be tolerant of missing build/runtime environment by silently returning early when a tool isn't found — let the test panic when a required tool is absent
Prefer#[cfg_attr(target_os = "windows", ignore = "...")](or matching#[cfg(unix)]gates) over runtime probe-and-skip helpers for platform-locked tools
Port relevant pnpm tests to Rust tests whenever they translate when porting behavior from pnpm
Use snapshot tests withinstaand carefully review diffs when intentional changes alter snapshots; accept withcargo insta reviewonly after careful review
Tests that need the mocked registry should startpnprthroughpacquet-testing-utils;cargo test/cargo nextest runshould not require a separatejust registry-mock launchstep
Files:
pacquet/crates/cli/tests/tarball_url_dependency.rs
🧠 Learnings (3)
📚 Learning: 2026-05-20T19:40:55.051Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11774
File: pacquet/crates/resolving-deps-resolver/src/resolve_peers.rs:0-0
Timestamp: 2026-05-20T19:40:55.051Z
Learning: In the pacquet Rust code, ensure the semver implementation uses the `node-semver` crate (not `nodejs-semver`). `node-semver`’s public API does not include a `satisfies_with_prerelease`-style method; prerelease-tolerant matching should be implemented inline by first calling `Range::satisfies`, and when it rejects a prerelease version, retry matching against a stripped `MAJOR.MINOR.PATCH` base of the prerelease version.
Applied to files:
pacquet/crates/resolving-tarball-resolver/src/tarball_resolver/tests.rspacquet/crates/package-manager/src/install_package_from_registry.rspacquet/crates/resolving-tarball-resolver/src/lib.rspacquet/crates/package-manager/src/dependencies_graph_to_lockfile.rspacquet/crates/resolving-tarball-resolver/src/tarball_resolver.rspacquet/crates/tarball/src/tests.rspacquet/crates/package-manager/src/install_with_fresh_lockfile.rspacquet/crates/cli/tests/tarball_url_dependency.rspacquet/crates/tarball/src/lib.rs
📚 Learning: 2026-05-22T00:08:44.646Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11837
File: pacquet/crates/resolving-npm-resolver/src/pick_package.rs:33-51
Timestamp: 2026-05-22T00:08:44.646Z
Learning: In the pnpm/pnpm repo’s pacquet Rust crates, do not flag Unicode ellipsis characters (U+2026, `…`) in Rust doc comments (`///` / `/** */`) as a lint violation. The pacquet crate’s `dylint.toml` only enables `perfectionist::derive_ordering`, and the Dylint `unicode-ellipsis` rule is not enabled for this project—so `…` in doc comments is an intentional, repo-consistent style.
Applied to files:
pacquet/crates/resolving-tarball-resolver/src/tarball_resolver/tests.rspacquet/crates/package-manager/src/install_package_from_registry.rspacquet/crates/resolving-tarball-resolver/src/lib.rspacquet/crates/package-manager/src/dependencies_graph_to_lockfile.rspacquet/crates/resolving-tarball-resolver/src/tarball_resolver.rspacquet/crates/tarball/src/tests.rspacquet/crates/package-manager/src/install_with_fresh_lockfile.rspacquet/crates/cli/tests/tarball_url_dependency.rspacquet/crates/tarball/src/lib.rs
📚 Learning: 2026-05-20T23:07:58.444Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11784
File: pacquet/crates/resolving-deps-resolver/src/hoist_peers.rs:120-133
Timestamp: 2026-05-20T23:07:58.444Z
Learning: When reviewing code in this pacquet Rust port, follow the upstream pnpm compatibility rule: only match pnpm’s behavior exactly. Do not propose review changes that intentionally deviate from pnpm’s documented/observed behavior, even if pnpm appears buggy. If you identify a real bug in pnpm behavior, the review should prioritize fixing it upstream in pnpm first, and avoid implementing a pnpm-behavior workaround here unless the same fix has already landed upstream.
Applied to files:
pacquet/crates/resolving-tarball-resolver/src/tarball_resolver/tests.rspacquet/crates/package-manager/src/install_package_from_registry.rspacquet/crates/resolving-tarball-resolver/src/lib.rspacquet/crates/package-manager/src/dependencies_graph_to_lockfile.rspacquet/crates/resolving-tarball-resolver/src/tarball_resolver.rspacquet/crates/tarball/src/tests.rspacquet/crates/package-manager/src/install_with_fresh_lockfile.rspacquet/crates/cli/tests/tarball_url_dependency.rspacquet/crates/tarball/src/lib.rs
🔇 Additional comments (4)
pacquet/crates/package-manager/src/dependencies_graph_to_lockfile.rs (1)
17-20: LGTM!Also applies to: 337-364
pacquet/crates/package-manager/src/install_package_from_registry.rs (1)
128-133: LGTM!Also applies to: 224-239
pacquet/crates/package-manager/src/install_with_fresh_lockfile.rs (1)
37-38: LGTM!Also applies to: 454-483, 1325-1359
pacquet/crates/cli/tests/tarball_url_dependency.rs (1)
1-22: LGTM!Also applies to: 33-40, 61-117
…ter-error/test handling Address review feedback on #12076: - Attach auth headers to the TarballResolver HEAD preflight so private tarball hosts aren't rejected before FetchTarballForResolution runs. - Handle Ok(Err(_)) from the store-index writer task on the --lockfile-only path, mirroring the materializing path. - Stop package_integrity at the next sibling lockfile entry so a tarball entry that lost its integrity can't borrow another package's.
|
Addressed the outside-diff finding on the HEAD preflight in ce5214c: The two inline findings (test-helper sibling scan, lockfile-only writer-error handling) are addressed in the same commit; replies are on their threads. Verified: Written by an agent (Claude Code, claude-opus-4-8). |
What
Implements end-to-end support in pacquet for remote (non-registry)
http(s)-tarball direct dependencies — e.g."foo": "https://cdn.example.com/foo-1.0.0.tgz". Fixes #12053.The bug
A direct dependency whose specifier is a non-registry tarball URL panicked while building the lockfile:
TarballResolverreturned noname/version/integrity(those live in the tarball'spackage.json), so the dep path was just the URL (no@) and the importer-dep-path parse hitMissingSuffix. (A registry-host tarball URL is claimed by the npm resolver instead and was already fine.)The fix
pnpm learns name/version/integrity in the package-requester after the fetch, but pacquet builds the lockfile before the install pass. So the
TarballResolvernow fetches the tarball during resolution: it downloads, computes the sha512 integrity, extracts to the store, and reads the bundled manifest, warming the sharedMemCache(keyed by URL) so the install pass reuses the extraction without a second download (one download total).name_verstaysNoneso the depPath isname@<url>— matching pnpm (seeinstalling/deps-installer/test/lockfile.ts, "packages installed via tarball URL … are normalized"). Thename_ver-dependent call sites fall back to the manifest (mirroring pnpm'sgetManifestFromResponse).Changes (pacquet-only)
crates/tarball— new publicFetchTarballForResolutionentry point that downloads + computes integrity (refactored the integrity step offetch_and_extract_onceto verify-or-compute), extracts to the store, reads the manifest, queues the store-index row, and warmsMemCache.TarballResolver— fetches during resolution when given aTarballFetchContext; legacy HEAD-only shape when none (unit tests).install_with_fresh_lockfile.rs— store handles opened before resolver construction; context wired in.name_verwas required (install_package_from_registry, prefetch-key collector, and — scoped to remote http tarballs only —real_namein the lockfile builder, leavingfile:/git deps untouched).remote_tarball_integrity_survives_unrelated_install. Its original premise was inverted: the test registry binds to127.0.0.1, so its.replace("localhost", "127.0.0.1")was a no-op and routed through the npm resolver — corrected so the tarball host islocalhost(registry stays127.0.0.1) andTarballResolveris actually exercised.Verification
name@<url>key → unrelatedaddpreserves it verbatim →--frozen-lockfilesucceeds). Confirmed it fails when the integrity is dropped.pacquet-cli(152) andpacquet-package-manager(370) suites pass.cargo check/clippy -D warnings/fmt/taplo/cargo doc -D warnings/typos/dylintall clean.Notes / out of scope
version:field on URL-keyedpackages:entries, and its YAML emitter quotes keys containing://+:port(pnpm leaves them bare). Worth a follow-up.--lockfile-only, a remote tarball direct dep now writes CAS files to the store (the lockfile needs the integrity, which requires the download). Minor; possible follow-up.Written by an agent (Claude Code, claude-opus-4-8).
Summary by CodeRabbit
New Features
Bug Fixes
Improvements