From 123950ffa9fc08911a349be6fc1abeaa267ecaf7 Mon Sep 17 00:00:00 2001 From: Ben Konyi Date: Thu, 16 Sep 2021 20:31:45 +0000 Subject: [PATCH] [ CLI / VM ] Add support for disabling DDS Allows for tooling (e.g., DAP) to spawn their own DDS instance without having to disable the CLI. Fixes https://github.com/dart-lang/sdk/issues/47059 TEST=pkg/dartdev/test/commands/run_test.dart Change-Id: Ie9a4832d424edae67f32560399d3b0a6ca9f1dc0 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/213660 Reviewed-by: Siva Annamalai Commit-Queue: Ben Konyi --- pkg/dartdev/lib/src/commands/run.dart | 6 +++ pkg/dartdev/test/commands/run_test.dart | 58 +++++++++++++++++++++++-- runtime/bin/main.cc | 10 +++-- runtime/bin/main_options.cc | 8 +++- runtime/bin/main_options.h | 1 + 5 files changed, 75 insertions(+), 8 deletions(-) diff --git a/pkg/dartdev/lib/src/commands/run.dart b/pkg/dartdev/lib/src/commands/run.dart index 22e9e1a6dfc..ace72858db4 100644 --- a/pkg/dartdev/lib/src/commands/run.dart +++ b/pkg/dartdev/lib/src/commands/run.dart @@ -165,6 +165,12 @@ class RunCommand extends DartdevCommand { negatable: false, help: 'Enables tracing of library and script loading.', ) + ..addFlag('dds', + hide: !verbose, + help: 'Use the Dart Development Service (DDS) for enhanced debugging ' + 'functionality. Note: Disabling DDS may break some functionality ' + 'in IDEs and other tooling.', + defaultsTo: true) ..addFlag( 'debug-dds', hide: true, diff --git a/pkg/dartdev/test/commands/run_test.dart b/pkg/dartdev/test/commands/run_test.dart index 45eb00c4757..613a508cd3d 100644 --- a/pkg/dartdev/test/commands/run_test.dart +++ b/pkg/dartdev/test/commands/run_test.dart @@ -12,6 +12,9 @@ import 'package:test/test.dart'; import '../utils.dart'; const String soundNullSafetyMessage = 'Info: Compiling with sound null safety'; +const devToolsMessagePrefix = + 'The Dart DevTools debugger and profiler is available at: http://127.0.0.1:'; +const observatoryMessagePrefix = 'Observatory listening on http://127.0.0.1:'; void main() { group('run', run, timeout: longTimeout); @@ -347,10 +350,59 @@ void main(List args) => print("$b $args"); expect(result.exitCode, 0); }); - group('DevTools', () { - const devToolsMessagePrefix = - 'The Dart DevTools debugger and profiler is available at: http://127.0.0.1:'; + group('DDS', () { + group('disable', () { + test('dart run simple', () { + p = project(mainSrc: "void main() { print('Hello World'); }"); + ProcessResult result = p.runSync([ + 'run', + '--no-dds', + '--enable-vm-service', + p.relativeFilePath, + ]); + expect(result.stdout, isNot(contains(devToolsMessagePrefix))); + expect(result.stdout, contains(observatoryMessagePrefix)); + }); + test('dart simple', () { + p = project(mainSrc: "void main() { print('Hello World'); }"); + ProcessResult result = p.runSync([ + '--no-dds', + '--enable-vm-service', + p.relativeFilePath, + ]); + expect(result.stdout, isNot(contains(devToolsMessagePrefix))); + expect(result.stdout, contains(observatoryMessagePrefix)); + }); + }); + + group('explicit enable', () { + test('dart run simple', () { + p = project(mainSrc: "void main() { print('Hello World'); }"); + ProcessResult result = p.runSync([ + 'run', + '--dds', + '--enable-vm-service', + p.relativeFilePath, + ]); + expect(result.stdout, contains(devToolsMessagePrefix)); + expect(result.stdout, contains(observatoryMessagePrefix)); + }); + + test('dart simple', () { + p = project(mainSrc: "void main() { print('Hello World'); }"); + ProcessResult result = p.runSync([ + '--dds', + '--enable-vm-service', + p.relativeFilePath, + ]); + expect(result.stdout, contains(devToolsMessagePrefix)); + expect(result.stdout, contains(observatoryMessagePrefix)); + }); + }); + }); + + group('DevTools', () { test('dart run simple', () async { p = project(mainSrc: "void main() { print('Hello World'); }"); ProcessResult result = p.runSync([ diff --git a/runtime/bin/main.cc b/runtime/bin/main.cc index d3ca835c8a6..64f17153323 100644 --- a/runtime/bin/main.cc +++ b/runtime/bin/main.cc @@ -538,7 +538,7 @@ static Dart_Isolate CreateAndSetupServiceIsolate(const char* script_uri, CHECK_RESULT(result); int vm_service_server_port = INVALID_VM_SERVICE_SERVER_PORT; - if (Options::disable_dart_dev()) { + if (Options::disable_dart_dev() || Options::disable_dds()) { vm_service_server_port = Options::vm_service_server_port(); } else if (Options::vm_service_server_port() != INVALID_VM_SERVICE_SERVER_PORT) { @@ -549,12 +549,14 @@ static Dart_Isolate CreateAndSetupServiceIsolate(const char* script_uri, // the following scenarios: // - The DartDev CLI is disabled (CLI isolate starts DDS) and VM service is // enabled. + // - DDS is disabled. // TODO(bkonyi): do we want to tie DevTools / DDS to the CLI in the long run? - bool wait_for_dds_to_advertise_service = !Options::disable_dart_dev(); + bool wait_for_dds_to_advertise_service = + !(Options::disable_dart_dev() || Options::disable_dds()); // Load embedder specific bits and return. if (!VmService::Setup( - Options::disable_dart_dev() ? Options::vm_service_server_ip() - : DEFAULT_VM_SERVICE_SERVER_IP, + !wait_for_dds_to_advertise_service ? Options::vm_service_server_ip() + : DEFAULT_VM_SERVICE_SERVER_IP, vm_service_server_port, Options::vm_service_dev_mode(), Options::vm_service_auth_disabled(), Options::vm_write_service_info_filename(), Options::trace_loading(), diff --git a/runtime/bin/main_options.cc b/runtime/bin/main_options.cc index 44386ec0487..e213e706b46 100644 --- a/runtime/bin/main_options.cc +++ b/runtime/bin/main_options.cc @@ -454,6 +454,11 @@ bool Options::ParseArguments(int argc, } else if (IsOption(argv[i], "no-serve-devtools")) { serve_devtools = false; skipVmOption = true; + } else if (IsOption(argv[i], "dds")) { + // This flag is set by default in dartdev, so we ignore it. --no-dds is + // a VM flag as disabling DDS changes how we configure the VM service, + // so we don't need to handle that case here. + skipVmOption = true; } if (!skipVmOption) { temp_vm_options.AddArgument(argv[i]); @@ -623,7 +628,8 @@ bool Options::ParseArguments(int argc, if (!run_command && strcmp(argv[i - 1], "run") == 0) { run_command = true; } - if (!Options::disable_dart_dev() && enable_vm_service_ && run_command) { + if (!Options::disable_dart_dev() && !Options::disable_dds() && + enable_vm_service_ && run_command) { const char* dds_format_str = "--launch-dds=%s\\:%d"; size_t size = snprintf(nullptr, 0, dds_format_str, vm_service_server_ip(), diff --git a/runtime/bin/main_options.h b/runtime/bin/main_options.h index 66e6fde64e4..e1daacfa83b 100644 --- a/runtime/bin/main_options.h +++ b/runtime/bin/main_options.h @@ -44,6 +44,7 @@ namespace bin { V(suppress_core_dump, suppress_core_dump) \ V(enable_service_port_fallback, enable_service_port_fallback) \ V(disable_dart_dev, disable_dart_dev) \ + V(no_dds, disable_dds) \ V(long_ssl_cert_evaluation, long_ssl_cert_evaluation) \ V(bypass_trusting_system_roots, bypass_trusting_system_roots) \ V(delayed_filewatch_callback, delayed_filewatch_callback) \