node-usb / node-usb

Improved USB library for Node.js
https://node-usb.github.io/node-usb/
MIT License
1.57k stars 277 forks source link

Need to disable the prebuild #558

Closed theofficialgman closed 1 year ago

theofficialgman commented 1 year ago

Bug Description:

prebuilds are linking to glibc 2.29+ so can not be run on bionic and buster and other distros

Steps to Reproduce:

  1. npm install usb

I have tried multiple options such as --no-binary-cache and --build-from-source when installing from npm with my project and always get the broken prebuilds.

how can this be disabled so I always build from source?

linking webpack since it is not folling the rebuild request https://github.com/webpack/webpack/issues/16470

mildsunrise commented 1 year ago

umm, the prebuild should not be a problem in the situation you mention, because node-gyp-build should fail to load it (thus falling back to a build from source). let me check...

mildsunrise commented 1 year ago

background: the problem with prebuild portability has been brought back before. I brought it up with #444, but then I'm seeing I failed to follow up on that (@thegecko sorry). regardless, the fallback should be working...

theofficialgman commented 1 year ago

I can tell you from experience what is happening.

I execute npm install --build-from-source on a project (which includes usb) the packages install to node_modules. at this point, the usb package still has the prebuilts/linux-arm64 folder and its contents intact, they don't get deleted or updated with a build from source. thats what I assume should be happening.

I then execute npm run webpack which among other things, copies native node modules directly from the prebuilts folder into the root directorys generated folder.

again, not really sure if the issue here is webpack, node-usb, node-gyp-build, or prebuildify. something in the chain is not working as it should since the prebuilts are getting copied over to the generated folder (which files get used by the electron binary)

I have written some nasty script to get around this (and lzma-native that literally puts x86_64 binaries in the linux-arm64 prebuilds folder... ugh. didn't include the build-from-source in the command since it didn't work with webpack anyway) https://github.com/Pi-Apps-Coders/files/blob/main/CompileCommands.md#balenaetcher

mildsunrise commented 1 year ago

Okay, the missing piece of info is that this is for the arm64 architecture, and that this is for an Electron pack. I tested on an ubuntu:18.04 x86_64 Docker container, with Node v10.x, and the prebuild was compatible so there wasn't even a need to fall back to source. Maybe arm64 prebuilds target a more recent glibc, this is something we should fix.

