fix: run commands in single shell (#369)

This change removes `runInShell: true` from `Process.start` when
executing commands. This option is unnecessary since Melos is spawning
a shell itself to execute the command. Using `runInShell: true`
becomes a problem for command chains (e.g. '&&' or '||').
In this case only the first command is executed in the shell that Melos
is spawning and the rest of the commands are executed in the shell
spawned by `Process.start`.

The change also cleans up a few other related bits of code.

Fixes #367

packages/melos/lib/src/commands/bootstrap.dart

@@ -173,37 +173,24 @@ mixin _BootstrapMixin on _CleanMixin {
     Package package, {
     bool inTemporaryProject = false,
   }) async {
-    final execArgs = [
+    final command = [
       ...pubCommandExecArgs(
         useFlutter: package.isFlutterPackage,
         workspace: workspace,
       ),
       'get',
       if (workspace.config.commands.bootstrap.runPubGetOffline) '--offline'
-    ];
+    ].join(' ');
 
-    final executable = currentPlatform.isWindows ? 'cmd' : '/bin/sh';
     final packagePath = inTemporaryProject
         ? join(workspace.melosToolPath, package.pathRelativeToWorkspace)
         : package.path;
-    final process = await Process.start(
-      executable,
-      currentPlatform.isWindows ? ['/C', '%MELOS_SCRIPT%'] : [],
+
+    final process = await startCommandRaw(
+      command,
       workingDirectory: packagePath,
-      environment: {
-        utils.envKeyMelosTerminalWidth: utils.terminalWidth.toString(),
-        'MELOS_SCRIPT': execArgs.join(' '),
-      },
-      runInShell: true,
     );
 
-    if (!currentPlatform.isWindows) {
-      // Pipe in the arguments to trigger the script to run.
-      process.stdin.writeln(execArgs.join(' '));
-      // Exit the process with the same exit code as the previous command.
-      process.stdin.writeln(r'exit $?');
-    }
-
     const logTimeout = Duration(seconds: 10);
     final packagePrefix = '[${AnsiStyles.blue.bold(package.name)}]: ';
     void Function(String) logLineTo(void Function(String) log) =>

packages/melos/lib/src/commands/exec.dart

@@ -74,7 +74,7 @@ mixin _ExecMixin on _Melos {
       environment.remove(envKeyMelosTerminalWidth);
     }
 
-    return startProcess(
+    return startCommand(
       execArgs,
       logger: logger,
       environment: environment,

packages/melos/lib/src/commands/run.dart

@@ -158,7 +158,7 @@ mixin _RunMixin on _Melos {
         .child(runningLabel)
         .newLine();
 
-    return startProcess(
+    return startCommand(
       scriptParts..addAll(extraArgs),
       logger: logger,
       environment: environment,

packages/melos/lib/src/common/utils.dart

@@ -277,59 +277,76 @@ bool isWorkspaceDirectory(String directory) =>
 bool isPackageDirectory(String directory) =>
     fileExists(pubspecPathForDirectory(directory));
 
-Future<int> startProcess(
-  List<String> execArgs, {
-  String? prefix,
-  Map<String, String> environment = const {},
+Future<Process> startCommandRaw(
+  String command, {
   String? workingDirectory,
-  bool onlyOutputOnError = false,
+  Map<String, String> environment = const {},
   bool includeParentEnvironment = true,
-  required MelosLogger logger,
-}) async {
-  final workingDirectoryPath = workingDirectory ?? Directory.current.path;
-  final executable = currentPlatform.isWindows ? 'cmd' : '/bin/sh';
-  final filteredArgs = execArgs.map((arg) {
-    // Remove empty args.
-    if (arg.trim().isEmpty) {
-      return null;
-    }
-
-    // Attempt to make line continuations Windows & Linux compatible.
-    if (arg.trim() == r'\') {
-      return currentPlatform.isWindows ? arg.replaceAll(r'\', '^') : arg;
-    }
-    if (arg.trim() == '^') {
-      return currentPlatform.isWindows ? arg : arg.replaceAll('^', r'\');
-    }
-
-    // Inject MELOS_* variables if any.
-    environment.forEach((key, value) {
-      if (key.startsWith('MELOS_')) {
-        arg = arg.replaceAll('\$$key', value);
-        arg = arg.replaceAll(key, value);
-      }
-    });
-
-    return arg;
-  }).where((element) => element != null);
+}) {
+  final executable = currentPlatform.isWindows ? 'cmd.exe' : '/bin/sh';
+  workingDirectory ??= Directory.current.path;
 
-  final execProcess = await Process.start(
+  return Process.start(
     executable,
     currentPlatform.isWindows
         ? ['/C', '%MELOS_SCRIPT%']
         : ['-c', r'eval "$MELOS_SCRIPT"'],
-    workingDirectory: workingDirectoryPath,
+    workingDirectory: workingDirectory,
     environment: {
       ...environment,
       envKeyMelosTerminalWidth: terminalWidth.toString(),
-      'MELOS_SCRIPT': filteredArgs.join(' '),
+      'MELOS_SCRIPT': command,
     },
     includeParentEnvironment: includeParentEnvironment,
-    runInShell: true,
+  );
+}
+
+Future<int> startCommand(
+  List<String> command, {
+  String? prefix,
+  Map<String, String> environment = const {},
+  String? workingDirectory,
+  bool onlyOutputOnError = false,
+  bool includeParentEnvironment = true,
+  required MelosLogger logger,
+}) async {
+  final processedCommand = command
+      .map((arg) {
+        // Remove empty args.
+        if (arg.trim().isEmpty) {
+          return null;
+        }
+
+        // Attempt to make line continuations Windows & Linux compatible.
+        if (arg.trim() == r'\') {
+          return currentPlatform.isWindows ? arg.replaceAll(r'\', '^') : arg;
+        }
+        if (arg.trim() == '^') {
+          return currentPlatform.isWindows ? arg : arg.replaceAll('^', r'\');
+        }
+
+        // Inject MELOS_* variables if any.
+        environment.forEach((key, value) {
+          if (key.startsWith('MELOS_')) {
+            arg = arg.replaceAll('\$$key', value);
+            arg = arg.replaceAll(key, value);
+          }
+        });
+
+        return arg;
+      })
+      .where((element) => element != null)
+      .join(' ');
+
+  final process = await startCommandRaw(
+    processedCommand,
+    workingDirectory: workingDirectory,
+    environment: environment,
+    includeParentEnvironment: includeParentEnvironment,
   );
 
-  var stdoutStream = execProcess.stdout;
-  var stderrStream = execProcess.stderr;
+  var stdoutStream = process.stdout;
+  var stderrStream = process.stderr;
 
   if (prefix != null && prefix.isNotEmpty) {
     final pluginPrefixTransformer =
@@ -344,12 +361,12 @@ Future<int> startProcess(
       },
     );
 
-    stdoutStream = execProcess.stdout
+    stdoutStream = process.stdout
         .transform<String>(utf8.decoder)
         .transform<String>(pluginPrefixTransformer)
         .transform<List<int>>(utf8.encoder);
 
-    stderrStream = execProcess.stderr
+    stderrStream = process.stderr
         .transform<String>(utf8.decoder)
         .transform<String>(pluginPrefixTransformer)
         .transform<List<int>>(utf8.encoder);
@@ -381,7 +398,7 @@ Future<int> startProcess(
 
   await processStdoutCompleter.future;
   await processStderrCompleter.future;
-  final exitCode = await execProcess.exitCode;
+  final exitCode = await process.exitCode;
 
   if (onlyOutputOnError && exitCode > 0) {
     logger.stdout(utf8.decode(processStdout, allowMalformed: true));

packages/melos/lib/src/workspace.dart

@@ -183,7 +183,7 @@ class MelosWorkspace {
       if (childProcessPath != null) 'PATH': childProcessPath!,
     };
 
-    return utils.startProcess(
+    return utils.startCommand(
       execArgs,
       logger: logger,
       environment: environment,
@@ -203,7 +203,7 @@ class MelosWorkspace {
       if (childProcessPath != null) 'PATH': childProcessPath!,
     };
 
-    return utils.startProcess(
+    return utils.startCommand(
       execArgs,
       logger: logger,
       environment: environment,

packages/melos/test/utils_test.dart

@@ -1,11 +1,14 @@
 import 'dart:io';
 
+import 'package:melos/src/common/io.dart';
 import 'package:melos/src/common/utils.dart';
+import 'package:melos/src/logging.dart';
 import 'package:path/path.dart';
 import 'package:path/path.dart' as p;
 import 'package:pub_semver/pub_semver.dart';
 import 'package:test/test.dart';
 
+import 'matchers.dart';
 import 'utils.dart';
 
 void main() {
@@ -72,4 +75,30 @@ void main() {
       );
     });
   });
+
+  group('startProcess', () {
+    test('runs command chain in single shell', () async {
+      final workspaceDir = createTemporaryWorkspaceDirectory();
+      final testDir = p.join(workspaceDir.path, 'test');
+
+      ensureDir(testDir);
+
+      final logger = TestLogger();
+      await startCommand(
+        [
+          'cd',
+          'test',
+          '&&',
+          if (Platform.isWindows) 'cd' else 'pwd',
+        ],
+        logger: logger.toMelosLogger(),
+        workingDirectory: workspaceDir.path,
+      );
+
+      expect(
+        logger.output.normalizeNewLines(),
+        ignoringAnsii('$testDir\n'),
+      );
+    });
+  });
 }