RFC: prebuilt addon dependencies

[mscdex]

I'm working on making some addon dependencies that provide prebuilt versions to avoid the need for a build environment. So far I'm only testing this with cpu-features. If the entire process (including prebuilt addons working on the systems they were designed for and falling back to building from source) proves to be reliable, I will merge the necessary changes into the master branch.

I've tested the experimental branch on a few different systems of various configurations with success, but it would be great if others could test it as well, especially on platforms like mac and Windows (as I don't own such systems). All that's needed is to npm install mscdex/ssh2#prebuilt and then npm test from within the resulting node_modules/ssh2 directory. That should be a good test to make sure everything is working as expected.

[ferbs]

It fails in a node:14.18-bullseye-slim container with: gyp ERR! find Python

Maybe take a look at how node-ffmpeg-installer handles their prebuilt binaries?

Also, I'd suggest linking here from #1083. Many people will land there directly after a search and won't otherwise see your request.

[mscdex]

Can you elaborate a bit on the error? Is it failing on the optional dependency or the addon bundled with ssh2?

Unfortunately node-ffmpeg-installer utilizes a less than ideal approach for binary handling. It requires a separate npm package for each binary and even then you can/will still have issues with binary compatibility because it's only taking CPU and OS into account. You also need to take into account libc type, libc version, node modules ABI version and/or n-api version, etc. Believe me, if there was an existing project that dealt with all of that plus had a solution for taking advantage of Github Actions to do all of the building for each configuration, I would have just used that instead of creating my own system.

[mikl]

I tried this with Node.js 14.18.1 + npm v6.14.15 on macOS 12.1 (Monterey) on an Apple Silicon MacBook Pro, with this result:

⌁ [mikl:~/Work/Node.js/tmp] % npm install mscdex/ssh2#prebuilt

> cpu-features@0.0.2 install /Volumes/Work/Node.js/tmp/node_modules/cpu-features
> node-install-addons

[install-addons] OS version could not be determined
[install-addons] Falling back to build ...
[install-addons] Checking for available minbuild...
[install-addons] Fetching release info at https://api.github.com/repos/mscdex/cpu-features/releases/tags/v0.0.2
[...snip, proceeds to build cpu-features successfully]

I assume OS version could not be determined + Falling back to build was not the result you were looking for? The tests pass.

[ferbs]

elaborate a bit on the error

Sure, I ran docker run --rm --interactive --tty --user=root node:14.18-bullseye-slim bash and then:

# add git to the container so npm can install #prebuilt branch
apt-get update && apt-get install git 

su node 
cd
mkdir try
cd try

npm init -y

Then tried the install:

node@fe8dba47e8e0:~/try$ npm install mscdex/ssh2#prebuilt

> ssh2@1.5.0 install /home/node/try/node_modules/ssh2
> node install.js

gyp ERR! find Python
gyp ERR! find Python Python is not set from command line or npm configuration
gyp ERR! find Python Python is not set from environment variable PYTHON
gyp ERR! find Python checking if "python" can be used
gyp ERR! find Python - "python" is not in PATH or produced an error
gyp ERR! find Python checking if "python2" can be used
gyp ERR! find Python - "python2" is not in PATH or produced an error
gyp ERR! find Python checking if "python3" can be used
gyp ERR! find Python - "python3" is not in PATH or produced an error
gyp ERR! find Python
gyp ERR! find Python **********************************************************
gyp ERR! find Python You need to install the latest version of Python.
gyp ERR! find Python Node-gyp should be able to find and use Python. If not,
gyp ERR! find Python you can try one of the following options:
gyp ERR! find Python - Use the switch --python="/path/to/pythonexecutable"
gyp ERR! find Python   (accepted by both node-gyp and npm)
gyp ERR! find Python - Set the environment variable PYTHON
gyp ERR! find Python - Set the npm configuration variable python:
gyp ERR! find Python   npm config set python "/path/to/pythonexecutable"
gyp ERR! find Python For more information consult the documentation at:
gyp ERR! find Python https://github.com/nodejs/node-gyp#installation
gyp ERR! find Python **********************************************************
gyp ERR! find Python
gyp ERR! configure error
gyp ERR! stack Error: Could not find any Python installation to use
gyp ERR! stack     at PythonFinder.fail (/usr/local/lib/node_modules/npm/node_modules/node-gyp/lib/find-python.js:307:47)
gyp ERR! stack     at PythonFinder.runChecks (/usr/local/lib/node_modules/npm/node_modules/node-gyp/lib/find-python.js:136:21)
gyp ERR! stack     at PythonFinder.<anonymous> (/usr/local/lib/node_modules/npm/node_modules/node-gyp/lib/find-python.js:179:16)
gyp ERR! stack     at PythonFinder.execFileCallback (/usr/local/lib/node_modules/npm/node_modules/node-gyp/lib/find-python.js:271:16)
gyp ERR! stack     at exithandler (child_process.js:390:5)
gyp ERR! stack     at ChildProcess.errorhandler (child_process.js:402:5)
gyp ERR! stack     at ChildProcess.emit (events.js:400:28)
gyp ERR! stack     at Process.ChildProcess._handle.onexit (internal/child_process.js:280:12)
gyp ERR! stack     at onErrorNT (internal/child_process.js:469:16)
gyp ERR! stack     at processTicksAndRejections (internal/process/task_queues.js:82:21)
gyp ERR! System Linux 5.10.25-linuxkit
gyp ERR! command "/usr/local/bin/node" "/usr/local/lib/node_modules/npm/node_modules/node-gyp/bin/node-gyp.js" "--target=v14.18.2" "rebuild"
gyp ERR! cwd /home/node/try/node_modules/ssh2/lib/protocol/crypto
gyp ERR! node -v v14.18.2
gyp ERR! node-gyp -v v5.1.0
gyp ERR! not ok
Failed to build optional crypto binding
npm WARN try@1.0.0 No description
npm WARN try@1.0.0 No repository field.

