feat(pacquet/config): port peersSuffixMaxLength setting#12026
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds peersSuffixMaxLength support across config defaults/env/workspace parsing, exposes Config.peers_suffix_max_length, rounds the setting through lockfile serialization, extends freshness checks and optimistic-repeat fast-path, threads the value into resolver/lockfile generation, and adds tests. ChangespeersSuffixMaxLength Configuration and Validation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
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 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 |
Contributor
Micro-Benchmark ResultsLinux |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #12026 +/- ##
==========================================
+ Coverage 88.38% 88.41% +0.03%
==========================================
Files 230 230
Lines 29116 29177 +61
==========================================
+ Hits 25734 25798 +64
+ Misses 3382 3379 -3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Review Summary by QodoPort peersSuffixMaxLength setting end-to-end to match pnpm
WalkthroughsDescription• Port peersSuffixMaxLength setting end-to-end matching pnpm behavior • Read from pnpm-workspace.yaml, global config.yaml, and PNPM_CONFIG_PEERS_SUFFIX_MAX_LENGTH • Thread into resolver to cap peer-suffix rendering at configured value • Write to lockfile settings.peersSuffixMaxLength only when non-default • Detect drift via StalenessReason::PeersSuffixMaxLengthChanged in freshness checks • Invalidate optimistic-repeat-install fast path on peersSuffixMaxLength changes Diagramflowchart LR
Config["Config reads peersSuffixMaxLength<br/>default 1000"]
Resolver["ResolveImporterOptions<br/>threads to resolver"]
PeerSuffix["Peer-suffix capped<br/>at configured value"]
Lockfile["Lockfile settings<br/>serializes when non-default"]
Freshness["check_lockfile_settings<br/>detects drift"]
OptimisticRepeat["optimistic_repeat_install<br/>invalidates on change"]
Config -->|"env > yaml > default"| Resolver
Resolver -->|"peers_suffix_max_length"| PeerSuffix
PeerSuffix -->|"strip-on-default"| Lockfile
Lockfile -->|"compare recorded vs current"| Freshness
Freshness -->|"PeersSuffixMaxLengthChanged"| OptimisticRepeat
File Changes1. pacquet/crates/config/src/defaults.rs
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/package-manager/src/install_with_fresh_lockfile.rs`:
- Line 585: The code currently does an unchecked narrowing cast of
config.peers_suffix_max_length (u64) to usize when building
ResolveImporterOptions (peers_suffix_max_length: config.peers_suffix_max_length
as usize); replace this with a checked conversion using
usize::try_from(config.peers_suffix_max_length) and handle the Result (e.g.,
return an appropriate error if conversion fails or the value is out of range) so
the resolver behavior can't silently change on 32-bit targets; update the
construction site of ResolveImporterOptions (where peers_suffix_max_length is
set) to perform the try_from and propagate or map the error.
In `@pacquet/crates/package-manager/src/optimistic_repeat_install.rs`:
- Line 277: The code unsafely casts Config.peers_suffix_max_length (u64) to u32
with as u32 which can truncate large values; replace the direct cast in the
workspace state construction (the peers_suffix_max_length field) with a safe
conversion such as using u32::try_from(Config.peers_suffix_max_length).ok() (or
equivalent checked conversion) so the result is an Option<u32> that is None on
overflow rather than silently truncated.
🪄 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: 9cb076c4-245a-4a10-9be7-d9c797634ba8
📒 Files selected for processing (17)
pacquet/crates/config/src/defaults.rspacquet/crates/config/src/env_overlay.rspacquet/crates/config/src/lib.rspacquet/crates/config/src/workspace_yaml.rspacquet/crates/lockfile/src/freshness.rspacquet/crates/lockfile/src/freshness/tests.rspacquet/crates/lockfile/src/lib.rspacquet/crates/lockfile/src/save_lockfile/tests.rspacquet/crates/package-manager/src/dependencies_graph_to_lockfile.rspacquet/crates/package-manager/src/dependencies_graph_to_lockfile/tests.rspacquet/crates/package-manager/src/install.rspacquet/crates/package-manager/src/install_package_from_registry/tests.rspacquet/crates/package-manager/src/install_with_fresh_lockfile.rspacquet/crates/package-manager/src/optimistic_repeat_install.rspacquet/crates/package-manager/src/optimistic_repeat_install/tests.rspacquet/crates/resolving-deps-resolver/src/resolve_importer.rspacquet/crates/resolving-deps-resolver/src/resolve_importer/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). (2)
- GitHub Check: Lint and Test (windows-latest)
- GitHub Check: Compile & Lint
🧰 Additional context used
📓 Path-based instructions (1)
pacquet/**/*.rs
📄 CodeRabbit inference engine (pacquet/AGENTS.md)
pacquet/**/*.rs: When porting a function that firespnpm:<channel>events throughglobalLogger,logger.debug(), orstreamParser.write(), mirror the call site, payload, and ordering so the reporter parses pacquet's NDJSON the same way it parses pnpm's.
Declare a newtype wrapper for branded string types. Do not collapse the brand into a plainStringor&str.
If upstream always validates before construction, validate in pacquet's wrapper too. The wrapper must construct only viaTryFrom<String>and/orFromStr. Do not provide an infallible public constructor.
If upstream never validates, just brand for type-safety. Expose an infallibleFrom<String>(andFrom<&str>when convenient).
If upstream occasionally constructs without validation, exposefrom_str_uncheckedas an escape hatch alongside the validating constructor.
Match upstream serde behavior for branded types that cross JSON, YAML, or INI boundaries. Use#[serde(try_from = "String")]for deserialization and#[serde(into = "String")]for serialization.
Use#[derive(derive_more::From)]and#[derive(derive_more::Into)]for mechanical conversion impls. Fall back to manualimplonly when conversion needs custom logic.
String-literal unions should becomeenums, not newtype wrappers. Model closed sets of valid string values as enums.
Template literal types should be treated as branded strings with validation discipline from rules 2-5.
Choose owned vs. borrowed parameters to minimize copies. Widen to the most encompassing type (&Pathover&PathBuf,&strover&String) when it doesn't force extra copies.
PreferArc::clone(&x)/Rc::clone(&x)overx.clone()for reference-counted types, so the cost is visible at the call site.
Follow Rust API Guidelines for naming conventions.
Do not use star imports inside module bodies. Writeuse super::{Foo, bar}instead ofuse super::*;. Two forms stay allowed: external-crate preludes likeuse rayon::prelude::*;and root-of-module re-...
Files:
pacquet/crates/resolving-deps-resolver/src/resolve_importer/tests.rspacquet/crates/config/src/defaults.rspacquet/crates/config/src/env_overlay.rspacquet/crates/lockfile/src/save_lockfile/tests.rspacquet/crates/resolving-deps-resolver/src/resolve_importer.rspacquet/crates/lockfile/src/lib.rspacquet/crates/config/src/workspace_yaml.rspacquet/crates/package-manager/src/install.rspacquet/crates/package-manager/src/install_package_from_registry/tests.rspacquet/crates/package-manager/src/install_with_fresh_lockfile.rspacquet/crates/package-manager/src/dependencies_graph_to_lockfile.rspacquet/crates/lockfile/src/freshness.rspacquet/crates/package-manager/src/dependencies_graph_to_lockfile/tests.rspacquet/crates/config/src/lib.rspacquet/crates/package-manager/src/optimistic_repeat_install/tests.rspacquet/crates/package-manager/src/optimistic_repeat_install.rspacquet/crates/lockfile/src/freshness/tests.rs
🧠 Learnings (6)
📚 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-deps-resolver/src/resolve_importer/tests.rspacquet/crates/config/src/defaults.rspacquet/crates/config/src/env_overlay.rspacquet/crates/lockfile/src/save_lockfile/tests.rspacquet/crates/resolving-deps-resolver/src/resolve_importer.rspacquet/crates/lockfile/src/lib.rspacquet/crates/config/src/workspace_yaml.rspacquet/crates/package-manager/src/install.rspacquet/crates/package-manager/src/install_package_from_registry/tests.rspacquet/crates/package-manager/src/install_with_fresh_lockfile.rspacquet/crates/package-manager/src/dependencies_graph_to_lockfile.rspacquet/crates/lockfile/src/freshness.rspacquet/crates/package-manager/src/dependencies_graph_to_lockfile/tests.rspacquet/crates/config/src/lib.rspacquet/crates/package-manager/src/optimistic_repeat_install/tests.rspacquet/crates/package-manager/src/optimistic_repeat_install.rspacquet/crates/lockfile/src/freshness/tests.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-deps-resolver/src/resolve_importer/tests.rspacquet/crates/config/src/defaults.rspacquet/crates/config/src/env_overlay.rspacquet/crates/lockfile/src/save_lockfile/tests.rspacquet/crates/resolving-deps-resolver/src/resolve_importer.rspacquet/crates/lockfile/src/lib.rspacquet/crates/config/src/workspace_yaml.rspacquet/crates/package-manager/src/install.rspacquet/crates/package-manager/src/install_package_from_registry/tests.rspacquet/crates/package-manager/src/install_with_fresh_lockfile.rspacquet/crates/package-manager/src/dependencies_graph_to_lockfile.rspacquet/crates/lockfile/src/freshness.rspacquet/crates/package-manager/src/dependencies_graph_to_lockfile/tests.rspacquet/crates/config/src/lib.rspacquet/crates/package-manager/src/optimistic_repeat_install/tests.rspacquet/crates/package-manager/src/optimistic_repeat_install.rspacquet/crates/lockfile/src/freshness/tests.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-deps-resolver/src/resolve_importer/tests.rspacquet/crates/config/src/defaults.rspacquet/crates/config/src/env_overlay.rspacquet/crates/lockfile/src/save_lockfile/tests.rspacquet/crates/resolving-deps-resolver/src/resolve_importer.rspacquet/crates/lockfile/src/lib.rspacquet/crates/config/src/workspace_yaml.rspacquet/crates/package-manager/src/install.rspacquet/crates/package-manager/src/install_package_from_registry/tests.rspacquet/crates/package-manager/src/install_with_fresh_lockfile.rspacquet/crates/package-manager/src/dependencies_graph_to_lockfile.rspacquet/crates/lockfile/src/freshness.rspacquet/crates/package-manager/src/dependencies_graph_to_lockfile/tests.rspacquet/crates/config/src/lib.rspacquet/crates/package-manager/src/optimistic_repeat_install/tests.rspacquet/crates/package-manager/src/optimistic_repeat_install.rspacquet/crates/lockfile/src/freshness/tests.rs
📚 Learning: 2026-05-24T21:10:50.292Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11915
File: pacquet/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs:553-617
Timestamp: 2026-05-24T21:10:50.292Z
Learning: When implementing dependency “revisit”/optional-folding in pacquet, mirror pnpm’s `resolveDependencies.ts` behavior: on revisit, update the `optional` flag only for the directly visited package, and do not automatically change `optional` for transitive descendants. pnpm corrects stale optional flags later via `copyDependencySubGraph` (a BFS in `lockfile/pruner/src/index.ts`/`copyDependencySubGraph`). Until pacquet has an equivalent pruner/copy-subgraph step, be aware that the raw `node.optional` can flow through to the lockfile/virtual store via `dependencies_graph_to_lockfile.rs` → `create_virtual_store.rs` → `installability.rs`, so review changes that affect optional flag propagation for this mismatch.
Applied to files:
pacquet/crates/resolving-deps-resolver/src/resolve_importer/tests.rspacquet/crates/resolving-deps-resolver/src/resolve_importer.rs
📚 Learning: 2026-05-24T21:10:50.292Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11915
File: pacquet/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs:553-617
Timestamp: 2026-05-24T21:10:50.292Z
Learning: In pacquet’s Rust resolver layer, the behavior of `ResolvedPackage.optional` “AND-folding”/revisit updates is intentionally aligned with pnpm: when a package is directly revisited, only that package’s `optional` flag is updated; transitive descendants are not re-walked/rescored in this stage. pnpm later corrects any stale optional flags downstream via its pruner/BFS logic (`copyDependencySubGraph` + `nonOptional` reachability stamping in the lockfile pruner). Since pacquet does not yet have that pruner equivalent, review should NOT flag the downstream/stale optional propagation as a bug in pacquet PRs; treat the resolver-layer optional propagation gap as expected parity until the pruning/stamping step is ported.
Applied to files:
pacquet/crates/resolving-deps-resolver/src/resolve_importer/tests.rspacquet/crates/resolving-deps-resolver/src/resolve_importer.rs
📚 Learning: 2026-05-20T23:23:40.606Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11788
File: pacquet/crates/lockfile/src/snapshot_dep_ref.rs:52-56
Timestamp: 2026-05-20T23:23:40.606Z
Learning: In the pacquet lockfile crate, the link target is intentionally represented as a raw `Link(String)` (not a `LinkTarget` newtype) for both `SnapshotDepRef::Link` and `ImporterDepVersion::Link`. pnpm upstream preserves `link:` targets verbatim with only a prefix check (e.g., `refToRelative`), so do not suggest introducing a `LinkTarget` newtype in a bug-fix/targeted PR. If type branding is ever desired, it should be done together for both variants in a separate dedicated refactor PR rather than as part of a small fix.
Applied to files:
pacquet/crates/lockfile/src/save_lockfile/tests.rspacquet/crates/lockfile/src/lib.rspacquet/crates/lockfile/src/freshness.rspacquet/crates/lockfile/src/freshness/tests.rs
🔇 Additional comments (17)
pacquet/crates/config/src/defaults.rs (1)
245-256: LGTM!pacquet/crates/config/src/lib.rs (1)
26-28: LGTM!Also applies to: 397-415, 2578-2632
pacquet/crates/config/src/env_overlay.rs (1)
133-133: LGTM!pacquet/crates/config/src/workspace_yaml.rs (1)
118-118: LGTM!Also applies to: 502-502
pacquet/crates/lockfile/src/lib.rs (1)
48-55: LGTM!Also applies to: 70-75
pacquet/crates/lockfile/src/save_lockfile/tests.rs (1)
164-235: LGTM!pacquet/crates/lockfile/src/freshness.rs (1)
102-113: LGTM!Also applies to: 207-207, 246-260
pacquet/crates/lockfile/src/freshness/tests.rs (2)
594-601: LGTM!Also applies to: 617-625, 638-665, 687-701, 719-727, 741-747, 764-766, 783-789, 814-820
827-904: LGTM!pacquet/crates/package-manager/src/dependencies_graph_to_lockfile.rs (1)
71-78: LGTM!Also applies to: 109-110, 130-130
pacquet/crates/package-manager/src/dependencies_graph_to_lockfile/tests.rs (1)
41-41: LGTM!Also applies to: 207-207, 227-227, 872-872, 994-994, 1145-1145
pacquet/crates/package-manager/src/install.rs (1)
1060-1065: LGTM!pacquet/crates/package-manager/src/install_package_from_registry/tests.rs (1)
38-38: LGTM!pacquet/crates/package-manager/src/install_with_fresh_lockfile.rs (1)
1047-1049: LGTM!pacquet/crates/package-manager/src/optimistic_repeat_install/tests.rs (1)
428-468: LGTM!Also applies to: 513-516
pacquet/crates/resolving-deps-resolver/src/resolve_importer.rs (1)
100-104: LGTM!Also applies to: 149-152, 196-197, 277-278
pacquet/crates/resolving-deps-resolver/src/resolve_importer/tests.rs (1)
95-95: LGTM!
|
Actionable comments posted: 0 |
|
Actionable comments posted: 0 |
1 similar comment
|
Actionable comments posted: 0 |
Contributor
Integrated-Benchmark Report (Linux)Scenario: Isolated linker: fresh restore, cold cache + cold store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 2.0829644863599994,
"stddev": 0.066850377637756,
"median": 2.07319647856,
"user": 2.7405716000000004,
"system": 3.28341106,
"min": 1.96466871306,
"max": 2.19583654706,
"times": [
2.03870811706,
2.07763887206,
2.04054321106,
2.19583654706,
2.16780214506,
1.96466871306,
2.0553097500599997,
2.06875408506,
2.10502354006,
2.11535988306
]
},
{
"command": "pacquet@main",
"mean": 2.0501086858600006,
"stddev": 0.06773101142726648,
"median": 2.04265495806,
"user": 2.7345702,
"system": 3.24459106,
"min": 1.94556523906,
"max": 2.14632614606,
"times": [
2.14632614606,
2.14398202906,
1.94556523906,
2.10453131106,
2.00634076606,
2.06727412506,
2.00898186006,
2.07238725406,
2.01803579106,
1.98766233706
]
}
]
}Scenario: Isolated linker: fresh restore, hot cache + hot store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 0.6640286777800001,
"stddev": 0.01651908171406663,
"median": 0.65973530138,
"user": 0.36720569999999997,
"system": 1.3296974599999998,
"min": 0.6465930938800001,
"max": 0.7069451008800001,
"times": [
0.7069451008800001,
0.6679661878800001,
0.6465930938800001,
0.65260282688,
0.6685361068800001,
0.6601446188800001,
0.6646459368800001,
0.6593259838800001,
0.6562241968800001,
0.6573027248800001
]
},
{
"command": "pacquet@main",
"mean": 0.67380093608,
"stddev": 0.015156202735964724,
"median": 0.6714170648800001,
"user": 0.3667261,
"system": 1.3566348599999998,
"min": 0.6585095648800001,
"max": 0.71299072088,
"times": [
0.71299072088,
0.6648468098800001,
0.6585095648800001,
0.6717942518800001,
0.6747359988800001,
0.6678670988800001,
0.6802546028800001,
0.6710398778800001,
0.67348624988,
0.66248418488
]
}
]
}Scenario: Isolated linker: fresh install, cold cache + cold store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 2.37970467302,
"stddev": 0.028337228508125208,
"median": 2.3774029937200005,
"user": 3.9135182199999994,
"system": 3.05420308,
"min": 2.33849218972,
"max": 2.4259499647200005,
"times": [
2.4259499647200005,
2.3447556827200002,
2.3841794437200003,
2.4103765427200003,
2.35926099072,
2.36952723072,
2.3706265437200003,
2.39913974872,
2.33849218972,
2.3947383927200003
]
},
{
"command": "pacquet@main",
"mean": 2.3571826400200004,
"stddev": 0.024721918657338224,
"median": 2.35556256572,
"user": 3.88855052,
"system": 3.02855998,
"min": 2.33049934472,
"max": 2.4120440217200003,
"times": [
2.34776603772,
2.3652732537200003,
2.33382419672,
2.37282567672,
2.35993365372,
2.36746244972,
2.4120440217200003,
2.35119147772,
2.33049934472,
2.33100628772
]
}
]
}Scenario: Isolated linker: fresh install, hot cache + hot store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 1.5940279595800002,
"stddev": 0.03695883060730748,
"median": 1.60445512738,
"user": 1.7826519799999996,
"system": 1.83808012,
"min": 1.52357037538,
"max": 1.64914838638,
"times": [
1.60294257538,
1.61800616538,
1.64914838638,
1.56161844738,
1.61461834838,
1.6059676793800002,
1.59694086738,
1.61372753838,
1.5537392123800002,
1.52357037538
]
},
{
"command": "pacquet@main",
"mean": 1.5305973458800002,
"stddev": 0.04288695918586298,
"median": 1.5243736488800002,
"user": 1.7103001799999997,
"system": 1.8004232200000003,
"min": 1.4883538383800001,
"max": 1.6395307833800001,
"times": [
1.53776632838,
1.6395307833800001,
1.5274620323800001,
1.50099136138,
1.5040994123800002,
1.5429542203800002,
1.4883538383800001,
1.5212852653800002,
1.54258949738,
1.5009407193800002
]
}
]
} |
Contributor
|
Bencher Report| Branch | pr/12026 |
| 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,379.70 ms(+2.19%)Baseline: 2,328.67 ms | 2,794.40 ms (85.16%) |
| isolated-linker.fresh-install.hot-cache.hot-store | 📈 view plot 🚷 view threshold | 1,594.03 ms(+9.53%)Baseline: 1,455.27 ms | 1,746.33 ms (91.28%) |
| isolated-linker.fresh-restore.cold-cache.cold-store | 📈 view plot 🚷 view threshold | 2,082.96 ms(+1.18%)Baseline: 2,058.58 ms | 2,470.30 ms (84.32%) |
| isolated-linker.fresh-restore.hot-cache.hot-store | 📈 view plot 🚷 view threshold | 664.03 ms(-2.25%)Baseline: 679.34 ms | 815.21 ms (81.45%) |
|
Actionable comments posted: 0 |
Adds `Config.peers_suffix_max_length` (default 1000) plumbed through the resolver, lockfile settings, and optimistic-repeat-install drift gate so pacquet matches pnpm's behavior for the [`peersSuffixMaxLength`](https://pnpm.io/settings#peerssuffixmaxlength) setting end-to-end: - read from `pnpm-workspace.yaml`, global `config.yaml`, and `PNPM_CONFIG_PEERS_SUFFIX_MAX_LENGTH` - threaded into `ResolveImporterOptions` so the deps resolver caps rendered peer suffixes at the configured value - written to `pnpm-lock.yaml`'s `settings.peersSuffixMaxLength` only when non-default (matching pnpm's strip-on-default in `convertToLockfileFile`) - `check_lockfile_settings` now reports drift as `StalenessReason::PeersSuffixMaxLengthChanged` - joins `optimistic_repeat_install`'s `settings_match` comparison via `current_settings` Tracks #12009.
…x_max_length casts Address CodeRabbit review feedback: `config.peers_suffix_max_length` is u64 but two narrowing casts could truncate on extreme values — `as usize` at the resolver-options boundary (on 32-bit targets) and `as u32` at the workspace-state writer. Replace both with `try_from(..).unwrap_or(<TYPE>::MAX)` so a configured value larger than the target type can't silently change resolver behavior.
`pacquet-config` doesn't depend on `pacquet-deps-path` (and shouldn't, to avoid a dep cycle), so an intra-doc link to `pacquet_deps_path::create_peer_dep_graph_hash` couldn't be resolved and broke the Doc CI job under `-D warnings`. Downgrade to a plain code reference.
`perfectionist::macro-trailing-comma` flags a trailing comma in a single-line macro invocation; cargo fmt collapsed the original multi-line `assert_eq!` into one line without dropping the comma. Drop it so Dylint passes.
Six multi-line `assert!(...)` calls in freshness/tests.rs need a trailing comma after the final expression, and two single-line `assert_eq!(...)` calls (one in freshness/tests.rs, one in save_lockfile/tests.rs) have stray trailing commas. Both flavors trip Dylint's `perfectionist::macro-trailing-comma`.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Ports the
peersSuffixMaxLengthsetting (default1000) to pacquet end-to-end, matching pnpm:pnpm-workspace.yaml, globalconfig.yaml, andPNPM_CONFIG_PEERS_SUFFIX_MAX_LENGTHResolveImporterOptionsso the deps resolver caps the rendered peer-suffix at the configured value (was hardcoded1000viaResolvePeersOptions::default()before)pnpm-lock.yaml'ssettings.peersSuffixMaxLengthonly when non-default — mirrors pnpm's strip-on-default inconvertToLockfileFilecheck_lockfile_settingsnow reports drift asStalenessReason::PeersSuffixMaxLengthChangedoptimistic_repeat_install'ssettings_matchcomparison viacurrent_settings, so a change here invalidates the fast pathCloses one of the checkboxes tracked in #12009.
Test plan
cargo nextest run -p pacquet-config -p pacquet-lockfile -p pacquet-resolving-deps-resolver -p pacquet-package-manager— 807 tests passjust fmt/just check/just lint— cleanpacquet-config: default-1000, yaml override, env-var-wins-over-yamlpacquet-lockfile: lockfile-side drift (PeersSuffixMaxLengthChanged) for both unset-vs-default and explicit-vs-explicit, plus serialization strip-on-default + round-trip when non-defaultpacquet-package-manager: optimistic-repeat-install drift gate (portscheckDepsStatus.test.ts:175-203)Written by an agent (Claude Code, claude-opus-4-7).
Summary by CodeRabbit
New Features
Lockfile
Tests