From f7726369b011bfdcc6b1532eb0f92f77d802e112 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Thu, 31 Jan 2019 03:14:59 +0800 Subject: [PATCH 1/3] process: exit on --debug and --debug-brk after option parsing Moves the exit of `--debug` and `--debug-brk` earlier, that is, after the option parsing is done in the C++ land. Also removes `process._invalidDebug`. --- lib/internal/bootstrap/node.js | 8 +------- src/node_options.cc | 6 ++++++ src/node_options.h | 4 ---- src/node_process_object.cc | 6 ------ 4 files changed, 7 insertions(+), 17 deletions(-) diff --git a/lib/internal/bootstrap/node.js b/lib/internal/bootstrap/node.js index 23d72547ae900f..05a123feb9cfca 100644 --- a/lib/internal/bootstrap/node.js +++ b/lib/internal/bootstrap/node.js @@ -204,13 +204,7 @@ Object.defineProperty(process, 'argv0', { process.argv[0] = process.execPath; // Handle `--debug*` deprecation and invalidation. -if (process._invalidDebug) { - process.emitWarning( - '`node --debug` and `node --debug-brk` are invalid. ' + - 'Please use `node --inspect` or `node --inspect-brk` instead.', - 'DeprecationWarning', 'DEP0062', undefined, true); - process.exit(9); -} else if (process._deprecatedDebugBrk) { +if (process._deprecatedDebugBrk) { process.emitWarning( '`node --inspect --debug-brk` is deprecated. ' + 'Please use `node --inspect-brk` instead.', diff --git a/src/node_options.cc b/src/node_options.cc index 8bd8b827faa109..8e49fdbf389d60 100644 --- a/src/node_options.cc +++ b/src/node_options.cc @@ -30,6 +30,12 @@ void DebugOptions::CheckOptions(std::vector* errors) { "--without-v8-platform"); } #endif + + if (deprecated_debug && !inspector_enabled) { + errors->push_back("[DEP0062]: `node --debug` and `node --debug-brk` " + "are invalid. Please use `node --inspect` or " + "`node --inspect-brk` instead."); + } } void PerProcessOptions::CheckOptions(std::vector* errors) { diff --git a/src/node_options.h b/src/node_options.h index 50c66ce890bf4c..d4e373d05d9280 100644 --- a/src/node_options.h +++ b/src/node_options.h @@ -81,10 +81,6 @@ class DebugOptions : public Options { break_first_line; } - bool invalid_invocation() const { - return deprecated_debug && !inspector_enabled; - } - bool wait_for_connect() const { return break_first_line || break_node_first_line; } diff --git a/src/node_process_object.cc b/src/node_process_object.cc index c1f8806110ffef..89abe19a84ff57 100644 --- a/src/node_process_object.cc +++ b/src/node_process_object.cc @@ -265,12 +265,6 @@ MaybeLocal CreateProcessObject( "_deprecatedDebugBrk", True(env->isolate())); } - // --debug or, --debug-brk without --inspect - if (env->options()->debug_options().invalid_invocation()) { - READONLY_DONT_ENUM_PROPERTY(process, - "_invalidDebug", True(env->isolate())); - } - // --security-revert flags #define V(code, _, __) \ do { \ From 3657440760aeb320e219cd4434e38fad63d37f08 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Thu, 31 Jan 2019 03:29:30 +0800 Subject: [PATCH 2/3] process: move DEP0062 (node --debug) to end-of-life This has already been practically end-of-life since `node --debug` alone would exit the process. This patch drops support of `node --inspect --debug-brk` as well. `node --inspect --debug-brk` has been deprecated since v8, it has been maintained so that vendors can target Node.js v6 and above without detecting versions. The support of `--inspect`, which starts from v6, will reach end-of-life in April 2019, it should be safe to drop the support of `--inspect --debug-brk` altogether in v12. Also removes `process._deprecatedDebugBrk` --- doc/api/deprecations.md | 5 +++- lib/internal/bootstrap/node.js | 8 ------- src/node_options.cc | 5 ++++ src/node_options.h | 6 ----- src/node_process_object.cc | 6 ----- test/sequential/test-debugger-debug-brk.js | 27 ++++++---------------- 6 files changed, 16 insertions(+), 41 deletions(-) diff --git a/doc/api/deprecations.md b/doc/api/deprecations.md index ecaea8f4d9f7ae..273b1131018cfa 100644 --- a/doc/api/deprecations.md +++ b/doc/api/deprecations.md @@ -1279,9 +1279,12 @@ changes: - version: v8.0.0 pr-url: https://github.com/nodejs/node/pull/10970 description: Runtime deprecation. + - version: REPLACEME + pr-url: REPLACEME + description: End-of-Life. --> -Type: Runtime +Type: End-Of-Life `--debug` activates the legacy V8 debugger interface, which was removed as of V8 5.8. It is replaced by Inspector which is activated with `--inspect` diff --git a/lib/internal/bootstrap/node.js b/lib/internal/bootstrap/node.js index 05a123feb9cfca..4b630085e349d1 100644 --- a/lib/internal/bootstrap/node.js +++ b/lib/internal/bootstrap/node.js @@ -203,14 +203,6 @@ Object.defineProperty(process, 'argv0', { }); process.argv[0] = process.execPath; -// Handle `--debug*` deprecation and invalidation. -if (process._deprecatedDebugBrk) { - process.emitWarning( - '`node --inspect --debug-brk` is deprecated. ' + - 'Please use `node --inspect-brk` instead.', - 'DeprecationWarning', 'DEP0062', undefined, true); -} - const { deprecate } = NativeModule.require('internal/util'); { // Install legacy getters on the `util` binding for typechecking. diff --git a/src/node_options.cc b/src/node_options.cc index 8e49fdbf389d60..2398ba2bb3cd69 100644 --- a/src/node_options.cc +++ b/src/node_options.cc @@ -36,6 +36,11 @@ void DebugOptions::CheckOptions(std::vector* errors) { "are invalid. Please use `node --inspect` or " "`node --inspect-brk` instead."); } + + if (deprecated_debug && inspector_enabled && break_first_line) { + errors->push_back("[DEP0062]: `node --inspect --debug-brk` is deprecated. " + "Please use `node --inspect-brk` instead."); + } } void PerProcessOptions::CheckOptions(std::vector* errors) { diff --git a/src/node_options.h b/src/node_options.h index d4e373d05d9280..e68e1cdfd734ca 100644 --- a/src/node_options.h +++ b/src/node_options.h @@ -75,12 +75,6 @@ class DebugOptions : public Options { HostPort host_port{"127.0.0.1", kDefaultInspectorPort}; - bool deprecated_invocation() const { - return deprecated_debug && - inspector_enabled && - break_first_line; - } - bool wait_for_connect() const { return break_first_line || break_node_first_line; } diff --git a/src/node_process_object.cc b/src/node_process_object.cc index 89abe19a84ff57..f83f553c997b66 100644 --- a/src/node_process_object.cc +++ b/src/node_process_object.cc @@ -259,12 +259,6 @@ MaybeLocal CreateProcessObject( "_breakNodeFirstLine", True(env->isolate())); } - // --inspect --debug-brk - if (env->options()->debug_options().deprecated_invocation()) { - READONLY_DONT_ENUM_PROPERTY(process, - "_deprecatedDebugBrk", True(env->isolate())); - } - // --security-revert flags #define V(code, _, __) \ do { \ diff --git a/test/sequential/test-debugger-debug-brk.js b/test/sequential/test-debugger-debug-brk.js index 086ee2788dee1c..33a7539b554c60 100644 --- a/test/sequential/test-debugger-debug-brk.js +++ b/test/sequential/test-debugger-debug-brk.js @@ -2,32 +2,19 @@ const common = require('../common'); common.skipIfInspectorDisabled(); -// This test ensures that the debug-brk flag will spin up a new process and -// wait, rather than exit. - +// This test ensures that the --debug-brk flag will exit the process const assert = require('assert'); const fixtures = require('../common/fixtures'); -const spawn = require('child_process').spawn; +const { spawnSync } = require('child_process'); -// file name here doesn't actually matter since -// debugger will connect regardless of file name arg +// file name here doesn't actually matter the process will exit on start. const script = fixtures.path('empty.js'); function test(arg) { - const child = spawn(process.execPath, ['--inspect', arg, script]); - const argStr = child.spawnargs.join(' '); - const fail = () => assert.fail(true, false, `'${argStr}' should not quit`); - child.on('exit', fail); - - // give node time to start up the debugger - setTimeout(function() { - child.removeListener('exit', fail); - child.kill(); - }, 2000); - - process.on('exit', function() { - assert(child.killed); - }); + const child = spawnSync(process.execPath, ['--inspect', arg, script]); + const stderr = child.stderr.toString(); + assert(stderr.includes('DEP0062')); + assert.strictEqual(child.status, 9); } test('--debug-brk'); From 1cfc45a399c34265659ede4d230a9edec9c4a3bd Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Thu, 31 Jan 2019 04:06:13 +0800 Subject: [PATCH 3/3] fixup! process: move DEP0062 (node --debug) to end-of-life --- doc/api/deprecations.md | 2 +- test/sequential/test-debugger-debug-brk.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/api/deprecations.md b/doc/api/deprecations.md index 273b1131018cfa..169b5a51315465 100644 --- a/doc/api/deprecations.md +++ b/doc/api/deprecations.md @@ -1280,7 +1280,7 @@ changes: pr-url: https://github.com/nodejs/node/pull/10970 description: Runtime deprecation. - version: REPLACEME - pr-url: REPLACEME + pr-url: https://github.com/nodejs/node/pull/25828 description: End-of-Life. --> diff --git a/test/sequential/test-debugger-debug-brk.js b/test/sequential/test-debugger-debug-brk.js index 33a7539b554c60..7a13572a06a2a5 100644 --- a/test/sequential/test-debugger-debug-brk.js +++ b/test/sequential/test-debugger-debug-brk.js @@ -7,7 +7,7 @@ const assert = require('assert'); const fixtures = require('../common/fixtures'); const { spawnSync } = require('child_process'); -// file name here doesn't actually matter the process will exit on start. +// File name here doesn't actually matter the process will exit on start. const script = fixtures.path('empty.js'); function test(arg) {