+ ssh2@1.5.0
updated 1 package and audited 15 packages in 3.464s
found 0 vulnerabilities

node@fe8dba47e8e0:~/try$

[mikl]

With Node v16.13.1 + npm v8.1.2 (on macOS 12.1 (Monterey) on an Apple Silicon MacBook Pro) the npm logs are a bit more cryptic, can’t really tell if it’s successfully using a prebuilt binary, but the tests pass.

[ferbs]

You also need to take into account libc type, libc version, node modules ABI version and/or n-api version, etc

I didn't write up enough detail on this either, sorry. I haven't actually looked at either implementation myself, just mentioned that installer because ffmpeg also cares about hardware features and so thought their approach might be worth investigating.

Along similar lines of rough ideas/suggestions, a fallback idea comes to mind. Something to consider if the build headaches get unpleasant. You could use an explicit opt-in approach where someone npm installs the plugin externally themselves. Then either it gets picked up using feature detection or the dev passes it in directly (like useCpuFeaturesPlugin(features).) With the downside that it's prob a major semver increment.

[mscdex]

Sure, I ran docker run --rm --interactive --tty --user=root node:14.18-bullseye-slim bash and then:

That's the bundled addon, so that makes sense.

npm 7 or 8 started making optional dependency install scripts silent unless there's a failure, so it appears cpu-features was installed fine.

It would be interesting to see if it found a compatible binary or if it built from source. Not sure if adding --loglevel verbose to the npm install command would show that or not....

You could use an explicit opt-in approach where someone npm installs the plugin externally themselves.

That's not really an ideal approach as I think most people wouldn't know to do that or would want to go through the extra effort. Having npm pull it in as an optional dependency is the ideal mechanism and is what we're currently using (and doing detection at runtime as you mention). The whole point to having this dependency at all is to make the default handshake parameters be more optimal "out of the box" depending on the CPU and its features.

I assume OS version could not be determined + Falling back to build was not the result you were looking for?

It's good to see it fell back to building and that worked. Does sw_vers -productVersion work on that mac? That's currently what we're using to determine the macOS version. Perhaps there is a different mechanism for newer macOS versions?

[mikl]

Does sw_vers -productVersion work on that mac? That's currently what we're using to determine the macOS version. Perhaps there is a different mechanism for newer macOS versions?

It does, but the return value is different on Apple Silicon Macs as I understand it. On Intel Macs, it still returns a 10.x number for the latest OS (I guess 10.19 or something like that), but on Apple Silicon, it gives the new versioning scheme, so on my machine it returns 12.1.

(I just checked, on my Intel Mac, it also returns 12.1)

[ferbs]

Having npm pull it in as an optional dependency is the ideal mechanism

Yeah. I wonder if it's possible to stuff that part of the install into a child process? Then filter/modify the actual output.

[priceaj]

Windows 11 Logs attached

ssh2-npm-test-log.txt

[mscdex]

Ok, I see the bug with getting the macOS version. I will fix that soon.

