Open jlarmstrongiv opened 6 months ago
for reference: https://github.com/vercel/ncc/issues/1183
From the ncc dev: "We specifically include support for bindings and node-pre-gyp". I am pretty sure they don't anticipate multiple architectures.
Those libraries support prebuilt binaries with multiple architectures though right?
It’s just that only the correct binary for the particular os/architecture is downloaded using npm scripts at install time, rather than prebuildify’s new pattern of including all the architectures in the npm package.
But I'll see what they do.
I doubt @vercel/ncc
or esbuild
will move quickly to support prebuildify. They already bundle most .node files successfully if they follow the common pattern.
The only solution I can think of so far is to have a build script to move the correct binary to a location that common bundlers like esbuild
, @vercel/ncc
, and webpack
expect.
If build scripts are enabled and the correct binary is renamed and moved to the expected location, all should work fine. If build scripts are disabled, then the new logic for locating the binary can be the fallback.
That way, bundling will still work with build scripts enabled and usearch will still work even if build scripts are disabled.
What do you think @sroussey ? I’m trying to think of other compromises, but that’s the only one that’s come to mind. If you have any other ideas or workarounds, please share!
I asked them what I can do on my side. I don't expect them to write any code, and would likely take too long anyhow.
I've been trying to use the folders they expect, but no success so far. Next try is to also add a require to something they expect and see if that triggers it. If I do find a folder and require combo that works, we may have to rename all the .node files since they all have the same name. Then I can collapse the folder structure. Github actions doesn't like that though. It likes assets to have their own folder. Maybe I can add another step after the builds are done but before the packaging.
BTW: ncc
uses webpack-asset-relocator-loader
.
So, some such around here: https://github.com/vercel/webpack-asset-relocator-loader/blob/bda4108a3aeb672fe42da85650349ded73fba6e1/src/asset-relocator.js#L207-L215
My thinking at the moment is that I don't use the prebuildify library, but instead create my own. Instead of using variables to figure out the path, I will switch on them and have hard coded paths that the code scanner will find. I think this will work across build tools.
I don't think it will be hard, but it will have to wait until tomorrow.
BTW: have you tried this?
npx --yes @vercel/ncc build test.js -o ncc-usearch-bundled --debug --external usearch
cd ncc-usearch-bundled
npm init -y
npm i usearch
That should work. At least to get you through the weekend. Also you see how external works and helps in tricky situations.
I asked them what I can do on my side. I don't expect them to write any code, and would likely take too long anyhow.
My apologies. I’m sorry I didn’t fully read the linked issue. You’re entirely right.
My thinking at the moment is that I don't use the prebuildify library, but instead create my own. Instead of using variables to figure out the path, I will switch on them and have hard coded paths that the code scanner will find. I think this will work across build tools.
That should work. If the paths can be analyzed, they can be bundled.
In previous js bundlers, a partial dynamic syntax was supported, but esbuild, vite, and others have not implemented it https://github.com/evanw/esbuild/issues/700 so I think your method is best.
If I do find a folder and require combo that works, we may have to rename all the .node files since they all have the same name. Then I can collapse the folder structure. Github actions doesn't like that though. It likes assets to have their own folder. Maybe I can add another step after the builds are done but before the packaging.
I’ve done the same thing where I add another build step to move the assets before the publishing. It’s good to do if needed.
BTW: have you tried this?
I like your code examples! (especially the last one with --external
flag) But, the main package I’m trying to bundle is usearch
. I’ve already excluded it from the build, and I just need to bundle the package together into a single js + single .node file.
So, if I add this dummy code, it will work in ncc. But unfortunately, not esbuild:
if (process.uptime() < 0) {
require(__dirname + "/../../../prebuilds/darwin-arm64+x64/usearch.node");
require(__dirname + "/../../../prebuilds/linux-arm64/usearch.node");
require(__dirname + "/../../../prebuilds/linux-x64/usearch.node");
require(__dirname + "/../../../prebuilds/win32-ia32/usearch.node");
require(__dirname + "/../../../prebuilds/win32-x64/usearch.node");
}
and maybe something like
require(__dirname + "/../../../build/Release/usearch.node");
in case people did their own build.
Sadly
require(__dirname + `/../../../build/${os.platform}-${os.arch}/usearch.node`);
did not work.
BTW: did you try just copying over the prebuilds
folder in your adventures?
cosmetic changes
Maybe a red herring? It uses
windows-latest
so maybe that changed instead?
@ashvardanian -- I did a force push of the last commit had a good windows build.
Both of these are the same commit:
https://github.com/unum-cloud/usearch/actions/runs/8625104966/job/23641178068
https://github.com/sroussey/usearch/actions/runs/8681208236/job/23803506624
This is a good reason to pin your dependencies and not use -latest
etc.
@sroussey I am not sure I understand correctly.
The diff between our versions is ubuntu-latest
vs ubuntu-22.04
.
But in our case only the Windows runner is failing, Ubuntu is fine.
Moreover, ubuntu-latest
and ubuntu-22.04
should map to the same config.
and maybe something like
require(__dirname + "/../../../build/Release/usearch.node");
in case people did their own build.
That’s smart
Sadly
require(__dirname +
/../../../build/${os.platform}-${os.arch}/usearch.node);` did not work.
Still waiting to support variables in dynamic imports for esbuild https://github.com/evanw/esbuild/issues/700
Would using relative paths allow esbuild
to bundle correctly?
BTW: did you try just copying over the prebuilds folder in your adventures?
I try to let the bundlers do that, because I’ve had problems with other libraries where the paths change due to bundling and it’s better just to let them handle it.
But in our case only the Windows runner is failing, Ubuntu is fine.
My guess is that they changed something in the windows runner.
I try to let the bundlers do that, because I’ve had problems with other libraries where the paths change due to bundling and it’s better just to let them handle it.
Yeah, looks like ncc will change it to be relative to the combined js file. So the copy of the prebuilds folder is adjacent to the index.js file (which just happens to be where we expect it to be).
Seems like you are right, all of our Windows prebuild pipelines fail across repos. @sroussey whats the downside of disabling prebuild pipeline?
You could disable for windows which should cause everyone to rebuild on their machine on that platform.
If they don't allow lifecycle scripts it will fail, same with bun, etc. They will need to make changes to accommodate.
I'm not a big windows build expert but I'll have another look this weekend. I'm curious if other people are having windows build issues on GitHub actions in 2024. What was the date it started failing? Might be a clue.
@sroussey started failing about a month ago, same in SimSIMD. Thank you!
Did the paths to the output binaries change? It seems the require
added for the bundler no longer work
Are you using main
or main-dev
?
I’m using the latest npm package 2.11.1
. I will investigate further
I tried uploading all usearch.node
prebuilds, and I get invalid ELF header
, which means it’s not selecting the right binary either
Using TypeScript and prebuilt binaries was a mistake IMHO. It’s an extremely complicated pipeline. I have absolutely no clue about how it works. Would it be better if we switch back to pure JS and let everyone build locally from sources? Our build time should be pretty low.
I can remove that next week.
Sent from my iPhone
On Fri, Jun 7, 2024 at 10:30 AM Ash Vardanian @.***> wrote:
Using TypeScript and prebuilt binaries was a mistake IMHO. It’s an extremely complicated pipeline. I have absolutely no clue about how it works. Would it be better if we switch back to pure JS and let everyone build locally from sources? Our build time should be pretty low.
— Reply to this email directly, view it on GitHub https://github.com/unum-cloud/usearch/issues/377#issuecomment-2155246792, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAA7C5J34XRP5XHYHSCII3DZGHUZVAVCNFSM6AAAAABFSMBACWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNJVGI2DMNZZGI . You are receiving this because you were mentioned.Message ID: @.***>
I tried switching from arm64 to x86_64 and it did pick the right binary that time, so I’m not sure why it isn’t selecting arm64
I usually have a good experience with prebuilds, and they definitely save time with npm install
and heartache dealing with all those SIMSIMD_TARGET
environment variables.
I’d love for prebuilds + usearch to work together. Is it possible to have an escape hatch for users to select the path to the prebuilt usearch.node
file? Would that be the best of both worlds?
Here’s an example of that option from better-sqlite3
:
`options.nativeBinding`: if you're using a complicated build system
that moves, transforms, or concatenates your JS files,
better-sqlite3 might have trouble locating its native C++ addon (`better_sqlite3.node`).
If you get an error that looks like [#534](https://github.com/JoshuaWise/better-sqlite3/issues/534#issuecomment-757907190),
you can solve it by using this option to provide the file path of `better_sqlite3.node`
(relative to the current working directory).
It’s also very common for wasm libraries to offer that option as well.
It would also allow me to select the right arm64 architecture and allow us to ignore bundling altogether.
I just feel like @sroussey put a lot of effort into the high value feature of prebuilds, and it’d be a shame to remove it when it’s so close. Adding this escape hatch will mean that the current code will work in many cases, and advanced cases will always have a workaround to get their binary.
You can always just build, it is not automatic if it thinks it has a prebuild though.
Tests on builds don't catch anything wrong since this is a cross-build as Github doesn't have arm64 for actions. Otherwise it should get caught.
I am packing my life into boxes at the moment, but I should be in a hotel next week and able to use more than an ipad... i will check on things then.
BTW: you can do something hacky... like (npm install && cd node_modules/usearch && npm run prebuild-arm64)
Describe the bug
NPM install fails on arm64 linux. I presume it couldn’t find a new prebuilt binary (exciting new feature from @sroussey), and fell back to compiling from scratch, which it can no longer do because the source and gyp files have been removed from the npm package.
Steps to reproduce
On an arm64 machine, run
Inside the container, run
View the error:
Expected behavior
NPM install to work with the
linux-arm64
prebuildsRegardless, it’s important for falling back to compiling from scratch with
node-gyp
to continue to work.USearch version
2.10.x
Operating System
Amazon Linux 2023
Hardware architecture
Arm
Which interface are you using?
Other bindings
Contact Details
Ping me in Discord
Is there an existing issue for this?
Code of Conduct