From 59c67c7192319378f5d6cf067e75522539c091da Mon Sep 17 00:00:00 2001 From: Jake Macdonald Date: Tue, 29 Jul 2025 18:04:12 +0000 Subject: [PATCH 01/11] support and test parens on windows --- pkgs/process/lib/src/interface/common.dart | 4 +- .../test/src/interface/common_test.dart | 55 +++++++++++++++++++ 2 files changed, 58 insertions(+), 1 deletion(-) diff --git a/pkgs/process/lib/src/interface/common.dart b/pkgs/process/lib/src/interface/common.dart index d374631cb..e49c66fc8 100644 --- a/pkgs/process/lib/src/interface/common.dart +++ b/pkgs/process/lib/src/interface/common.dart @@ -28,7 +28,9 @@ String sanitizeExecutablePath(String executable, if (!platform.isWindows) { return executable; } - if (executable.contains(' ') && !executable.contains('"')) { + if ((executable.contains(' ') || + (executable.contains('(') || executable.contains(')'))) && + !executable.contains('"')) { // Use quoted strings to indicate where the file name ends and the arguments begin; // otherwise, the file name is ambiguous. return '"$executable"'; diff --git a/pkgs/process/test/src/interface/common_test.dart b/pkgs/process/test/src/interface/common_test.dart index 8290b5ef4..b382f3ba5 100644 --- a/pkgs/process/test/src/interface/common_test.dart +++ b/pkgs/process/test/src/interface/common_test.dart @@ -260,6 +260,36 @@ void main() { r'C:\"Program Files"\bla.exe'); }); + test('when path has parenthesis', () { + expect( + sanitizeExecutablePath(r'ProgramFiles(x86)\bla.exe', + platform: platform), + r'"ProgramFiles(x86)\bla.exe"'); + expect( + sanitizeExecutablePath(r'"ProgramFiles(x86)\bla.exe"', + platform: platform), + r'"ProgramFiles(x86)\bla.exe"'); + expect( + sanitizeExecutablePath(r'C:\"ProgramFiles(x86)"\bla.exe', + platform: platform), + r'C:\"ProgramFiles(x86)"\bla.exe'); + }); + + test('when path has parenthesis and spaces', () { + expect( + sanitizeExecutablePath(r'Program Files (x86)\bla.exe', + platform: platform), + r'"Program Files (x86)\bla.exe"'); + expect( + sanitizeExecutablePath(r'"Program Files (x86)\bla.exe"', + platform: platform), + r'"Program Files (x86)\bla.exe"'); + expect( + sanitizeExecutablePath(r'C:\"Program Files (x86)"\bla.exe', + platform: platform), + r'C:\"Program Files (x86)"\bla.exe'); + }); + test('with absolute path when currentDirectory getter throws', () { final FileSystem fsNoCwd = MemoryFileSystemNoCwd(fs); final String command = fs.path.join(dir3.path, 'bla.exe'); @@ -571,6 +601,31 @@ void main() { ' ${tmpDir.path}/path4\n' ' ${tmpDir.path}/path5\n')))); }); + + test('can execute files with spaces and parens', () async { + final crazyDir = tmpDir.childDirectory('crazy P()ath'); + final crazyMain = crazyDir.childFile('main.dart') + ..createSync(recursive: true) + ..writeAsStringSync(''' +void main() { + print('hello'); +}'''); + final crazyExe = crazyDir.childFile('main.exe'); + final localProcessManager = LocalProcessManager(); + expect( + localProcessManager.runSync([ + io.Platform.resolvedExecutable, + 'compile', + 'exe', + crazyMain.path, + '-o', + crazyExe.path + ]).exitCode, + 0); + final result = localProcessManager.runSync([crazyExe.path]); + expect(result.exitCode, 0); + expect(result.stdout, 'hello\n'); + }); }); } From c607971ddd3e4b1f3c69329d6eafc6d5c2103933 Mon Sep 17 00:00:00 2001 From: Jake Macdonald Date: Tue, 29 Jul 2025 18:10:11 +0000 Subject: [PATCH 02/11] update pubspec/changelog --- pkgs/process/CHANGELOG.md | 4 ++++ pkgs/process/pubspec.yaml | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/pkgs/process/CHANGELOG.md b/pkgs/process/CHANGELOG.md index bfa1e9664..6eb48fe22 100644 --- a/pkgs/process/CHANGELOG.md +++ b/pkgs/process/CHANGELOG.md @@ -1,3 +1,7 @@ +## 5.0.5 + +* Wrap commands with parenthesis in them with quotes. + ## 5.0.4 * Updates minimum supported SDK version to Flutter 3.22/Dart 3.4. diff --git a/pkgs/process/pubspec.yaml b/pkgs/process/pubspec.yaml index 2a4194cfc..32a977708 100644 --- a/pkgs/process/pubspec.yaml +++ b/pkgs/process/pubspec.yaml @@ -1,6 +1,6 @@ name: process description: A pluggable, mockable process invocation abstraction for Dart. -version: 5.0.4 +version: 5.0.5 repository: https://github.com/dart-lang/tools/tree/main/pkgs/process issue_tracker: https://github.com/dart-lang/tools/issues?q=is%3Aissue+is%3Aopen+label%3Apackage%3Aprocess From 8fa83c5649a1404a0dae5d6e35f75f30312eb333 Mon Sep 17 00:00:00 2001 From: Jake Macdonald Date: Tue, 29 Jul 2025 18:22:31 +0000 Subject: [PATCH 03/11] run with runInShell true and false, and run package:process tests on windows --- .github/workflows/process.yaml | 4 +-- .../test/src/interface/common_test.dart | 29 ++++++++++--------- 2 files changed, 18 insertions(+), 15 deletions(-) diff --git a/.github/workflows/process.yaml b/.github/workflows/process.yaml index 98044335b..c0c8f3f88 100644 --- a/.github/workflows/process.yaml +++ b/.github/workflows/process.yaml @@ -22,10 +22,10 @@ defaults: jobs: build: - runs-on: ubuntu-latest + runs-on: ${{ matrix.os }} strategy: matrix: - os: [ubuntu-latest] + os: [ubuntu-latest, windows] sdk: [stable, dev] steps: - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 diff --git a/pkgs/process/test/src/interface/common_test.dart b/pkgs/process/test/src/interface/common_test.dart index b382f3ba5..d95c6302f 100644 --- a/pkgs/process/test/src/interface/common_test.dart +++ b/pkgs/process/test/src/interface/common_test.dart @@ -612,19 +612,22 @@ void main() { }'''); final crazyExe = crazyDir.childFile('main.exe'); final localProcessManager = LocalProcessManager(); - expect( - localProcessManager.runSync([ - io.Platform.resolvedExecutable, - 'compile', - 'exe', - crazyMain.path, - '-o', - crazyExe.path - ]).exitCode, - 0); - final result = localProcessManager.runSync([crazyExe.path]); - expect(result.exitCode, 0); - expect(result.stdout, 'hello\n'); + for (final runInShell in const [true, false]) { + expect( + localProcessManager.runSync([ + io.Platform.resolvedExecutable, + 'compile', + 'exe', + crazyMain.path, + '-o', + crazyExe.path + ]).exitCode, + 0); + final result = localProcessManager + .runSync([crazyExe.path], runInShell: runInShell); + expect(result.exitCode, 0); + expect(result.stdout, 'hello\n'); + } }); }); } From 3bd53a0b2f380c6b6840c4d666bc5d5edfba6914 Mon Sep 17 00:00:00 2001 From: Jake Macdonald Date: Tue, 29 Jul 2025 18:26:30 +0000 Subject: [PATCH 04/11] only compile the app once, log stdout/stderr for more info --- .../test/src/interface/common_test.dart | 27 ++++++++++--------- 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/pkgs/process/test/src/interface/common_test.dart b/pkgs/process/test/src/interface/common_test.dart index d95c6302f..28f4d919b 100644 --- a/pkgs/process/test/src/interface/common_test.dart +++ b/pkgs/process/test/src/interface/common_test.dart @@ -612,21 +612,24 @@ void main() { }'''); final crazyExe = crazyDir.childFile('main.exe'); final localProcessManager = LocalProcessManager(); + // Create an executable we can actually run. + expect( + localProcessManager.runSync([ + io.Platform.resolvedExecutable, + 'compile', + 'exe', + crazyMain.path, + '-o', + crazyExe.path + ]).exitCode, + 0); + for (final runInShell in const [true, false]) { - expect( - localProcessManager.runSync([ - io.Platform.resolvedExecutable, - 'compile', - 'exe', - crazyMain.path, - '-o', - crazyExe.path - ]).exitCode, - 0); final result = localProcessManager .runSync([crazyExe.path], runInShell: runInShell); - expect(result.exitCode, 0); - expect(result.stdout, 'hello\n'); + expect(result.exitCode, 0, + reason: 'stdout: ${result.stdout}\nstderr: ${result.stderr}'); + expect(result.stdout, contains('hello')); } }); }); From 50c0b7b6dfeb655f77f16cb68d96947d15f4e26e Mon Sep 17 00:00:00 2001 From: Jake Macdonald Date: Tue, 29 Jul 2025 19:24:32 +0000 Subject: [PATCH 05/11] remove sanitizeExecutable entirely, its not necessary and actually breaks things --- pkgs/process/lib/src/interface/common.dart | 20 --- .../src/interface/local_process_manager.dart | 12 +- .../test/src/interface/common_test.dart | 139 +++++++----------- 3 files changed, 60 insertions(+), 111 deletions(-) diff --git a/pkgs/process/lib/src/interface/common.dart b/pkgs/process/lib/src/interface/common.dart index e49c66fc8..65fbd1777 100644 --- a/pkgs/process/lib/src/interface/common.dart +++ b/pkgs/process/lib/src/interface/common.dart @@ -18,26 +18,6 @@ const Map _osToPathStyle = { 'windows': 'windows', }; -/// Sanitizes the executable path on Windows. -/// https://github.com/dart-lang/sdk/issues/37751 -String sanitizeExecutablePath(String executable, - {Platform platform = const LocalPlatform()}) { - if (executable.isEmpty) { - return executable; - } - if (!platform.isWindows) { - return executable; - } - if ((executable.contains(' ') || - (executable.contains('(') || executable.contains(')'))) && - !executable.contains('"')) { - // Use quoted strings to indicate where the file name ends and the arguments begin; - // otherwise, the file name is ambiguous. - return '"$executable"'; - } - return executable; -} - /// Searches the `PATH` for the executable that [executable] is supposed to launch. /// /// This first builds a list of candidate paths where the executable may reside. diff --git a/pkgs/process/lib/src/interface/local_process_manager.dart b/pkgs/process/lib/src/interface/local_process_manager.dart index ab7e96a31..81e3eca98 100644 --- a/pkgs/process/lib/src/interface/local_process_manager.dart +++ b/pkgs/process/lib/src/interface/local_process_manager.dart @@ -40,11 +40,11 @@ class LocalProcessManager implements ProcessManager { }) { try { return Process.start( - sanitizeExecutablePath(_getExecutable( + _getExecutable( command, workingDirectory, runInShell, - )), + ), _getArguments(command), workingDirectory: workingDirectory, environment: environment, @@ -70,11 +70,11 @@ class LocalProcessManager implements ProcessManager { }) { try { return Process.run( - sanitizeExecutablePath(_getExecutable( + _getExecutable( command, workingDirectory, runInShell, - )), + ), _getArguments(command), workingDirectory: workingDirectory, environment: environment, @@ -101,11 +101,11 @@ class LocalProcessManager implements ProcessManager { }) { try { return Process.runSync( - sanitizeExecutablePath(_getExecutable( + _getExecutable( command, workingDirectory, runInShell, - )), + ), _getArguments(command), workingDirectory: workingDirectory, environment: environment, diff --git a/pkgs/process/test/src/interface/common_test.dart b/pkgs/process/test/src/interface/common_test.dart index 28f4d919b..926c3cb5f 100644 --- a/pkgs/process/test/src/interface/common_test.dart +++ b/pkgs/process/test/src/interface/common_test.dart @@ -238,58 +238,6 @@ void main() { ' C:\\.tmp_rand0\\dir2_rand0\n')))); }); - test('when path has spaces', () { - expect( - sanitizeExecutablePath(r'Program Files\bla.exe', - platform: platform), - r'"Program Files\bla.exe"'); - expect( - sanitizeExecutablePath(r'ProgramFiles\bla.exe', platform: platform), - r'ProgramFiles\bla.exe'); - expect( - sanitizeExecutablePath(r'"Program Files\bla.exe"', - platform: platform), - r'"Program Files\bla.exe"'); - expect( - sanitizeExecutablePath(r'"Program Files\bla.exe"', - platform: platform), - r'"Program Files\bla.exe"'); - expect( - sanitizeExecutablePath(r'C:\"Program Files"\bla.exe', - platform: platform), - r'C:\"Program Files"\bla.exe'); - }); - - test('when path has parenthesis', () { - expect( - sanitizeExecutablePath(r'ProgramFiles(x86)\bla.exe', - platform: platform), - r'"ProgramFiles(x86)\bla.exe"'); - expect( - sanitizeExecutablePath(r'"ProgramFiles(x86)\bla.exe"', - platform: platform), - r'"ProgramFiles(x86)\bla.exe"'); - expect( - sanitizeExecutablePath(r'C:\"ProgramFiles(x86)"\bla.exe', - platform: platform), - r'C:\"ProgramFiles(x86)"\bla.exe'); - }); - - test('when path has parenthesis and spaces', () { - expect( - sanitizeExecutablePath(r'Program Files (x86)\bla.exe', - platform: platform), - r'"Program Files (x86)\bla.exe"'); - expect( - sanitizeExecutablePath(r'"Program Files (x86)\bla.exe"', - platform: platform), - r'"Program Files (x86)\bla.exe"'); - expect( - sanitizeExecutablePath(r'C:\"Program Files (x86)"\bla.exe', - platform: platform), - r'C:\"Program Files (x86)"\bla.exe'); - }); - test('with absolute path when currentDirectory getter throws', () { final FileSystem fsNoCwd = MemoryFileSystemNoCwd(fs); final String command = fs.path.join(dir3.path, 'bla.exe'); @@ -408,13 +356,6 @@ void main() { ' /.tmp_rand0/dir1_rand0\n' ' /.tmp_rand0/dir2_rand0\n')))); }); - - test('when path has spaces', () { - expect( - sanitizeExecutablePath('/usr/local/bin/foo bar', - platform: platform), - '/usr/local/bin/foo bar'); - }); }); }); group('Real Filesystem', () { @@ -602,35 +543,63 @@ void main() { ' ${tmpDir.path}/path5\n')))); }); - test('can execute files with spaces and parens', () async { - final crazyDir = tmpDir.childDirectory('crazy P()ath'); - final crazyMain = crazyDir.childFile('main.dart') - ..createSync(recursive: true) - ..writeAsStringSync(''' + group('can actually execute files', () { + void testCompileAndExecute(File mainFile) { + final localProcessManager = LocalProcessManager(); + final exePath = '${mainFile.path}.exe'; + // Create an executable we can actually run. + expect( + localProcessManager.runSync([ + io.Platform.resolvedExecutable, + 'compile', + 'exe', + mainFile.path, + '-o', + exePath + ]).exitCode, + 0); + + for (final runInShell in const [true, false]) { + final result = + localProcessManager.runSync([exePath], runInShell: runInShell); + expect(result.exitCode, 0, + reason: 'stdout: ${result.stdout}\nstderr: ${result.stderr}'); + expect(result.stdout, contains('hello')); + } + } + + test('with spaces in the command name', () { + final dir = tmpDir.childDirectory('the path'); + final main = dir.childFile('main.dart') + ..createSync(recursive: true) + ..writeAsStringSync(''' void main() { print('hello'); }'''); - final crazyExe = crazyDir.childFile('main.exe'); - final localProcessManager = LocalProcessManager(); - // Create an executable we can actually run. - expect( - localProcessManager.runSync([ - io.Platform.resolvedExecutable, - 'compile', - 'exe', - crazyMain.path, - '-o', - crazyExe.path - ]).exitCode, - 0); - - for (final runInShell in const [true, false]) { - final result = localProcessManager - .runSync([crazyExe.path], runInShell: runInShell); - expect(result.exitCode, 0, - reason: 'stdout: ${result.stdout}\nstderr: ${result.stderr}'); - expect(result.stdout, contains('hello')); - } + testCompileAndExecute(main); + }); + + test('with parenthesis in the command name', () async { + final dir = tmpDir.childDirectory('theP()ath'); + final main = dir.childFile('main.dart') + ..createSync(recursive: true) + ..writeAsStringSync(''' +void main() { + print('hello'); +}'''); + testCompileAndExecute(main); + }); + + test('with spaces and parenthesis in the command name', () async { + final dir = tmpDir.childDirectory('crazy P()ath'); + final main = dir.childFile('main.dart') + ..createSync(recursive: true) + ..writeAsStringSync(''' +void main() { + print('hello'); +}'''); + testCompileAndExecute(main); + }); }); }); } From 3bd1ed18e49af562783bd76fc3d8e5bbc3aa733c Mon Sep 17 00:00:00 2001 From: Jake Macdonald Date: Tue, 29 Jul 2025 19:26:39 +0000 Subject: [PATCH 06/11] add a test for a space inside parenthesis as well --- pkgs/process/test/src/interface/common_test.dart | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/pkgs/process/test/src/interface/common_test.dart b/pkgs/process/test/src/interface/common_test.dart index 926c3cb5f..960d0f48c 100644 --- a/pkgs/process/test/src/interface/common_test.dart +++ b/pkgs/process/test/src/interface/common_test.dart @@ -591,7 +591,18 @@ void main() { }); test('with spaces and parenthesis in the command name', () async { - final dir = tmpDir.childDirectory('crazy P()ath'); + final dir = tmpDir.childDirectory('the P()ath'); + final main = dir.childFile('main.dart') + ..createSync(recursive: true) + ..writeAsStringSync(''' +void main() { + print('hello'); +}'''); + testCompileAndExecute(main); + }); + + test('with spaces inside parenthesis in the command name', () async { + final dir = tmpDir.childDirectory('the P( )ath'); final main = dir.childFile('main.dart') ..createSync(recursive: true) ..writeAsStringSync(''' From 0812c0ddae407189b674099b0a3996c6e1c3018e Mon Sep 17 00:00:00 2001 From: Jake Macdonald Date: Tue, 29 Jul 2025 19:40:32 +0000 Subject: [PATCH 07/11] log the runInShell variable also on failure --- pkgs/process/test/src/interface/common_test.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkgs/process/test/src/interface/common_test.dart b/pkgs/process/test/src/interface/common_test.dart index 960d0f48c..db6ecdd7a 100644 --- a/pkgs/process/test/src/interface/common_test.dart +++ b/pkgs/process/test/src/interface/common_test.dart @@ -563,7 +563,7 @@ void main() { final result = localProcessManager.runSync([exePath], runInShell: runInShell); expect(result.exitCode, 0, - reason: 'stdout: ${result.stdout}\nstderr: ${result.stderr}'); + reason: 'runInShell: $runInShell\nstdout: ${result.stdout}\nstderr: ${result.stderr}'); expect(result.stdout, contains('hello')); } } From 38a326ab3dea23b7eddd9df4a3657db37bb53004 Mon Sep 17 00:00:00 2001 From: Jake Macdonald Date: Tue, 29 Jul 2025 19:42:25 +0000 Subject: [PATCH 08/11] fix long line --- pkgs/process/test/src/interface/common_test.dart | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkgs/process/test/src/interface/common_test.dart b/pkgs/process/test/src/interface/common_test.dart index db6ecdd7a..67d406bf5 100644 --- a/pkgs/process/test/src/interface/common_test.dart +++ b/pkgs/process/test/src/interface/common_test.dart @@ -563,7 +563,8 @@ void main() { final result = localProcessManager.runSync([exePath], runInShell: runInShell); expect(result.exitCode, 0, - reason: 'runInShell: $runInShell\nstdout: ${result.stdout}\nstderr: ${result.stderr}'); + reason: 'runInShell: $runInShell\nstdout: ${result.stdout}\n' + 'stderr: ${result.stderr}'); expect(result.stdout, contains('hello')); } } From 45bbebbed2358652c7ecf83ff6ade49355e54d21 Mon Sep 17 00:00:00 2001 From: Jake Macdonald Date: Tue, 29 Jul 2025 19:48:25 +0000 Subject: [PATCH 09/11] udpate changelog --- pkgs/process/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkgs/process/CHANGELOG.md b/pkgs/process/CHANGELOG.md index 6eb48fe22..311d021de 100644 --- a/pkgs/process/CHANGELOG.md +++ b/pkgs/process/CHANGELOG.md @@ -1,6 +1,6 @@ ## 5.0.5 -* Wrap commands with parenthesis in them with quotes. +* Fix mixtures of parentheses and spaces in windows command paths. ## 5.0.4 From 35dc4f41d15b3f5ca1706a1d5e2065665718efbe Mon Sep 17 00:00:00 2001 From: Jake Macdonald Date: Wed, 30 Jul 2025 14:29:46 +0000 Subject: [PATCH 10/11] use windows-latest as the os name --- .github/workflows/process.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/process.yaml b/.github/workflows/process.yaml index c0c8f3f88..4d05828b4 100644 --- a/.github/workflows/process.yaml +++ b/.github/workflows/process.yaml @@ -25,7 +25,7 @@ jobs: runs-on: ${{ matrix.os }} strategy: matrix: - os: [ubuntu-latest, windows] + os: [ubuntu-latest, windows-latest] sdk: [stable, dev] steps: - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 From 3d873465b72b72ed7d39a027ee241aaad2784db1 Mon Sep 17 00:00:00 2001 From: Jake Macdonald Date: Wed, 30 Jul 2025 14:35:08 +0000 Subject: [PATCH 11/11] skip failing test --- pkgs/process/test/src/interface/common_test.dart | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/pkgs/process/test/src/interface/common_test.dart b/pkgs/process/test/src/interface/common_test.dart index 67d406bf5..58265895f 100644 --- a/pkgs/process/test/src/interface/common_test.dart +++ b/pkgs/process/test/src/interface/common_test.dart @@ -589,7 +589,10 @@ void main() { print('hello'); }'''); testCompileAndExecute(main); - }); + }, + skip: io.Platform.isWindows + ? 'https://github.com/dart-lang/tools/issues/2139' + : null); test('with spaces and parenthesis in the command name', () async { final dir = tmpDir.childDirectory('the P()ath');