From 51d73d6413e37d2e0e01c72533b4b310b746b6d8 Mon Sep 17 00:00:00 2001 From: Felix Angelov Date: Wed, 19 Mar 2025 12:29:58 -0500 Subject: [PATCH] fix(shorebird_cli): `gradlew` execution on `windows` (#2997) --- .../lib/src/executables/gradlew.dart | 6 ++++++ .../lib/src/shorebird_process.dart | 11 ++++++++--- .../test/src/executables/gradlew_test.dart | 7 +++++++ .../test/src/shorebird_process_test.dart | 17 +++++++++++++++++ 4 files changed, 38 insertions(+), 3 deletions(-) diff --git a/packages/shorebird_cli/lib/src/executables/gradlew.dart b/packages/shorebird_cli/lib/src/executables/gradlew.dart index 5a67645f..0bf7abbd 100644 --- a/packages/shorebird_cli/lib/src/executables/gradlew.dart +++ b/packages/shorebird_cli/lib/src/executables/gradlew.dart @@ -105,6 +105,9 @@ class Gradlew { final result = await process.run( executablePath, args, + // Never run in shell because we always have a fully resolved + // executable path. + runInShell: false, workingDirectory: p.dirname(executablePath), environment: {if (!javaHome.isNullOrEmpty) 'JAVA_HOME': javaHome!}, ); @@ -138,6 +141,9 @@ class Gradlew { return process.stream( executablePath, args, + // Never run in shell because we always have a fully resolved + // executable path. + runInShell: false, workingDirectory: p.dirname(executablePath), environment: {if (!javaHome.isNullOrEmpty) 'JAVA_HOME': javaHome!}, ); diff --git a/packages/shorebird_cli/lib/src/shorebird_process.dart b/packages/shorebird_cli/lib/src/shorebird_process.dart index e79bfbf1..c7d93366 100644 --- a/packages/shorebird_cli/lib/src/shorebird_process.dart +++ b/packages/shorebird_cli/lib/src/shorebird_process.dart @@ -33,12 +33,14 @@ class ShorebirdProcess { String executable, List arguments, { Map? environment, + bool? runInShell, String? workingDirectory, }) async { final process = await start( executable, arguments, environment: environment, + runInShell: runInShell, workingDirectory: workingDirectory, mode: ProcessStartMode.inheritStdio, ); @@ -50,9 +52,9 @@ class ShorebirdProcess { String executable, List arguments, { Map? environment, + bool? runInShell, String? workingDirectory, bool useVendedFlutter = true, - bool? runInShell, }) async { final resolvedEnvironment = _resolveEnvironment( environment, @@ -129,6 +131,7 @@ class ShorebirdProcess { List arguments, { Map? environment, bool useVendedFlutter = true, + bool? runInShell, String? workingDirectory, ProcessStartMode mode = ProcessStartMode.normal, }) { @@ -154,6 +157,7 @@ class ShorebirdProcess { resolvedExecutable, resolvedArguments, environment: resolvedEnvironment, + runInShell: runInShell, workingDirectory: workingDirectory, mode: mode, ); @@ -310,7 +314,6 @@ class ProcessWrapper { executable, arguments, environment: environment, - runInShell: Platform.isWindows, workingDirectory: workingDirectory, ); return ShorebirdProcessResult( @@ -325,13 +328,15 @@ class ProcessWrapper { String executable, List arguments, { Map? environment, + bool? runInShell, String? workingDirectory, ProcessStartMode mode = ProcessStartMode.normal, }) { return Process.start( executable, arguments, - runInShell: Platform.isWindows, + // TODO(felangel): refactor to never runInShell + runInShell: runInShell ?? Platform.isWindows, environment: environment, workingDirectory: workingDirectory, mode: mode, diff --git a/packages/shorebird_cli/test/src/executables/gradlew_test.dart b/packages/shorebird_cli/test/src/executables/gradlew_test.dart index b229671a..fd0d2088 100644 --- a/packages/shorebird_cli/test/src/executables/gradlew_test.dart +++ b/packages/shorebird_cli/test/src/executables/gradlew_test.dart @@ -41,6 +41,7 @@ void main() { () => process.run( any(), any(), + runInShell: any(named: 'runInShell'), workingDirectory: any(named: 'workingDirectory'), environment: any(named: 'environment'), ), @@ -119,6 +120,7 @@ Make sure you have run "flutter build apk" at least once.'''); () => process.run( p.join(tempDir.path, 'android', 'gradlew'), ['app:tasks', '--all', '--console=auto'], + runInShell: false, workingDirectory: p.join(tempDir.path, 'android'), environment: {'JAVA_HOME': javaHome}, ), @@ -147,6 +149,7 @@ Make sure you have run "flutter build apk" at least once.'''); () => process.run( p.join(tempDir.path, 'android', 'gradlew'), ['app:tasks', '--all', '--console=auto'], + runInShell: false, workingDirectory: p.join(tempDir.path, 'android'), environment: {'JAVA_HOME': javaHome}, ), @@ -234,6 +237,7 @@ BUILD FAILED in 3s () => process.run( p.join(tempDir.path, 'android', 'gradlew'), ['app:tasks', '--all', '--console=auto'], + runInShell: false, workingDirectory: p.join(tempDir.path, 'android'), environment: {'JAVA_HOME': javaHome}, ), @@ -552,6 +556,7 @@ No daemons are running. () => process.stream( any(), any(), + runInShell: any(named: 'runInShell'), workingDirectory: any(named: 'workingDirectory'), environment: any(named: 'environment'), ), @@ -568,6 +573,7 @@ No daemons are running. () => process.stream( any(), any(), + runInShell: any(named: 'runInShell'), workingDirectory: any(named: 'workingDirectory'), environment: any(named: 'environment'), ), @@ -580,6 +586,7 @@ No daemons are running. () => process.stream( p.join(projectRoot.path, 'android', 'gradlew'), ['--daemon'], + runInShell: false, workingDirectory: p.join(projectRoot.path, 'android'), environment: {'JAVA_HOME': javaHome}, ), diff --git a/packages/shorebird_cli/test/src/shorebird_process_test.dart b/packages/shorebird_cli/test/src/shorebird_process_test.dart index ac47351d..cb55eeac 100644 --- a/packages/shorebird_cli/test/src/shorebird_process_test.dart +++ b/packages/shorebird_cli/test/src/shorebird_process_test.dart @@ -460,6 +460,7 @@ void main() { () => processWrapper.start( any(), any(), + runInShell: any(named: 'runInShell'), environment: any(named: 'environment'), ), ).thenAnswer((_) async => startProcess); @@ -473,6 +474,22 @@ void main() { ).called(1); }); + test('can override runInShell', () async { + await runWithOverrides( + () => shorebirdProcess.start('git', ['pull'], runInShell: true), + ); + + verify( + () => processWrapper.start( + 'git', + ['pull'], + environment: {}, + workingDirectory: any(named: 'workingDirectory'), + runInShell: true, + ), + ).called(1); + }); + test('sanitizes executable on windows', () { when(() => platform.isWindows).thenReturn(true); const executable =