Npm: download the required binaries during installation

[aminya]

• The binaries are now downloaded upon installation. This reduces the package size significantly, which is very important for open-source repositories.

• Fixes finding the binaries on Windows inside the hooks. Previously the code fell back to using npx

Closes #43

[Envek]

Hello! Thank you for your contribution and sorry for the late review!

Recently we had discussions on whether we should change packaging to download binaries on install or not and finally decided that we should. So your pull request is just in time!

However, there are some issues that should be addressed first:

1. Download is broken on Linux and MacOS: file has wrong name due to incorrect filename extension detection and missing executable bit.

2. In case if download failed, error message is unclear:

   [5/5] Rebuilding all packages...
   error /home/envek/test/node_modules/@arkweid/lefthook: Command failed.
   Exit code: 1
   Command: node postinstall.js
   Arguments: 
   Directory: /home/envek/test/node_modules/@arkweid/lefthook
   Output:
   events.js:292
         throw er; // Unhandled 'error' event
         ^
   
   Error: Response status was 404
       at ClientRequest.<anonymous> (/home/envek/evl.ms/test/node_modules/node-downloader-helper/dist/index.js:1:7153)
       at Object.onceWrapper (events.js:422:26)
       …
   

   To help users to understand what is going wrong (and to help us debug it), error message (Response status was 404 in this example) should be prefixed with details about what and when failed. Something like this:

   Failed to download lefthook_0.7.5_Linux_x86_64: Response status was 404 while fetching https://github.com/evilmartians/lefthook/releases/download/v0.7.5/lefthook_0.7.5_Linux_x86_64
   

[aminya]

Thank you. I addressed the comments.

[Envek]

Yes, we know that. In that case other installation methods can be used instead.

I wish that NPM supported platform-specific packages, but in current form it is already way too heavy,

[Envek]

I extracted everything about executable extension into separate commit and pushed it to master, so here we can focus on NPM package (no action needed).

[bbodenmiller]

Yes, we know that. In that case other installation methods can be used instead.

I wish that NPM supported platform-specific packages, but in current form it is already way too heavy,

Perhaps a net install (this PR) vs full install (current package) could be maintained then. Alternatively it appears there is options to handle this using OS dependencies: https://stackoverflow.com/questions/15176082/npm-package-json-os-specific-dependency

[aminya]

You could make a fork, and add proxy support similar to prebuild-install

[bbodenmiller]

You could make a fork, and add proxy support similar to prebuild-install

Proxy support does not address organizations that forbid external downloads by policy and/or block external internet access other than via npm mirror.

Do what you want, I'm just telling you have this will break some workflows and it appears I'm not the first to raise this concern.

[aminya]

If the company blocks the internet, it should mock it using its proxy server. https://github.com/evilmartians/lefthook/releases/download URL can be simply redirected to a local server that hots the binaries. This is probably similar to how that company is mocking the URLs for the packages that use prebuild-install and node-pre-gyp (~6 million downloads a week).

[Envek]

We probably would like to support two packages (one with bundled executables and one that downloads them), but it requires changing how hook scripts invoke lefthook binary.

[Envek]

Thank you! We will release it soon. Maybe next week.

[aminya]

Thanks! 🎉

[privatenumber]

Since this fetches binaries from GitHub, I want to raise the concern that this could interfere with environments that rely on a npm proxy registry (originally raised here).

I think this issue is more relevant than it appears. Many environments use a npm proxy in case npm goes down or becomes inaccessible (eg. CI deploys, China, etc). In the case where GitHub becomes unavailable, (which happens more than it should...), this will break npm install.

I think this can be easily addressed by publishing the binaries to npm and downloading from npm instead of GitHub (like playwright-chromium).

[aminya]

Thank you! We will release it soon. Maybe next week.

Any plan to release this?

[aminya]

@Envek

[Envek]

Sorry, hadn't a chance to prepare a release (it is pretty big compared to previous ones), so it is postponed until I will have a day or two to polish things out. Can't say any specific date or time now.

[Envek]

It finally was released in 0.8.0 and now available as @evilmartians/lefthook-installer package.

Thank you very much for you contribution and sorry for such a long delay.