Yeah. I wonder if it's possible to stuff that part of the install into a child process? Then filter/modify the actual output.

I'm not sure what you mean here. What I was saying is npm is already doing this, as we cpu-features is an optionalDependency in package.json. The problem we're trying to solve here is twofold:

1. Build output during install (npm v8 -- and possibly v7 -- now hide build output if the build was successful)
2. No build environment during install (thus leading to case 1)

Technically (especially since the "build" process is now greatly expanded) we could always hide the output and always return zero so that nothing ever gets seen, this would work for all npm versions. However, my concern is the lack of any messaging. With that kind of setup, during installation you won't know whether dependencies will be available or not. The alternative is to display something or emit a warning using the standard node.js core mechanism for doing so, but I think that would annoy people even more.

Currently the only way to know if the default handshake parameters will be optimized out of the box is by looking at debug output, since I do include that information there.

[mscdex]

Windows 11 Logs attached

A couple of interesting issues there. Thanks.

I will probably add a flag for printing environment/debug information to help identify why no compatible binary was found, etc.

[ferbs]

Filtering not hiding. There will be ongoing failure cases: new hardware and OS releases, build dependencies missing in environment, etc. I'd suggest tuning the messaging, with no stacktrace displayed in stdout/stderr. Eg:

"Unable to install or build optional 'cpu-features' ssh2 dependency. This can have a significant impact on performance, see /tmp/ssh2_install_error/ssh2_cpuFeatures_optionalInstallError.20220106abc123.log for details. More information at https://github.com/mscdex/ssh2/wiki/cpu-features"

[mscdex]

Filtering not hiding. There will be ongoing failure cases: new hardware and OS releases, build dependencies missing in environment, etc. I'd suggest tuning the messaging, with no stacktrace displayed in stdout/stderr. Eg:

The mechanism being used here (install-addons) is generic, so including hardcoded URLs like that is not really an option. Additionally, writing a log to disk is not something I'm very keen on, especially from a maintenance perspective (e.g. handling failures, where to put the log, etc.). If there was a way to incorporate the output into the npm log, I'd be open to that, but as far as I know there is no way to do that.

If we want to display anything we need to intentionally fail the install script, otherwise npm v8 users will never see anything. That's not exactly ideal as it's a workaround, but the real problem is figuring out what to display, especially when falling back to compilation and it fails. install-addons already has a command line option to control the amount of output. We could expand that, but then you have problems when you're not installing the addon directly, such as a dependency of some other package and the user may not know to pass such optional flags.

[aside] It would be nice if npm (and node.js package managers in general) would just incorporate a better, flexible solution for installing addons. Then we wouldn't have to worry about doing so many hacky workarounds and whatnot. [/aside]

[Julusian]

I strongly recommend not inventing your own native dependency install library (install-addons). The main reason being that various electron toolings (eg electron-builder) will need to know to invoke it when cross compiling a build, so its going to cause a load of pain for anyone packaging this in electron until those toolings are aware of how to drive your installer to install the correct thing. I don't know if you are even providing prebuilt versions for electron right now either, but thats a separate thing.

I would suggest looking at prebuildify. I don't know if it does everything you want, so it might not be a replacement, but the way that tool bundles all the prebuilt binaries inside of the npm package would negate the need to invent and maintain a new install tool, with the added benefit of faster installs (no uncached http request to fetch the native binary) at the cost of a couple of mb more disk space

[mscdex]

@Julusian

I strongly recommend not inventing your own native dependency install library (install-addons).

Too late.

The main reason being that various electron toolings (eg electron-builder) will need to know to invoke it when cross compiling a build, so its going to cause a load of pain for anyone packaging this in electron until those toolings are aware of how to drive your installer to install the correct thing.

I'm not sure what you're describing here exactly. As far as Electron support goes, there should be no difference between prebuildify and build/install-addons. They'd both generate binaries for Electron and then either use a compatible binary or fall back to building if none exist.

I don't know if you are even providing prebuilt versions for electron right now either, but thats a separate thing.

build-addons supports this.

I would suggest looking at prebuildify

