Conversation
|
Warning Review limit reached
Next review available in: 10 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughYarn lock migration now normalizes package names across npm aliases and direct URLs. It updates entry consolidation, package identity selection, and alias indexing. Migration tests cover registry, GitHub, tarball, and distinct-resolution alias cases. ChangesYarn alias migration
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
|
Status: reproduced all three shapes with CI: build 98671 (current head, rebased on main) has no failing tests; |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
I reviewed this PR and didn't find any bugs. Because it redesigns how the yarn migrator derives package names (which drives what bun install downloads) and the description flags overlap with three in-flight PRs (#38795, #38803, #37244), a human look would still be worthwhile.
What was reviewed
parse_npm_aliasscope handling — verifiednpm:@scope/pkg@rangenow splits after the scope; traced both callers (process_deps, theversionparse branch) to confirm the returned.versionchange is safe.consolidate_and_append_entrykeying onentry.name— checked it separates same-alias/different-target entries and does not over-merge non-alias entries relative to the old first-spec-name key.- The two naming passes and the alias-index loop — confirmed the
entry.name/git_repo_name/has_direct_url_specreshaping is behavior-preserving for plain, git, and direct-URL entries.
Extended reasoning...
Overview
The PR replaces the tarball-URL heuristic (get_package_name_from_resolved_url) with a new Entry.name field derived from the yarn key line's alias@npm:<name>@<range> spec. parse_npm_alias gains a name return and now skips a leading scope @ before splitting. Consolidation keys on (entry.name, version); both package-id naming passes and the alias-index loop switch to entry.name. The two hand-rolled "is direct URL" scans become Entry::has_direct_url_spec. Two new tests assert the full bun.lock packages object plus, via a loopback registry, which manifest names the post-migration pass fetches; the existing scoped-alias test is refactored onto the shared migrateYarnLock helper.
Security risks
None identified. Input is a local yarn.lock and package.json; the new parsing operates on borrowed byte slices with bounds-checked helpers (strip_prefix, split_at, index_of_char_usize) and no allocation or unsafe. The change does not touch integrity verification, network, or filesystem paths.
Level of scrutiny
Medium-high. The migrator's package name is what bun install later downloads and what fetch_necessary_package_metadata_after_yarn_or_pnpm_migration requests, so a wrong name silently installs the wrong package. This is a deliberate redesign (spec-derived vs URL-derived) rather than a mechanical fix, and I traced each of the four rewritten call sites against the old branching to confirm non-alias entries name the same as before. I also spot-checked npm_alias_target against @my/alias@npm:@scope/pkg@1.0.0, foo@npm:npm@1.0.0, and foo@npm: (empty target → falls back to first-spec name).
Other factors
- The PR description explicitly notes small conflicts with #38795, #38803 (which edits the deleted function and pins a test row this PR changes) and #37244 (which keys a consolidation map on the spec name). Coordinating merge order is a human decision.
- Test coverage is strong:
toStrictEqualon the fullpackagesobject plus registry-fetch verification, and the PR states the 14 existing snapshots (includingyarn-cli-repoandyarn-stuff) are unchanged. The extractedmigrateYarnLockhelper follows harness conventions (drains stdout/stderr concurrently,await using,port: 0, per-test cache dir). - No prior review comments from me on this PR.
|
Update: #38803 no longer touches |
f9cfd2b to
b04b530
Compare
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
The git-vs-alias regression I flagged earlier is fixed — dedupe_name() now keys git entries by git_repo_name (mirroring how the old package-id pass identified them), and the new "git dependency keyed by a package name stays separate from an alias of it" test pins it. No further issues found this run.
Because this reworks the identity used for both consolidation and the package-id pass in install-correctness code (a wrong merge silently installs a different package), and because it needs coordination with #38803/#37244 on the deleted get_package_name_from_resolved_url, a human look is still worthwhile.
What was reviewed
dedupe_name()runs aftergit_repo_nameis populated (consolidation happens on the fully-parsed entry) and its!has_direct_url_spec()guard matches the old package-id pass ordering.parse_npm_aliasscope-@handling and the range-lessnpm:@corp/barecase.has_direct_url_spec()correctly excludes@git+https://specs; the doc-comment length flags from comment-cop are addressed in the latest commits.
Extended reasoning...
Overview
Follow-up to #38806 that stops the yarn v1 migrator from naming npm: alias packages via a tarball-URL heuristic and instead reads the target name straight from the alias spec. Entry gains a name field computed at parse time, parse_npm_alias is fixed to handle scoped targets and range-less specs, and consolidation + the package-id pass now share one identity function dedupe_name(). get_package_name_from_resolved_url is deleted (no callers left in this PR's tree). Three new tests cover the GitHub Packages layout, /-/-after-host, same-alias-name-different-packages, and the git-vs-alias case my earlier review raised; the existing scoped-alias test is refactored onto a shared migrateYarnLock helper.
Prior review addressed
My earlier inline comment (now resolved) flagged that keying consolidation on entry.name alone would merge a git fork (lodash@git+…/lodash-fork.git → name lodash) with an npm alias targeting lodash at the same version. The revision introduces dedupe_name(), which returns git_repo_name for git entries (guarded by !has_direct_url_spec() to preserve the old package-id-pass ordering) and entry.name otherwise; both consolidation and the package-id pass now use it. I traced the parse loop to confirm git_repo_name is set before consolidate_and_append_entry runs (it's called on the previous, fully-populated entry when the next header is seen). The new git-vs-alias test pins exactly the scenario I described and the PR description confirms it failed on the first revision and passes now. The one remaining gap — non-GitHub git hosts without a parsable repo name still fall through to entry.name — is pre-existing (the old package-id pass had the same behavior) and the description says it's tracked separately.
Security risks
This is package-install correctness: a wrong dedupe_name merge means bun.lock records the wrong package under a dependency, and a later install fetches it silently. That's the class this PR fixes for the alias-vs-alias case, and the identity function used for merging is the load-bearing invariant. No new untrusted-input parsing surface beyond what the migrator already had; parse_npm_alias's new slicing is bounds-safe (strip_prefix, split_at on an index found within the slice).
Level of scrutiny
Medium-high. The diff is compact and well-reasoned, existing snapshots (yarn-cli-repo, yarn-stuff) are unchanged, and the tests assert full packages objects plus the manifest names actually requested. But identity/consolidation changes in the installer have subtle interaction with every other resolution kind (git, file:, direct-URL, workspace), one such interaction already surfaced in review, and the PR description itself flags a one-hunk merge conflict with two in-flight PRs (#38803 deletes-vs-keeps get_package_name_from_resolved_url, #37244 keys its consolidation map differently). That coordination is a human call.
Other factors
The comment-cop bot flagged several long doc comments; the two follow-up commits (shorten the yarn Entry doc comments, one-line doc comments on the yarn Entry helpers) reduced them to single lines, which is what the current diff shows. robobun independently reproduced all three failure shapes on main and confirmed the new tests fail without / pass with the src change.
|
Updated 3:05 PM PT - Aug 15th, 2026
❌ @robobun, your commit 272e010 has some failures in 🧪 To try this PR locally: bunx bun-pr 38948That installs a local version of the PR into your bun-38948 --bun |
… the tarball URL An alias entry in a yarn v1 lockfile is keyed alias@npm:<name>@<range>, so the key line carries the real package name. The migrator instead guessed it from the resolved tarball URL and fell back to the alias name, which named the package wrongly for registries without the <name>/-/ tarball layout and for URLs with /-/ directly after the host, and it consolidated entries by alias name and version, merging aliases of the same name that point at different packages into one package. Each entry now records the package name taken from its alias spec (or the spec name when the key line has no alias), and consolidation, both naming passes and the alias index registration use it. The URL lookup for alias names is deleted; tarball entries are still named by the name_to_use block.
…ge pass uses Keying consolidation on the package name alone merged a git entry keyed under a package's name (a fork keeping upstream's name and version) with an npm: alias of that package. The package-id pass identifies git entries by their repository; consolidation now uses the same Entry::dedupe_name, which keeps the two apart as before.
a4abd56 to
1072523
Compare
There was a problem hiding this comment.
I reviewed this PR again after the dedupe_name() fix in 7519f61 and found no new issues — the git-vs-alias merge I flagged earlier is now keyed by repository and pinned by the new test, and I verified the non-GitHub git-host case the description calls out was already collapsed by the package-id pass on main (so consolidating it one step earlier is not a new regression). Because this changes the identity that consolidation and the package-id pass share, and it interacts with #38803 and #37244, a human look would still be worthwhile.
Extended reasoning...
Overview
This PR changes how the yarn.lock migrator names npm: alias packages: instead of parsing the package name out of the resolved tarball URL (which fails for GitHub Packages and other registries without the <name>/-/<file>.tgz layout), it reads the target name directly from the alias@npm:<name>@<range> spec on the entry's key line. Entry gains a name field computed at parse time, parse_npm_alias is fixed to skip a scoped name's leading @ when locating the range separator, consolidation and the package-id pass now share Entry::dedupe_name() (repository for git entries, package name otherwise), and get_package_name_from_resolved_url is deleted. Three new hermetic tests cover the alias-spec naming, same-alias-name-different-target consolidation, and the git-vs-alias separation that my first-pass review flagged.
Security risks
None identified. The change only affects how package names are derived from a yarn v1 lockfile the user already trusts; no new external inputs are parsed and the loopback-registry tests confirm the migrator requests the intended package names.
Level of scrutiny
Medium-high. Lockfile migration is a correctness-critical path — a wrong consolidation key silently installs a different package than yarn resolved. The first revision of this PR introduced exactly that class of regression (git fork keyed under a package name merged with an npm: alias of the same name/version), which was caught in review and fixed with dedupe_name() plus a pinning test. I re-traced the fix: git_repo_name is set from the resolved line before consolidate_and_append_entry runs on that entry, so the git entry is correctly identified as lodash-fork rather than lodash at consolidation time. I also traced the non-GitHub-host edge the description mentions and confirmed the old package-id pass already gave both entries the same id, so the observable output is unchanged.
Other factors
The fix is well-tested (three new test.concurrent cases asserting the full packages object and the exact manifest names fetched, plus 14 unchanged snapshots including yarn-cli-repo and yarn-stuff), the comment-cop feedback about long doc comments has been addressed, and CI is green apart from an unrelated Windows bake failure. Deferring rather than approving because the identity change is subtle enough to have needed one round of correction already, and the description notes one-hunk conflicts with #38803 and #37244 that whoever merges second will need to resolve.
There was a problem hiding this comment.
The git-vs-alias merge I flagged is fixed by dedupe_name() (7519f61) and pinned by the new test; I have no further findings. Because this reworks the identity that migration keys packages on — where a wrong merge silently installs a different package — and interacts with #38795/#38803/#37244, a maintainer sign-off is still worthwhile.
What was reviewed:
dedupe_name()now matches the package-id pass's git-entry identity, so the fork/alias case stays two packages; the non-GitHub-git collision is confirmed pre-existing (old package-id pass already keyed both onlodash).parse_npm_aliasscope handling and the range-lessnpm:@corp/barecase.- The alias-registration loop now runs without a
resolvedURL; for non-alias entriesget_name_from_spec(specs[0]) == entry.nameso it's a no-op there. - New tests assert the full
packagesobject plus the manifest names actually fetched, and usetest.concurrentwith a per-test loopback registry.
Extended reasoning...
Overview
Changes how the yarn v1 → bun.lock migrator names npm: alias packages: instead of parsing the tarball URL, it reads the target name from the alias@npm:<name>@<range> key spec. Adds an Entry::name field set at parse time, a dedupe_name() helper so consolidation and the package-id pass key on the same identity (repository for git entries, package name otherwise), and deletes the now-unused URL-name extractor. ~156 lines changed in src/install/yarn.rs (net −30), ~+200 lines of tests.
Security risks
None. This is offline lockfile-to-lockfile migration; no new parsing of untrusted network data, no path handling, no auth. The correctness risk (wrong package installed after a bad merge) is a supply-chain-adjacent concern but is what the change fixes, not introduces.
Level of scrutiny
Medium-high. Package-manager identity logic where a wrong consolidation silently resolves a dependency to a different package. My first-pass review found exactly that class of regression (git fork merged with an npm alias of the same name/version); it was fixed with dedupe_name() and a regression test. The remaining non-GitHub-git-host edge case is verified as pre-existing behavior (the old package-id pass already collapsed those). The fix is well-reasoned and the tests are strong (full packages object + observed manifest fetches), but the change is not mechanical and the PR notes one-hunk conflicts with three other in-flight PRs touching adjacent code.
Other factors
- All prior review threads (mine and comment-cop's) are resolved; comments were shortened to one line each.
- CI is green on all lanes per robobun; the one Windows failure is unrelated and failing on main.
- 21 tests in the file pass including 14 unchanged snapshots (
yarn-cli-repo,yarn-stuff). - Given the first revision had a real regression and the identity change touches every migrated entry, I'm deferring rather than auto-approving so a maintainer can confirm the
dedupe_nameapproach and the coordination with #38803/#37244.
…ht after the host (#43189) Replaces #38803: the same commit on current main, plus the review changes. ### Problem - `bun install` and `bun pm migrate` abort on a `yarn.lock` entry such as `"x@https://evil.example/-/registry.npmjs.org/x.tgz"`: `panic: slice index starts at 42 but ends at 20`, top frame `bun_install::yarn::migrate_yarn_lockfile`. A `/-/` directly after the registry host panics too (`27..26`). The tarball URL of the real package `-` has that shape. - Cause: the `name_to_use` block (`src/install/yarn.rs:941` on main) searches the URL for `/-/` and for `registry.` independently, then slices between the two indexes. ### Fix - `Entry::get_package_name_from_default_registry_url` parses from left to right: the scheme, then `registry.npmjs.org/` or `registry.yarnpkg.com/`, then one name segment (two for `@scope/name`), then `/-/`. `strings::is_npm_package_name` must accept the name. Every other URL keeps the spec name, as before. - Each slice bound comes from one forward scan, so the bounds are in order. Real registry URLs give the same names as before. - From the review: `@scope/-` now keeps its full name. The first `/-/` of that URL is inside the name. - Verified: the new case in `test/cli/install/migration/yarn-lock-migration.test.ts` fails on canary b52d513 with the panic and passes here. Also ran `migrate.test.ts`, `lockfile-only.test.ts`, and the yarn cases of `nested-overrides.test.ts`. ### Background - A yarn v1 entry has spec keys (`name@range`, or `name@https://...` for a URL dependency) and a `resolved` tarball URL. The migrator cannot download the tarball to learn the package name. - A registry tarball URL is `<registry>/<name>/-/<file>.tgz`. On the two default registries `<name>` is the real package name, so the migrator uses it. - A `bun.lock` package entry starts with `"<name>@<version or url>"`. <details><summary>Notes</summary> No issue reports this crash. It was found by a read of the code. Every shape except the `-` package needs a hand-edited `yarn.lock`, which `bun install` must still survive. Each URL as the only dependency of a project, `bun pm migrate -f`, name recorded in `bun.lock`: | `resolved` URL | canary b52d513 | this PR | |---|---|---| | `https://evil.example/-/registry.npmjs.org/x.tgz` | panic `42..20` | spec name | | `https://registry.npmjs.org/-/y.tgz` | panic `27..26` | spec name | | `https://registry.npmjs.org/-/-/--0.0.1.tgz` | panic `27..26` | `-` | | `https://registry.npmjs.org//-/z.tgz` | empty name | spec name | | `https://registry.mirror.example/registry.npmjs.org/other/-/other-1.0.0.tgz` | `registry.npmjs.org/other` | spec name | | `https://registry.npmjs.org/@scope/-/-/--0.0.1.tgz` | `@scope` | `@scope/-` | | `https://registry.npmjs.org/@scope/-/y.tgz` | `@scope` | spec name | | `https://registry.npmjs.org/@scope//-/z.tgz` | `@scope/` | spec name | | `https://registry.npmjs.org/a/b/-/b-1.0.0.tgz` | `a/b` | spec name | | `https://registry.npmjs.org/../-/x-1.0.0.tgz` | `..` | spec name | | `https://registry.npmjs.org/w.tgz` | spec name | spec name | | `https://registry.npmjs.org/@scope/real/-/real-1.0.0.tgz` | `@scope/real` | `@scope/real` | | `http://registry.npmjs.org/real-a/-/real-a-1.0.0.tgz` | `real-a` | `real-a` | | `https://registry.yarnpkg.com/real-b/-/real-b-1.0.0.tgz` | `real-b` | `real-b` | | `http://registry.yarnpkg.com/real-c/-/real-c-1.0.0.tgz` | `real-c` | `real-c` | The test runs one `bun pm migrate` over all of these rows. It asserts the full `packages` object of `bun.lock`, and that the loopback registry received no request. Sites not in this PR: - `Entry::get_package_name_from_resolved_url`, the parser for `npm:` alias entries. It does not panic. For a URL with `/-/` directly after the host it returns the host as the name. #38948 names an alias from its spec and deletes that parser, so this PR does not change it. The two PRs merge cleanly in either order (`git merge-tree`). - The `is_default_registry` check in the resolution block of `migrate_yarn_lockfile`. It decides how the URL is stored and accepts `https://` only. A shared predicate would change one of the two behaviors. - I searched `yarn.rs` for other slices between two independent searches. This block was the only one. Relation to #41908: it changes the condition around this block to `is_tarball_dep` and keeps the index arithmetic, so the two PRs conflict in this one block. The resolution is the `is_tarball_dep` condition around the one helper call. No test row depends on that PR. `strings::is_npm_package_name` does not accept `~ ' ! ( ) *` in an unscoped name. npm accepted such names in the past. A URL dependency on such a package keeps the spec name, as a tarball on any other host does. Test runs with the debug build: `yarn-lock-migration.test.ts` 19 pass, 14 snapshots unchanged. `migrate.test.ts` and `lockfile-only.test.ts` 148 pass, 1 todo. `nested-overrides.test.ts -t yarn` 6 pass. `test/internal/source-lints/byte-search.test.ts` 3 pass. The earlier revisions and their review are in #38803. </details>
This branch has not been deployed
1 participant
Follow-up to #38806 (#27781): that change fixed what the tarball URL heuristic returns for scoped packages; this one stops the yarn migrator from using a URL heuristic for alias names at all. There is no user report for the three outputs below; they were found while following up #38806. Affected users are those migrating a
yarn.lockwithnpm:aliases resolved from a registry without the<name>/-/tarball layout (GitHub Packages and similar) and those whoseyarn.lockuses one alias name for two different packages at the same version; the second case installs the wrong package silently.Problem
bun pm migrate/bun installon a yarn v1 lockfile names annpm:alias package after itsresolvedtarball URL (Entry::get_package_name_from_resolved_url,src/install/yarn.rs:241on main, used at:830,:927and:1588) and falls back to the alias name, and consolidates entries by alias name (YarnLock::consolidate_and_append_entry,:472). The alias spec that keys every such entry,alias@npm:<name>@<range>, is never read for the name. Three wrong outputs follow, all reproduced on the released build and on main:<name>/-/<file>.tgztarball layout (GitHub Packages,https://npm.pkg.github.com/download/@corp/foo/1.0.0/<hex>):"gh-alias": ["gh-alias@1.0.0", ...]instead of@corp/foo@1.0.0, and the post-migration manifest pass asks the registry forgh-alias./-/directly follows the host (https://registry.npmjs.org/-/bar-1.0.0.tgz#sha1):"host-alias": ["registry.npmjs.org@1.0.0", ...]instead ofbar@1.0.0.xdepends onsame "npm:foo@1.0.0",yonsame "npm:baz@1.0.0", two yarn entries): consolidation merges the two entries, one package disappears frombun.lock, and the other parent's dependency silently resolves to the wrong package.Entry::parse_npm_alias(:221) also splitnpm:@scope/name@rangeat the scope's@; nothing observable consumed that, but it is the parser the fix builds on.Fix
Entrygets anamefield, set when the key line is parsed: the target of the firstalias@npm:<name>@<range>spec on the line, otherwise the name of the first spec (Entry::package_name_of).parse_npm_aliasnow returns the target name as well and skips a leading scope@before looking for the separator, the same splitdependency::parse_with_taguses fornpm:versions; a spec without a range (alias@npm:@corp/bare) yields the whole target as the name.(Entry::dedupe_name(), version): the repository for a git entry (which is how the package-id pass already identified git entries) andentry.namefor everything else. The second naming pass usesentry.namein place of the URL lookup, and the index pass registers every spec whose name differs fromentry.nameas an alias.get_package_name_from_resolved_urlhas no callers left and is deleted; the two copies of the "is this a direct URL dependency" spec scan becomeEntry::has_direct_url_spec.dedupe_nameis there because keying consolidation on the package name alone (this PR's first revision) merged a git entry keyed under a package's name (lodash@git+.../lodash-fork.git, a fork keeping upstream's name and version) withmy-lodash@npm:lodash@4.17.21; the package-id pass kept those apart by repository, and consolidation now uses the same identity. Git entries without a parsable repository (non-GitHub hosts) are still identified by their spec name exactly as before this PR; that such an entry can collapse into a registry package of the same name and version is pre-existing and reported separately.same "npm:foo@1.0.0", are matched against exactly these specs), it does not depend on the registry's URL layout, and it is present even whenresolvedis not. The URL only ever agreed with it by convention, so for every entry the old code named correctly the new name is the same, and the existing alias, scoped-alias andyarn-stuffcoverage (aliases sharing a key line with plain specs, aliases on separate lines deduplicating into one package, two versions of the same aliased package) is unchanged.lodash@4.17.21entry with amy-lodash@npm:lodash@4.17.21entry on another line. The package-id pass already gave those one package before, so the output is the same; it now happens one step earlier.name_to_useblock, which still reads the name out of default-registry URLs; that block is untouched here because install: fix default-trusted lifecycle scripts being blocked after yarn.lock migration #38795 and install: fix yarn.lock migration panic on tarball URLs with "/-/" right after the host #38803 are reworking it. install: fix yarn.lock migration panic on tarball URLs with "/-/" right after the host #38803 currently modifies the deleted function for that block, and its new test pins the alias-name fallback for the/-/-after-host alias row (alias-of-odd-tarball@1.0.0), which this change turns intowhatever@1.0.0; install: make yarn.lock migration linear in lock size #37244 keys its consolidation map on the spec name and would switch toentry.name. Whichever of these lands second has a one-hunk conflict to resolve.test/cli/install/migration/yarn-lock-migration.test.ts, "yarn.lock npm aliases are named after the alias spec, not the tarball URL" (an alias without a range, the GitHub Packages layout with a scoped target,/-/after the host, and yarn's usual"x-cjs@npm:x@^4.2.0", x@^4.1.0:line) and "yarn.lock aliases of one name and version to different packages stay separate". Both assert the fullpackagesobject of the migratedbun.lockand, through a loopback registry, the names the migration fetches manifests for. Without thesrc/change the first fails withgh-alias@1.0.0/registry.npmjs.org@1.0.0(the range-less row passes on main and is there to cover the parser), the second with both parents resolving tobaz@1.0.0andfoomissing; both pass with this change. "yarn.lock git dependency keyed by a package name stays separate from an alias of it" pins the git case above: it passes on main, failed on this PR's first revision (my-lodashresolved to the fork), and passes now. The scoped-alias test from install: keep the scope when a yarn.lock npm: alias points at a scoped package #38806 stays as the regression guard and now shares the loopback-registry helper with the new tests.yarn-lock-migration.test.ts(21 tests in total, 14 snapshots includingyarn-cli-repoandyarn-stuff),lockfile-only.test.ts, the yarn cases ofnested-overrides.test.tsandmigrate.test.tspass unchanged with the debug build (yarn-cli-reponeeds--timeouton a loaded machine, the pre-existing debug+ASAN timing tracked in test(install): fix yarn-lock-migration yarn-cli-repo case under debug+ASAN #35377; its snapshot is unchanged);cargo clippy -p bun_installand the source lints are clean.Background
name@range; annpm:alias ("my-lodash": "npm:lodash@4"in package.json) givesmy-lodash@npm:lodash@^4.0.0. yarn writes every spec that resolved to the same tarball on one key line, so an alias spec and plain specs of the same package can share an entry. The entry body has the version, the tarball URL and integrity, but no name.bun.lockpackage,["<name>@<version>", "<registry url or empty>", {meta}, "<integrity>"], keyed by its node_modules path, and hangs it in the tree under the alias. The name in the first element is whatbun installlater downloads and what the post-migration manifest pass (fetch_necessary_package_metadata_after_yarn_or_pnpm_migration, which fills in bin/os/cpu) requests, so a wrong name there becomes a 404 or a different package.consolidate_and_append_entrymerges entries that describe the same package at the same version into one (yarn can list the same package under several key lines); it needs the package's real name to tell two aliases of the same name apart.[review] gate passed · iteration 5 · 2 files touched
fails on main (without fix)
passes on PR (with fix)
diff hotspot
gate history · 1 passed · 0 rejected · iteration 5
evidence per changed file