remove binding.gyp from npm package

[rochdev]

This file is no longer needed now that prebuilds exist for all supported platforms and architectures. It also has the benefit of removing the need for an install script which was triggering security tools.

[mkaufmaner]

This has broken our builds since we proxy the public npm registry with an instance of jFrog's Artifactory. Our instance artifactory does not support pre built binaries. Thus, we get the following errors upon install;

npm i
npm ERR! code 1
npm ERR! path /Users/developer/Documents/projects/temp/node_modules/@datadog/native-metrics
npm ERR! command failed
npm ERR! command sh /var/folders/bl/1fx3stt151n1glpkcwp0cvwm0000gp/T/install-1ee2bfe0.sh
npm ERR! gyp info it worked if it ends with ok
npm ERR! gyp info using node-gyp@9.0.0
npm ERR! gyp info using node@16.16.0 | darwin | x64
npm ERR! gyp info find Python using Python version 3.9.13 found at "/usr/local/opt/python@3.9/bin/python3.9"
npm ERR! gyp info spawn /usr/local/opt/python@3.9/bin/python3.9
npm ERR! gyp info spawn args [
npm ERR! gyp info spawn args   '/Users/developer/.nvm/versions/node/v16.16.0/lib/node_modules/npm/node_modules/node-gyp/gyp/gyp_main.py',
npm ERR! gyp info spawn args   'binding.gyp',
npm ERR! gyp info spawn args   '-f',
npm ERR! gyp info spawn args   'make',
npm ERR! gyp info spawn args   '-I',
npm ERR! gyp info spawn args   '/Users/developer/Documents/projects/temp/node_modules/@datadog/native-metrics/build/config.gypi',
npm ERR! gyp info spawn args   '-I',
npm ERR! gyp info spawn args   '/Users/developer/.nvm/versions/node/v16.16.0/lib/node_modules/npm/node_modules/node-gyp/addon.gypi',
npm ERR! gyp info spawn args   '-I',
npm ERR! gyp info spawn args   '/Users/developer/Library/Caches/node-gyp/16.16.0/include/node/common.gypi',
npm ERR! gyp info spawn args   '-Dlibrary=shared_library',
npm ERR! gyp info spawn args   '-Dvisibility=default',
npm ERR! gyp info spawn args   '-Dnode_root_dir=/Users/developer/Library/Caches/node-gyp/16.16.0',
npm ERR! gyp info spawn args   '-Dnode_gyp_dir=/Users/developer/.nvm/versions/node/v16.16.0/lib/node_modules/npm/node_modules/node-gyp',
npm ERR! gyp info spawn args   '-Dnode_lib_file=/Users/developer/Library/Caches/node-gyp/16.16.0/<(target_arch)/node.lib',
npm ERR! gyp info spawn args   '-Dmodule_root_dir=/Users/developer/Documents/projects/temp/node_modules/@datadog/native-metrics',
npm ERR! gyp info spawn args   '-Dnode_engine=v8',
npm ERR! gyp info spawn args   '--depth=.',
npm ERR! gyp info spawn args   '--no-parallel',
npm ERR! gyp info spawn args   '--generator-output',
npm ERR! gyp info spawn args   'build',
npm ERR! gyp info spawn args   '-Goutput_dir=.'
npm ERR! gyp info spawn args ]
npm ERR! gyp: binding.gyp not found (cwd: /Users/developer/Documents/projects/temp/node_modules/@datadog/native-metrics) while trying to load binding.gyp
npm ERR! gyp ERR! configure error
npm ERR! gyp ERR! stack Error: `gyp` failed with exit code: 1
npm ERR! gyp ERR! stack     at ChildProcess.onCpExit (/Users/developer/.nvm/versions/node/v16.16.0/lib/node_modules/npm/node_modules/node-gyp/lib/configure.js:261:16)
npm ERR! gyp ERR! stack     at ChildProcess.emit (node:events:527:28)
npm ERR! gyp ERR! stack     at Process.ChildProcess._handle.onexit (node:internal/child_process:291:12)
npm ERR! gyp ERR! System Darwin 21.6.0
npm ERR! gyp ERR! command "/Users/developer/.nvm/versions/node/v16.16.0/bin/node" "/Users/developer/.nvm/versions/node/v16.16.0/lib/node_modules/npm/node_modules/node-gyp/bin/node-gyp.js" "rebuild"
npm ERR! gyp ERR! cwd /Users/developer/Documents/projects/temp/node_modules/@datadog/native-metrics
npm ERR! gyp ERR! node -v v16.16.0
npm ERR! gyp ERR! node-gyp -v v9.0.0
npm ERR! gyp ERR! not ok

