fix(lockfile): parse git and github deps that carry an integrity hash - #104
Closed
robertodr wants to merge 1 commit into
Closed
fix(lockfile): parse git and github deps that carry an integrity hash#104robertodr wants to merge 1 commit into
robertodr wants to merge 1 commit into
Conversation
Newer versions of bun append an integrity hash to `github:` and `git+`
package entries, giving them arity 4 - the same shape as an npm package.
`deserialize_package` dispatches purely on arity, so those entries were
handed to `deserialize_npm_package`, which asked `to_npm_url` to build a
registry url out of a git specifier.
For a scoped package that silently produced a bogus registry.npmjs.org
url. For an unscoped one it failed outright, because `to_npm_url` looks
for the scope separator by splitting on the first `/` - which for
`kata@github:kenn-io/kata#c668572` consumes the `/` between owner and
repo, leaving `kata#c668572` with no `@` left to split on:
Failed to deserialize package: Missing @ for package name and
version declaration.
Dispatch arity 4 entries on their specifier, so git and github packages
reach the git/github deserializer whether or not bun recorded a hash for
them. `to_npm_url` now finds the name/version separator the same way
`drain_package_specifier` does instead of inferring the scope from a
`/`, so a specifier containing a slash can no longer be misread.
Assisted-by: ClaudeCode:claude-opus-5
Contributor
|
Is this different than what #101 resolves? |
Author
|
No, I think this is a duplicate. Should have looked at the open PRs before submitting... |
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
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.
🤖 AI text below 🤖
Problem
deserialize_packagedispatches lockfile entries purely on their arity. Newer versions of bun append an integrity hash togithub:andgit+entries, which gives them arity 4 — the same shape as an npm package:Those entries reach
deserialize_npm_package, which asksto_npm_urlto build a registry url out of a git specifier. There are two outcomes:https://registry.npmjs.org/@scope/pkg/-/pkg-github:owner/repo#rev.tgz, which fails much later at fetch time.to_npm_urllooks for the scope separator by splitting on the first/, so forkata@github:kenn-io/kata#c668572it consumes the/between owner and repo and is left withkata#c668572, which has no@to split on:The advice in that message can't help: the lockfile is valid and a fresh
bun installreproduces it. Any project with an unscoped github dependency is stuck.Fix
Dispatch arity-4 entries on their specifier — a new
4 if deserializer.has_git_or_github_specifier()arm sends them todeserialize_tarball_git_or_github_package, so git and github packages are recognised whether or not bun recorded a hash for them. That method only reads the identifier, so the trailing hash is harmlessly ignored. Backed by a new publicis_git_or_github_identifierhelper with doctests.Harden
to_npm_url— it now finds the name/version separator the waydrain_package_specifieralready does, instead of inferring the scope from a/, so a specifier containing a slash can no longer be misread. This is defence in depth: after (1) a git specifier no longer reaches it.Testing
Built and tested against rustc 1.95.
cargo test --locked— 7 doctests pass, 2 of them new.rustfmt --checkclean on the touched files.cargo clippy --all-targetsreports onecollapsible_ifwarning in an untouched block; warning count is unchanged from the base commit.No regression: generated output for
templates/git-deps/bun.lock(npm +git++ two arity-3github:deps) is byte-identical before and after this change.Reproduced and fixed, using a synthetic lockfile with arity-4 github entries, scoped and unscoped:
Before, this fails with
Missing @ for package name and version declaration. After, it produces the same entry the arity-3 form does:I did not add a template or
nix flake checkfixture for the arity-4 shape, since the lockfile bun generates depends on the bun version in the build environment — happy to add one if you'd prefer it pinned as a static fixture.