From 1d76ba1b3deaeda6b4dbf3f3f224128ee2f77460 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Wed, 30 Jan 2019 23:19:45 +0800 Subject: [PATCH] process: expose process.features.inspector Instead of using process.config.variables.v8_enable_inspector to detect whether inspector is enabled in the build. PR-URL: https://github.com/nodejs/node/pull/25819 Refs: https://github.com/nodejs/node/issues/25343 Reviewed-By: Eugene Ostroukhov Reviewed-By: Gus Caplan Reviewed-By: James M Snell Reviewed-By: Richard Lau Reviewed-By: Colin Ihrig Reviewed-By: Anna Henningsen --- lib/internal/bootstrap/node.js | 3 ++- lib/internal/util/inspector.js | 2 +- src/node_config.cc | 12 ++++++------ test/common/index.js | 5 ++--- .../test-inspector-cluster-port-clash.js | 2 +- test/parallel/test-cli-bad-options.js | 2 +- test/parallel/test-cli-node-print-help.js | 3 +-- test/parallel/test-module-cjs-helpers.js | 4 +--- test/parallel/test-process-env-allowed-flags.js | 2 +- test/parallel/test-process-features.js | 1 + test/parallel/test-repl-tab-complete.js | 2 +- test/parallel/test-v8-coverage.js | 2 +- test/sequential/test-async-wrap-getasyncid.js | 3 +-- .../sequential/test-inspector-has-inspector-false.js | 6 +++--- tools/test.py | 4 ++-- 15 files changed, 25 insertions(+), 28 deletions(-) diff --git a/lib/internal/bootstrap/node.js b/lib/internal/bootstrap/node.js index 24f7033f714845..534f43bad6f241 100644 --- a/lib/internal/bootstrap/node.js +++ b/lib/internal/bootstrap/node.js @@ -292,12 +292,13 @@ process.assert = deprecate( // TODO(joyeecheung): this property has not been well-maintained, should we // deprecate it in favor of a better API? -const { isDebugBuild, hasOpenSSL } = config; +const { isDebugBuild, hasOpenSSL, hasInspector } = config; Object.defineProperty(process, 'features', { enumerable: true, writable: false, configurable: false, value: { + inspector: hasInspector, debug: isDebugBuild, uv: true, ipv6: true, // TODO(bnoordhuis) ping libuv diff --git a/lib/internal/util/inspector.js b/lib/internal/util/inspector.js index 68c20108bd317b..e237db16a64800 100644 --- a/lib/internal/util/inspector.js +++ b/lib/internal/util/inspector.js @@ -3,8 +3,8 @@ let session; function sendInspectorCommand(cb, onError) { const { hasInspector } = internalBinding('config'); - const inspector = hasInspector ? require('inspector') : undefined; if (!hasInspector) return onError(); + const inspector = require('inspector'); if (session === undefined) session = new inspector.Session(); try { session.connect(); diff --git a/src/node_config.cc b/src/node_config.cc index 1f851ef91cdd45..6e443230f74df3 100644 --- a/src/node_config.cc +++ b/src/node_config.cc @@ -57,12 +57,6 @@ static void Initialize(Local target, READONLY_TRUE_PROPERTY(target, "hasTracing"); #endif -#if HAVE_INSPECTOR - READONLY_TRUE_PROPERTY(target, "hasInspector"); -#else - READONLY_FALSE_PROPERTY(target, "hasInspector"); -#endif - #if !defined(NODE_WITHOUT_NODE_OPTIONS) READONLY_TRUE_PROPERTY(target, "hasNodeOptions"); #endif @@ -73,6 +67,12 @@ static void Initialize(Local target, #endif // NODE_HAVE_I18N_SUPPORT +#if HAVE_INSPECTOR + READONLY_TRUE_PROPERTY(target, "hasInspector"); +#else + READONLY_FALSE_PROPERTY(target, "hasInspector"); +#endif + READONLY_PROPERTY(target, "bits", Number::New(env->isolate(), 8 * sizeof(intptr_t))); diff --git a/test/common/index.js b/test/common/index.js index 2c1eae7e410650..3d72a876eac281 100644 --- a/test/common/index.js +++ b/test/common/index.js @@ -80,8 +80,7 @@ if (process.argv.length === 2 && hasCrypto && // If the binary is build without `intl` the inspect option is // invalid. The test itself should handle this case. - (process.config.variables.v8_enable_inspector !== 0 || - !flag.startsWith('--inspect'))) { + (process.features.inspector || !flag.startsWith('--inspect'))) { throw new Error(`Test has to be started with the flag: '${flag}'`); } } @@ -629,7 +628,7 @@ function expectsError(fn, settings, exact) { } function skipIfInspectorDisabled() { - if (process.config.variables.v8_enable_inspector === 0) { + if (!process.features.inspector) { skip('V8 inspector is disabled'); } } diff --git a/test/known_issues/test-inspector-cluster-port-clash.js b/test/known_issues/test-inspector-cluster-port-clash.js index 0333ffcb98751c..bc04e9907b168b 100644 --- a/test/known_issues/test-inspector-cluster-port-clash.js +++ b/test/known_issues/test-inspector-cluster-port-clash.js @@ -13,7 +13,7 @@ const assert = require('assert'); // This following check should be replaced by common.skipIfInspectorDisabled() // if moved out of the known_issues directory. -if (process.config.variables.v8_enable_inspector === 0) { +if (!process.features.inspector) { // When the V8 inspector is disabled, using either --without-inspector or // --without-ssl, this test will not fail which it is expected to do. // The following fail will allow this test to be skipped by failing it. diff --git a/test/parallel/test-cli-bad-options.js b/test/parallel/test-cli-bad-options.js index 7abd330aa4726d..3fc8980c142509 100644 --- a/test/parallel/test-cli-bad-options.js +++ b/test/parallel/test-cli-bad-options.js @@ -6,7 +6,7 @@ require('../common'); const assert = require('assert'); const spawn = require('child_process').spawnSync; -if (process.config.variables.v8_enable_inspector === 1) { +if (process.features.inspector) { requiresArgument('--inspect-port'); requiresArgument('--inspect-port='); requiresArgument('--debug-port'); diff --git a/test/parallel/test-cli-node-print-help.js b/test/parallel/test-cli-node-print-help.js index 433a95faf20542..e115124b0487fe 100644 --- a/test/parallel/test-cli-node-print-help.js +++ b/test/parallel/test-cli-node-print-help.js @@ -22,10 +22,9 @@ function startPrintHelpTest() { } function validateNodePrintHelp() { - const config = process.config; const HAVE_OPENSSL = common.hasCrypto; const NODE_HAVE_I18N_SUPPORT = common.hasIntl; - const HAVE_INSPECTOR = config.variables.v8_enable_inspector === 1; + const HAVE_INSPECTOR = process.features.inspector; const cliHelpOptions = [ { compileConstant: HAVE_OPENSSL, diff --git a/test/parallel/test-module-cjs-helpers.js b/test/parallel/test-module-cjs-helpers.js index ccf95f3e81c14a..12de65598e54e1 100644 --- a/test/parallel/test-module-cjs-helpers.js +++ b/test/parallel/test-module-cjs-helpers.js @@ -5,7 +5,5 @@ require('../common'); const assert = require('assert'); const { builtinLibs } = require('internal/modules/cjs/helpers'); -const hasInspector = process.config.variables.v8_enable_inspector === 1; - -const expectedLibs = hasInspector ? 34 : 33; +const expectedLibs = process.features.inspector ? 34 : 33; assert.strictEqual(builtinLibs.length, expectedLibs); diff --git a/test/parallel/test-process-env-allowed-flags.js b/test/parallel/test-process-env-allowed-flags.js index 00c44e869455f1..dbc3b1ccd8c3c6 100644 --- a/test/parallel/test-process-env-allowed-flags.js +++ b/test/parallel/test-process-env-allowed-flags.js @@ -16,7 +16,7 @@ require('../common'); 'r', '--stack-trace-limit=100', '--stack-trace-limit=-=xX_nodejs_Xx=-', - ].concat(process.config.variables.v8_enable_inspector ? [ + ].concat(process.features.inspector ? [ '--inspect-brk', 'inspect-brk', '--inspect_brk', diff --git a/test/parallel/test-process-features.js b/test/parallel/test-process-features.js index 7752cc53b2a1ca..a24d366ba8e30a 100644 --- a/test/parallel/test-process-features.js +++ b/test/parallel/test-process-features.js @@ -6,6 +6,7 @@ const assert = require('assert'); const keys = new Set(Object.keys(process.features)); assert.deepStrictEqual(keys, new Set([ + 'inspector', 'debug', 'uv', 'ipv6', diff --git a/test/parallel/test-repl-tab-complete.js b/test/parallel/test-repl-tab-complete.js index 0325678ff2e200..5438c960b84cb1 100644 --- a/test/parallel/test-repl-tab-complete.js +++ b/test/parallel/test-repl-tab-complete.js @@ -29,7 +29,7 @@ const { } = require('../common/hijackstdio'); const assert = require('assert'); const fixtures = require('../common/fixtures'); -const hasInspector = process.config.variables.v8_enable_inspector === 1; +const hasInspector = process.features.inspector; if (!common.isMainThread) common.skip('process.chdir is not available in Workers'); diff --git a/test/parallel/test-v8-coverage.js b/test/parallel/test-v8-coverage.js index a7a318c34d4e96..3db8b81d5504b4 100644 --- a/test/parallel/test-v8-coverage.js +++ b/test/parallel/test-v8-coverage.js @@ -1,6 +1,6 @@ 'use strict'; -if (!process.config.variables.v8_enable_inspector) return; +if (!process.features.inspector) return; const common = require('../common'); const assert = require('assert'); diff --git a/test/sequential/test-async-wrap-getasyncid.js b/test/sequential/test-async-wrap-getasyncid.js index 6a6bf1b407e50f..a5e2bf64d82fc2 100644 --- a/test/sequential/test-async-wrap-getasyncid.js +++ b/test/sequential/test-async-wrap-getasyncid.js @@ -289,8 +289,7 @@ if (common.hasCrypto) { // eslint-disable-line node-core/crypto-check testInitialized(req, 'SendWrap'); } -if (process.config.variables.v8_enable_inspector !== 0 && - common.isMainThread) { +if (process.features.inspector && common.isMainThread) { const binding = internalBinding('inspector'); const handle = new binding.Connection(() => {}); testInitialized(handle, 'Connection'); diff --git a/test/sequential/test-inspector-has-inspector-false.js b/test/sequential/test-inspector-has-inspector-false.js index cdb7ca9e19e79b..56a50408bb8ea1 100644 --- a/test/sequential/test-inspector-has-inspector-false.js +++ b/test/sequential/test-inspector-has-inspector-false.js @@ -3,10 +3,10 @@ const common = require('../common'); -// This is to ensure that the sendInspectorCommand function calls the error -// function if its called with the v8_enable_inspector is disabled +if (process.features.inspector) { + common.skip('V8 inspector is enabled'); +} -process.config.variables.v8_enable_inspector = 0; const inspector = require('internal/util/inspector'); inspector.sendInspectorCommand( diff --git a/tools/test.py b/tools/test.py index e457075a1c5395..e6fc7be73f11b1 100755 --- a/tools/test.py +++ b/tools/test.py @@ -1661,8 +1661,8 @@ def Main(): # We want to skip the inspector tests if node was built without the inspector. has_inspector = Execute([vm, - '-p', 'process.config.variables.v8_enable_inspector'], context) - if has_inspector.stdout.rstrip() == '0': + '-p', 'process.features.inspector'], context) + if has_inspector.stdout.rstrip() == 'false': context.v8_enable_inspector = False has_crypto = Execute([vm,