From ddddb850a65dbd9e05e0337125afe47364f6162f Mon Sep 17 00:00:00 2001 From: Guillaume Besson Date: Thu, 30 Jan 2020 16:18:09 +0100 Subject: [PATCH 01/11] Adds support for Node native addons --- README.md | 15 +++++++++++---- lib/packer.js | 9 ++------- lib/walker.js | 15 +-------------- prelude/bootstrap.js | 17 +++++++++++++++++ 4 files changed, 31 insertions(+), 25 deletions(-) diff --git a/README.md b/README.md index 862f583a9..249b79ebb 100644 --- a/README.md +++ b/README.md @@ -200,10 +200,17 @@ This way you may even avoid creating `pkg` config for your project. ## Native addons -Native addons (`.node` files) use is supported, but packaging -`.node` files inside the executable is not resolved yet. You have -to deploy native addons used by your project to the same directory -as the executable. +Native addons (`.node` files) use is supported. When `pkg` encounters +a `.node` file in a `require` call, it will package this like an asset. +In some cases (like with the `bindings` package), the module path is generated +dynamicaly and `pkg` won't be able to detect it. In this case, you should +add the `.node` file directly in the `assets` field in `package.json`. + +The way NodeJS requires native addon is different from a classic JS +file. It needs to have a file on disk to load it but `pkg` only generate +one file. To circumvent this, `pkg` will create a temporary file on the +disk. These files will stay on the disk after the process has exited +and will be used again on the next process launch. When a package, that contains a native module, is being installed, the native module is compiled against current system-wide Node.js diff --git a/lib/packer.js b/lib/packer.js index 1f609cc4b..e63c47572 100644 --- a/lib/packer.js +++ b/lib/packer.js @@ -2,7 +2,7 @@ import { STORE_BLOB, STORE_CONTENT, STORE_LINKS, - STORE_STAT, isDotJS, isDotJSON, isDotNODE + STORE_STAT, isDotJS, isDotJSON, } from '../prelude/common.js'; import { log, wasReported } from './log.js'; @@ -34,18 +34,13 @@ function hasAnyStore (record) { export default function ({ records, entrypoint, bytecode }) { const stripes = []; - for (const snap in records) { const record = records[snap]; const { file } = record; if (!hasAnyStore(record)) continue; assert(record[STORE_STAT], 'packer: no STORE_STAT'); - if (isDotNODE(file)) { - continue; - } else { - assert(record[STORE_BLOB] || record[STORE_CONTENT] || record[STORE_LINKS]); - } + assert(record[STORE_BLOB] || record[STORE_CONTENT] || record[STORE_LINKS]); if (record[STORE_BLOB] && !bytecode) { delete record[STORE_BLOB]; diff --git a/lib/walker.js b/lib/walker.js index 7e66208dc..2cea2ef9c 100644 --- a/lib/walker.js +++ b/lib/walker.js @@ -512,24 +512,11 @@ class Walker { store: STORE_STAT }); - if (isDotNODE(record.file)) { - // provide explicit deployFiles to override - // native addon deployment place. see 'sharp' - if (!marker.hasDeployFiles) { - log.warn('Cannot include addon %1 into executable.', [ - 'The addon must be distributed with executable as %2.', - '%1: ' + record.file, - '%2: path-to-executable/' + path.basename(record.file) ]); - } - return; // discard - } - const derivatives1 = []; await this.stepActivate(marker, derivatives1); await this.stepDerivatives(record, marker, derivatives1); - if (store === STORE_BLOB) { - if (unlikelyJavascript(record.file)) { + if (unlikelyJavascript(record.file) || isDotNODE(record.file)) { this.append({ file: record.file, marker, diff --git a/prelude/bootstrap.js b/prelude/bootstrap.js index 216579e8d..585b4d700 100644 --- a/prelude/bootstrap.js +++ b/prelude/bootstrap.js @@ -1299,6 +1299,23 @@ function payloadFileSync (pointer) { return filename; } + if (common.isDotNODE(filename)) { + // Node addon files are not classic modules, they are loaded with process.dlopen which needs a filesystem path + // we need to write the file somewhere on disk first and then load it + const fs = require('fs'); + const moduleContent = fs.readFileSync(filename); + const moduleBaseName = require('path').basename(filename); + const hash = require('crypto').createHash('sha256').update(moduleContent).digest('hex'); + const tmpModulePath = `${require('os').tmpdir()}/${hash}_${moduleBaseName}`; + try { + fs.statSync(tmpModulePath); + } catch (e) { + // Most likely this means the module is not on disk yet + fs.writeFileSync(tmpModulePath, moduleContent, {mode: 0o444}); + } + return tmpModulePath; + } + if (flagWasOn) { FLAG_ENABLE_PROJECT = true; try { From aa35e5e98c38e4ba9287b54b84d5e139da9bef65 Mon Sep 17 00:00:00 2001 From: Guillaume Besson Date: Thu, 30 Jan 2020 16:31:05 +0100 Subject: [PATCH 02/11] Fix linting problems --- lib/packer.js | 2 +- prelude/bootstrap.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/packer.js b/lib/packer.js index e63c47572..edbd0150b 100644 --- a/lib/packer.js +++ b/lib/packer.js @@ -2,7 +2,7 @@ import { STORE_BLOB, STORE_CONTENT, STORE_LINKS, - STORE_STAT, isDotJS, isDotJSON, + STORE_STAT, isDotJS, isDotJSON } from '../prelude/common.js'; import { log, wasReported } from './log.js'; diff --git a/prelude/bootstrap.js b/prelude/bootstrap.js index 585b4d700..cb3e09965 100644 --- a/prelude/bootstrap.js +++ b/prelude/bootstrap.js @@ -1311,7 +1311,7 @@ function payloadFileSync (pointer) { fs.statSync(tmpModulePath); } catch (e) { // Most likely this means the module is not on disk yet - fs.writeFileSync(tmpModulePath, moduleContent, {mode: 0o444}); + fs.writeFileSync(tmpModulePath, moduleContent, { mode: 0o444 }); } return tmpModulePath; } From 1e3fa24945dfb1cb650f2b2d1f45cf27524a09cc Mon Sep 17 00:00:00 2001 From: Guillaume Besson Date: Thu, 30 Jan 2020 16:44:56 +0100 Subject: [PATCH 03/11] Change test-50-cannot-include-addon to test-50-can-include-addon --- .../main.js | 4 ++-- .../test-x-index.js | 0 .../time.node | 0 3 files changed, 2 insertions(+), 2 deletions(-) rename test/{test-50-cannot-include-addon => test-50-can-include-addon}/main.js (83%) rename test/{test-50-cannot-include-addon => test-50-can-include-addon}/test-x-index.js (100%) rename test/{test-50-cannot-include-addon => test-50-can-include-addon}/time.node (100%) diff --git a/test/test-50-cannot-include-addon/main.js b/test/test-50-can-include-addon/main.js similarity index 83% rename from test/test-50-cannot-include-addon/main.js rename to test/test-50-can-include-addon/main.js index f59d21738..5a13acd27 100644 --- a/test/test-50-cannot-include-addon/main.js +++ b/test/test-50-can-include-addon/main.js @@ -26,6 +26,6 @@ right = utils.pkg.sync([ assert(right.indexOf('\x1B\x5B') < 0, 'colors detected'); right = right.replace(/\\/g, '/'); -assert(right.indexOf('test-50-cannot-include-addon/time.node') >= 0); -assert(right.indexOf('path-to-executable/time.node') >= 0); +assert(right.indexOf('test-50-can-include-addon/time.node') === -1); +assert(right.indexOf('path-to-executable/time.node') === -1); utils.vacuum.sync(output); diff --git a/test/test-50-cannot-include-addon/test-x-index.js b/test/test-50-can-include-addon/test-x-index.js similarity index 100% rename from test/test-50-cannot-include-addon/test-x-index.js rename to test/test-50-can-include-addon/test-x-index.js diff --git a/test/test-50-cannot-include-addon/time.node b/test/test-50-can-include-addon/time.node similarity index 100% rename from test/test-50-cannot-include-addon/time.node rename to test/test-50-can-include-addon/time.node From c88fd41ab80b480f688b6dbcac2b8adca7debdc6 Mon Sep 17 00:00:00 2001 From: Guillaume Besson Date: Thu, 30 Jan 2020 16:45:37 +0100 Subject: [PATCH 04/11] Add new test case for native addon packaged in executable --- test/test-50-native-addon-4/lib/time.node | 1 + test/test-50-native-addon-4/main.js | 37 +++++++++++++++++++++ test/test-50-native-addon-4/test-x-index.js | 10 ++++++ 3 files changed, 48 insertions(+) create mode 100644 test/test-50-native-addon-4/lib/time.node create mode 100644 test/test-50-native-addon-4/main.js create mode 100644 test/test-50-native-addon-4/test-x-index.js diff --git a/test/test-50-native-addon-4/lib/time.node b/test/test-50-native-addon-4/lib/time.node new file mode 100644 index 000000000..37a464842 --- /dev/null +++ b/test/test-50-native-addon-4/lib/time.node @@ -0,0 +1 @@ +module.exports = 'test'; diff --git a/test/test-50-native-addon-4/main.js b/test/test-50-native-addon-4/main.js new file mode 100644 index 000000000..9252a81ac --- /dev/null +++ b/test/test-50-native-addon-4/main.js @@ -0,0 +1,37 @@ +#!/usr/bin/env node + +'use strict'; + +const fs = require('fs'); +const path = require('path'); +const assert = require('assert'); +const utils = require('../utils.js'); + +assert(!module.parent); +assert(__dirname === process.cwd()); + +const host = 'node' + process.version.match(/^v(\d+)/)[1]; +const target = process.argv[2] || host; +const input = './test-x-index.js'; +const output = './run-time/test-output.exe'; + +let left, right; +utils.mkdirp.sync(path.dirname(output)); + +left = utils.spawn.sync( + 'node', [ path.basename(input) ], + { cwd: path.dirname(input) } +); + +utils.pkg.sync([ + '--target', target, + '--output', output, input +]); + +right = utils.spawn.sync( + './' + path.basename(output), [], + { cwd: path.dirname(output) } +); + +assert.equal(left, right); +utils.vacuum.sync(path.dirname(output)); diff --git a/test/test-50-native-addon-4/test-x-index.js b/test/test-50-native-addon-4/test-x-index.js new file mode 100644 index 000000000..b66cc79e2 --- /dev/null +++ b/test/test-50-native-addon-4/test-x-index.js @@ -0,0 +1,10 @@ +/* eslint-disable no-underscore-dangle */ + +'use strict'; + +var fs = require('fs'); +var path = require('path'); +var Module = require('module'); +Module._extensions['.node'] = Module._extensions['.js']; +console.log(fs.existsSync(path.join(__dirname, 'lib/time.node'))); +console.log(require('./lib/time.node')); From d824f40adcb087ad06bc86f8ad653dc37eb2912a Mon Sep 17 00:00:00 2001 From: Guillaume Besson Date: Thu, 30 Jan 2020 16:58:34 +0100 Subject: [PATCH 05/11] Fix lint error --- test/test-50-native-addon-4/main.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test/test-50-native-addon-4/main.js b/test/test-50-native-addon-4/main.js index 9252a81ac..30f878589 100644 --- a/test/test-50-native-addon-4/main.js +++ b/test/test-50-native-addon-4/main.js @@ -2,7 +2,6 @@ 'use strict'; -const fs = require('fs'); const path = require('path'); const assert = require('assert'); const utils = require('../utils.js'); From 2f9a3943c4f7f6f460827e0e49f30084985335f8 Mon Sep 17 00:00:00 2001 From: Guillaume Besson Date: Thu, 30 Jan 2020 17:10:53 +0100 Subject: [PATCH 06/11] Fix tests using basename in native addon mock --- test/test-50-can-include-addon/time.node | 2 +- test/test-50-native-addon-2/node_modules/dependency/time.node | 2 +- test/test-50-native-addon-3/lib/community/time-y.node | 2 +- test/test-50-native-addon-3/lib/enterprise/time-z.node | 2 +- test/test-50-native-addon-3/lib/time-x.node | 2 +- test/test-50-native-addon/lib/time.node | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/test/test-50-can-include-addon/time.node b/test/test-50-can-include-addon/time.node index aa43fbb13..37a464842 100644 --- a/test/test-50-can-include-addon/time.node +++ b/test/test-50-can-include-addon/time.node @@ -1 +1 @@ -module.exports = require('path').basename(__filename); +module.exports = 'test'; diff --git a/test/test-50-native-addon-2/node_modules/dependency/time.node b/test/test-50-native-addon-2/node_modules/dependency/time.node index aa43fbb13..37a464842 100644 --- a/test/test-50-native-addon-2/node_modules/dependency/time.node +++ b/test/test-50-native-addon-2/node_modules/dependency/time.node @@ -1 +1 @@ -module.exports = require('path').basename(__filename); +module.exports = 'test'; diff --git a/test/test-50-native-addon-3/lib/community/time-y.node b/test/test-50-native-addon-3/lib/community/time-y.node index aa43fbb13..7a4cb5f89 100644 --- a/test/test-50-native-addon-3/lib/community/time-y.node +++ b/test/test-50-native-addon-3/lib/community/time-y.node @@ -1 +1 @@ -module.exports = require('path').basename(__filename); +module.exports = 'time-y'; diff --git a/test/test-50-native-addon-3/lib/enterprise/time-z.node b/test/test-50-native-addon-3/lib/enterprise/time-z.node index aa43fbb13..1e395f40d 100644 --- a/test/test-50-native-addon-3/lib/enterprise/time-z.node +++ b/test/test-50-native-addon-3/lib/enterprise/time-z.node @@ -1 +1 @@ -module.exports = require('path').basename(__filename); +module.exports = 'time-z'; diff --git a/test/test-50-native-addon-3/lib/time-x.node b/test/test-50-native-addon-3/lib/time-x.node index aa43fbb13..37a464842 100644 --- a/test/test-50-native-addon-3/lib/time-x.node +++ b/test/test-50-native-addon-3/lib/time-x.node @@ -1 +1 @@ -module.exports = require('path').basename(__filename); +module.exports = 'test'; diff --git a/test/test-50-native-addon/lib/time.node b/test/test-50-native-addon/lib/time.node index aa43fbb13..37a464842 100644 --- a/test/test-50-native-addon/lib/time.node +++ b/test/test-50-native-addon/lib/time.node @@ -1 +1 @@ -module.exports = require('path').basename(__filename); +module.exports = 'test'; From 748a1793feb010db29ace17db81843d2f9b92d98 Mon Sep 17 00:00:00 2001 From: Guillaume Besson Date: Thu, 30 Jan 2020 17:22:49 +0100 Subject: [PATCH 07/11] Fix native addon test --- test/test-50-native-addon-3/node_modules/dependency/time-d.node | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test-50-native-addon-3/node_modules/dependency/time-d.node b/test/test-50-native-addon-3/node_modules/dependency/time-d.node index aa43fbb13..5b8b4f773 100644 --- a/test/test-50-native-addon-3/node_modules/dependency/time-d.node +++ b/test/test-50-native-addon-3/node_modules/dependency/time-d.node @@ -1 +1 @@ -module.exports = require('path').basename(__filename); +module.exports = 'time-d'; From 9cc42533b05dd8358b0c0a5764e8f73a168235e8 Mon Sep 17 00:00:00 2001 From: Guillaume Besson Date: Mon, 10 Feb 2020 16:16:03 +0100 Subject: [PATCH 08/11] Patch dlopen to work with other modules than .node --- prelude/bootstrap.js | 47 ++++++++++++++++++++++++++++---------------- 1 file changed, 30 insertions(+), 17 deletions(-) diff --git a/prelude/bootstrap.js b/prelude/bootstrap.js index cb3e09965..4857c9761 100644 --- a/prelude/bootstrap.js +++ b/prelude/bootstrap.js @@ -1299,23 +1299,6 @@ function payloadFileSync (pointer) { return filename; } - if (common.isDotNODE(filename)) { - // Node addon files are not classic modules, they are loaded with process.dlopen which needs a filesystem path - // we need to write the file somewhere on disk first and then load it - const fs = require('fs'); - const moduleContent = fs.readFileSync(filename); - const moduleBaseName = require('path').basename(filename); - const hash = require('crypto').createHash('sha256').update(moduleContent).digest('hex'); - const tmpModulePath = `${require('os').tmpdir()}/${hash}_${moduleBaseName}`; - try { - fs.statSync(tmpModulePath); - } catch (e) { - // Most likely this means the module is not on disk yet - fs.writeFileSync(tmpModulePath, moduleContent, { mode: 0o444 }); - } - return tmpModulePath; - } - if (flagWasOn) { FLAG_ENABLE_PROJECT = true; try { @@ -1529,3 +1512,33 @@ function payloadFileSync (pointer) { }); } }()); + +// ///////////////////////////////////////////////////////////////// +// PATCH PROCESS /////////////////////////////////////////////////// +// ///////////////////////////////////////////////////////////////// + +(function() { + const fs = require('fs'); + var ancestor = {}; + ancestor.dlopen = process.dlopen; + + process.dlopen = function () { + const args = cloneArgs(arguments); + if (insideSnapshot(args[1])) { + // Node addon files and .so cannot be read with fs directly, they are loaded with process.dlopen which needs a filesystem path + // we need to write the file somewhere on disk first and then load it + const moduleContent = fs.readFileSync(args[1]); + const moduleBaseName = require('path').basename(args[1]); + const hash = require('crypto').createHash('sha256').update(moduleContent).digest('hex'); + const tmpModulePath = `${require('os').tmpdir()}/${hash}_${moduleBaseName}`; + try { + fs.statSync(tmpModulePath); + } catch (e) { + // Most likely this means the module is not on disk yet + fs.writeFileSync(tmpModulePath, moduleContent, { mode: 0o444 }); + } + args[1] = tmpModulePath; + } + return ancestor.dlopen.apply(process, args); + } +}()); From c3cac037e601244db51b40114916d8031e378558 Mon Sep 17 00:00:00 2001 From: Guillaume Besson Date: Mon, 10 Feb 2020 17:19:11 +0100 Subject: [PATCH 09/11] Handle .node modules requiring other .so modules --- prelude/bootstrap.js | 40 ++++++++++++++++++++++++++++++++++++---- 1 file changed, 36 insertions(+), 4 deletions(-) diff --git a/prelude/bootstrap.js b/prelude/bootstrap.js index 4857c9761..f3233e095 100644 --- a/prelude/bootstrap.js +++ b/prelude/bootstrap.js @@ -1524,11 +1524,13 @@ function payloadFileSync (pointer) { process.dlopen = function () { const args = cloneArgs(arguments); - if (insideSnapshot(args[1])) { + const modulePath = args[1]; + const moduleDirname = require('path').dirname(modulePath); + if (insideSnapshot(modulePath)) { // Node addon files and .so cannot be read with fs directly, they are loaded with process.dlopen which needs a filesystem path // we need to write the file somewhere on disk first and then load it - const moduleContent = fs.readFileSync(args[1]); - const moduleBaseName = require('path').basename(args[1]); + const moduleContent = fs.readFileSync(modulePath); + const moduleBaseName = require('path').basename(modulePath); const hash = require('crypto').createHash('sha256').update(moduleContent).digest('hex'); const tmpModulePath = `${require('os').tmpdir()}/${hash}_${moduleBaseName}`; try { @@ -1539,6 +1541,36 @@ function payloadFileSync (pointer) { } args[1] = tmpModulePath; } - return ancestor.dlopen.apply(process, args); + + const unknownModuleErrorRegex = /([^:]+): cannot open shared object file: No such file or directory/; + const tryImporting = function (previousErrorMessage) { + try { + const res = ancestor.dlopen.apply(process, args); + return res; + } catch (e) { + if (e.message === previousErrorMessage) { + // we already tried to fix this and it didn't work, give up + throw e; + } + if (e.message.match(unknownModuleErrorRegex)) { + // some modules are packaged with dynamic linking and needs to open other files that should be in + // the same directory, in this case, we write this file in the same /tmp directory and try to + // import the module again + const moduleName = e.message.match(unknownModuleErrorRegex)[1]; + const modulePath = `${moduleDirname}/${moduleName}`; + const moduleContent = fs.readFileSync(modulePath); + const moduleBaseName = require('path').basename(modulePath); + const tmpModulePath = `${require('os').tmpdir()}/${moduleBaseName}`; + try { + fs.statSync(tmpModulePath); + } catch (e) { + fs.writeFileSync(tmpModulePath, moduleContent, { mode: 0o444 }); + } + return tryImporting(e.message); + } + throw e; + } + } + tryImporting(); } }()); From 54644ba324cfd860476dae2304f9a678a9a2f7ec Mon Sep 17 00:00:00 2001 From: Guillaume Besson Date: Fri, 22 May 2020 10:32:19 +0200 Subject: [PATCH 10/11] Fix linting errors --- prelude/bootstrap.js | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/prelude/bootstrap.js b/prelude/bootstrap.js index f1013b227..4234f8d8d 100644 --- a/prelude/bootstrap.js +++ b/prelude/bootstrap.js @@ -1576,7 +1576,7 @@ function payloadFileSync (pointer) { // PATCH PROCESS /////////////////////////////////////////////////// // ///////////////////////////////////////////////////////////////// -(function() { +(function () { const fs = require('fs'); var ancestor = {}; ancestor.dlopen = process.dlopen; @@ -1616,20 +1616,20 @@ function payloadFileSync (pointer) { // the same directory, in this case, we write this file in the same /tmp directory and try to // import the module again const moduleName = e.message.match(unknownModuleErrorRegex)[1]; - const modulePath = `${moduleDirname}/${moduleName}`; - const moduleContent = fs.readFileSync(modulePath); - const moduleBaseName = require('path').basename(modulePath); + const importModulePath = `${moduleDirname}/${moduleName}`; + const moduleContent = fs.readFileSync(importModulePath); + const moduleBaseName = require('path').basename(importModulePath); const tmpModulePath = `${require('os').tmpdir()}/${moduleBaseName}`; try { fs.statSync(tmpModulePath); - } catch (e) { + } catch (err) { fs.writeFileSync(tmpModulePath, moduleContent, { mode: 0o444 }); } return tryImporting(e.message); } throw e; } - } + }; tryImporting(); - } + }; }()); From 6855243d0503f1c2c287483b0af1e37c35133786 Mon Sep 17 00:00:00 2001 From: Lee Robinson Date: Tue, 2 Mar 2021 08:09:16 -0600 Subject: [PATCH 11/11] Apply suggestions from code review --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 2688d2947..d5e4ba2db 100644 --- a/README.md +++ b/README.md @@ -203,8 +203,8 @@ In some cases (like with the `bindings` package), the module path is generated dynamicaly and `pkg` won't be able to detect it. In this case, you should add the `.node` file directly in the `assets` field in `package.json`. -The way NodeJS requires native addon is different from a classic JS -file. It needs to have a file on disk to load it but `pkg` only generate +The way Node.js requires native addon is different from a classic JS +file. It needs to have a file on disk to load it, but `pkg` only generates one file. To circumvent this, `pkg` will create a temporary file on the disk. These files will stay on the disk after the process has exited and will be used again on the next process launch.