install: name folder, tarball and git packages without a package.json name after their source - #38681
install: name folder, tarball and git packages without a package.json name after their source#38681robobun wants to merge 2 commits into
Conversation
…ame is invalid
Package::parse copied the name out of a non-root package's package.json
verbatim, so a tarball, folder, git or workspace package named e.g. "a:b"
was written to bun.lock as "a:b@<resolution>". The bun.lock parser rejects
such names ("Invalid package name"), so every following install ignored the
lockfile and rewrote it, --frozen-lockfile always failed and bun pm ls
failed with InvalidLockfile.
Apply the lockfile parser's check (dependency::is_safe_install_folder_name)
when the name is read, log an error pointing at the name in that
package.json, and fail the install before anything is saved. The root
package is exempt: its name is not a bun.lock packages entry.
The tarball and git arms of process_extracted_tarball_package exit through
PackageManager::crash so the logged reason is printed before exiting.
… name after their source
bun.lock stores a package as "<name>@<resolution>" and splits it apart again
at the first "@" after an optional scope, so an empty name ("@file:../dir")
or a name containing another "@" is written out but cannot be loaded back:
every following install prints "Ignoring lockfile" and --frozen-lockfile
always fails.
Replace the git-only new_name hook on ResolverContext with fallback_name(),
implemented by the git, tarball and folder/link resolvers: a package whose
package.json has no name is named after the last component of its
repository, tarball URL or folder (dependency::fallback_package_name), or
"unnamed-package" when that is not storable either. Git packages keep the
name they were given before; the SHA-1 fallback for an empty repository
name is replaced by the shared one.
The parse-time name check now uses is_safe_lockfile_package_name, which
also rejects names with an extra "@", so folder, tarball, git and
workspace packages with such a name fail to install instead of saving a
lockfile that never loads.
|
Warning Review limit reached
Next review available in: 31 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 (12)
Comment |
|
Status: reproduced on bun 1.4.0 with Fix is in this PR (#38681). It is stacked on #38633, whose commit is included here: #38633 rejects invalid names, this PR derives a name for packages that have none and extends the rejection to names the Tests: |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
|
Found 2 issues this PR may fix:
🤖 Generated with Claude Code |
|
#17060 is exactly the folder case fixed here ( #13861 is not claimed. Its yarn.lock shows a bare |
1 participant
Problem
file:folder,link:target, local or remote tarball whose own package.json has nonameinstalls, but bun.lock ends up with"x": ["@file:../dir", {}](or["@../pkg.tgz", ...],["@https://...", ...]). The nextbun installfails to parse that entry (error: Invalid package resolution), printswarn: Ignoring lockfileand re-resolves everything,bun install --frozen-lockfilealways fails withlockfile had changes, but lockfile is frozen, andbun pm lsfails witherror: Error loading lockfile: InvalidLockfile. Same result for a git dependency without a package.json name whose repository name is not usable as a package name. This is bun.lock failed on file:../xxx #17060 ("$": ["@file:../srv/gen/js", ...]).@(a@b,@scope/a@b) in a folder, tarball, git or workspace package produces["a@b@file:../dir", {}], which the parser splits into nameaand resolutionb@file:../dir(error: Missing git dependency tag/Unexpected resolution), with the same consequences.src/install/lockfile/bun.lock.rs, the"[\"{}@{}\", "writes) emitsname@resolutionand the reader recovers the name withdependency::split_name_and_version, so an empty name or one with a second@cannot round-trip.Package::parse_with_json_impl(src/install/lockfile/Package.rs) only derived a name for git packages without one; folder and tarball packages were left unnamed, and names with@were copied as-is.bun add ../nameless-dirwrote"": "../nameless-dir"into package.json and then failed to install, and the isolated linker stored a nameless folder package asnode_modules/.bun/@file+pkg/node_modules/with the package files spilled directly into thatnode_modules.Fix
ResolverContext::fallback_name()replaces the git-onlynew_name/set_new_name/take_new_name/resolution()/dep_id()surface.Package::parse_with_json_impluses it whenever package.json has no (or an empty) name; the git, tarball and folder/link:resolvers implement it, while the root, workspace members (registered by name inWorkspaceMapbefore this runs) and the npm cache keep returningNone, so their behavior is unchanged.dependency::fallback_package_name(location)is the shared derivation: the last path component of the repository, tarball path/URL (query string and.tgz/.tar.gzstripped) or folder, andunnamed-packagewhen that component is not storable either (file:../odd@dir,file:..). It always satisfies the check below, so whatever bun writes it can read back. Git packages get exactly the name they got before for every real repository (Repository::create_dependency_name_from_version_literalis removed; its SHA-1-bytes fallback for an empty repository name is replaced by the shared fallback, which unlike raw SHA-1 bytes is loadable).ResolutionType::ZEROEDand thedep_idparameter ofprocess_extracted_tarball_packageonly served the removed surface and are removed too.get_package_idfinds the entry already in the lockfile instead of appending a second package (the bun-lock test adds a second alias for the same folder after the first install and checks it resolves to the samefolder-pkg@file:...entry).dependency::is_safe_lockfile_package_name, i.e.is_safe_install_folder_name(what the reader applies) plus "no@other than a scope marker" (what thename@resolutionencoding needs). Names with an extra@are therefore rejected with the sameInvalid package name "a@b"error instead of being invented around, consistent with thea:bcase, and this also covers workspace members. This branch contains install: reject tarball, folder and git packages whose package.json name is invalid #38633's commit and is meant to land after it; once it is merged the diff here shrinks to the changes described above.test/cli/install/bun-lock.test.ts: folder, empty-string name, unstorable folder name, local tarball, remote tarball and remote tarball with a query string are written asfolder-pkg@file:pkgs/folder-pkg,unnamed-package@file:pkgs/odd@folder,tarball-pkg@./tarball-pkg.tgz,remote-pkg@http://...,signed-pkg@http://...?token=abc/def; a second install and--frozen-lockfileload the file unchanged without any warning, and a second alias for the same folder reuses the entry.test/cli/install/bun-install-git-deps.test.ts: nameless git package fromshared-repo.gitkeeps the nameshared-repo.git; fromodd@repo.gitit becomesunnamed-package;--frozen-lockfileloads the result.test/cli/install/bun-install.test.ts:a@bfolder,@scope/a@btarball anda@bworkspace member are rejected, added next to install: reject tarball, folder and git packages whose package.json name is invalid #38633's tests.test/cli/install/isolated-install.test.ts: nameless folder dependency is stored as.bun/pkg-1@file+pkg-1/node_modules/pkg-1andrequire()works.test/cli/install/bun-add.test.ts:bun add file:../x/pkg-without-nameadds"pkg-without-name"and installs it.a@bonly hits this encoding if something depends on it withfile:.), andverify_package_json_name_and_version, which already reinstalls packages whose package.json has no name on every run and keeps doing so.Background
packagessection stores each installed package as"<path in node_modules>": ["<name>@<resolution>", ...]. On load the first element is split withsplit_name_and_version: at the first@, or at the second one when the string starts with@(a scope). The recovered name is then checked withis_safe_install_folder_name, because package names from the lockfile become folder names (npm cache, isolated store). A single entry failing either step makes the whole lockfile unloadable, which bun reports asIgnoring lockfile.Package::parse_with_json_implturns a package.json into a lockfilePackagefor everything that is not a registry manifest: the root, workspace members,file:folders,link:targets, and the package.json extracted from tarball and git dependencies. It runs in two passes over aStringBuilder(count the bytes, allocate once, then append), which is why the name is decided once up front and appended later.ResolverContextis the per-source strategy object passed into that function (GitResolver,TarballResolver,NewResolver<Folder|Symlink|Workspace>,CacheFolderResolver,()for the root); it supplies the package'sResolution, and now also the name to use when package.json has none.lockfile.package_index; a dependency that resolves to a folder or tarball already present in the lockfile is matched throughget_package_id(name_hash, ..., resolution), so the name bun derives has to be the same every time the same source is resolved.Fixes #17060