[bun install] manual os overide flags + basic level of libc filtering for linux#21174
[bun install] manual os overide flags + basic level of libc filtering for linux#21174
Conversation
|
Updated 1:52 AM PT - Aug 2nd, 2025
❌ @RiskyMH, your commit dd67609 has 4 failures in
🧪 To try this PR locally: bunx bun-pr 21174That installs a local version of the PR into your bun-21174 --bun |
| /// `"libc"` field in package.json, not exposed in npm registry api yet. | ||
| libc: Libc = Libc.none, | ||
| /// `"libc"` field in package.json | ||
| libc: Libc = Libc.all, |
There was a problem hiding this comment.
This should continue to be none, and we should treat none as "all"
There was a problem hiding this comment.
we should add a test with an old binary lockfile that makes sure packages are still installed now that we use the libc bits
There was a problem hiding this comment.
Ah yeah, I half assumed it was already tested. Will add!
| if (name[0] == '@') { | ||
| if (strings.indexOfChar(name, '/')) |i| { | ||
| const scope_and_pkg = name[i + 1 ..]; | ||
| inline for (platform_keywords) |keyword| { | ||
| if (strings.containsComptime(scope_and_pkg, keyword)) { | ||
| return true; | ||
| } | ||
| } | ||
| } | ||
| } else if (strings.indexOfChar(name, '-')) |i| { |
There was a problem hiding this comment.
| if (name[0] == '@') { | |
| if (strings.indexOfChar(name, '/')) |i| { | |
| const scope_and_pkg = name[i + 1 ..]; | |
| inline for (platform_keywords) |keyword| { | |
| if (strings.containsComptime(scope_and_pkg, keyword)) { | |
| return true; | |
| } | |
| } | |
| } | |
| } else if (strings.indexOfChar(name, '-')) |i| { | |
| if (strings.indexOfChar(name, '/')) |i| { | |
| const scope_and_pkg = name[i + 1 ..]; | |
| inline for (platform_keywords) |keyword| { | |
| if (strings.containsComptime(scope_and_pkg, keyword)) { | |
| return true; | |
| } | |
| } | |
| } else if (strings.indexOfChar(name, '-')) |i| { |
just makes it a little more simple. we can assume the package name is valid. also, when the package has a '/', should we also check for '-' before looking for keywords?
There was a problem hiding this comment.
I was half trying to have best perf as who knows how many times this fn gets ran (yes i know its not too much, but every little thing right)
Yeah we could check for - there too but im not sure if its going to gain much and I haven't checked enough to see whats in use (ie is there a @mybuild/win32 or smth)
There was a problem hiding this comment.
@mybuild/win32 is a good example, wouldn't be surprised if this is common. let's not check for '-' if it's scoped
There was a problem hiding this comment.
I don't think(/hope) anyone puts the specific stuff in the org part, so I think its fine to only search the actual name part
In anycase, this should yield a default true for unmatched packages in this rudimentary search (as if bun doesn't see libc, it assumes all)
| // try Negatable(Npm.Libc).toJson(meta.libc, writer); | ||
| // try writer.writeAll("], "); | ||
| // } | ||
| if (meta.libc != .all and meta.libc != .none) { |
There was a problem hiding this comment.
wouldn't we want to serialize either if it's .all or if it's .none because they're different?
There was a problem hiding this comment.
i think how the others do this is with "none" which isn't great but makes it not .all
There was a problem hiding this comment.
well it defaults to .none so we don't want to always show, and .all is all so like others don't need to include either (even though it should be impossible to have).
4 participants
What does this PR do?
for native packages, they often add
os,cpu, (and sometimeslibc) to control where it gets installed to. NPM has 3 similarly named flags where you can change the defaults. This also adds some common flags like --save-dev to help people migrate easier.Unfortunately NPM doesn't give the
libcarray in the abbreviated metadata (npm/registry#26), so we have to use the normal one to get it. However, this is a big speed issue for more to downloads and then more to json parse. So instead it does a basic guess of if its an optional dependency, is scoped" and and common key words like linux in it.#14091
also with #20945,
bunx bun@latestshould be faster on linux as doesn't need to download 4 editions and instead only 2How did you verify your code works?
made tests on local mock registry