npm ERR! A complete log of this run can be found in:
npm ERR!     /Users/developer/.npm/_logs/2022-08-02T20_46_05_921Z-debug-0.log

[rochdev]

Our instance artifactory does not support pre built binaries.

@mkaufmaner The prebuilt binaries are included in the archive from npm, we don't pull the binaries at install time from for example s3 or GitHub, so the prebuilds should be present even if that's not supported. It seems that it could be caused by caching as described in DataDog/dd-trace-js#2239 (comment).

[mkaufmaner]

@rochdev Understood. We are in the process of attempting to "fix" our internal registry to use the prebuilt binaries from the npm registry properly. However, I am concerned that any functionality around pulling and caching binaries may have been disabled purposefully by our security team.

Could you please elaborate on "triggering security tools."? Was that the reason for this being removed in the first place? If so, then security should have added an exception.

Regardless of the reason for removing the bindings from the published package, could you please add them back? Instead, change the install script to detect the prebuilt binaries like it did before?

Furthermore, this is a breaking change because it drops support for building the supporting libraries from source using through the existing package managers. Thus, the major version of this package should have been incremented to reflect.

[Qard]

The presence of an install script is what makes security tools flag it. Install scripts can do anything, including malicious things.

[rochdev]

this is a breaking change because it drops support for building the supporting libraries from source using through the existing package managers

It shouldn't be a breaking change because the prebuilds are directly in the package, so they will always be pulled at install time regardless of any external access. This is usually not true as most packages use node-pre-gyp which would download them using AWS S3 or GitHub in an install script, but we add them directly to npm instead to avoid this very issue. Can you try maybe to just clear the cache?

[artfiedler]

This is a real breaking change... as in, its BREAKING our deployments and npm install's from project pulls...

Please revert back, as many people do not have access to reset cache or force their administrators to accept what is happening here.

If its not reverted back, a working solution needs to be provided that does not involve modifying artifactory etc, one solution I've tried is package.json overrides but I believe I need to find and add more then just dd-native-metrics for npm install dd-trace override

[rochdev]

Please revert back, as many people do not have access to reset cache or force their administrators to accept what is happening here.

Package managers support building native add-ons based on the presence of the binding.gyp file. If the file is present, then it builds the add-on using the file. If the file is not present, then it doesn't. There are no other mechanisms (that I'm aware of at least) that it uses to determine whether something needs to be built or not, so it shouldn't try to build in the first place. At worse some metrics would be unavailable, but it should never fail the installation.

If we were to remove the native code completely, which we plan to do in the future when Node provides all the metrics, then the same issue would still happen even though there would be no native code at all. In that sense, even if we were to bring back the file, this would only be a temporary fix until the issue comes back later. We need to figure out exactly what is happening here, and once we know the root cause of this bug then we can provide a working solution.

If its not reverted back, a working solution needs to be provided that does not involve modifying artifactory etc, one solution I've tried is package.json overrides but I believe I need to find and add more then just dd-native-metrics for npm install dd-trace override

I completely agree with this, which is why I've been trying to reproduce for the last few days. I want to fix this, but I need to know how to reproduce. Now that it seems that private registries are pretty much always involved, I'll try to see if I can come up with a reproduction and understand what is going on.

[rochdev]

@artfiedler Can you share which private registry you are using and whether it's hosted as a service or on-premise?

[rochdev]

@artfiedler I was able to reproduce with an OSS private registry. You can follow progress in npm/cli#5234 but I'll also update all threads about the issue when there is a resolution.

[mkaufmaner]

@artfiedler I was able to reproduce with an OSS private registry. You can follow progress in npm/cli#5234 but I'll also update all threads about the issue when there is a resolution.

@rochdev Could you add the bindings.gyp file back to the published package in the meantime?

[artfiedler]

@artfiedler Can you share which private registry you are using and whether it's hosted as a service or on-premise?

On premise JFrog 7.35.2 rev 73502900

[rochdev]

@mkaufmaner @artfiedler Please see DataDog/dd-trace-js#2239 (comment) for a workaround.