I won't go into all of the reasons why existing solutions did not work, but specifically comparing with prebuildify here are a few reasons why I didn't use it (through the lens of using build-addons/install-addons generally, not specific to ssh2's usage):

• It embeds all possible platform configuration binaries into the package -- this is bad for two reasons:
  1. Users already complain about packages being too large and including things they believe shouldn't be in there. This will only make that situation worse.
  2. When your binaries are quite large individually (especially if you statically link some dependencies), this makes your download significantly larger.
• Nothing out of the box to make it easy to generate binaries for various configurations using Github Actions
• Still requires you to have all required source code in your package, even if an existing binary is usable
• Has missing support for various platform-specific considerations, like minimum kernel/OS version (Linux, macOS, Windows) and minimum libc version (Linux, Windows).

Believe me, the last thing I wanted to do was create my own binary generator and consumer, but after looking at all of the currently available solutions, nothing met all of my criteria. Ideally it would be great to have some of this logic (e.g. the install-addons step in this case) built into node package managers so that package maintainers just have to provide the binaries in one form or another.

[Julusian]

I'm not sure what you're describing here exactly. As far as Electron support goes, there should be no difference between prebuildify and build/install-addons. They'd both generate binaries for Electron and then either use a compatible binary or fall back to building if none exist.

So, I can run electron-builder --win from a mac to produce a windows build of an electron app. In order to do this, electron-builder has a step which finds each dependency that has a native component, and 'rebuilds' it. The problem is that it isnt possible to build a windows version of cpu-features on macos (I guess it probably is, but its not realistic to expect it to be possible), so if electron-builder tries to compile it we can expect that to fail. But as you have prebuilds, it would be nice if they could be used. However, in order for electron-builder to know how to instruct your package to get the right prebuild, it checks for a few different tools used to do this, as it needs to be able to instruct it which build it should fetch (done here). To me it feels like it should be possible to do via some environment variables or something, but right now this is what they are doing electron-rebuild has a similar knowledge of the tooling. What this means is that a library using prebuild-install will reinstall the correct version when doing a cross-compile of an electron project, whereas install-addons will do nothing, or might end up rebuilding for the current (aka wrong) environment.

If you don't even have a bindings.gyp in your repository then expect the tools to not even realise it is a native library, which will likely break electron builds for the current environment as it wont know to do anything to it.

It embeds all possible platform configuration binaries into the package -- this is bad for two reasons:

• Users already complain about packages being too large and including things they believe shouldn't be in there. This will only make that situation worse.
• When your binaries are quite large individually (especially if you statically link some dependencies), this makes your download significantly larger.

Yeah, its a balancing act of install speed vs disk space. The prebuildify approach may not be the right fit for cpu-features, mostly because it is nan based which makes the number of required variants rather ridiculous. But even then the cpu-features binaries are rather small, so are barely noticable.

• Nothing out of the box to make it easy to generate binaries for various configurations using Github Actions
• Still requires you to have all required source code in your package, even if an existing binary is usable

Yes, I agree these are pain point of most of the tools I have looked at.

Has missing support for various platform-specific considerations, like minimum kernel/OS version (Linux, macOS, Windows) and minimum libc version (Linux, Windows).

In my experience with a few other native libraries, most of these have never come up as an issue that requires different builds. It has occasionally been an issue of the binary requiring a too new libc or macos, but that has been easily fixed by building against older versions, then they work for everyone. I rarely see projects with even musl prebuilds.

But maybe cpu-features is different and needs to hook into many more places in the os. I cant answer that, but this has been my experience with other projects.

Ideally it would be great to have some of this logic (e.g. the install-addons step in this case) built into node package managers so that package maintainers just have to provide the binaries in one form or another.

I agree. Ive read at least one proposal for support for this, but I havent seen anything happen on this front yet

[jeffrson]

Fails on Windows with Node 18.0.0, Visual Studio 2022 if that matters

>npm test

> ssh2@1.5.0 test
> node test/test.js

> Running test-exec.js ...
> Running test-integration-openssh.js ...
Skipping OpenSSH integration tests on Windows
> Running test-misc-client-server.js ...
> Running test-openssh.js ...
> Running test-protocol-crypto.js ...
Crypto binding not available
Testing cipher: null, mac: <none> (native encrypt, native decrypt) ...
Testing cipher: chacha20-poly1305@openssh.com, mac: <implicit> (native encrypt, native decrypt) ...
Testing cipher: aes128-gcm@openssh.com, mac: <implicit> (native encrypt, native decrypt) ...
Testing cipher: aes128-cbc, mac: hmac-sha1-etm@openssh.com (native encrypt, native decrypt) ...
Testing cipher: aes128-ctr, mac: hmac-sha1 (native encrypt, native decrypt) ...
Testing cipher: arcfour, mac: hmac-sha2-256-96 (native encrypt, native decrypt) ...
node:internal/crypto/cipher:116
    this[kHandle].initiv(cipher, credential, iv, authTagLength);
                  ^

Error: error:0308010C:digital envelope routines::unsupported
    at Cipheriv.createCipherBase (node:internal/crypto/cipher:116:19)
    at Cipheriv.createCipherWithIV (node:internal/crypto/cipher:135:3)
    at new Cipheriv (node:internal/crypto/cipher:243:3)
    at createCipheriv (node:crypto:141:10)
    at new GenericCipherNative (C:\temp\ssh2test\node_modules\ssh2\lib\protocol\crypto.js:394:28)
    at createCipher (C:\temp\ssh2test\node_modules\ssh2\lib\protocol\crypto.js:1511:17)
    at C:\temp\ssh2test\node_modules\ssh2\test\test-protocol-crypto.js:122:20
    at Array.forEach (<anonymous>)
    at C:\temp\ssh2test\node_modules\ssh2\test\test-protocol-crypto.js:37:7 {
  library: 'digital envelope routines',
  reason: 'unsupported',
  code: 'ERR_OSSL_EVP_UNSUPPORTED'
}

Node.js v18.0.0
> Running test-protocol-keyparser.js ...
> Running test-server-hostkeys.js ...
> Running test-sftp.js ...
node:assert:399
    throw err;
    ^

AssertionError [ERR_ASSERTION]: WriteStream: Unexpected client error: TypeError: Cannot set property closed of #<Writable> which has only a getter
    at new WriteStream (C:\temp\ssh2test\node_modules\ssh2\lib\protocol\SFTP.js:3585:15)
    at SFTP.createWriteStream (C:\temp\ssh2test\node_modules\ssh2\lib\protocol\SFTP.js:311:12)
    at C:\temp\ssh2test\node_modules\ssh2\test\test-sftp.js:689:25
    at wrapped (C:\temp\ssh2test\node_modules\ssh2\test\common.js:79:15)
    at C:\temp\ssh2test\node_modules\ssh2\test\test-sftp.js:743:7
    at wrapped (C:\temp\ssh2test\node_modules\ssh2\test\common.js:79:15)
    at C:\temp\ssh2test\node_modules\ssh2\test\test-sftp.js:753:7
    at wrapped (C:\temp\ssh2test\node_modules\ssh2\test\common.js:79:15)
    at SFTP.onReady (C:\temp\ssh2test\node_modules\ssh2\lib\client.js:1483:11)
    at SFTP.emit (node:events:527:28)

    at Client.onError (C:\temp\ssh2test\node_modules\ssh2\test\common.js:195:5)
    at Client.emit (node:events:527:28)
    at Socket.<anonymous> (C:\temp\ssh2test\node_modules\ssh2\lib\client.js:715:20)
    at Socket.emit (node:events:527:28)
    at addChunk (node:internal/streams/readable:324:12)
    at readableAddChunk (node:internal/streams/readable:297:9)
    at Readable.push (node:internal/streams/readable:234:10)
    at TCP.onStreamRead (node:internal/stream_base_commons:190:23) {
  generatedMessage: false,
  code: 'ERR_ASSERTION',
  actual: false,
  expected: true,
  operator: '=='
}

Node.js v18.0.0
> Running test-shell.js ...
> Running test-userauth-agent-openssh.js ...
Skipping ssh-agent test on Windows
> Running test-userauth-agent.js ...
> Running test-userauth.js ...

[mscdex]

@jeffrson Those errors are unrelated and are already fixed in the current version of ssh2.

[jeffrson]

Okay.

You've seen that "Crypto binding not available"?

[mscdex]

@jeffrson That just means the bundled addon failed to build during ssh2 install. The bundled addon does not have a prebuilt version available, only ssh2's optional addon dependencies do (namely cpu-features).

[dlong500]

@mscdex How are things going with the prebuilt testing? Any idea on when this might get merged into a mainline release?

[mscdex]

@dlong500 I haven't touched it in a long while because of various roadblocks. At this point I'm not optimistic that something like this is very feasible given the current state of node, package managers, and other pieces that need to come together to make prebuilt addon installations as smooth as possible (or in some cases, work at all).

Maybe at some point in the future things will be different for the better, but until then building from scratch is really the best we have unfortunately.

[mscdex]

I'm going to go ahead and close this because unfortunately the current node.js package management ecosystem makes what I wanted to achieve impossible. Maybe some day in the future that will no longer be the case. Thank you to everyone who participated in my little addon experiment.