[DAP] Normalize Directory.current in DAP before launching the process
We normalize paths to have uppercase drive letters in many places in DAP to avoid mismatches of things like breakpoint paths if the client sends mixed casing.
It's not clear why the change to not host DDS internally affected this, but it seems sensible to perform this normalization here because we're normalizing everywhere else and right now the paths are treated as case-sensitive in many places.
Fixes https://github.com/dart-lang/sdk/issues/56965
Fixes https://github.com/dart-lang/sdk/issues/57011
Change-Id: Ie63ef66058ff9d37955659a780f7bd40cdfc2dc6
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/392442
Reviewed-by: Ben Konyi <bkonyi@google.com>
Commit-Queue: Ben Konyi <bkonyi@google.com>
Reviewed-by: Derek Xu <derekx@google.com>
diff --git a/pkg/dds/lib/src/dap/adapters/dart.dart b/pkg/dds/lib/src/dap/adapters/dart.dart
index 8c6b66a..69f5ebb 100644
--- a/pkg/dds/lib/src/dap/adapters/dart.dart
+++ b/pkg/dds/lib/src/dap/adapters/dart.dart
@@ -394,7 +394,7 @@
if (args is DartLaunchRequestArguments)
path.dirname((args as DartLaunchRequestArguments).program),
...?args.additionalProjectPaths,
- ].nonNulls.toList();
+ ].nonNulls.map(normalizePath).toList();
/// Whether we have already sent the [TerminatedEvent] to the client.
///
@@ -2682,6 +2682,10 @@
Directory.current = Directory(cwd);
}
+ // Also ensure the cwd always has uppercase drive letters to match what
+ // we'll normalize to everywhere else.
+ Directory.current = Directory(normalizePath(Directory.current.path));
+
// Notify IsolateManager if we'll be debugging so it knows whether to set
// up breakpoints etc. when isolates are registered.
final debug = !(noDebug ?? false);
diff --git a/pkg/dds/test/dap/integration/dart_test_test.dart b/pkg/dds/test/dap/integration/dart_test_test.dart
index c967cf2..4011824 100644
--- a/pkg/dds/test/dap/integration/dart_test_test.dart
+++ b/pkg/dds/test/dap/integration/dart_test_test.dart
@@ -115,7 +115,7 @@
.nonNulls
.toList();
// Ensure we had a frame with the absolute path of the test script.
- expect(stackFramePaths, contains(testFile.path));
+ expect(stackFramePaths, contains(uppercaseDriveLetter(testFile.path)));
});
test('can hit and resume from a breakpoint', () async {
diff --git a/pkg/dds/test/dap/integration/debug_breakpoints_test.dart b/pkg/dds/test/dap/integration/debug_breakpoints_test.dart
index f5e578c..0d5a126 100644
--- a/pkg/dds/test/dap/integration/debug_breakpoints_test.dart
+++ b/pkg/dds/test/dap/integration/debug_breakpoints_test.dart
@@ -207,26 +207,6 @@
await testHitBreakpointAndResume();
});
- test(
- 'stops at a line breakpoint and can be resumed '
- 'when breakpoint requests have lowercase drive letters '
- 'and program/VM have uppercase drive letters', () async {
- final client = dap.client;
- client.forceDriveLetterCasingUpper = true;
- client.forceBreakpointDriveLetterCasingLower = true;
- await testHitBreakpointAndResume();
- }, skip: !Platform.isWindows);
-
- test(
- 'stops at a line breakpoint and can be resumed '
- 'when breakpoint requests have uppercase drive letters '
- 'and program/VM have lowercase drive letters', () async {
- final client = dap.client;
- client.forceDriveLetterCasingLower = true;
- client.forceBreakpointDriveLetterCasingUpper = true;
- await testHitBreakpointAndResume();
- }, skip: !Platform.isWindows);
-
test('stops at a line breakpoint and can step over (next)', () async {
final testFile = dap.createTestFile('''
void main(List<String> args) async {
diff --git a/pkg/dds/test/dap/integration/debug_exceptions_test.dart b/pkg/dds/test/dap/integration/debug_exceptions_test.dart
index 7e3f728..3d94abf 100644
--- a/pkg/dds/test/dap/integration/debug_exceptions_test.dart
+++ b/pkg/dds/test/dap/integration/debug_exceptions_test.dart
@@ -104,7 +104,7 @@
// expect.
expect(
mainStackFrameEvent.source?.path,
- dap.client.uppercaseDriveLetter(testFile.path),
+ uppercaseDriveLetter(testFile.path),
);
expect(mainStackFrameEvent.line, exceptionLine);
expect(mainStackFrameEvent.column, 5);
diff --git a/pkg/dds/test/dap/integration/debug_test.dart b/pkg/dds/test/dap/integration/debug_test.dart
index ebb3805..8ec0fd1 100644
--- a/pkg/dds/test/dap/integration/debug_test.dart
+++ b/pkg/dds/test/dap/integration/debug_test.dart
@@ -112,7 +112,7 @@
runInTerminalArgs!.args,
containsAllInOrder([
Platform.resolvedExecutable,
- dap.client.uppercaseDriveLetter(testFile.path),
+ uppercaseDriveLetter(testFile.path),
]),
);
expect(proc!.pid, isPositive);
@@ -520,9 +520,8 @@
({String name, Uri fileLikeUri}) getExpectedMacroSource(File testFile) {
// Drive letters are always normalized to uppercase so expect
// uppercase in the path part of the macro URI.
- final fileLikeUri =
- Uri.file(dap.client.uppercaseDriveLetter(testFile.path))
- .replace(scheme: 'dart-macro+file');
+ final fileLikeUri = Uri.file(uppercaseDriveLetter(testFile.path))
+ .replace(scheme: 'dart-macro+file');
// The expected source name will differ for inside/outside the lib
// folder.
final name = folderName == 'lib'
diff --git a/pkg/dds/test/dap/integration/no_debug_test.dart b/pkg/dds/test/dap/integration/no_debug_test.dart
index 7641222..807e9ba 100644
--- a/pkg/dds/test/dap/integration/no_debug_test.dart
+++ b/pkg/dds/test/dap/integration/no_debug_test.dart
@@ -72,7 +72,7 @@
runInTerminalArgs!.args,
containsAllInOrder([
Platform.resolvedExecutable,
- dap.client.uppercaseDriveLetter(testFile.path),
+ uppercaseDriveLetter(testFile.path),
]),
);
});
diff --git a/pkg/dds/test/dap/integration/test_client.dart b/pkg/dds/test/dap/integration/test_client.dart
index 59a540b..6b6234b 100644
--- a/pkg/dds/test/dap/integration/test_client.dart
+++ b/pkg/dds/test/dap/integration/test_client.dart
@@ -16,6 +16,7 @@
import 'package:vm_service/vm_service.dart' as vm;
import 'test_server.dart';
+import 'test_support.dart';
/// A helper class to simplify acting as a client for interacting with the
/// [DapTestServer] in tests.
@@ -51,19 +52,17 @@
late final Future<Uri?> vmServiceUri;
- /// Used to control drive letter casing on Windows for testing.
- bool? forceDriveLetterCasingUpper;
-
- /// Used to control drive letter casing on Windows for testing.
- bool? forceDriveLetterCasingLower;
-
- /// Used to control drive letter casing for breakpoint requests on Windows for
+ /// Used to control drive letter casing for the 'program' value on Windows for
/// testing.
- bool? forceBreakpointDriveLetterCasingUpper;
+ DriveLetterCasing? forceProgramDriveLetterCasing;
- /// Used to control drive letter casing for breakpoint requests on Windows for
+ /// Used to control drive letter casing for the 'cwd' value on Windows for
/// testing.
- bool? forceBreakpointDriveLetterCasingLower;
+ DriveLetterCasing? forceCwdDriveLetterCasing;
+
+ /// Used to control drive letter casing for breakpoint paths on Windows for
+ /// testing.
+ DriveLetterCasing? forceBreakpointDriveLetterCasing;
/// All stderr OutputEvents that have occurred so far.
final StringBuffer _stderr = StringBuffer();
@@ -174,8 +173,11 @@
DartAttachRequestArguments(
vmServiceUri: vmServiceUri,
vmServiceInfoFile: vmServiceInfoFile,
- cwd: cwd != null ? _normalizePath(cwd) : null,
- additionalProjectPaths: additionalProjectPaths,
+ cwd: cwd != null
+ ? setDriveLetterCasing(cwd, forceCwdDriveLetterCasing)
+ : null,
+ additionalProjectPaths:
+ additionalProjectPaths?.map(uppercaseDriveLetter).toList(),
debugSdkLibraries: debugSdkLibraries,
debugExternalPackageLibraries: debugExternalPackageLibraries,
showGettersInDebugViews: showGettersInDebugViews,
@@ -339,13 +341,15 @@
return sendRequest(
DartLaunchRequestArguments(
noDebug: noDebug,
- program: _normalizePath(program),
- cwd: cwd != null ? _normalizePath(cwd) : null,
+ program: setDriveLetterCasing(program, forceProgramDriveLetterCasing),
+ cwd: cwd != null
+ ? setDriveLetterCasing(cwd, forceCwdDriveLetterCasing)
+ : null,
args: args,
vmAdditionalArgs: vmAdditionalArgs,
toolArgs: toolArgs,
additionalProjectPaths:
- additionalProjectPaths?.map(_normalizePath).toList(),
+ additionalProjectPaths?.map(uppercaseDriveLetter).toList(),
console: console,
debugSdkLibraries: debugSdkLibraries,
debugExternalPackageLibraries: debugExternalPackageLibraries,
@@ -777,50 +781,11 @@
);
}
- /// Normalizes a non-breakpoint path being sent to the debug adapter based on
- /// the values of [forceDriveLetterCasingUpper] and
- /// [forceDriveLetterCasingLower].
- String _normalizePath(String path) {
- return _forceDriveLetterCasing(
- path,
- upper: forceDriveLetterCasingUpper,
- lower: forceDriveLetterCasingLower,
- );
- }
-
- /// Normalizes a a path to have an uppercase drive letter. All paths verified
- /// that come out of the adapter are normalized this way so test expectations
- /// should be normalized the same way before comparing.
- String uppercaseDriveLetter(String path) {
- return _forceDriveLetterCasing(
- path,
- upper: true,
- );
- }
-
/// Normalizes a breakpoint path being sent to the debug adapter based on
/// the values of [forceBreakpointDriveLetterCasingUpper] and
/// [forceBreakpointDriveLetterCasingLower].
String _normalizeBreakpointPath(String path) {
- return _forceDriveLetterCasing(
- path,
- upper: forceBreakpointDriveLetterCasingUpper,
- lower: forceBreakpointDriveLetterCasingLower,
- );
- }
-
- String _forceDriveLetterCasing(String path, {bool? upper, bool? lower}) {
- assert(upper != true || lower != true);
- if (!Platform.isWindows || path.isEmpty) {
- return path;
- }
- if (upper ?? false) {
- return path.substring(0, 1).toUpperCase() + path.substring(1);
- } else if (lower ?? false) {
- return path.substring(0, 1).toLowerCase() + path.substring(1);
- } else {
- return path;
- }
+ return setDriveLetterCasing(path, forceBreakpointDriveLetterCasing);
}
/// Sets the exception pause mode to [pauseMode] and expects to pause after
diff --git a/pkg/dds/test/dap/integration/test_support.dart b/pkg/dds/test/dap/integration/test_support.dart
index 4b34242..27391d6 100644
--- a/pkg/dds/test/dap/integration/test_support.dart
+++ b/pkg/dds/test/dap/integration/test_support.dart
@@ -365,3 +365,28 @@
});
}
}
+
+/// Sets the casing of a drive letter according to [casing].
+String setDriveLetterCasing(String path, DriveLetterCasing? casing) {
+ if (!Platform.isWindows || path.isEmpty) {
+ return path;
+ }
+ var (driveLetter, rest) = (path.substring(0, 1), path.substring(1));
+ return switch (casing) {
+ DriveLetterCasing.uppercase => driveLetter.toUpperCase() + rest,
+ DriveLetterCasing.lowercase => driveLetter.toLowerCase() + rest,
+ null => path,
+ };
+}
+
+/// Normalizes a a path to have an uppercase drive letter. All paths verified
+/// that come out of the adapter are normalized this way so test expectations
+/// should be normalized the same way before comparing.
+String uppercaseDriveLetter(String path) {
+ return setDriveLetterCasing(path, DriveLetterCasing.uppercase);
+}
+
+enum DriveLetterCasing {
+ uppercase,
+ lowercase,
+}
diff --git a/pkg/dds/test/dap/integration/windows_path_casing_test.dart b/pkg/dds/test/dap/integration/windows_path_casing_test.dart
new file mode 100644
index 0000000..a84aaeb
--- /dev/null
+++ b/pkg/dds/test/dap/integration/windows_path_casing_test.dart
@@ -0,0 +1,66 @@
+// Copyright (c) 2024, 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:io';
+
+import 'package:test/test.dart';
+
+import 'test_client.dart';
+import 'test_scripts.dart';
+import 'test_support.dart';
+
+main() {
+ group('windows path casing', () {
+ DapTestSession? dap;
+ tearDown(() => dap?.tearDown());
+
+ for (final actualCwdCasing in [null, ...DriveLetterCasing.values]) {
+ for (final programCasing in DriveLetterCasing.values) {
+ for (final cwdCasing in [null, ...DriveLetterCasing.values]) {
+ for (final breakpointCasing in DriveLetterCasing.values) {
+ test(
+ 'stops at a breakpoint and can resume (casing: '
+ 'actualCwd: $actualCwdCasing, '
+ 'program: $programCasing, '
+ 'cwd: $cwdCasing, '
+ 'breakpoint requests: $breakpointCasing)', () async {
+ // Set the correct casing of drive letters for this test.
+ if (actualCwdCasing != null) {
+ Directory.current = Directory(
+ setDriveLetterCasing(Directory.current.path, actualCwdCasing),
+ );
+ }
+
+ dap = await DapTestSession.setUp();
+ final client = dap!.client;
+
+ client.forceProgramDriveLetterCasing = programCasing;
+ client.forceCwdDriveLetterCasing = cwdCasing;
+ client.forceBreakpointDriveLetterCasing = breakpointCasing;
+
+ final testFile = dap!.createTestFile(simpleBreakpointProgram);
+ final breakpointLine = lineWith(testFile, breakpointMarker);
+
+ // Hit the initial breakpoint.
+ final stop = await client.hitBreakpoint(
+ testFile,
+ breakpointLine,
+ cwd: cwdCasing == null ? null : dap!.testAppDir.path,
+ );
+
+ // Resume and expect termination (as the script will get to the end).
+ await Future.wait([
+ client.event('terminated'),
+ client.continue_(stop.threadId!),
+ ], eagerError: true);
+ });
+ }
+ }
+ }
+ }
+
+ // These tests can be slow due to starting up the external server process.
+ }, timeout: Timeout.none, skip: !Platform.isWindows);
+}
