[dds] Refactor some DAP classes in preperation for "dart test" adapter
Change-Id: Ice48b45c0ad11a1981e8e4115bb8de6d009d7b5a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/212062
Reviewed-by: Ben Konyi <bkonyi@google.com>
Commit-Queue: Ben Konyi <bkonyi@google.com>
diff --git a/pkg/dds/lib/src/dap/adapters/dart.dart b/pkg/dds/lib/src/dap/adapters/dart.dart
index 5099eef..1343db2 100644
--- a/pkg/dds/lib/src/dap/adapters/dart.dart
+++ b/pkg/dds/lib/src/dap/adapters/dart.dart
@@ -1654,10 +1654,18 @@
/// enabling debugging.
final bool? noDebug;
+ /// The program/Dart script to be run.
final String program;
+
+ /// Arguments to be passed to [program].
final List<String>? args;
+
+ /// Arguments to be passed to the tool that will run [program] (for example,
+ /// the VM or Flutter tool).
+ final List<String>? toolArgs;
+
final int? vmServicePort;
- final List<String>? vmAdditionalArgs;
+
final bool? enableAsserts;
/// Which console to run the program in.
@@ -1678,7 +1686,7 @@
required this.program,
this.args,
this.vmServicePort,
- this.vmAdditionalArgs,
+ this.toolArgs,
this.console,
this.enableAsserts,
String? name,
@@ -1705,7 +1713,7 @@
noDebug = obj['noDebug'] as bool?,
program = obj['program'] as String,
args = (obj['args'] as List?)?.cast<String>(),
- vmAdditionalArgs = (obj['vmAdditionalArgs'] as List?)?.cast<String>(),
+ toolArgs = (obj['toolArgs'] as List?)?.cast<String>(),
vmServicePort = obj['vmServicePort'] as int?,
console = obj['console'] as String?,
enableAsserts = obj['enableAsserts'] as bool?,
@@ -1718,7 +1726,7 @@
if (noDebug != null) 'noDebug': noDebug,
'program': program,
if (args != null) 'args': args,
- if (vmAdditionalArgs != null) 'vmAdditionalArgs': vmAdditionalArgs,
+ if (toolArgs != null) 'toolArgs': toolArgs,
if (vmServicePort != null) 'vmServicePort': vmServicePort,
if (console != null) 'console': console,
if (enableAsserts != null) 'enableAsserts': enableAsserts,
diff --git a/pkg/dds/lib/src/dap/adapters/dart_cli.dart b/pkg/dds/lib/src/dap/adapters/dart_cli.dart
index bf0eab9..2a3f351 100644
--- a/pkg/dds/lib/src/dap/adapters/dart_cli.dart
+++ b/pkg/dds/lib/src/dap/adapters/dart_cli.dart
@@ -10,25 +10,18 @@
import 'package:pedantic/pedantic.dart';
import 'package:vm_service/vm_service.dart' as vm;
-import '../exceptions.dart';
import '../logging.dart';
import '../protocol_generated.dart';
import '../protocol_stream.dart';
import 'dart.dart';
+import 'mixins.dart';
/// A DAP Debug Adapter for running and debugging Dart CLI scripts.
class DartCliDebugAdapter extends DartDebugAdapter<DartLaunchRequestArguments,
- DartAttachRequestArguments> {
+ DartAttachRequestArguments>
+ with PidTracker, VmServiceInfoFileUtils, PackageConfigUtils {
Process? _process;
- /// Process IDs to terminate during shutdown.
- ///
- /// This may be populated with pids from the VM Service to ensure we clean up
- /// properly where signals may not be passed through the shell to the
- /// underlying VM process.
- /// https://github.com/Dart-Code/Dart-Code/issues/907
- final pidsToTerminate = <int>{};
-
@override
final parseLaunchArgs = DartLaunchRequestArguments.fromJson;
@@ -75,45 +68,7 @@
/// Called by [disconnectRequest] to request that we forcefully shut down the
/// app being run (or in the case of an attach, disconnect).
Future<void> disconnectImpl() async {
- // TODO(dantup): In Dart-Code DAP, we first try again with sigint and wait
- // for a few seconds before sending sigkill.
- pidsToTerminate.forEach(
- (pid) => Process.killPid(pid, ProcessSignal.sigkill),
- );
- }
-
- /// Waits for [vmServiceInfoFile] to exist and become valid before returning
- /// the VM Service URI contained within.
- Future<Uri> waitForVmServiceInfoFile(File vmServiceInfoFile) async {
- final completer = Completer<Uri>();
- late final StreamSubscription<FileSystemEvent> vmServiceInfoFileWatcher;
-
- Uri? tryParseServiceInfoFile(FileSystemEvent event) {
- final uri = _readVmServiceInfoFile(vmServiceInfoFile);
- if (uri != null && !completer.isCompleted) {
- vmServiceInfoFileWatcher.cancel();
- completer.complete(uri);
- }
- }
-
- vmServiceInfoFileWatcher = vmServiceInfoFile.parent
- .watch(events: FileSystemEvent.all)
- .where((event) => event.path == vmServiceInfoFile.path)
- .listen(
- tryParseServiceInfoFile,
- onError: (e) => logger?.call('Ignoring exception from watcher: $e'),
- );
-
- // After setting up the watcher, also check if the file already exists to
- // ensure we don't miss it if it was created right before we set the
- // watched up.
- final uri = _readVmServiceInfoFile(vmServiceInfoFile);
- if (uri != null && !completer.isCompleted) {
- unawaited(vmServiceInfoFileWatcher.cancel());
- completer.complete(uri);
- }
-
- return completer.future;
+ terminatePids(ProcessSignal.sigkill);
}
/// Called by [launchRequest] to request that we actually start the app to be
@@ -128,25 +83,15 @@
final debug = !(args.noDebug ?? false);
if (debug) {
- // Create a temp folder for the VM to write the service-info-file into.
- // Using tmpDir.createTempory() is flakey on Windows+Linux (at least
- // on GitHub Actions) complaining the file does not exist when creating a
- // watcher. Creating/watching a folder and writing the file into it seems
- // to be reliable.
- final serviceInfoFilePath = path.join(
- Directory.systemTemp.createTempSync('dart-vm-service').path,
- 'vm.json',
- );
-
- vmServiceInfoFile = File(serviceInfoFilePath);
- unawaited(waitForVmServiceInfoFile(vmServiceInfoFile)
+ vmServiceInfoFile = generateVmServiceInfoFile();
+ unawaited(waitForVmServiceInfoFile(logger, vmServiceInfoFile)
.then((uri) => connectDebugger(uri, resumeIfStarting: true)));
}
final vmArgs = <String>[
if (debug) ...[
'--enable-vm-service=${args.vmServicePort ?? 0}${ipv6 ? '/::1' : ''}',
- '--pause_isolates_on_start=true',
+ '--pause_isolates_on_start',
if (!enableAuthCodes) '--disable-service-auth-codes'
],
'--disable-dart-dev',
@@ -156,10 +101,11 @@
],
// Default to asserts on, this seems like the most useful behaviour for
// editor-spawned debug sessions.
- if (args.enableAsserts ?? true) '--enable-asserts'
+ if (args.enableAsserts ?? true) '--enable-asserts',
];
final processArgs = [
...vmArgs,
+ ...?args.toolArgs,
args.program,
...?args.args,
];
@@ -171,7 +117,7 @@
var possibleRoot = path.isAbsolute(args.program)
? path.dirname(args.program)
: path.dirname(path.normalize(path.join(args.cwd ?? '', args.program)));
- final packageConfig = _findPackageConfigFile(possibleRoot);
+ final packageConfig = findPackageConfigFile(possibleRoot);
if (packageConfig != null) {
this.usePackageConfigFile(packageConfig);
}
@@ -209,9 +155,12 @@
final vmServiceInfoFile = args.vmServiceInfoFile;
if ((vmServiceUri == null) == (vmServiceInfoFile == null)) {
- throw DebugAdapterException(
- 'To attach, provide exactly one of vmServiceUri/vmServiceInfoFile',
+ sendOutput(
+ 'console',
+ '\nTo attach, provide exactly one of vmServiceUri/vmServiceInfoFile',
);
+ handleSessionTerminate();
+ return;
}
// Find the package_config file for this script.
@@ -220,7 +169,7 @@
// necessary.
final cwd = args.cwd;
if (cwd != null) {
- final packageConfig = _findPackageConfigFile(cwd);
+ final packageConfig = findPackageConfigFile(cwd);
if (packageConfig != null) {
this.usePackageConfigFile(packageConfig);
}
@@ -228,7 +177,7 @@
final uri = vmServiceUri != null
? Uri.parse(vmServiceUri)
- : await waitForVmServiceInfoFile(File(vmServiceInfoFile!));
+ : await waitForVmServiceInfoFile(logger, File(vmServiceInfoFile!));
unawaited(connectDebugger(uri, resumeIfStarting: false));
}
@@ -297,42 +246,10 @@
unawaited(process.exitCode.then(_handleExitCode));
}
- /// Find the `package_config.json` file for the program being launched.
- ///
- /// TODO(dantup): Remove this once
- /// https://github.com/dart-lang/sdk/issues/45530 is done as it will not be
- /// necessary.
- File? _findPackageConfigFile(String possibleRoot) {
- File? packageConfig;
- while (true) {
- packageConfig =
- File(path.join(possibleRoot, '.dart_tool', 'package_config.json'));
-
- // If this packageconfig exists, use it.
- if (packageConfig.existsSync()) {
- break;
- }
-
- final parent = path.dirname(possibleRoot);
-
- // If we can't go up anymore, the search failed.
- if (parent == possibleRoot) {
- packageConfig = null;
- break;
- }
-
- possibleRoot = parent;
- }
-
- return packageConfig;
- }
-
/// Called by [terminateRequest] to request that we gracefully shut down the
/// app being run (or in the case of an attach, disconnect).
Future<void> terminateImpl() async {
- pidsToTerminate.forEach(
- (pid) => Process.killPid(pid, ProcessSignal.sigint),
- );
+ terminatePids(ProcessSignal.sigint);
await _process?.exitCode;
}
@@ -349,19 +266,4 @@
void _handleStdout(List<int> data) {
sendOutput('stdout', utf8.decode(data));
}
-
- /// Attempts to read VM Service info from a watcher event.
- ///
- /// If successful, returns the URI. Otherwise, returns null.
- Uri? _readVmServiceInfoFile(File file) {
- try {
- final content = file.readAsStringSync();
- final json = jsonDecode(content);
- return Uri.parse(json['uri']);
- } catch (e) {
- // It's possible we tried to read the file before it was completely
- // written so ignore and try again on the next event.
- logger?.call('Ignoring error parsing vm-service-info file: $e');
- }
- }
}
diff --git a/pkg/dds/lib/src/dap/adapters/mixins.dart b/pkg/dds/lib/src/dap/adapters/mixins.dart
new file mode 100644
index 0000000..8c73716
--- /dev/null
+++ b/pkg/dds/lib/src/dap/adapters/mixins.dart
@@ -0,0 +1,139 @@
+// Copyright (c) 2021, the Dart project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+import 'dart:async';
+import 'dart:convert';
+import 'dart:io';
+
+import 'package:path/path.dart' as path;
+import 'package:pedantic/pedantic.dart';
+
+import '../logging.dart';
+
+/// A mixin providing some utility functions for locating/working with
+/// package_config.json files.
+mixin PackageConfigUtils {
+ /// Find the `package_config.json` file for the program being launched.
+ ///
+ /// TODO(dantup): Remove this once
+ /// https://github.com/dart-lang/sdk/issues/45530 is done as it will not be
+ /// necessary.
+ File? findPackageConfigFile(String possibleRoot) {
+ File? packageConfig;
+ while (true) {
+ packageConfig =
+ File(path.join(possibleRoot, '.dart_tool', 'package_config.json'));
+
+ // If this packageconfig exists, use it.
+ if (packageConfig.existsSync()) {
+ break;
+ }
+
+ final parent = path.dirname(possibleRoot);
+
+ // If we can't go up anymore, the search failed.
+ if (parent == possibleRoot) {
+ packageConfig = null;
+ break;
+ }
+
+ possibleRoot = parent;
+ }
+
+ return packageConfig;
+ }
+}
+
+/// A mixin for tracking additional PIDs that can be shut down at the end of a
+/// debug session.
+mixin PidTracker {
+ /// Process IDs to terminate during shutdown.
+ ///
+ /// This may be populated with pids from the VM Service to ensure we clean up
+ /// properly where signals may not be passed through the shell to the
+ /// underlying VM process.
+ /// https://github.com/Dart-Code/Dart-Code/issues/907
+ final pidsToTerminate = <int>{};
+
+ /// Terminates all processes with the PIDs registered in [pidsToTerminate].
+ void terminatePids(ProcessSignal signal) {
+ // TODO(dantup): In Dart-Code DAP, we first try again with sigint and wait
+ // for a few seconds before sending sigkill.
+ pidsToTerminate.forEach(
+ (pid) => Process.killPid(pid, signal),
+ );
+ }
+}
+
+/// A mixin providing some utility functions for working with vm-service-info
+/// files such as ensuring a temp folder exists to create them in, and waiting
+/// for the file to become valid parsable JSON.
+mixin VmServiceInfoFileUtils {
+ /// Creates a temp folder for the VM to write the service-info-file into and
+ /// returns the [File] to use.
+ File generateVmServiceInfoFile() {
+ // Using tmpDir.createTempory() is flakey on Windows+Linux (at least
+ // on GitHub Actions) complaining the file does not exist when creating a
+ // watcher. Creating/watching a folder and writing the file into it seems
+ // to be reliable.
+ final serviceInfoFilePath = path.join(
+ Directory.systemTemp.createTempSync('dart-vm-service').path,
+ 'vm.json',
+ );
+
+ return File(serviceInfoFilePath);
+ }
+
+ /// Waits for [vmServiceInfoFile] to exist and become valid before returning
+ /// the VM Service URI contained within.
+ Future<Uri> waitForVmServiceInfoFile(
+ Logger? logger,
+ File vmServiceInfoFile,
+ ) async {
+ final completer = Completer<Uri>();
+ late final StreamSubscription<FileSystemEvent> vmServiceInfoFileWatcher;
+
+ Uri? tryParseServiceInfoFile(FileSystemEvent event) {
+ final uri = _readVmServiceInfoFile(logger, vmServiceInfoFile);
+ if (uri != null && !completer.isCompleted) {
+ vmServiceInfoFileWatcher.cancel();
+ completer.complete(uri);
+ }
+ }
+
+ vmServiceInfoFileWatcher = vmServiceInfoFile.parent
+ .watch(events: FileSystemEvent.all)
+ .where((event) => event.path == vmServiceInfoFile.path)
+ .listen(
+ tryParseServiceInfoFile,
+ onError: (e) => logger?.call('Ignoring exception from watcher: $e'),
+ );
+
+ // After setting up the watcher, also check if the file already exists to
+ // ensure we don't miss it if it was created right before we set the
+ // watched up.
+ final uri = _readVmServiceInfoFile(logger, vmServiceInfoFile);
+ if (uri != null && !completer.isCompleted) {
+ unawaited(vmServiceInfoFileWatcher.cancel());
+ completer.complete(uri);
+ }
+
+ return completer.future;
+ }
+
+ /// Attempts to read VM Service info from a watcher event.
+ ///
+ /// If successful, returns the URI. Otherwise, returns null.
+ Uri? _readVmServiceInfoFile(Logger? logger, File file) {
+ try {
+ final content = file.readAsStringSync();
+ final json = jsonDecode(content);
+ return Uri.parse(json['uri']);
+ } catch (e) {
+ // It's possible we tried to read the file before it was completely
+ // written so ignore and try again on the next event.
+ logger?.call('Ignoring error parsing vm-service-info file: $e');
+ }
+ }
+}
diff --git a/pkg/dds/lib/src/dap/protocol_generated.dart b/pkg/dds/lib/src/dap/protocol_generated.dart
index 30620e4..a39f56f 100644
--- a/pkg/dds/lib/src/dap/protocol_generated.dart
+++ b/pkg/dds/lib/src/dap/protocol_generated.dart
@@ -4,8 +4,8 @@
// This code was auto-generated by tool/dap/generate_all.dart - do not hand-edit!
-import 'package:dds/src/dap/protocol_common.dart';
-import 'package:dds/src/dap/protocol_special.dart';
+import 'protocol_common.dart';
+import 'protocol_special.dart';
/// Arguments for 'attach' request. Additional attributes are implementation
/// specific.
diff --git a/pkg/dds/test/dap/integration/test_support.dart b/pkg/dds/test/dap/integration/test_support.dart
index 4c804c6..a8cbd0f 100644
--- a/pkg/dds/test/dap/integration/test_support.dart
+++ b/pkg/dds/test/dap/integration/test_support.dart
@@ -88,7 +88,7 @@
vmArgs ??= [];
vmArgs.addAll([
'--enable-vm-service=0',
- '--pause_isolates_on_start=true',
+ '--pause_isolates_on_start',
]);
final processArgs = [
...vmArgs,