As for implementing support for --build-from-source, it is not our place to add it. Instead node-gyp-build should implement that support (there's https://github.com/prebuild/node-gyp-build/issues/20 open). Once it's implemented there, ping us in case a version upgrade is needed on our side. If the authors are unresponsive, we can switch to another loader (sad, because node-gyp-build seems to be the 'standard', but whatever).

I execute npm install --build-from-source on a project (which includes usb) the packages install to node_modules. at this point, the usb package still has the prebuilts/linux-arm64 folder and its contents intact, they don't get deleted or updated with a build from source.

why should they? the prebuilds come from the package. they don't hurt, as they will only be used if build/Release/*.node does not exist. the point is that node-gyp-build should ignore the prebuilds and launch node-gyp is --build-from-source is used.

mildsunrise commented 1 year ago

actually... I see that https://github.com/prebuild/node-gyp-build/issues/20 is only for --build-from-source=<specific package>. node-gyp-build should already implement support for a general --build-from-source.

did a small test and it works. as you can see node_modules/usb/build exists, so that means --build-from-source was obeyed:

Screenshot from 2022-11-13 22-42-30

theofficialgman commented 1 year ago

yes I see that, but that doesn't solve the problem. the problem is that the prebuilts still exist and it appears that nothing cares about the build folder existing.

what part of the of the software stacks job is it to decide to pull the binary from the build folder vs the prebuilts/linux-arm64 folder?

in my experience (not a node developer, I'm a user here attempting to build software), webpack appears to simply copy from the prebuilts/linux-arm64 folder, regardless of the build folder existing. I don't know if this is correct and node-gyp-build should be copying the files from the build folder over to the correct system's prebuilts folder or not. that is why I have cross listed this bug at webpack as linked in my inital posting https://github.com/webpack/webpack/issues/16470

theofficialgman commented 1 year ago

also it might be because the files inside the Release folder are not named .node which is why the build folder contents aren't being used

mildsunrise commented 1 year ago

what part of the of the software stacks job is it to decide to pull the binary from the build folder vs the prebuilts/linux-arm64 folder?

node-gyp-build is what we use to both do the conditional building, and to load the binary.

node-gyp-build always prefers a binary from build if it exists, by design. what makes you think it is picking the prebuilds, does it react differently if you remove the prebuilds folder? can you share some logs so we can get a better idea?

theofficialgman commented 1 year ago

what makes you think it is picking the prebuilds, does it react differently if you remove the prebuilds folder? can you share some logs so we can get a better idea?

I know it is using the prebuilts folder because I know what the binary hash is of the prebuilts binary vs the the one from the build folder.

mildsunrise commented 1 year ago

also it might be because the files inside the Release folder are not named .node which is why the build folder contents aren't being used

they are not? then this explains it, yes. node-gyp-build searches for a .node file in build/Release or build/Debug, so maybe something on your side / webpack isn't respecting that?

I know it is using the prebuilts folder because I know what the binary hash is of the prebuilts binary vs the the one from the build folder.

I am not following, sorry :_

theofficialgman commented 1 year ago

I know it is using the prebuilts folder because I know what the binary hash is of the prebuilts binary vs the the one from the build folder.

I am not following, sorry :_

here is an example, the binary from the prebuilds folder:

~/etcher/node_modules/usb/prebuilds/linux-arm64$ sha1sum node.napi.armv8.node 
0a17c054b1df0d149ea6ed5651bebea292d975a5  node.napi.armv8.node

here is the only .node file that got generated in the build folder

~/etcher/node_modules/usb/build/Release$ sha1sum usb_bindings.node 
e4517f1a90d2a7c6f49a3d21dea6b0bce67a0cf1  usb_bindings.node

npm run webpack coped the binary from the prebuilts/linux-arm64 folder over to to the generated folder.... not the one from the build/Release folder as I said. you can tell because the sha1sum for those files matches

~/etcher/generated/modules/usb/prebuilds/binding$ sha1sum node.napi.armv8.node 
0a17c054b1df0d149ea6ed5651bebea292d975a5  node.napi.armv8.node
mildsunrise commented 1 year ago

I see. then it seems like webpack is picking the wrong binary? given that it's not simply copying the usb directory to the generated files, but applying its own structure, I'm afraid this isn't something I can help with sorry :(

theofficialgman commented 1 year ago

this appears to be a similar/related case to https://github.com/node-usb/node-usb/issues/465 https://github.com/node-usb/node-usb/issues/463

theofficialgman commented 1 year ago

does it react differently if you remove the prebuilds folder?

I removed the prebuilds folder entirely for linux-arm64. I think this error message from webpack tells us all we need to know

ubuntu@instance-20220801-1619:~/etcher$ rm -rf node_modules/usb/prebuilds/linux-arm64
ubuntu@instance-20220801-1619:~/etcher$ npm run webpack

> balena-etcher@1.10.0 webpack
> webpack

[webpack-cli] Failed to load '/home/ubuntu/etcher/webpack.config.ts' config
[webpack-cli] Error: Could not find usb prebuild. Should try fallback to node-gyp and use /build/Release instead of /prebuilds
    at findUsbPrebuild (/home/ubuntu/etcher/webpack.config.ts:88:9)
    at Object.<anonymous> (/home/ubuntu/etcher/webpack.config.ts:125:50)
    at Module._compile (node:internal/modules/cjs/loader:1155:14)
    at Module.m._compile (/home/ubuntu/etcher/node_modules/ts-node/src/index.ts:1056:23)
    at Module._extensions..js (node:internal/modules/cjs/loader:1209:10)
    at Object.require.extensions.<computed> [as .ts] (/home/ubuntu/etcher/node_modules/ts-node/src/index.ts:1059:12)
    at Module.load (node:internal/modules/cjs/loader:1033:32)
    at Function.Module._load (node:internal/modules/cjs/loader:868:12)
    at Module.require (node:internal/modules/cjs/loader:1057:19)
    at require (node:internal/modules/cjs/helpers:103:18)
theofficialgman commented 1 year ago

hmm... this seems oddly specific

https://github.com/balena-io/etcher/blob/7de99003ca74fe7561119ab7131f1780aeeaf71f/webpack.config.ts#L80-L128

did you write it (or another developer here) and etcher just copy/pasted it into their repo? this seems to be why the build files aren't getting copied. I'm starting to think webpack doesn't have any way of automatically copying over the .node native modules and the etcher developers simply added their own hacks to a script inorder to do so.

mildsunrise commented 1 year ago

no, I definitely did not write this. judging by the blame output it seems to be @ab77, maybe they didn't anticipate people wanting a build from source? edit: the commit is pretty big though (https://github.com/balena-io/etcher/commit/2e53feb38cd548bd2fe68957797bffe66b832eaf) so not sure if it was copy/pasted from somewhere else

ab77 commented 1 year ago

FIY @mcraa @zwhitchcox

mcraa commented 1 year ago

Sorry for the slow response i am not around my computer these days.

IIUC this issue can be closed. If the OP wants to build etcher on an architecture without a prebuild the webpack config needs to be modified. Yes, electron-builder (so etcher as well) needs specific webpack config to keep native modules working. We have the current config to make it build on our CI, for supported the architectures and platforms of github actions.

Of course PRs welcome in the etcher repo, or an issue can be opened there, and I can extend it as soon as i am back.

theofficialgman commented 1 year ago

I'll go ahead and close this given the feedback ofc it would be nice if ARMhf/ARM64 prebuilds could be as compatible as the x86_64 ones with a lower GLIC requirement (there are multiple ways to build on bionic and even older disros, such as a chroot, bionic build servers, cross compiling on bionic with github actions, etc)

thegecko commented 1 year ago

there are multiple ways to build on bionic and even older disros, such as a chroot, bionic build servers, cross compiling on bionic with github actions, etc

We use prebuildify-cross to try to get this as low as possible, with images over at https://github.com/node-usb/docker-images

Any help with this would be appreciated.

theofficialgman commented 1 year ago

We use prebuildify-cross to try to get this as low as possible, with images over at https://github.com/node-usb/docker-images

Any help with this would be appreciated.

use a different image then. use the -lts image variants instead of the regular ones which will have a lower GLIBC requirement. https://github.com/dockcross/dockcross is used by https://github.com/prebuild/docker-images which in turn is used by https://github.com/node-usb/docker-images