node-gyp-build
node-gyp-build copied to clipboard
prebuild
fix: fix the preinstall function logic
It simplifies the preinstall logic to call the load function and report errors in case of not finding a prebuild. It then falls back to the build.
Fixes #54
Test result:
This is not the right place to implement this. But before we dive into that, let's first discuss if we even want this, because I'm not sure.
On the one hand, per the unix philosophy, output should not include unnecessary information. That a build was not found is unnecessary information because there's a fallback to compiling from source. If that succeeds (and we expect it to) then output should be empty. Also because "no native build was found" can be misinterpreted as an error (https://github.com/prebuild/node-gyp-build/issues/42). If it doesn't succeed, then npm i --verbose can be used to opt-in to that extra information.
On the other hand, npm is swallowing the output of install scripts nowadays, so we already get the desired behavior of "silent on success". So we could decide to always be verbose.
But before we dive into that, let's first discuss if we even want this, because I'm not sure.
This helps the developers to understand the reasons why their package installation goes through the build process instead of just loading the file. I'll open an issue for npm if npm doesn't report the error logs by default.
On the one hand, per the unix philosophy, output should not include unnecessary information
As per my government's rules, error logging must be enabled on all Information Technology (IT) assets throughout the enterprise. So, not sure if the philosophical opinions come into play here.
Also, the previous code was executing an unvalidated process.argv[2]. It can be exploited as remote code execution. I haven't spent much time looking into the code, but I think it is a CVE target. I have sent an email to the author.
As per my government's rules, error logging must be enabled
Interesting, but the point is, it's not an error per se.
Also, the previous code was executing an unvalidated process.argv[2]. I am not sure if it can be exploited as remote code execution. I haven't spent much time looking into the code, but it might be a CVE target.
No, because it's sourced from an npm script. I.e. there's no difference in risk between node-gyp-build dangerous-command and node-gyp-build && dangerous-command. But, if I'm wrong and there is a real vulnerability, then please contact one of us privately with details.
I'll open an issue for npm if npm doesn't report the error logs by default.
It swallows the output of an install script unless that script exited with a non-zero code. Which I think fits your requirements.
Yeah, so when the build also fails, I can know why the to prebuild was not picked up in the first place.
The intent of node-gyp-build seems to be to be to make loading native bindings a smooth experience. Adding a message here doesn't really do that and it gets buried under the already-confusing log messages of node-gyp that follow.
I would definitely be for making the situation less confusing. Maybe better guidance for --build-from-source/--fallback-to-build? Or if your package does have to be built on install instead of on demand, use something like https://www.npmjs.com/package/bindings ?
